diff options
author | Ludovic Courtès <ludo@gnu.org> | 2019-10-16 11:51:42 +0200 |
---|---|---|
committer | Ludovic Courtès <ludo@gnu.org> | 2019-10-16 22:53:40 +0200 |
commit | 81c580c8664bfeeb767e2c47ea343004e88223c7 (patch) | |
tree | 682678e9f32fd6c38f78ccd5cd2ab58ee736e1be /nix | |
parent | a1aaca314ca94700ebe3449d6bd73522f2d243bc (diff) | |
download | guix-81c580c8664bfeeb767e2c47ea343004e88223c7.tar.gz |
daemon: Make 'profiles/per-user' non-world-writable.
Fixes <https://bugs.gnu.org/37744>. Reported at <https://www.openwall.com/lists/oss-security/2019/10/09/4>. Based on Nix commit 5a303093dcae1e5ce9212616ef18f2ca51020b0d by Eelco Dolstra <edolstra@gmail.com>. * nix/libstore/local-store.cc (LocalStore::LocalStore): Set 'perUserDir' to #o755 instead of #o1777. (LocalStore::createUser): New function. * nix/libstore/local-store.hh (LocalStore): Add it. * nix/libstore/store-api.hh (StoreAPI): Add it. * nix/nix-daemon/nix-daemon.cc (performOp): In 'wopSetOptions', add condition to handle "user-name" property and honor it. (processConnection): Add 'userId' parameter. Call 'store->createUser' when userId is not -1. * guix/profiles.scm (ensure-profile-directory): Note that this is now handled by the daemon. * guix/store.scm (current-user-name): New procedure. (set-build-options): Add #:user-name parameter and pass it to the daemon. * tests/guix-daemon.sh: Test the creation of 'profiles/per-user' when listening on a TCP socket. * tests/store.scm ("profiles/per-user exists and is not writable") ("profiles/per-user/$USER exists"): New tests.
Diffstat (limited to 'nix')
-rw-r--r-- | nix/libstore/local-store.cc | 17 | ||||
-rw-r--r-- | nix/libstore/local-store.hh | 2 | ||||
-rw-r--r-- | nix/libstore/store-api.hh | 4 | ||||
-rw-r--r-- | nix/nix-daemon/nix-daemon.cc | 24 |
4 files changed, 43 insertions, 4 deletions
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc index 3b08492c64..3793382361 100644 --- a/nix/libstore/local-store.cc +++ b/nix/libstore/local-store.cc @@ -88,8 +88,9 @@ LocalStore::LocalStore(bool reserveSpace) Path perUserDir = profilesDir + "/per-user"; createDirs(perUserDir); - if (chmod(perUserDir.c_str(), 01777) == -1) - throw SysError(format("could not set permissions on '%1%' to 1777") % perUserDir); + if (chmod(perUserDir.c_str(), 0755) == -1) + throw SysError(format("could not set permissions on '%1%' to 755") + % perUserDir); mode_t perm = 01775; @@ -1642,4 +1643,16 @@ void LocalStore::vacuumDB() } +void LocalStore::createUser(const std::string & userName, uid_t userId) +{ + auto dir = settings.nixStateDir + "/profiles/per-user/" + userName; + + createDirs(dir); + if (chmod(dir.c_str(), 0755) == -1) + throw SysError(format("changing permissions of directory '%s'") % dir); + if (chown(dir.c_str(), userId, -1) == -1) + throw SysError(format("changing owner of directory '%s'") % dir); +} + + } diff --git a/nix/libstore/local-store.hh b/nix/libstore/local-store.hh index 4113fafcb5..2e48cf03e6 100644 --- a/nix/libstore/local-store.hh +++ b/nix/libstore/local-store.hh @@ -180,6 +180,8 @@ public: void setSubstituterEnv(); + void createUser(const std::string & userName, uid_t userId); + private: Path schemaPath; diff --git a/nix/libstore/store-api.hh b/nix/libstore/store-api.hh index 2d9dcbd573..7d2ad2270d 100644 --- a/nix/libstore/store-api.hh +++ b/nix/libstore/store-api.hh @@ -289,6 +289,10 @@ public: /* Check the integrity of the Nix store. Returns true if errors remain. */ virtual bool verifyStore(bool checkContents, bool repair) = 0; + + /* Create a profile for the given user. This is done by the daemon + because the 'profiles/per-user' directory is not writable by users. */ + virtual void createUser(const std::string & userName, uid_t userId) = 0; }; diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc index 1163a249d1..3dd156ba77 100644 --- a/nix/nix-daemon/nix-daemon.cc +++ b/nix/nix-daemon/nix-daemon.cc @@ -613,6 +613,17 @@ static void performOp(bool trusted, unsigned int clientVersion, || name == "build-repeat" || name == "multiplexed-build-output") settings.set(name, value); + else if (name == "user-name" + && settings.clientUid == (uid_t) -1) { + /* Create the user profile. This is necessary if + clientUid = -1, for instance because the client + connected over TCP. */ + struct passwd *pw = getpwnam(value.c_str()); + if (pw != NULL) + store->createUser(value, pw->pw_uid); + else + printMsg(lvlInfo, format("user name %1% not found") % value); + } else settings.set(trusted ? name : "untrusted-" + name, value); } @@ -731,7 +742,7 @@ static void performOp(bool trusted, unsigned int clientVersion, } -static void processConnection(bool trusted) +static void processConnection(bool trusted, uid_t userId) { canSendStderr = false; _writeToStderr = tunnelStderr; @@ -778,6 +789,15 @@ static void processConnection(bool trusted) /* Open the store. */ store = std::shared_ptr<StoreAPI>(new LocalStore(reserveSpace)); + if (userId != (uid_t) -1) { + /* Create the user profile. */ + struct passwd *pw = getpwuid(userId); + if (pw != NULL && pw->pw_name != NULL) + store->createUser(pw->pw_name, userId); + else + printMsg(lvlInfo, format("user with UID %1% not found") % userId); + } + stopWork(); to.flush(); @@ -963,7 +983,7 @@ static void acceptConnection(int fdSocket) /* Handle the connection. */ from.fd = remote; to.fd = remote; - processConnection(trusted); + processConnection(trusted, clientUid); exit(0); }, false, "unexpected build daemon error: ", true); |