Why is /gnu/store writable by the guixbuild group?

Why is /gnu/store writable by the guixbuild group?

[Steven Allen]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

All,

While the builders run in containers, it still feels like a really bad
idea. Being able to write to /gnu/store gives one the power to overwrite
any binary. Furthermore, it makes grsecurity's TPE mad :(.

So, why exactly does the guixbuild group need write access to this
directory? I'd think that the guix-daemon would be responsible for
moving finished builds into the store, not the builders themselves.

On a related note, why do all builders use guixbuild as their primary
group. It would be safer to make guixbuild a supplementary group and
give every build user it's own primary group. This way, any group
writable files that the build process happens to create will not be
writable by all build users.


-- 
Steven Allen
((Do Not Email <honeypot@stebalien.com>))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Why is /gnu/store writable by the guixbuild group?

[Thompson, David]

On Fri, Jan 22, 2016 at 9:41 AM, Steven Allen <steven@stebalien.com> wrote:
> All,
>
> While the builders run in containers, it still feels like a really bad
> idea. Being able to write to /gnu/store gives one the power to overwrite
> any binary. Furthermore, it makes grsecurity's TPE mad :(.

On GuixSD, /gnu/store is mounted *read-only* and remounted read/write
for the purposes of the daemon only.  So, for any particular build, a
build user can *only* write to their specific output directories and
nothing else.

Note as well that the items in the store are owned by root and cannot
be touched.  The only user that can trash things is the superuser, if
they so choose.

> So, why exactly does the guixbuild group need write access to this
> directory? I'd think that the guix-daemon would be responsible for
> moving finished builds into the store, not the builders themselves.

Builders write directly to their output directories.  In GNU terms,
this is the directory used for './configure --prefix=/gnu/store/foo'.
I don't see an issue with this.

> On a related note, why do all builders use guixbuild as their primary
> group. It would be safer to make guixbuild a supplementary group and
> give every build user it's own primary group. This way, any group
> writable files that the build process happens to create will not be
> writable by all build users.

In the long term, it would be cool to just use user namespaces instead
of build users, but this would cause issues for a number of Guix users
(and some of our donated build slaves) who do not have a new enough
kernel.  Some day.

- Dave

Re: Why is /gnu/store writable by the guixbuild group?

[Steven Allen]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

On 01-22-16, Thompson, David wrote:
> On GuixSD, /gnu/store is mounted *read-only* and remounted read/write
> for the purposes of the daemon only.  So, for any particular build, a
> build user can *only* write to their specific output directories and
> nothing else.

Got it. Off to fix the Arch package... Unfortunately, I doubt this will
make grsecurity happy (and TPE is a really nice security feature)
because the store *could* be mounted read-write somewhere.

> Note as well that the items in the store are owned by root and cannot
> be touched.  The only user that can trash things is the superuser, if
> they so choose.

FYI, in my Arch install (not GuixSD, as far as I can tell), some of the
files in /gnu/store/ files are owned by the guixbuild group (but not
group writable). I assume these are failed in-progress builds (for some
reason, Guix on Arch keeps on trying to build gcc on my poor laptop even
though I've enabled substitutes but that's another issue...)

> > So, why exactly does the guixbuild group need write access to this
> > directory? I'd think that the guix-daemon would be responsible for
> > moving finished builds into the store, not the builders themselves.
> 
> Builders write directly to their output directories.  In GNU terms,
> this is the directory used for './configure --prefix=/gnu/store/foo'.

Then why does /gnu/store need to be writable by the guixbuild group?  If
the builders can only write to their output directories, e.g.
/gnu/store/foo, /gnu/store shouldn't need to be writable by guixbuild.

> I don't see an issue with this.

There isn't any. I was under the impression that store directories were
named after the hash of the output so I was assuming that the guix
builder was creating them. Now I understand that they are named after
the hash of the inputs which is *really* cool.

My only reservation with this is that directories in /gnu/store may or
may not be "complete" (one could have half-completed builds). However,
given that no build can go from complete to in-progress (builds are
deterministic so there are no rebuilds), this isn't really a problem as
long as programs never assume that all builds in the store are complete.

> > On a related note, why do all builders use guixbuild as their primary
> > group.
> In the long term, it would be cool to just use user namespaces...

In the short term, is there any reason not to give each of these users
its own group?

-- 
Steven Allen
((Do Not Email <honeypot@stebalien.com>))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Why is /gnu/store writable by the guixbuild group?

[Andreas Enge]

On Fri, Jan 22, 2016 at 10:45:17AM -0500, Steven Allen wrote:
> My only reservation with this is that directories in /gnu/store may or
> may not be "complete" (one could have half-completed builds).

I think that the atomic operation that validates a directory in /gnu/store
is registering it in the database. Before that, a build is not complete,
and also not used by the daemon (when creating profiles, deciding whether
a package still needs to be built, and so on).

Andreas

Re: Why is /gnu/store writable by the guixbuild group?

[Ludovic Courtès]

Steven Allen <steven@stebalien.com> skribis:

> On 01-22-16, Thompson, David wrote:
>> On GuixSD, /gnu/store is mounted *read-only* and remounted read/write
>> for the purposes of the daemon only.  So, for any particular build, a
>> build user can *only* write to their specific output directories and
>> nothing else.
>
> Got it. Off to fix the Arch package... Unfortunately, I doubt this will
> make grsecurity happy (and TPE is a really nice security feature)
> because the store *could* be mounted read-write somewhere.

What’s TPE (sorry for asking) and how does it complain exactly?

>> Note as well that the items in the store are owned by root and cannot
>> be touched.  The only user that can trash things is the superuser, if
>> they so choose.
>
> FYI, in my Arch install (not GuixSD, as far as I can tell), some of the
> files in /gnu/store/ files are owned by the guixbuild group (but not
> group writable). I assume these are failed in-progress builds (for some
> reason,

Exactly.

> Guix on Arch keeps on trying to build gcc on my poor laptop even
> though I've enabled substitutes but that's another issue...)

That shouldn’t happen, unless you’re using an old version of Guix for
which substitutes are no longer available at hydra.gnu.org.

>> > So, why exactly does the guixbuild group need write access to this
>> > directory? I'd think that the guix-daemon would be responsible for
>> > moving finished builds into the store, not the builders themselves.
>> 
>> Builders write directly to their output directories.  In GNU terms,
>> this is the directory used for './configure --prefix=/gnu/store/foo'.
>
> Then why does /gnu/store need to be writable by the guixbuild group?  If
> the builders can only write to their output directories, e.g.
> /gnu/store/foo, /gnu/store shouldn't need to be writable by guixbuild.

That’s because initially build processes write to their chroot, but when
the build completes, the build process moves the outputs (the results)
back to the store.  See nix/libstore/build.cc in ‘registerOutputs’:

--8<---------------cut here---------------start------------->8---
      if (pathExists(actualPath)) {
          /* Move output paths from the chroot to the Nix store. */
          if (buildMode == bmRepair)
              replaceValidPath(path, actualPath);
          else
              if (buildMode != bmCheck && rename(actualPath.c_str(), path.c_str()) == -1)
                  throw SysError(format("moving build output `%1%' from the chroot to the Nix store") % path);
      }
--8<---------------cut here---------------end--------------->8---

If you look at ‘strace -f -p $(pidof guix-daemon)’ while running ‘guix
build grue-hunter’, the above lines of code translate to:

--8<---------------cut here---------------start------------->8---
7519  --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=7544, si_status=0, si_utime=0, si_stime=0} ---
7519  lstat("/gnu/store/660hdld3sc7laz8kw871pd3yyg9khs5m-grue-hunter-1.0.drv.chroot/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
7519  rename("/gnu/store/660hdld3sc7laz8kw871pd3yyg9khs5m-grue-hunter-1.0.drv.chroot/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0", "/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0") = 0
--8<---------------cut here---------------end--------------->8---

> My only reservation with this is that directories in /gnu/store may or
> may not be "complete" (one could have half-completed builds). However,
> given that no build can go from complete to in-progress (builds are
> deterministic so there are no rebuilds), this isn't really a problem as
> long as programs never assume that all builds in the store are complete.

As Andreas said, there’s a database (/var/guix/db/db.sqlite by default)
that contains a table listing valid store entries, among other things.

>> > On a related note, why do all builders use guixbuild as their primary
>> > group.
>> In the long term, it would be cool to just use user namespaces...
>
> In the short term, is there any reason not to give each of these users
> its own group?

Would it make a difference?

I should point out that this part (the daemon) is inherited from the Nix
project, which was started long ago, and notably long before user name
spaces came into existence.

HTH,
Ludo’.

Re: Why is /gnu/store writable by the guixbuild group?

[Steven Allen]

[-- Attachment #1: Type: text/plain, Size: 3205 bytes --]

On 01-22-16, Ludovic Courtès wrote:
> What’s TPE (sorry for asking) and how does it complain exactly?

Nevermind. This is a false positive and I've reported it to the
grsecurity people (although they may not fix it...).

FYI...

TPE is Trusted Path Execution. Basically, it means that unprivileged
users can only execute files that are not writable, or in directories
writable, by users other than the current user or root. This is to help
make it harder to trick users into executing files written by a
malicious user.

However, after thinking about it, I this case is a false positive
because:

1. The /gnu/store/xxx/ directories and all files under them are not
   group writable and are owned by root.
2. /gnu/store has the sticky bit set.

This means that any files in /gnu/store that are owned by root must have
"blessed" by root (either linked-in or chowned by root). Therefore, the
"no group/other writable parent directory" constraint is unnecessary.

> > Guix on Arch keeps on trying to build gcc on my poor laptop even
> > though I've enabled substitutes but that's another issue...)
> 
> That shouldn’t happen, unless you’re using an old version of Guix for
> which substitutes are no longer available at hydra.gnu.org.

I'm using guix from git and I'll look into it. In my build logs, it
appears that tar is complaining about an invalid flat ("--sort=name") so
I think guix is having trouble extracting the substitutes.

> That’s because initially build processes write to their chroot, but when
> the build completes, the build process moves the outputs (the results)
> back to the store. 
...
> If you look at ‘strace -f -p $(pidof guix-daemon)’ while running ‘guix
> build grue-hunter’, the above lines of code translate to:
> 
> --8<---------------cut here---------------start------------->8---
> 7519  --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=7544, si_status=0, si_utime=0, si_stime=0} ---
> 7519  lstat("/gnu/store/660hdld3sc7laz8kw871pd3yyg9khs5m-grue-hunter-1.0.drv.chroot/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
> 7519  rename("/gnu/store/660hdld3sc7laz8kw871pd3yyg9khs5m-grue-hunter-1.0.drv.chroot/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0", "/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0") = 0
> --8<---------------cut here---------------end--------------->8---

I just did a local experiment (running pstree alongside strace) and,
unless I'm mistaken, 7519 is running as root.

> >> > On a related note, why do all builders use guixbuild as their primary
> >> > group.
> >> In the long term, it would be cool to just use user namespaces...
> >
> > In the short term, is there any reason not to give each of these users
> > its own group?
> 
> Would it make a difference?

No. I was concerned that some files might end up group writable but the
ro bind mount of /gnu/store means that even world writable files are
read only (and nix will make sure that no files in the final build are
writable by users other than root.

-- 
Steven Allen
((Do Not Email <honeypot@stebalien.com>))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Why is /gnu/store writable by the guixbuild group?

[Ludovic Courtès]

Steven Allen <steven@stebalien.com> skribis:

> On 01-22-16, Ludovic Courtès wrote:
>> What’s TPE (sorry for asking) and how does it complain exactly?
>
> Nevermind. This is a false positive and I've reported it to the
> grsecurity people (although they may not fix it...).
>
> FYI...
>
> TPE is Trusted Path Execution. Basically, it means that unprivileged
> users can only execute files that are not writable, or in directories
> writable, by users other than the current user or root. This is to help
> make it harder to trick users into executing files written by a
> malicious user.

OK.

> However, after thinking about it, I this case is a false positive
> because:
>
> 1. The /gnu/store/xxx/ directories and all files under them are not
>    group writable and are owned by root.
> 2. /gnu/store has the sticky bit set.
>
> This means that any files in /gnu/store that are owned by root must have
> "blessed" by root (either linked-in or chowned by root). Therefore, the
> "no group/other writable parent directory" constraint is unnecessary.

Indeed.  The mental model is that /gnu/store is populated by root, on
behalf of other users.  The build users act as a poor way to enforce the
principle of least authority.

>> > Guix on Arch keeps on trying to build gcc on my poor laptop even
>> > though I've enabled substitutes but that's another issue...)
>> 
>> That shouldn’t happen, unless you’re using an old version of Guix for
>> which substitutes are no longer available at hydra.gnu.org.
>
> I'm using guix from git and I'll look into it. In my build logs, it
> appears that tar is complaining about an invalid flat ("--sort=name") so
> I think guix is having trouble extracting the substitutes.

I don’t think this is related.

>> That’s because initially build processes write to their chroot, but when
>> the build completes, the build process moves the outputs (the results)
>> back to the store. 
> ...
>> If you look at ‘strace -f -p $(pidof guix-daemon)’ while running ‘guix
>> build grue-hunter’, the above lines of code translate to:
>> 
>> --8<---------------cut here---------------start------------->8---
>> 7519  --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=7544, si_status=0, si_utime=0, si_stime=0} ---
>> 7519  lstat("/gnu/store/660hdld3sc7laz8kw871pd3yyg9khs5m-grue-hunter-1.0.drv.chroot/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>> 7519  rename("/gnu/store/660hdld3sc7laz8kw871pd3yyg9khs5m-grue-hunter-1.0.drv.chroot/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0", "/gnu/store/h6sdfqzv4xbydwiafiqvrw0d5505l1l8-grue-hunter-1.0") = 0
>> --8<---------------cut here---------------end--------------->8---
>
> I just did a local experiment (running pstree alongside strace) and,
> unless I'm mistaken, 7519 is running as root.

Hmm, I think you’re right.  In that case the thing does not need to be
world-writable, we should try and check!

Thanks,
Ludo’.