[RFC] UX/Consistency: the state of credential spec

thaJeztah commented 7 years ago

While working on https://github.com/moby/moby/pull/34002, I noticed there's a lot of confusing bits in this feature that we may want to improve on;

on `docker run` / `docker create`

set through --security-opt credentialspec=file://foo (https://github.com/moby/moby/pull/23389)
flag --credentialspec was not implemented, and removed in https://github.com/moby/moby/pull/31976
compose-file (version: 2 - undocumented https://docs.docker.com/compose/compose-file/compose-file-v2/#security_opt):
```
version: '2'
services:
  sqlserver:
    ...
    security_opt:
      - "credentialspec=file://sqlserverapp.json"
```

on `docker service create` / `docker service update`

set through dedicated flag: --credential-spec=file://foo (https://github.com/moby/moby/pull/32339)
compose-file (version 3): https://docs.docker.com/compose/compose-file/#credential_spec
top-level option for service

Docker compose docs / implementation (version 3):

Documentation needs some updating;

    credential_spec:
      file: c:/WINDOWS/my-credential-spec.txt

    credential_spec:
      registry: HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Virtualization\Containers\CredentialSpecs

Using:

registry: "HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Virtualization\Containers\CredentialSpecs"

Produces an error:

yaml: line 6: found unknown escape character

And has to be changed to;

registry: "HKLM\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Virtualization\\Containers\\CredentialSpecs"

In addition:

The example file in the documentation is an absolute path, but paths should be specified relative to C:\ProgramData\docker\CredentialSpecs\, or more factually <docker root>/CredentialSpecs (docker root is configurable)
The example file in the documentation is specified using a Linux path, but should use Windows (backslash, instead of forward slash)
We should explain if "file" refers to a file on the local host, the daemon host, or inside the container
The registry option only expects a registry key name, not the full path (i.e. it searches for a key named "the-key-I-specified" inside HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Virtualization\Containers\CredentialSpecs)
There's no example of the expected format of the credential specs file
- the example shows .txt, but the credential specs file is actually a JSON file, in which case, our example should use my-credential-spec.json
- can we provide an example file, and how to create/generate one (or refer to Windows documentation on creating one)

Error message consistency

Using this compose file;

version: '3.3'
services:
  web:
    image: "nginx:alpine"
    credential_spec:
        file: c:/WINDOWS/my-credential-spec.txt
        registry: "HKLM\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Virtualization\\Containers\\CredentialSpecs"

produces an error, because both file: and registry: is specified;

service web: Invalid credential spec - must provide one of `File` or `Registry`

The error is correct, but given that this error is generated client-side;

should File and Registry be changed to file: and registry: ?
do we perform the same validation server-side? (we should)

thaJeztah commented 7 years ago

ping @friism @aluzzardi @johnstep @londoncalling for input / help

thaJeztah commented 7 years ago

On the format of the credential spec file; I found this gist https://gist.github.com/PatrickLang/27c743782fca17b19bf94490cbb6f960, referring to a tool here;

https://github.com/Microsoft/Virtualization-Documentation/blob/804008c172b80a4f354a92cd4d12a4e23e1d4328/windows-server-container-tools/ServiceAccounts/CredentialSpec.psm1

Should we have this information documented in the Docker documentation?
If these tools are needed to run native Windows containers, should they be included in releases? (could be in contrib/)

thaJeztah commented 7 years ago

Also; the requirement to have the specs uploaded to the daemon host (or present in the daemon host's registry) may make it more difficult to use in a Swarm; is there a way to distribute credential-specs in a Swarm (e.g. through secrets or configs)?

thaJeztah commented 7 years ago

/cc @PatrickLang @jhowardmsft

johnstep commented 7 years ago

You're right about the paths. For both files and registry keys, they should be relative, and I'm not even positive if sub-directories or sub-keys are supported, but will verify.

As for swarm, we are planning to add a config: option to reference a credential spec stored as a config instead of a file or a registry key. In 17.06, that is not supported. There is a workaround to distribute a credential spec to all nodes in the swarm, using a config, but it is certainly not ideal:

copy-credspec.yml

version: '3.3'
configs:
  credspec:
    file: ${ProgramData}/Docker/CredentialSpecs/${CREDSPEC:-gmsa.json}
services:
  copy:
    command: cmd /c copy credspec C:\\out\\${CREDSPEC:-gmsa.json}
    configs:
      - credspec
    deploy:
      mode: global
      restart_policy:
        condition: none
    image: microsoft/nanoserver
    volumes:
      - source: ${ProgramData}/Docker/CredentialSpecs
        target: C:/out
        type: bind

That compose file defaults to gmsa.json but a variable $env:CREDSPEC overrides the file name. The file is stored in the default location, as per the script that creates it in the first place.

The above can be deployed in a swarm with:

docker stack deploy -c copy-credspec.yml copy-credspec

We are hoping to have a config: option soon, for swarm.

friism commented 7 years ago

@thaJeztah thanks for typing this up.

I agree with the docs recommendations you make.

I think there was a debate about whether to security_opt or not to security_opt. Afaik, we can back-port --credential-spec to docker run for consistency.

thaJeztah commented 7 years ago

Yes, we should try to keep it consistent at least

@johnstep interesting; what does a docker service inspect look like for your example? Because this part of the code looks to indicate you cannot pass a full path; https://github.com/moby/moby/blob/9aecbbf9bf50dd5c3d250e8dc1c74360a9f30d8e/daemon/start_windows.go#L201-L208

johnstep commented 7 years ago

Sorry for the confusion. The service above does not use a credential spec; it copies the credential spec file to each node in the swarm, thus allowing any other service to make use of the credential spec.

thaJeztah commented 7 years ago

Oh! Sorry, yes, I mis-read the example

friism commented 7 years ago

@patricklang When this feature was first added to docker run we ended up with a model where it was documented in Microsoft docs with links from Docker docs. Our experience is that lots of people are using this with Docker Windows containers, which is why we made sure it would work with docker service create and docker-compose. How would you feel about contributing your guide to the Docker docs and working on extending it together?

thaJeztah commented 6 years ago

ping @gbarr01 - just recalled this one was still open, and also relates to missing documentation for this feature

docker / cli