Bryan Steele <brynet@gmail.com> writes:

> On Tue, Feb 08, 2022 at 12:00:05AM -0500, Bryan Steele wrote:
>> On Sun, Feb 06, 2022 at 04:59:05PM +0100, Omar Polo wrote:
>> > Thomas Adam <thomas@xteddy.org> writes:
>> > 
>> > > On Sun, Feb 06, 2022 at 12:06:27PM +0100, Omar Polo wrote:
>> > >> here's a revised diff.  it's equivalent in practice to the previous one,
>> > >> but hopefully less scary :)
>> > >
>> > > Thanks for this.  I have no means of testing this though (the kernel version I
>> > > have here on Arch Linux doesn't seem to offer Landlock) but I have a few
>> > > comment in-line below.
>> > 
>> > unfortunately it doesn't seem to be that much available yet.  I can
>> > confirm that fedora has it, and I'm told nixos ship with landlock
>> > enabled too.
>> > 
>> > I just tried on a devuan ceres (unstable) vm and they ship
>> > linux/landlock.h, but the actual kernel has landlock disabled!  I've
>> > added a check for ENOSYS and ENOTSUP to handle this situation where
>> > landlock was found at compile time but it's not present at runtime.
>> 
>> I'd agrue that in the ENOTSUP case it shouldn't fail silently and
>> should instead return an error, that way distributions attempting
>> to ship packages have a chance of catching it, rather than shipping
>> out binaries that don't actually provide any sort of protection.

in one hand, i wholeheartedly agree.  any failure here should be treated
as fatal error and abort everything, as you noted with the pledge
example.

on the other hand, we're talking about linux here >.>

I've got a report from a user (on a different project) that a program I
wrote was failing for him because of landlock.  The package was built in
an environment with landlock enabled but then ran on a previous linux
version and thus failing.  These kind of monstrosities are probably not
so uncommon I guess.

I don't want got to hard-fail at everything if the user boots with a
previous kernel version, that's the main reason.

Furthermore, it's not that easy to disable a kernel feature, it requires
root access on the machine and probably a reboot too.

Also, see the patch for gtar from the landlock author, they don't seem
to be too much concerned of it failing :D

https://lists.gnu.org/archive/html/bug-tar/2021-04/msg00002.html

what about keeping the status quo for a while still and then enforcing
it properly when more people are running >=5.13 ?

>> At least in that case the distros adding --disable-landlock will be
>> easier spot as well.
>
> I guess that's not actually even a flag, but I guess they might
> try and override cv_header_linux_landlock_h=no during builds.
>
> Still if it's always going to get compiled on Linux, there should be
> some kind of indication it is actually doing something and isn't
> just a no-op.
>
> I know this has already been committed, but just had a chance to
> reply.

An indicator wouldn't be bad, it's difficult to make it not extremely
verbose thought

op@devuan:~/w/got-portable$ got st -S?
got-read-gotconfig: landlock disabled, continuing nevertheless
got-read-gitconfig: landlock disabled, continuing nevertheless
got-read-pack: landlock disabled, continuing nevertheless
got-read-gotconfig: landlock disabled, continuing nevertheless
got-read-gitconfig: landlock disabled, continuing nevertheless
got-read-pack: landlock disabled, continuing nevertheless
M  compat/landlock.c
op@devuan:~/w/got-portable$ got di compat/
got-read-gotconfig: landlock disabled, continuing nevertheless
got-read-gitconfig: landlock disabled, continuing nevertheless
got-read-pack: landlock disabled, continuing nevertheless
got-read-gotconfig: landlock disabled, continuing nevertheless
got-read-gitconfig: landlock disabled, continuing nevertheless
got-read-pack: landlock disabled, continuing nevertheless
diff 97799ccd4b67a81f97039305d4fdd66588da9962 /home/op/w/got-portable
blob - 47a5209dbfe20f149a142f16faaca86ca659120c
file + compat/landlock.c
--- compat/landlock.c
+++ compat/landlock.c
@@ -86,8 +86,10 @@ landlock_no_fs(void)
        fd = landlock_create_ruleset(&rattr, sizeof(rattr), 0);
        if (fd == -1) {
                /* this kernel doesn't have landlock built in */
-               if (errno == ENOSYS || errno == ENOTSUP)
+               if (errno == ENOSYS || errno == ENOTSUP) {
+                       warnx("landlock disabled, continuing nevertheless");
                        return 0;
+               }
                return -1;
        }