Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The push command sends a POST request with an empty Content-Type header #41621

Open
Smasherr opened this issue Nov 2, 2020 · 7 comments
Open

Comments

@Smasherr
Copy link

Smasherr commented Nov 2, 2020

Description

In our company, we have an internal docker registry behind a Web-Application-Firewall that handles the traffic incoming from the outside of our network. We had to analyze why docker push fails if a request has to go through the WAF. Quickly we found out that it receives a POST-request (after a HEAD-request) from the docker client and blocks it due to the empty Content-Type header.

Looks like this request is invoked from repository.go:739

resp, err := bs.client.Post(u, "", nil)

According to the protocol's RFC, if the header is set its value must not be empty:

Content-Type   = "Content-Type" ":" media-type

(otherwise, it would be 1#media-type for instance, see the conventions for #rule)

Hence I think for better compliance with the RFC, it is better to omit Content-Type completely rather than to have it with an empty value If the POST request does not have a body

Output of docker version:

Client: Docker Engine - Community
 Version:           19.03.12
 API version:       1.40
 Go version:        go1.13.10
 Git commit:        48a66213fe
 Built:             Mon Jun 22 15:45:50 2020
 OS/Arch:           linux/amd64
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          19.03.13
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       4484c46d9d
  Built:            Wed Sep 16 17:07:04 2020
  OS/Arch:          linux/amd64
  Experimental:     true
 containerd:
  Version:          v1.3.7
  GitCommit:        8fba4e9a7d01810a393d5d25a3621dc101981175
 runc:
  Version:          1.0.0-rc10
  GitCommit:        dc9208a3303feef5b3839f4323d9beb36df0a9dd
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683

Output of docker info:

Client:
 Debug Mode: false
 Plugins:
  app: Docker Application (Docker Inc., v0.8.0)
  buildx: Build with BuildKit (Docker Inc., v0.3.1-tp-docker)

Server:
 Containers: 15
  Running: 1
  Paused: 0
  Stopped: 14
 Images: 162
 Server Version: 19.03.13
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 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: runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 8fba4e9a7d01810a393d5d25a3621dc101981175
 runc version: dc9208a3303feef5b3839f4323d9beb36df0a9dd
 init version: fec3683
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 4.19.128-microsoft-standard
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 12.35GiB
 Name: docker-desktop
 ID: 6UPN:CSRO:W7PW:BWOX:2ROZ:4TQA:LS7D:OMZW:CEFK:3JAO:VX2N:24EU
 Docker Root Dir: /var/lib/docker
 Debug Mode: true
  File Descriptors: 47
  Goroutines: 52
  System Time: 2020-10-30T16:00:11.5716656Z
  EventsListeners: 3
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: true
 Insecure Registries:
  bin.ti8m.ch:80
  127.0.0.0/8
 Live Restore Enabled: false
 Product License: Community Engine

WARNING: bridge-nf-call-iptables is disabled
WARNING: bridge-nf-call-ip6tables is disabled

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

@thaJeztah
Copy link
Member

Thanks for reporting; looks like that code is coming from a dependency that we use; the source code for that is coming from the https://github.com/docker/distribution repository.

Looking for that line in the code, I see it was originally added in distribution/distribution@ce614b6, which was part of distribution/distribution#387 (https://github.com/docker/distribution/pull/387/files#diff-83220d00e31f5405e8bfa78525b47db3d1cf3c9376da699a84f953ff3a94b882R345)

@dmcgowan @tiborvass do you recall if there was a specific reason for this, or was it because no content-type header was expected for this specific request?

@dmcgowan
Copy link
Member

dmcgowan commented Nov 2, 2020

Not intentional, the Go library isn't doing what was expected in this case and just always sets the header even if empty. This could be fixed by replacing Post call with NewRequest + Do

@Smasherr
Copy link
Author

Smasherr commented Nov 2, 2020

The fix seems to be easy, should I open a pull request here or in docker/distribution?

@thaJeztah
Copy link
Member

Thanks for looking @dmcgowan !

@Smasherr fix should go into docker/distribution, because the code in vendor is "vendored in" from the upstream repository; thanks in advance for your pull request there!

@Smasherr
Copy link
Author

Smasherr commented Nov 3, 2020

@iamanv
Copy link

iamanv commented Feb 18, 2021

Has the issue been fixed and released to public? I have a similar use case wherein I am trying to push an image through a reverse proxy and noticed that the docker client still sends Content-Type: [] with Content-Length: [0] while doing a POST for blob uploads. Here are some more details about the version I am running:

 Cloud integration: 1.0.7
 Version:           20.10.2
 API version:       1.41
 Go version:        go1.13.15
 Git commit:        2291f61
 Built:             Mon Dec 28 16:12:42 2020
 OS/Arch:           darwin/amd64
 Context:           default
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          20.10.2
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       8891c58
  Built:            Mon Dec 28 16:15:28 2020
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.4.3
  GitCommit:        269548fa27e0089a8b8278fc4fc781d7f65a939b
 runc:
  Version:          1.0.0-rc92
  GitCommit:        ff819c7e9184c13b7c2607fe6c30ae19403a7aff
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0 

Also, I am not really sure if I completely understood what @thaJeztah meant by:

fix should go into docker/distribution, the code in vendor is "vendored in" from the upstream repository

Since it's the docker client that is sending the request with empty header, shouldn't the fix be applied in docker/engine? If someone could provide some information, would be of great help thanks.

@thaJeztah
Copy link
Member

Has the issue been fixed and released to public?

The code was fixed in the "distribution" library (distribution/distribution#3289 / distribution/distribution#3297), but has not yet been included in a release of that library

Since it's the docker client that is sending the request with empty header, shouldn't the fix be applied in docker/engine?

The bug is in the client library that's maintained in the https://github.com/distribution/distribution repository. The "docker engine" uses that library as a build-time dependency (so the source code for the function in which the bug is, compiled into the docker/engine).

This repository has a copy of that code in the "vendor" directory, but we don't make changes to vendored code (otherwise integrity validation would fail);

resp, err := bs.client.Post(u, "", nil)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants