Handle hard links to other files in the output

Also, change the file mode before changing the owner.  This prevents a
slight time window in which a setuid binary would be setuid root.

doc/dev/release-procedures.txt

@@ -1,33 +0,0 @@
-To produce a `stable' release from the trunk:
-
--1. Update the release notes; make sure that the release date is
-    correct.
-
-0. Make sure that the trunk builds in the release supervisor.
-
-1. Branch the trunk, e.g., `svn cp .../trunk
-   .../branches/0.5-release'.
-
-2. Switch to the branch, e.g., `svn switch .../branches/0.5-release'.
-
-3. In `configure.ac', change `STABLE=0' into `STABLE=1' and commit.
-
-4. In the release supervisor, add a one-time job to build
-   `.../branches/0.5-release'.
-
-5. Make sure that the release succeeds.
-
-6. Move the branch to a tag, e.g., `svn mv .../branches/0.5-release
-   .../tags/0.5'.
-
-   Note that the branch should not be used for maintenance; it should
-   be deleted after the release has been created.  A maintenance
-   branch (e.g., `.../branches/0.5') should be created from the
-   original revision of the trunk (since maintenance releases should
-   also be tested first; hence, we cannot have `STABLE=1').  The same
-   procedure can then be followed to produce maintenance releases;
-   just substitute `.../branches/VERSION' for the trunk.
-
-7. Switch back to the trunk.
-
-8. Bump the version number in `configure.ac' (in AC_INIT).

doc/manual/release-notes.xml

@@ -6,6 +6,16 @@
 
 
 
+<!--==================================================================-->
+
+<section xml:id="ssec-relnotes-1.5"><title>Release 1.5 (February 27, 2013)</title>
+
+<para>This is a brown paper bag release to fix a regression introduced
+by the hard link security fix in 1.4.</para>
+
+</section>
+
+
 <!--==================================================================-->
 
 <section xml:id="ssec-relnotes-1.4"><title>Release 1.4 (February 26, 2013)</title>

src/libstore/local-store.cc

@@ -465,39 +465,8 @@ void LocalStore::makeStoreWritable()
 const time_t mtimeStore = 1; /* 1 second into the epoch */
 
 
-void canonicalisePathMetaData(const Path & path, bool recurse, uid_t fromUid)
+static void canonicaliseTimestampAndPermissions(const Path & path, const struct stat & st)
 {
-    checkInterrupt();
-
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError(format("getting attributes of path `%1%'") % path);
-
-    /* Really make sure that the path is of a supported type.  This
-       has already been checked in dumpPath(). */
-    assert(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode));
-
-    if (fromUid != (uid_t) -1 && st.st_uid != fromUid)
-        throw BuildError(format("invalid ownership on file `%1%'") % path);
-
-    /* Change ownership to the current uid.  If it's a symlink, use
-       lchown if available, otherwise don't bother.  Wrong ownership
-       of a symlink doesn't matter, since the owning user can't change
-       the symlink and can't delete it because the directory is not
-       writable.  The only exception is top-level paths in the Nix
-       store (since that directory is group-writable for the Nix build
-       users group); we check for this case below. */
-    if (st.st_uid != geteuid()) {
-#if HAVE_LCHOWN
-        if (lchown(path.c_str(), geteuid(), (gid_t) -1) == -1)
-#else
-        if (!S_ISLNK(st.st_mode) &&
-            chown(path.c_str(), geteuid(), (gid_t) -1) == -1)
-#endif
-            throw SysError(format("changing owner of `%1%' to %2%")
-                % path % geteuid());
-    }
-
     if (!S_ISLNK(st.st_mode)) {
 
         /* Mask out all type related bits. */
@@ -528,18 +497,84 @@ void canonicalisePathMetaData(const Path & path, bool recurse, uid_t fromUid)
 #endif
             throw SysError(format("changing modification time of `%1%'") % path);
     }
+}
 
-    if (recurse && S_ISDIR(st.st_mode)) {
+
+void canonicaliseTimestampAndPermissions(const Path & path)
+{
+    struct stat st;
+    if (lstat(path.c_str(), &st))
+        throw SysError(format("getting attributes of path `%1%'") % path);
+    canonicaliseTimestampAndPermissions(path, st);
+}
+
+
+typedef std::pair<dev_t, ino_t> Inode;
+typedef set<Inode> InodesSeen;
+
+
+static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSeen & inodesSeen)
+{
+    checkInterrupt();
+
+    struct stat st;
+    if (lstat(path.c_str(), &st))
+        throw SysError(format("getting attributes of path `%1%'") % path);
+
+    /* Really make sure that the path is of a supported type.  This
+       has already been checked in dumpPath(). */
+    assert(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode));
+
+    /* Fail if the file is not owned by the build user.  This prevents
+       us from messing up the ownership/permissions of files
+       hard-linked into the output (e.g. "ln /etc/shadow $out/foo").
+       However, ignore files that we chown'ed ourselves previously to
+       ensure that we don't fail on hard links within the same build
+       (i.e. "touch $out/foo; ln $out/foo $out/bar"). */
+    if (fromUid != (uid_t) -1 && st.st_uid != fromUid) {
+        assert(!S_ISDIR(st.st_mode));
+        if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end())
+            throw BuildError(format("invalid ownership on file `%1%'") % path);
+        mode_t mode = st.st_mode & ~S_IFMT;
+        assert(st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && st.st_mtime == mtimeStore);
+        return;
+    }
+
+    inodesSeen.insert(Inode(st.st_dev, st.st_ino));
+
+    canonicaliseTimestampAndPermissions(path, st);
+
+    /* Change ownership to the current uid.  If it's a symlink, use
+       lchown if available, otherwise don't bother.  Wrong ownership
+       of a symlink doesn't matter, since the owning user can't change
+       the symlink and can't delete it because the directory is not
+       writable.  The only exception is top-level paths in the Nix
+       store (since that directory is group-writable for the Nix build
+       users group); we check for this case below. */
+    if (st.st_uid != geteuid()) {
+#if HAVE_LCHOWN
+        if (lchown(path.c_str(), geteuid(), (gid_t) -1) == -1)
+#else
+        if (!S_ISLNK(st.st_mode) &&
+            chown(path.c_str(), geteuid(), (gid_t) -1) == -1)
+#endif
+            throw SysError(format("changing owner of `%1%' to %2%")
+                % path % geteuid());
+    }
+
+    if (S_ISDIR(st.st_mode)) {
         Strings names = readDirectory(path);
         foreach (Strings::iterator, i, names)
-            canonicalisePathMetaData(path + "/" + *i, true, fromUid);
+            canonicalisePathMetaData_(path + "/" + *i, fromUid, inodesSeen);
     }
 }
 
 
 void canonicalisePathMetaData(const Path & path, uid_t fromUid)
 {
-    canonicalisePathMetaData(path, true, fromUid);
+    InodesSeen inodesSeen;
+
+    canonicalisePathMetaData_(path, fromUid, inodesSeen);
 
     /* On platforms that don't have lchown(), the top-level path can't
        be a symlink, since we can't change its ownership. */

src/libstore/local-store.hh

@@ -309,7 +309,7 @@ private:
      in a setuid Nix installation. */
 void canonicalisePathMetaData(const Path & path, uid_t fromUid);
 
-void canonicalisePathMetaData(const Path & path, bool recurse, uid_t fromUid);
+void canonicaliseTimestampAndPermissions(const Path & path);
 
 MakeError(PathInUse, Error);
 

src/libstore/optimise-store.cc

@@ -32,7 +32,7 @@ struct MakeReadOnly
     {
         try {
             /* This will make the path read-only. */
-            if (path != "") canonicalisePathMetaData(path, false, -1);
+            if (path != "") canonicaliseTimestampAndPermissions(path);
         } catch (...) {
             ignoreException();
         }

version

@@ -1 +1 @@
-1.4
+1.5