docker / cli

The Docker CLI
Apache License 2.0
4.95k stars 1.93k forks source link

Add `--tls-verify` option for `docker login` #3183

Open zdzielinski opened 3 years ago

zdzielinski commented 3 years ago

Description

The docker login command should ideally support a new --tls-verify option for marking insecure registries at runtime.

Current methodology:

Currently, any insecure registries must be added to the daemon.json file, and docker must be restarted to reflect changes. This works well for long-term registry usage on long-living machines, but this current feature-set could be extended to better support more ephemeral environments by removing manual configuration steps for marking insecure registries.

IE - https://docs.docker.com/registry/insecure/

{
  "insecure-registries" : ["myregistrydomain.com:5000"]
}

As an example, a tool in the same general area as docker is skopeo. This tool currently supports a --tls-verify option for skopeo login - and works as I would expect this potential new feature for docker login to work. During the performance of skopeo login, authentication credentials are stored along with a mark for the registry being "insecure", further use of this registry does not require valid CA trust for commands such as skopeo sync and skopeo copy.

IE - https://github.com/containers/skopeo/blob/main/docs/skopeo-login.1.md

Generally, when a user is logging into a registry - they have at least a basic understanding of the details of that registry, whether it be self hosted, upstream, etc. We should extend support for the user to mark this registry as "insecure" during the login action to help remove manual configuration steps on their end, as many users may overlook this manual action as a pre-configuration step during internal tool usage. This creates confusion in heavily automated environments, and removing this step would go a long way in easing the use of the docker cli.

Possible concerns:

Does this create a security regression concern? Should we be worried about this potentially being exploited in upstream and publicly available configuration scripts if implemented? As a potential solution, we could add output for docker pull or docker run to make it know to the user that the current registry is insecure, so at least some awareness is provided throughout its use. This could be extended to any potential log files in use as well.

For example, this output could look like this:

> docker pull 127.0.0.1:5000/nginx:latest
WARNING: The registry "127.0.0.1:5000" was marked as insecure during docker login.
WARNING: Please ensure this registry is known to be insecure, and that this is expected.
latest: Pulling from library/nginx
b4d181a07f80: Pull complete 
66b1c490df3f: Pull complete 
d0f91ae9b44c: Pull complete 
baf987068537: Pull complete 
6bbc76cbebeb: Pull complete 
32b766478bc2: Pull complete 
Digest: sha256:8df46d7414eda82c2a8c9c50926545293811ae59f977825845dda7d558b4125b
Status: Downloaded newer image for nginx:latest
127.0.0.1:5000/library/nginx:latest

The above warning could potentially trigger only if a registry was marked as insecure during docker login, which would distinguish this potentially insecure action from registries that were marked as insecure within the daemon.json file itself. One marker for the daemon.json file to distinguish this could look like the following:

{
  "insecure-registries" : [
    "myregistrydomain.com:5000",
    "runtime:myotherregistrydomain.com:5000"
  ]
}

Output of docker version:

Docker version 20.10.7, build f0df350

Output of docker info:

Client:
 Context:    desktop-linux
 Debug Mode: false
 Plugins:
  buildx: Build with BuildKit (Docker Inc., v0.5.1-docker)
  compose: Docker Compose (Docker Inc., v2.0.0-beta.6)
  scan: Docker Scan (Docker Inc., v0.8.0)

Server:
 Containers: 1
  Running: 0
  Paused: 0
  Stopped: 1
 Images: 20
 Server Version: 20.10.7
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: d71fcd7d8303cbf684402823e425e9dd2e99285d
 runc version: b9ee9c6314599f1b4a7f497e1f1f856fe433d3b7
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.10.25-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 2
 Total Memory: 3.844GiB
 Name: docker-desktop
 ID: 6TAU:Y375:2HDP:VAPH:36OD:VAVK:D4XR:PHAK:PNE6:4YQ6:LDLK:OXEK
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.1:5000
  test.localdomain:1234
  127.0.0.0/8
 Live Restore Enabled: false

Additional environment details (AWS, VirtualBox, physical, etc.):

MacOS Big Sur v11.4

thaJeztah commented 3 years ago

This is (roughly) a duplicate of https://github.com/moby/moby/issues/8887

Adding this flag to docker login likely won't help with this use-case, because every action interacting with the registry would require this flag to be added as well (docker run, docker pull, docker push, docker build, etc.)

Does this create a security regression concern? Should we be worried about this potentially being exploited in upstream and publicly available configuration scripts if implemented?

Yes, it does. Disabling TLS verify definitely affects security; a user could now docker pull --tls-verify=false ubuntu, and will be exposed MITM attacks.

Of course, there may be situations where "insecure" is considered "OK" (depending on the situation; e.g., test-environments, and registry running within an internal network);

Note that loopback IP addresses are already marked as "insecure" by default (127.0.0.0/8), so if this is for test-environments, and the ephemeral registry runs on the same machine, this would already work without changes.

If the ephemeral registries are on an internal network, it's possible to specify the internal ip-range as "insecure".

Or, if you like to live dangerous; specify the whole internet as "insecure", which would be the rough equivalent of fully disabling TLS protection https://github.com/moby/moby/issues/8887#issuecomment-549148054

{"insecure-registries": ["0.0.0.0/1","128.0.0.0/2","192.0.0.0/3","224.0.0.0/4"]}
zdzielinski commented 3 years ago

This is good information - although I am still on the side of the fence that supporting some method of runtime marking would be ideal. The main idea being that during automated setup of certain environments which already expect insecure registries to be marked, we could have docker mark them for us in a supported way, removing the need to manually template the daemon.json file.

My main question is - would the benefits of this type of feature outweigh any possible security concerns? Given that other tools (skopeo as an example) already support features in the same vein? And would the Docker team be interested in exploring a feature addition for this?

thaJeztah commented 3 years ago

I think one big difference between skopeo and the docker cli is that skopeo is (unless this changed) a local tool. In other words, any action it performs will run on the client side. The docker cli controls the docker daemon, which may be running locally on your machine, but could run elsewhere.

For that reason, the configuration is currently on the daemon side; the person responsible for managing the daemon controls wether or not connecting with insecure registries should be allowed.

zdzielinski commented 3 years ago

Would it be worth exploring an additional docker daemon or docker settings interaction then?

Potentially, a sub-command could be created to document and template any settings that are supported within the daemon.json file, and handle re-loading the ingest of this file as an alternative to restarting docker every time.

Just spitballing at this point, it's not the end of the world if this never comes to fruition. But I know that many heavily automated shops would leverage these types of features very quickly if implemented.

nigels-com commented 2 months ago

Need this. Having to touch daemon.json to workaround a glitchy setup is less than ideal.