8000 Merge pull request #38777 from wk8/wk8/raw_cred_specs · moby/moby@2925eb7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2925eb7

Browse files
thaJeztahthaJeztah
authored
Merge pull request #38777 from wk8/wk8/raw_cred_specs
Making it possible to pass Windows credential specs directly to the engine
2 parents 5635c2a + 7fdac7e commit 2925eb7

File tree

4 files changed

+431
-93
lines changed

4 files changed

+431
-93
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ validate: build ## validate DCO, Seccomp profile generation, gofmt,\n./pkg/ isol
184184
$(DOCKER_RUN_DOCKER) hack/validate/all
185185

186186
win: build ## cross build the binary for windows
187-
$(DOCKER_RUN_DOCKER) hack/make.sh win
187+
$(DOCKER_RUN_DOCKER) DOCKER_CROSSPLATFORMS=windows/amd64 hack/make.sh cross
188188

189189
.PHONY: swagger-gen
190190
swagger-gen:

‎daemon/oci_windows.go‎

Lines changed: 98 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
containertypes "github.com/docker/docker/api/types/container"
1111
"github.com/docker/docker/container"
12+
"github.com/docker/docker/errdefs"
1213
"github.com/docker/docker/oci"
1314
"github.com/docker/docker/oci/caps"
1415
"github.com/docker/docker/pkg/sysinfo"
@@ -253,68 +254,8 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
253254
setResourcesInSpec(c, s, isHyperV)
254255

255256
// Read and add credentials from the security options if a credential spec has been provided.
256-
if c.HostConfig.SecurityOpt != nil {
257-
cs := ""
258-
for _, sOpt := range c.HostConfig.SecurityOpt {
259-
sOpt = strings.ToLower(sOpt)
260-
if !strings.Contains(sOpt, "=") {
261-
return fmt.Errorf("invalid security option: no equals sign in supplied value %s", sOpt)
262-
}
263-
var splitsOpt []string
264-
splitsOpt = strings.SplitN(sOpt, "=", 2)
265-
if len(splitsOpt) != 2 {
266-
return fmt.Errorf("invalid security option: %s", sOpt)
267-
}
268-
if splitsOpt[0] != "credentialspec" {
269-
return fmt.Errorf("security option not supported: %s", splitsOpt[0])
270-
}
271-
272-
var (
273-
match bool
274-
csValue string
275-
err error
276-
)
277-
if match, csValue = getCredentialSpec("file://", splitsOpt[1]); match {
278-
if csValue == "" {
279-
return fmt.Errorf("no value supplied for file:// credential spec security option")
280-
}
281-
if cs, err = readCredentialSpecFile(c.ID, daemon.root, filepath.Clean(csValue)); err != nil {
282-
return err
283-
}
284-
} else if match, csValue = getCredentialSpec("registry://", splitsOpt[1]); match {
285-
if csValue == "" {
286-
return fmt.Errorf("no value supplied for registry:// credential spec security option")
287-
}
288-
if cs, err = readCredentialSpecRegistry(c.ID, csValue); err != nil {
289-
return err
290-
}
291-
} else if match, csValue = getCredentialSpec("config://", splitsOpt[1]); match {
292-
// if the container does not have a DependencyStore, then it
293-
// isn't swarmkit managed. In order to avoid creating any
294-
// impression that `config://` is a valid API, return the same
295-
// error as if you'd passed any other random word.
296-
if c.DependencyStore == nil {
297-
return fmt.Errorf("invalid credential spec security option - value must be prefixed file:// or registry:// followed by a value")
298-
}
299-
300-
// after this point, we can return regular swarmkit-relevant
301-
// errors, because we'll know this container is managed.
302-
if csValue == "" {
303-
return fmt.Errorf("no value supplied for config:// credential spec security option")
304-
}
305-
306-
csConfig, err := c.DependencyStore.Configs().Get(csValue)
307-
if err != nil {
308-
return errors.Wrap(err, "error getting value from config store")
309-
}
310-
// stuff the resulting secret data into a string to use as the
311-
// CredentialSpec
312-
cs = string(csConfig.Spec.Data)
313-
} else {
314-
return fmt.Errorf("invalid credential spec security option - value must be prefixed file:// or registry:// followed by a value")
315-
}
316-
}
317-
s.Windows.CredentialSpec = cs
257+
if err := daemon.setWindowsCredentialSpec(c, s); err != nil {
258+
return err
318259
}
319260

320261
// Do we have any assigned devices?
@@ -344,6 +285,78 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
344285
return nil
345286
}
346287

288+
var errInvalidCredentialSpecSecOpt = errdefs.InvalidParameter(fmt.Errorf("invalid credential spec security option - value must be prefixed by 'file://', 'registry://', or 'raw://' followed by a non-empty value"))
289+
290+
// setWindowsCredentialSpec sets the spec's `Windows.CredentialSpec`
291+
// field if relevant
292+
func (daemon *Daemon) setWindowsCredentialSpec(c *container.Container, s *specs.Spec) error {
293+
if c.HostConfig == nil || c.HostConfig.SecurityOpt == nil {
294+
return nil
295+
}
296+
297+
// TODO (jrouge/wk8): if provided with several security options, we silently ignore
298+
// all but the last one (provided they're all valid, otherwise we do return an error);
299+
// this doesn't seem like a great idea?
300+
credentialSpec := ""
301+
302+
for _, secOpt := range c.HostConfig.SecurityOpt {
303+
optSplits := strings.SplitN(secOpt, "=", 2)
304+
if len(optSplits) != 2 {
305+
return errdefs.InvalidParameter(fmt.Errorf("invalid security option: no equals sign in supplied value %s", secOpt))
306+
}
307+
if !strings.EqualFold(optSplits[0], "credentialspec") {
308+
return errdefs.InvalidParameter(fmt.Errorf("security option not supported: %s", optSplits[0]))
309+
}
310+
311+
credSpecSplits := strings.SplitN(optSplits[1], "://", 2)
312+
if len(credSpecSplits) != 2 || credSpecSplits[1] == "" {
313+
return errInvalidCredentialSpecSecOpt
314+
}
315+
value := credSpecSplits[1]
316+
317+
var err error
318+
switch strings.ToLower(credSpecSplits[0]) {
319+
case "file":
320+
if credentialSpec, err = readCredentialSpecFile(c.ID, daemon.root, filepath.Clean(value)); err != nil {
321+
return errdefs.InvalidParameter(err)
322+
}
323+
case "registry":
324+
if credentialSpec, err = readCredentialSpecRegistry(c.ID, value); err != nil {
325+
return errdefs.InvalidParameter(err)
326+
}
327+
case "config":
328+
// if the container does not have a DependencyStore, then it
329+
// isn't swarmkit managed. In order to avoid creating any
330+
// impression that `config://` is a valid API, return the same
331+
// error as if you'd passed any other random word.
332+
if c.DependencyStore == nil {
333+
return errInvalidCredentialSpecSecOpt
334+
}
335+
336+
csConfig, err := c.DependencyStore.Configs().Get(value)
337+
if err != nil {
338+
return errdefs.System(errors.Wrap(err, "error getting value from config store"))
339+
}
340+
// stuff the resulting secret data into a string to use as the
341+
// CredentialSpec
342+
credentialSpec = string(csConfig.Spec.Data)
343+
case "raw":
344+
credentialSpec = value
345+
default:
346+
return errInvalidCredentialSpecSecOpt
347+
}
348+
}
349+
350+
if credentialSpec != "" {
351+
if s.Windows == nil {
352+
s.Windows = &specs.Windows{}
353+
}
354+
s.Windows.CredentialSpec = credentialSpec
355+
}
356+
357+
return nil
358+
}
359+
347360
// Sets the Linux-specific fields of the OCI spec
348361
// TODO: @jhowardmsft LCOW Support. We need to do a lot more pulling in what can
349362
// be pulled in from oci_linux.go.
@@ -427,34 +440,37 @@ func (daemon *Daemon) mergeUlimits(c *containertypes.HostConfig) {
427440
return
428441
}
429442

430-
// getCredentialSpec is a helper function to get the value of a credential spec supplied
431-
// on the CLI, stripping the prefix
432-
func getCredentialSpec(prefix, value string) (bool, string) {
433-
if strings.HasPrefix(value, prefix) {
434-
return true, strings.TrimPrefix(value, prefix)
435-
}
436-
return false, ""
443+
// registryKey is an interface wrapper around `registry.Key`,
444+
// listing only the methods we care about here.
445+
// It's mainly useful to easily allow mocking the registry in tests.
446+
type registryKey interface {
447+
GetStringValue(name string) (val string, valtype uint32, err error)
448+
Close() error
449+
}
450+
451+
var registryOpenKeyFunc = func(baseKey registry.Key, path string, access uint32) (registryKey, error) {
452+
return registry.OpenKey(baseKey, path, access)
437453
}
438454

439455
// readCredentialSpecRegistry is a helper function to read a credential spec from
440456
// the registry. If not found, we return an empty string and warn in the log.
441457
// This allows for staging on machines which do not have the necessary components.
442458
func readCredentialSpecRegistry(id, name string) (string, error) {
443-
var (
444-
k registry.Key
445-
err error
446-
val string
447-
)
448-
if k, err = registry.OpenKey(registry.LOCAL_MACHINE, credentialSpecRegistryLocation, registry.QUERY_VALUE); err != nil {
449-
return "", fmt.Errorf("failed handling spec %q for container %s - %s could not be opened", name, id, credentialSpecRegistryLocation)
450-
}
451-
if val, _, err = k.GetStringValue(name); err != nil {
459+
key, err := registryOpenKeyFunc(registry.LOCAL_MACHINE, credentialSpecRegistryLocation, registry.QUERY_VALUE)
460+
if err != nil {
461+
return "", errors.Wrapf(err, "failed handling spec %q for container %s - registry key %s could not be opened", name, id, credentialSpecRegistryLocation)
462+
}
463+
defer key.Close()
464+
465+
value, _, err := key.GetStringValue(name)
466+
if err != nil {
452467
if err == registry.ErrNotExist {
453-
return "", fmt.Errorf("credential spec %q for container %s as it was not found", name, id)
468+
return "", fmt.Errorf("registry credential spec %q for container %s was not found", name, id)
454469
}
455-
return "", fmt.Errorf("error %v reading credential spec %q from registry for container %s", err, name, id)
470+
return "", errors.Wrapf(err, "error reading credential spec %q from registry for container %s", name, id)
456471
}
457-
return val, nil
472+
473+
return value, nil
458474
}
459475

460476
// readCredentialSpecFile is a helper function to read a credential spec from
@@ -471,7 +487,7 @@ func readCredentialSpecFile(id, root, location string) (string, error) {
471487
}
472488
bcontents, err := ioutil.ReadFile(full)
473489
if err != nil {
474-
return "", fmt.Errorf("credential spec '%s' for container %s as the file could not be read: %q", full, id, err)
490+
return "", errors.Wrapf(err, "credential spec for container %s could not be read from file %q", id, full)
475491
}
476492
return string(bcontents[:]), nil
477493
}

0 commit comments

Comments
 (0)
0