[PATCH] daemon: Set ownership of kept build directories to the calling user.

[Hartmut Goebel]

This also closes bug #15890.

* nix/libstore/worker-protocol.hh (PROTOCOL_VERSION): Increment it.
* guix/store.scm (protocol-version): Increment it to the same value.
  (set-build-options): Send uid and gid of evoking user.
* nix/libstore/globals.hh (Settings) Add clientUid and clientGid.
* nix/nix-daemon/nix-daemon.cc (performOp)[wopSetOptions] Read clientUid
  and clientGid.
* nix/libstore/build.cc (DerivationGoal::deleteTmpDir): Change
  ownership of build directory if it is kept.
---
 guix/store.scm                  |  7 ++++++-
 nix/libstore/build.cc           | 21 +++++++++++++++++++++
 nix/libstore/globals.hh         |  6 ++++++
 nix/libstore/worker-protocol.hh |  2 +-
 nix/nix-daemon/nix-daemon.cc    |  9 +++++++++
 5 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/guix/store.scm b/guix/store.scm
index 43cfda9..2023875 100644
--- a/guix/store.scm
+++ b/guix/store.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016 Ludovic Courtès <address@hidden>
+;;; Copyright © 2016 Hartmut Goebel <address@hidden>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -136,7 +137,7 @@
             direct-store-path
             log-file))
 
-(define %protocol-version #x10f)
+(define %protocol-version #x110)
 
 (define %worker-magic-1 #x6e697863)               ; "nixc"
 (define %worker-magic-2 #x6478696f)               ; "dxio"
@@ -580,6 +581,10 @@ encoding conversion errors."
                            `(("locale" . ,locale))
                            '()))))
         (send (string-pairs pairs))))
+    (when (>= (nix-server-minor-version server) 16)
+          ;; Send uid and gid of evoking user. If either of it is zero (root),
+          ;; send -1 wich means: do not change.
+          (send (integer (or (getuid) -1)) (integer (or (getgid) -1))))
     (let loop ((done? (process-stderr server)))
       (or done? (process-stderr server)))))
 
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index ae78e65..e819135 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2617,6 +2617,27 @@ void DerivationGoal::deleteTmpDir(bool force)
                 format("note: keeping build directory `%2%'")
                 % drvPath % tmpDir);
             chmod(tmpDir.c_str(), 0755);
+            // if clientUid and clientGid are set change the ownership
+            if (settings.clientUid != (uid_t) -1) {
+              // FIXME: Is this the correct way to to it? Or it there a better
+              // way?
+              Strings args;
+              char ids[32];
+              if (settings.clientGid != (gid_t) -1)
+                // both uid and gid are set, change owner and group
+                snprintf(ids, 31, "%lu:%lu",
+                         (unsigned long int) settings.clientUid,
+                         (unsigned long int) settings.clientGid);
+              else
+                // only uid is set, change only the owner
+                snprintf(ids, 31, "%lu",
+                         (unsigned long int) settings.clientUid);
+              args.push_back("-R");
+              args.push_back(ids);
+              args.push_back(tmpDir.c_str());
+              string signature = runProgram("chown", true, args);
+              // FIXME: Is it necessary to free signature
+            }
         }
         else
             deletePath(tmpDir);
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index 8c07e36..dc6a004 100644
--- a/nix/libstore/globals.hh
+++ b/nix/libstore/globals.hh
@@ -70,6 +70,12 @@ struct Settings {
        subgoal of the same goal) fails. */
     bool keepGoing;
 
+    /* User and groud id of the client issuing the buld request.  Used to set
+       the owner and group of the keept temporary directories of failed
+       builds. */
+    uid_t clientUid;
+    gid_t clientGid;
+
     /* Whether, if we cannot realise the known closure corresponding
        to a derivation, we should try to normalise the derivation
        instead. */
diff --git a/nix/libstore/worker-protocol.hh b/nix/libstore/worker-protocol.hh
index 7b7be4a..99c1ee2 100644
--- a/nix/libstore/worker-protocol.hh
+++ b/nix/libstore/worker-protocol.hh
@@ -6,7 +6,7 @@ namespace nix {
 #define WORKER_MAGIC_1 0x6e697863
 #define WORKER_MAGIC_2 0x6478696f
 
-#define PROTOCOL_VERSION 0x10f
+#define PROTOCOL_VERSION 0x110
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index 35c284f..a1fce25 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -571,6 +571,15 @@ static void performOp(bool trusted, unsigned int 
clientVersion,
                     settings.set(trusted ? name : "untrusted-" + name, value);
             }
         }
+        if (GET_PROTOCOL_MINOR(clientVersion) >= 16) {
+          // FIXME: Does readInt always fit into uid_t/gid_t?
+          settings.clientUid = (uid_t) readInt(from);
+          settings.clientGid = (gid_t) readInt(from);
+        } else {
+          settings.clientUid = (uid_t) -1;
+          settings.clientGid = (gid_t) -1;
+        }
+
         settings.update();
         startWork();
         stopWork();
-- 
2.7.4