docker / go-connections

Utility package to work with network connections
https://pkg.go.dev/github.com/docker/go-connections
Apache License 2.0
217 stars 101 forks source link

NewUnixSocketWithOpts(): reduce umask override time-window, document hack #75

Closed thaJeztah closed 3 years ago

thaJeztah commented 4 years ago

Relates to https://twitter.com/sferquel/status/1304071249870680066?s=20

~t.b.h., still wondering what would happen if we just omitted this entirely, and used the umask as defined on the host~

This hack was originally added in https://github.com/moby/moby/commit/24c73ce2d3d572313fe56bad08819e0ca8b74d26 (https://github.com/moby/moby/pull/5947), but was scarce on information, and this code was cause for some confusion.

net.Listen does not allow for permissions to be set. As a result, when specifying custom permissions ("WithChmod()"), there is a short time between creating the socket and applying the permissions, during which the socket permissions are Less restrictive than desired.

To work around this limitation of net.Listen(), we temporarily set the umask to 0777, which forces the socket to be created with 000 permissions (i.e.: no access for anyone). After that, WithChmod() must be used to set the desired permissions.

This patch also removes the use of defer here, so that we can reset the umask to its original value as soon as possible. Ideally we'd be able to detect if WithChmod() was passed as an option, and skip changing umask if default permissions are used.

thaJeztah commented 3 years ago

@tiborvass @chris-crone @ndeloof ptal 🤗