GNU bug report logs - #75810
[PATCH 0/6] Rootless guix-daemon

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Christopher Baines <guix <at> cbaines.net>, 75810 <at> debbugs.gnu.org
Subject: [bug#75810] [PATCH v4 06/14] daemon: Allow running as non-root with unprivileged user namespaces.
Date: Mon, 03 Mar 2025 18:24:39 +0100

Hello,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> Similarly to what Simon pointed in their comments, I'd drop time-related
> 'recently' wording, as it won't age well, and is already made obvious by
> the above being mentioned as the 'historical' approach.

Alright, noted for the next revision.

>> -        if (chown(chrootRootDir.c_str(), 0, buildUser.getGID()) == -1)
>> +        if (buildUser.enabled() && chown(chrootRootDir.c_str(), 0, buildUser.getGID()) == -1)

> I think adding the new check for buildUser.enabled() in the above ifs
> should be split into a distinct commit since it's not relevant to this
> specific new feature.

It’s in fact related: previously you could not run guix-daemon with
useChroot == true unless running as root, and buildUser.enabled() was
implied in this case.

With this change, you can end up in the “if (useChroot)” block without
running as root, which is why this distinction needs to be made.

>>  #if CHROOT_ENABLED
>>          if (useChroot) {
>> -            /* Initialise the loopback interface. */
>> -            AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP));
>> -            if (fd == -1) throw SysError("cannot open IP socket");
>> +	    if (!fixedOutput) {
>> +		/* Initialise the loopback interface. */
>> +		AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP));
>> +		if (fd == -1) throw SysError("cannot open IP socket");
>>  
>> -            struct ifreq ifr;
>> -            strcpy(ifr.ifr_name, "lo");
>> -            ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING;
>> -            if (ioctl(fd, SIOCSIFFLAGS, &ifr) == -1)
>> -                throw SysError("cannot set loopback interface flags");
>> +		struct ifreq ifr;
>> +		strcpy(ifr.ifr_name, "lo");
>> +		ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING;
>> +		if (ioctl(fd, SIOCSIFFLAGS, &ifr) == -1)
>> +		    throw SysError("cannot set loopback interface flags");
>>  
>> -            fd.close();
>> +		fd.close();
>> +	    }
>
> That hunk above is also orthogonal to this feature AFAICS, should be
> split into a different commit to keep its diff focused.

It’s also related: setting up ‘lo’ would always work before, because we
were running as root, but now it only works when running in a separate
net namespace.

Thanks for your feedback!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.