8000 Making it possible to pass Windows credential specs directly to the e… · moby/moby@d01164e · GitHub
[go: up one dir, main page]
Skip to content

Commit d01164e

Browse files
wk8wk8
committed
Making it possible to pass Windows credential specs directly to the engine
Instead of having to go through files or registry values as is currently the case. While adding GMSA support to Kubernetes (kubernetes/kubernetes#73726) I stumbled upon the fact that Docker currently only allows passing Windows credential specs through files or registry values, forcing the Kubelet to perform a rather awkward dance of writing-then-deleting to either the disk or the registry to be able to create a Windows container with cred specs. This patch solves this problem by making it possible to directly pass whole base64-encoded cred specs to the engine's API. I took the opportunity to slightly refactor the method responsible for Windows cred spec as it seemed hard to read to me. Added some unit tests on Windows credential specs handling, as there were previously none. I have also tested it manually: given a Windows container using a cred spec that you would normally start with e.g. ```powershell docker run --rm --security-opt "credentialspec=file://win.json" mcr.microsoft.com/windows/servercore:ltsc2019 nltest /parentdomain # output: # my.ad.domain.com. (1) # The command completed successfully ``` can now equivalently be started with ```powershell $b64CredSpec = [System.Convert]::ToBase64String([System.IO.File]::ReadAllBytes('C:\ProgramData\docker\credentialspecs\win.json')) docker run --rm --security-opt "credentialspec=base64://$b64CredSpec" mcr.microsoft.com/windows/servercore:ltsc2019 nltest /parentdomain # same output! ``` I'll do another PR on Swarmkit after this is merged to allow services to use the same option. (It's worth noting that @dperny faced the same problem adding GMSA support to Swarmkit, to which he came up with an interesting solution - see #38632 - but alas these tricks are not available to the Kubelet.) Signed-off-by: Jean Rouge <rougej+github@gmail.com>
1 parent cbb885b commit d01164e

File tree

2 files changed

+430
-82
lines changed

2 files changed

+430
-82
lines changed

‎daemon/oci_windows.go‎

Lines changed: 103 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package daemon // import "github.com/docker/docker/daemon"
22

33
import (
4+
"encoding/base64"
45
"fmt"
56
"io/ioutil"
67
"path/filepath"
@@ -9,6 +10,7 @@ import (
910

1011
containertypes "github.com/docker/docker/api/types/container"
1112
"github.com/docker/docker/container"
13+
"github.com/docker/docker/errdefs"
1214
"github.com/docker/docker/oci"
1315
"github.com/docker/docker/oci/caps"
1416
"github.com/docker/docker/pkg/sysinfo"
@@ -253,68 +255,8 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
253255
setResourcesInSpec(c, s, isHyperV)
254256

255257
// 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.Erro 10000 rf("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-
6D4E }
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
258+
if err := daemon.setWindowsCredentialSpec(c, s); err != nil {
259+
return err
318260
}
319261

320262
// Do we have any assigned devices?
@@ -344,6 +286,82 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
344286
return nil
345287
}
346288

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

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, ""
448+
// registryKey is an interface wrapper around `registry.Key`,
449+
// listing only the methods we care about here.
450+
// It's mainly useful to easily allow mocking the registry in tests.
451+
type registryKey interface {
452+
GetStringValue(name string) (val string, valtype uint32, err error)
453+
Close() error
454+
}
455+
456+
var registryOpenKeyFunc = func(baseKey registry.Key, path string, access uint32) (registryKey, error) {
457+
return registry.OpenKey(baseKey, path, access)
437458
}
438459

439460
// readCredentialSpecRegistry is a helper function to read a credential spec from
440461
// the registry. If not found, we return an empty string and warn in the log.
441462
// This allows for staging on machines which do not have the necessary components.
442463
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 {
464+
key, err := registryOpenKeyFunc(registry.LOCAL_MACHINE, credentialSpecRegistryLocation, registry.QUERY_VALUE)
465+
if err != nil {
466+
return "", errors.Wrapf(err, "failed handling spec %q for container %s - registry key %s could not be opened", name, id, credentialSpecRegistryLocation)
467+
}
468+
defer key.Close()
469+
470+
value, _, err := key.GetStringValue(name)
471+
if err != nil {
452472
if err == registry.ErrNotExist {
453-
return "", fmt.Errorf("credential spec %q for container %s as it was not found", name, id)
473+
return "", fmt.Errorf("registry credential spec %q for container %s was not found", name, id)
454474
}
455-
return "", fmt.Errorf("error %v reading credential spec %q from registry for container %s", err, name, id)
475+
return "", errors.Wrapf(err, "error reading credential spec %q from registry for container %s", name, id)
456476
}
457-
return val, nil
477+
478+
return value, nil
458479
}
459480

460481
// readCredentialSpecFile is a helper function to read a credential spec from
@@ -471,7 +492,7 @@ func readCredentialSpecFile(id, root, location string) (string, error) {
471492
}
472493
bcontents, err := ioutil.ReadFile(full)
473494
if err != nil {
474-
return "", fmt.Errorf("credential spec '%s' for container %s as the file could not be read: %q", full, id, err)
495+
return "", errors.Wrapf(err, "credential spec for container %s could not be read from file %q", id, full)
475496
}
476497
return string(bcontents[:]), nil
477498
}

0 commit comments

Comments
 (0)
0