diff mbox series

[bug#36605,v4] daemon: Set ownership of kept build directories to the calling user.

Message ID 20190711202644.32014-1-h.goebel@crazy-compilers.com
State Accepted
Headers show
Series [bug#36605,v4] daemon: Set ownership of kept build directories to the calling user. | expand

Commit Message

Hartmut Goebel July 11, 2019, 8:26 p.m. UTC
Fixes <http://bugs.gnu.org/15890>.

* nix/libstore/globals.hh (Settings) Add clientUid and clientGid.
* nix/nix-daemon/nix-daemon.cc (daemonLoop] Store UID and GID of the
  caller in settings.
* nix/libstore/build.cc (_chown): New function.
  (DerivationGoal::deleteTmpDir): Use it, change ownership of build
  directory if it is kept and the new owner is not root.
---
 nix/libstore/build.cc        | 21 +++++++++++++++++++++
 nix/libstore/globals.hh      |  6 ++++++
 nix/nix-daemon/nix-daemon.cc | 12 ++++++++++++
 3 files changed, 39 insertions(+)

Comments

Hartmut Goebel July 11, 2019, 8:26 p.m. UTC | #1
As discussed, I updated some comments. Additionally I added building teh
documentation (2nd patch).

Hartmut Goebel (2):
  gnu: Add gunicorn and gunicorn-python2.
  gnu: Build documentation for gunicorn and gunicorn-python2.

 gnu/packages/web.scm | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
Hartmut Goebel July 11, 2019, 8:26 p.m. UTC | #2
Enclosed please find two minor enhancments to the ant-build-system and a
small clean-up resulting from these.

Hartmut Goebel (3):
  guix: ant-build-system: put dummy project-name into default build.xml.
  guix: ant-build-system: add empty `tests` target to default build.xml.
  gnu: Remove now useless #:tests? #f from java-packages.

 gnu/packages/java.scm           | 7 ++-----
 guix/build/ant-build-system.scm | 4 +++-
 2 files changed, 5 insertions(+), 6 deletions(-)
Hartmut Goebel July 11, 2019, 8:26 p.m. UTC | #3
Enclosed please find some enhancemnets to the java/ant build-system and some
java packages.

For the changes to the build-systm I'd apprechiate ideas for better code.
There also is room for improovements, e.g. adding both a "test" (unsing junit)
and a "javadoc" target to the default build.xml. (I will noit implement this,
I'm done with Java).

Regarding the packages: Only very few packages have build.xml for ant. For the
others I'm only using a default build.xml to get the jar build. So not tests
nor javadocs. IMO it is important to have the java packages available at all.
After the enhancements described above are implemented, this should be fixed.

Hartmut Goebel (12):
  guix: ant-bulild-sytem: allow specifying the source directory.
  guix: ant-build-system: use abs path as basedir
  guix: Add java-utils.
  gnu: Add java-plexus-utils.
  gnu: Add java-plexus-interpolation.
  gnu: Add java-commons-cli.
  gnu: Add java-commons-codec.
  gnu: Add java-commons-daemon.
  gnu: Add java-commons-io.
  gnu: Add java-commons-lang.
  gnu: Add java-commons-lang3.
  gnu: Add java-commons-bcel.

 Makefile.am                     |   1 +
 doc/guix.texi                   |   3 +-
 gnu/packages/java.scm           | 332 ++++++++++++++++++++++++++++++++++++++++
 guix/build-system/ant.scm       |   4 +
 guix/build/ant-build-system.scm |  10 +-
 guix/build/java-utils.scm       |  52 +++++++
 6 files changed, 396 insertions(+), 6 deletions(-)
 create mode 100644 guix/build/java-utils.scm
diff mbox series

Patch

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 889ee3d..e823001 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2631,6 +2631,21 @@  void DerivationGoal::closeLogFile()
 }
 
 
+static void _chown(const Path & path, uid_t uid, gid_t gid)
+{
+    checkInterrupt();
+
+    if (lchown(path.c_str(), uid, gid) == -1) {
+	throw SysError(format("change owner and group of `%1%'") % path);
+    }
+    struct stat st = lstat(path);
+    if (S_ISDIR(st.st_mode)) {
+        for (auto & i : readDirectory(path))
+            _chown(path + "/" + i.name, uid, gid);
+    }
+}
+
+
 void DerivationGoal::deleteTmpDir(bool force)
 {
     if (tmpDir != "") {
@@ -2639,6 +2654,12 @@  void DerivationGoal::deleteTmpDir(bool force)
                 format("note: keeping build directory `%2%'")
                 % drvPath % tmpDir);
             chmod(tmpDir.c_str(), 0755);
+            // Change the ownership if clientUid is set. Never change the
+            // ownership or the group to "root" for security reasons.
+            if (settings.clientUid != (uid_t) -1 && settings.clientUid != 0) {
+                _chown(tmpDir, settings.clientUid,
+                       settings.clientGid != 0 ? settings.clientGid : -1);
+            }
         }
         else
             deletePath(tmpDir);
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index 8c07e36..7beb1a5 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 build request.  Used to set
+       the owner and group of the kept 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/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index 682f9a2..47b67d5 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -960,6 +960,18 @@  static void daemonLoop()
                     strncpy(argvSaved[1], processName.c_str(), strlen(argvSaved[1]));
                 }
 
+#if defined(SO_PEERCRED)
+                /* Store the client's user and group for this connection. This
+                   has to be done in the forked process since it is per
+                   connection. */
+                settings.clientUid = cred.uid;
+                settings.clientGid = cred.gid;
+#else
+                /* Setting these to -1 means: do not change */
+                settings.clientUid = (uid_t) -1;
+                settings.clientGid = (gid_t) -1;
+#endif
+
                 /* Handle the connection. */
                 from.fd = remote;
                 to.fd = remote;