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.
This commit is contained in:
		
							parent
							
								
									a1aaca314c
								
							
						
					
					
						commit
						81c580c866
					
				
					 8 changed files with 90 additions and 6 deletions
				
			
		|  | @ -1732,7 +1732,8 @@ because the NUMBER is zero.)" | |||
|   (string-append %profile-directory "/guix-profile")) | ||||
| 
 | ||||
| (define (ensure-profile-directory) | ||||
|   "Attempt to create /…/profiles/per-user/$USER if needed." | ||||
|   "Attempt to create /…/profiles/per-user/$USER if needed.  Nowadays this is | ||||
| taken care of by the daemon." | ||||
|   (let ((s (stat %profile-directory #f))) | ||||
|     (unless (and s (eq? 'directory (stat:type s))) | ||||
|       (catch 'system-error | ||||
|  |  | |||
|  | @ -748,6 +748,14 @@ encoding conversion errors." | |||
|            (cut string-append "http://" <>)) | ||||
|        '("ci.guix.gnu.org"))) | ||||
| 
 | ||||
| (define (current-user-name) | ||||
|   "Return the name of the calling user." | ||||
|   (catch #t | ||||
|     (lambda () | ||||
|       (passwd:name (getpwuid (getuid)))) | ||||
|     (lambda _ | ||||
|       (getenv "USER")))) | ||||
| 
 | ||||
| (define* (set-build-options server | ||||
|                             #:key keep-failed? keep-going? fallback? | ||||
|                             (verbosity 0) | ||||
|  | @ -759,6 +767,7 @@ encoding conversion errors." | |||
|                             (build-verbosity 0) | ||||
|                             (log-type 0) | ||||
|                             (print-build-trace #t) | ||||
|                             (user-name (current-user-name)) | ||||
| 
 | ||||
|                             ;; When true, provide machine-readable "build | ||||
|                             ;; traces" for use by (guix status).  Old clients | ||||
|  | @ -849,6 +858,9 @@ encoding conversion errors." | |||
|                            `(("build-repeat" | ||||
|                               . ,(number->string (max 0 (1- rounds))))) | ||||
|                            '()) | ||||
|                      ,@(if user-name | ||||
|                            `(("user-name" . ,user-name)) | ||||
|                            '()) | ||||
|                      ,@(if terminal-columns | ||||
|                            `(("terminal-columns" | ||||
|                               . ,(number->string terminal-columns))) | ||||
|  |  | |||
|  | @ -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); | ||||
| } | ||||
| 
 | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -180,6 +180,8 @@ public: | |||
| 
 | ||||
|     void setSubstituterEnv(); | ||||
| 
 | ||||
|     void createUser(const std::string & userName, uid_t userId); | ||||
| 
 | ||||
| private: | ||||
| 
 | ||||
|     Path schemaPath; | ||||
|  |  | |||
|  | @ -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; | ||||
| }; | ||||
| 
 | ||||
| 
 | ||||
|  |  | |||
|  | @ -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); | ||||
|  |  | |||
|  | @ -94,6 +94,27 @@ done | |||
| 
 | ||||
| kill "$daemon_pid" | ||||
| 
 | ||||
| # Make sure 'profiles/per-user' is created when connecting over TCP. | ||||
| 
 | ||||
| orig_GUIX_STATE_DIRECTORY="$GUIX_STATE_DIRECTORY" | ||||
| GUIX_STATE_DIRECTORY="$GUIX_STATE_DIRECTORY-2" | ||||
| 
 | ||||
| guix-daemon --disable-chroot --listen="localhost:9877" & | ||||
| daemon_pid=$! | ||||
| 
 | ||||
| GUIX_DAEMON_SOCKET="guix://localhost:9877" | ||||
| export GUIX_DAEMON_SOCKET | ||||
| 
 | ||||
| test ! -d "$GUIX_STATE_DIRECTORY/profiles/per-user" | ||||
| 
 | ||||
| guix build guile-bootstrap -d | ||||
| 
 | ||||
| test -d "$GUIX_STATE_DIRECTORY/profiles/per-user/$USER" | ||||
| 
 | ||||
| kill "$daemon_pid" | ||||
| unset GUIX_DAEMON_SOCKET | ||||
| GUIX_STATE_DIRECTORY="$orig_GUIX_STATE_DIRECTORY" | ||||
| 
 | ||||
| # Check the failed build cache. | ||||
| 
 | ||||
| guix-daemon --no-substitutes --listen="$socket" --disable-chroot	\ | ||||
|  |  | |||
|  | @ -18,6 +18,7 @@ | |||
| 
 | ||||
| (define-module (test-store) | ||||
|   #:use-module (guix tests) | ||||
|   #:use-module (guix config) | ||||
|   #:use-module (guix store) | ||||
|   #:use-module (guix utils) | ||||
|   #:use-module (guix monads) | ||||
|  | @ -102,7 +103,17 @@ | |||
|               "/283gqy39v3g9dxjy26rynl0zls82fmcg-guile-2.0.7/bin/guile"))) | ||||
|        (not (direct-store-path? (%store-prefix))))) | ||||
| 
 | ||||
| (test-skip (if %store 0 13)) | ||||
| (test-skip (if %store 0 15)) | ||||
| 
 | ||||
| (test-equal "profiles/per-user exists and is not writable" | ||||
|   #o755 | ||||
|   (stat:perms (stat (string-append %state-directory "/profiles/per-user")))) | ||||
| 
 | ||||
| (test-equal "profiles/per-user/$USER exists" | ||||
|   (list (getuid) #o755) | ||||
|   (let ((s (stat (string-append %state-directory "/profiles/per-user/" | ||||
|                                 (passwd:name (getpwuid (getuid))))))) | ||||
|     (list (stat:uid s) (stat:perms s)))) | ||||
| 
 | ||||
| (test-equal "add-data-to-store" | ||||
|   #vu8(1 2 3 4 5) | ||||
|  |  | |||
		Reference in a new issue