fix: set the listening socket mode instead of inheriting the umask - #42
Merged
Merged
Conversation
bind(2) on an AF_UNIX socket creates the inode with 0777 & ~umask, and none of the three implementations touched it afterwards. connect(2) requires write permission on the socket, so at the usual umask 022 the socket came out 0755 and the grant the README documents — put the caller in the socket's group — silently did not work; only the owning uid could connect. Under umask 0 it came out 0777 and any local uid could drive the full Docker API through it. With the TCP listener gone this mode is the entire access-control boundary. Adds --listen-socket-mode (default 0660) and --listen-socket-group, taking either a group name or a gid. The umask is narrowed to 0177 around the bind so the socket is created at 0600, then chown'd and chmod'd before the first accept. Setting the mode after bind instead would leave a window in which the socket is already listening at the ambient mode. A world-writable mode is refused at startup with exit 2. There is deliberately no opt-out: it removes the boundary entirely. Both flags are ignored for fd://3, where systemd owns the socket and SocketMode/SocketGroup in the .socket unit are the right controls. Group name lookup differs by runtime: Go uses os/user.LookupGroup and Rust uses getgrnam_r, both of which consult NSS. Node has no equivalent, so TypeScript reads /etc/group — enough for the container case, and the error points at passing a numeric gid otherwise. TypeScript's bind is extracted to ts/src/listen.ts so the mode can be tested at all; it was top-level module code with no export. Verified natively under both umasks, all three: mode 660 either way, and serving. Each new test was checked to fail without the fix (it reports 755 under umask 022 and 777 under umask 0, exactly as reported). Closes #40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #40.
Branched off
main(not stacked this time).The bug
bind(2)on anAF_UNIXsocket creates the inode with0777 & ~umask, and none of the three implementations touched it afterwards.unix(7): connecting to a filesystem-visible socket requires write permission on it.0755w, so the grant the README documents silently does not work — only the owning uid can connect0777With the TCP listener removed in #35, this mode is the entire access-control boundary.
Changes
--listen-socket-mode(default0660) and--listen-socket-group(group name or gid) in all three.0177around the bind, so the socket is created at0600, then chown'd and chmod'd before the firstaccept. Setting the mode after bind would leave a window in which the socket is already listening at the ambient mode. Chown precedes chmod so it is never briefly reachable by the wrong group.fd://3, where systemd owns the socket andSocketMode/SocketGroupare the right controls.Verified natively, both umasks
Rejection paths, all three exit 2:
One deliberate asymmetry
Group name lookup differs by runtime, and I could not make it uniform:
os/user.LookupGroup(NSS)getgrnam_r(NSS)/etc/group, because Node exposes nogetgrnamequivalent at allSo an NSS-backed group (LDAP, SSSD) resolves in Go and Rust but not in TypeScript. The TS error message says so and points at passing a numeric gid, which works everywhere. Flagging it explicitly rather than burying it — it is the one place the three genuinely differ.
libcadded to Rust forumask(2)andgetgrnam_r(3), neither of which std exposes. It was already inCargo.locktransitively via tokio, so it costs no new compile units.Tests
Every new test was checked to fail without its fix — they report
755under umask 022 and777under umask 0, reproducing the issue exactly.listen.tsumask restore and bind-failure paths--listen-socket-group, not world-writablets/src/listen.tsis extracted fromindex.tsso the mode is testable at all — it was top-level module code with no export, the same problemshutdownhad in #41.Verification
make test-all,make lint-all— passmake test-integration{,-rs,-ts}— 27/27 eachmake test-integration-sock{,-rs,-ts}— 12/12 eachDocs
README.mdcorrected — it documented a grant mechanism that did not work. Now states the mode is set regardless of umask, explains whyconnect(2)needs write, notes that the group is what makes the mode useful, and pointsfd://3users at the unit file.