Omar Polo <op@omarpolo.com> writes:

> 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.

Please discard everything, I misunderstood.  I thought you were talking
about dropping the check completely, not the ENOTSUP case alone.

I sort of agree then, leaving ENOSYS for older kernels but removing the
ENOTSUP to make sure that distros build the package correctly.

I'll do some testing on different distribution, but I agree on the idea,
thanks!

> 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;
>         }