From bd013b6f987c23c3b99b639ba7cdbc7b694a13f5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 15 Feb 2012 01:31:56 +0100 Subject: [PATCH] On Linux, make the Nix store really read-only by using the immutable bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I was bitten one time too many by Python modifying the Nix store by creating *.pyc files when run as root. On Linux, we can prevent this by setting the immutable bit on files and directories (as in ‘chattr +i’). This isn't supported by all filesystems, so it's not an error if setting the bit fails. The immutable bit is cleared by the garbage collector before deleting a path. The only tricky aspect is in optimiseStore(), since it's forbidden to create hard links to an immutable file. Thus optimiseStore() temporarily clears the immutable bit before creating the link. --- configure.ac | 12 ++++-- src/libstore/local-store.cc | 7 ++++ src/libstore/optimise-store.cc | 26 ++++++++++++- src/libutil/Makefile.am | 4 +- src/libutil/immutable.cc | 67 ++++++++++++++++++++++++++++++++++ src/libutil/immutable.hh | 19 ++++++++++ src/libutil/util.cc | 3 ++ 7 files changed, 130 insertions(+), 8 deletions(-) create mode 100644 src/libutil/immutable.cc create mode 100644 src/libutil/immutable.hh diff --git a/configure.ac b/configure.ac index 2a0a30f030..29fdfdb36f 100644 --- a/configure.ac +++ b/configure.ac @@ -113,8 +113,8 @@ AC_LANG_POP(C++) # Check for chroot support (requires chroot() and bind mounts). AC_CHECK_FUNCS([chroot]) AC_CHECK_FUNCS([unshare]) -AC_CHECK_HEADERS([sched.h], [], [], []) -AC_CHECK_HEADERS([sys/param.h], [], [], []) +AC_CHECK_HEADERS([sched.h]) +AC_CHECK_HEADERS([sys/param.h]) AC_CHECK_HEADERS([sys/mount.h], [], [], [#ifdef HAVE_SYS_PARAM_H # include @@ -124,7 +124,7 @@ AC_CHECK_HEADERS([sys/mount.h], [], [], # Check for . AC_LANG_PUSH(C++) -AC_CHECK_HEADERS([locale], [], [], []) +AC_CHECK_HEADERS([locale]) AC_LANG_POP(C++) @@ -138,9 +138,13 @@ AC_SUBST([bsddiff_compat_include]) AC_CHECK_HEADERS([sys/personality.h]) +# Check for (for immutable file support). +AC_CHECK_HEADERS([linux/fs.h]) + + # Check for tr1/unordered_set. AC_LANG_PUSH(C++) -AC_CHECK_HEADERS([tr1/unordered_set], [], [], []) +AC_CHECK_HEADERS([tr1/unordered_set]) AC_LANG_POP(C++) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index a30839643c..21b1bdceae 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -5,6 +5,7 @@ #include "pathlocks.hh" #include "worker-protocol.hh" #include "derivations.hh" +#include "immutable.hh" #include #include @@ -405,6 +406,10 @@ void canonicalisePathMetaData(const Path & path, bool recurse) 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)); + /* 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 @@ -451,6 +456,8 @@ void canonicalisePathMetaData(const Path & path, bool recurse) foreach (Strings::iterator, i, names) canonicalisePathMetaData(path + "/" + *i, true); } + + makeImmutable(path); } diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index 89be6ac652..2ca98f46dd 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -1,5 +1,6 @@ #include "util.hh" #include "local-store.hh" +#include "immutable.hh" #include #include @@ -19,6 +20,7 @@ static void makeWritable(const Path & path) struct stat st; if (lstat(path.c_str(), &st)) throw SysError(format("getting attributes of path `%1%'") % path); + if (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode)) makeMutable(path); if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1) throw SysError(format("changing writability of `%1%'") % path); } @@ -31,6 +33,8 @@ struct MakeReadOnly ~MakeReadOnly() { try { + /* This will make the path read-only (and restore the + immutable bit on platforms that support it). */ if (path != "") canonicalisePathMetaData(path, false); } catch (...) { ignoreException(); @@ -39,6 +43,14 @@ struct MakeReadOnly }; +struct MakeImmutable +{ + Path path; + MakeImmutable(const Path & path) : path(path) { } + ~MakeImmutable() { makeImmutable(path); } +}; + + static void hashAndLink(bool dryRun, HashToPath & hashToPath, OptimiseStats & stats, const Path & path) { @@ -96,14 +108,24 @@ static void hashAndLink(bool dryRun, HashToPath & hashToPath, /* Make the containing directory writable, but only if it's not the store itself (we don't want or need to - mess with its permissions). */ + mess with its permissions). */ bool mustToggle = !isStorePath(path); if (mustToggle) makeWritable(dirOf(path)); /* When we're done, make the directory read-only again and reset its timestamp back to 0. */ MakeReadOnly makeReadOnly(mustToggle ? dirOf(path) : ""); - + + /* If ‘prevPath’ is immutable, we can't create hard links + to it, so make it mutable first (and make it immutable + again when we're done). We also have to make ‘path’ + mutable, otherwise rename() will fail to delete it. */ + makeMutable(prevPath.first); + MakeImmutable mk1(prevPath.first); + + makeMutable(path); + MakeImmutable mk2(path); + if (link(prevPath.first.c_str(), tempLink.c_str()) == -1) { if (errno == EMLINK) { /* Too many links to the same file (>= 32000 on diff --git a/src/libutil/Makefile.am b/src/libutil/Makefile.am index 98f32633b8..a326060cfe 100644 --- a/src/libutil/Makefile.am +++ b/src/libutil/Makefile.am @@ -1,12 +1,12 @@ pkglib_LTLIBRARIES = libutil.la libutil_la_SOURCES = util.cc hash.cc serialise.cc \ - archive.cc xml-writer.cc + archive.cc xml-writer.cc immutable.cc libutil_la_LIBADD = ../boost/format/libformat.la ${aterm_lib} ${sqlite_lib} pkginclude_HEADERS = util.hh hash.hh serialise.hh \ - archive.hh xml-writer.hh types.hh + archive.hh xml-writer.hh types.hh immutable.hh if !HAVE_OPENSSL libutil_la_SOURCES += \ diff --git a/src/libutil/immutable.cc b/src/libutil/immutable.cc new file mode 100644 index 0000000000..f72f856254 --- /dev/null +++ b/src/libutil/immutable.cc @@ -0,0 +1,67 @@ +#include "config.h" + +#include "immutable.hh" +#include "util.hh" + +#include +#include +#include + +#if HAVE_LINUX_FS_H +#include +#include +#include +#endif + +namespace nix { + + +void changeMutable(const Path & path, bool mut) +{ +#if defined(FS_IOC_SETFLAGS) && defined(FS_IOC_GETFLAGS) && defined(FS_IMMUTABLE_FL) + + /* Don't even try if we're not root. One day we should support + the CAP_LINUX_IMMUTABLE capability. */ + if (getuid() != 0) return; + + /* The O_NOFOLLOW is important to prevent us from changing the + mutable bit on the target of a symlink (which would be a + security hole). */ + AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW); + if (fd == -1) { + if (errno == ELOOP) return; // it's a symlink + throw SysError(format("opening file `%1%'") % path); + } + + unsigned int flags = 0, old; + + /* Silently ignore errors getting/setting the immutable flag so + that we work correctly on filesystems that don't support it. */ + if (ioctl(fd, FS_IOC_GETFLAGS, &flags)) return; + + old = flags; + + if (mut) flags &= ~FS_IMMUTABLE_FL; + else flags |= FS_IMMUTABLE_FL; + + if (old == flags) return; + + if (ioctl(fd, FS_IOC_SETFLAGS, &flags)) return; + +#endif +} + + +void makeImmutable(const Path & path) +{ + changeMutable(path, false); +} + + +void makeMutable(const Path & path) +{ + changeMutable(path, true); +} + + +} diff --git a/src/libutil/immutable.hh b/src/libutil/immutable.hh new file mode 100644 index 0000000000..5a42a46107 --- /dev/null +++ b/src/libutil/immutable.hh @@ -0,0 +1,19 @@ +#ifndef __IMMUTABLE_H +#define __IMMUTABLE_H + +#include + +namespace nix { + +/* Make the given path immutable, i.e., prevent it from being modified + in any way, even by root. This is a no-op on platforms that do not + support this, or if the calling user is not privileged. On Linux, + this is implemented by doing the equivalent of ‘chattr +i path’. */ +void makeImmutable(const Path & path); + +/* Make the given path mutable. */ +void makeMutable(const Path & path); + +} + +#endif /* !__IMMUTABLE_H */ diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 42e5519b48..31322f9c48 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -12,6 +12,7 @@ #include #include "util.hh" +#include "immutable.hh" extern char * * environ; @@ -304,6 +305,8 @@ static void _deletePath(const Path & path, unsigned long long & bytesFreed, struct stat st = lstat(path); + if (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode)) makeMutable(path); + if (!S_ISDIR(st.st_mode) && st.st_nlink == 1) { bytesFreed += st.st_size; blocksFreed += st.st_blocks;