Conversation
src/nix/unix/daemon.cc
Outdated
| struct passwd * pw = peer.uidKnown ? getpwuid(peer.uid) : 0; | ||
| std::string user = pw ? pw->pw_name : std::to_string(peer.uid); | ||
| auto pw = peer.uid ? getpwuid(*peer.uid) : nullptr; | ||
| std::string user = pw ? pw->pw_name : peer.uid ? std::to_string(*peer.uid) : ""; |
There was a problem hiding this comment.
The empty string case seems suspicious
There was a problem hiding this comment.
It's better than the previous behavior where it would call std::to_string() on an uninitialized uid field. The empty string won't match with anything in trusted-user except *.
There was a problem hiding this comment.
But all the code paths from getPeerInfo initialized all members or failed with a SysError (which gets removed now). What's the rationale for it? That was needed for the TCP store somehow?
There was a problem hiding this comment.
Is in, shouldn't the unix socket keep getting the same error handling treatment?
There was a problem hiding this comment.
getPeerInfo() doesn't necessarily initialize all members, e.g. on macOS it only fills in uid.
I've replaced the empty string with an std::optional.
|
Yay thanks for extracting this! |
| if (getsockopt(remote, SOL_SOCKET, SO_PEERCRED, &cred, &credLen) == 0) { | ||
| peer.pid = cred.pid; | ||
| peer.uid = cred.uid; | ||
| peer.gid = cred.gid; | ||
| } |
There was a problem hiding this comment.
Now this will fail silent, but it will fail in a deauthorizing manner.
If we had a "disallow users" this would not be the case, but thankfully we don't.
3 participants
Motivation
Taken from #5265.
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.