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

go: fix binding of privileged UDP ports via vmnetd #485

Merged
merged 3 commits into from Aug 6, 2019

Conversation

djs55
Copy link
Collaborator

@djs55 djs55 commented Aug 5, 2019

Previously in 32ede13 we added a fix for TCP where we didn't set the socket to non-blocking mode. This PR replicates the fix for UDP and adds a unit test.

Related to docker/for-mac#3775

Furthermore this PR only uses the vmnetd code path if the bind actually fails with permission denied. In particular on Mojave, some binds will succeed (e.g. 0.0.0.0:80) without being root.

Signed-off-by: David Scott <dave.scott@docker.com>
This mirrors the fix that we already had for TCP.

Signed-off-by: David Scott <dave.scott@docker.com>
On Mojave, binding to 0.0.0.0:80 will work but binding to 127.0.0.1:80
will fail (and still require vmnetd)

Signed-off-by: David Scott <dave.scott@docker.com>
@djs55
Copy link
Collaborator Author

djs55 commented Aug 6, 2019

Ignoring the OCaml broken CIs, travis is green for the Go code. This also passes the Docker Desktop end-to-end tests for extra safety.

@djs55 djs55 merged commit 103ce99 into moby:master Aug 6, 2019
@djs55 djs55 deleted the fix-vmnet-udp branch November 28, 2019 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant