On Sat, Jul 15, 2023 at 03:28:42PM -0400, Bryan Steele wrote:
> On Sat, Jul 15, 2023 at 04:19:17PM +0200, Omar Polo wrote:
> > landlock ABI v2 introduced LANDLOCK_ACCESS_FS_REFER to link or rename
> > a file from or to a different directory.  Not a big deal, especially
> > since this access right seems to be the only one denied by default and
> > that needs to be explicitly listed when adding a rule.
> > 
> > landlock ABI v3 however introduced LANDLOCK_ACCESS_FS_TRUNCATE for
> > truncate(2).  It does not require write permissions, from the
> > documentation:
> > 
> > > It should also be noted that truncating files does not require the
> > > LANDLOCK_ACCESS_FS_WRITE_FILE right. Apart from the truncate(2)
> > > system call, this can also be done through open(2) with the flags
> > > O_RDONLY | O_TRUNC.
> > 
> > so it seems that we have to catch up with every landlock ABI update
> > otherwise our libexecs are silently allowed to do more things after a
> > kernel update.  Extra annoyances points for having to copy the magic
> > numbers.
> > 
> > briefly tested on devuan that has landlock v3 ABI.
> > 
> > ok?
> > 
> > -----------------------------------------------
> > commit 50e298d16389ca1854169f765a1ea4362521c056 (portable)
> > from: Omar Polo <op@omarpolo.com>
> > date: Sat Jul 15 13:57:18 2023 UTC
> >  
> >  support landlock ABI v2 and v3
> >  
> >  Add the right #ifdef for backward-compatibility and block REFER and
> >  TRUNCATE as well, otherwise they're silently and implicitly allowed.
> 
> Definitely saw this coming.. :\
> 
> If this is for backwards-compat, is there any reason to not just #define
> these to 0? Do older Linux kernels handle having "future" bits set?

Nevermind.. sigh..

> >  The funny part is that LANDLOCK_ACCESS_FS_TRUNCATE doesn't need
> >  write permissions.
> >  
> > diff 3530f6ee16aaaddef48c026e2dab926bd7f2ef36 50e298d16389ca1854169f765a1ea4362521c056
> > commit - 3530f6ee16aaaddef48c026e2dab926bd7f2ef36
> > commit + 50e298d16389ca1854169f765a1ea4362521c056
> > blob - b6c32dad6cdbc3675520fd2632841f96ab8be1c7
> > blob + 3a9158dd54f0f2e7eb4a9e9ec82f9927159eb553
> > --- compat/landlock.c
> > +++ compat/landlock.c
> > @@ -58,6 +58,19 @@ landlock_restrict_self(int ruleset_fd, __u32 flags)
> >  #endif
> >  
> >  /*
> > + * Maybe we should ship with a full copy of the linux headers because
> > + * you never know...
> > + */
> > +
> > +#ifndef LANDLOCK_ACCESS_FS_REFER
> > +#define LANDLOCK_ACCESS_FS_REFER	(1ULL << 13)
> > +#endif
> > +
> > +#ifndef LANDLOCK_ACCESS_FS_TRUNCATE
> > +#define LANDLOCK_ACCESS_FS_TRUNCATE	(1ULL << 14)
> > +#endif
> > +
> > +/*
> >   * Revoke any fs access.
> >   */
> >  int
> > @@ -81,21 +94,31 @@ landlock_no_fs(void)
> >  				     LANDLOCK_ACCESS_FS_MAKE_SOCK |
> >  				     LANDLOCK_ACCESS_FS_MAKE_FIFO |
> >  				     LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> > -				     LANDLOCK_ACCESS_FS_MAKE_SYM,
> > +				     LANDLOCK_ACCESS_FS_MAKE_SYM |
> > +				     LANDLOCK_ACCESS_FS_REFER |
> > +				     LANDLOCK_ACCESS_FS_TRUNCATE,
> >  	};
> > -	int fd, saved_errno;
> > +	int fd, abi, saved_errno;
> >  
> >  	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1)
> >  		return -1;
> >  
> > -	fd = landlock_create_ruleset(&rattr, sizeof(rattr), 0);
> > -	if (fd == -1) {
> > +	abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
> > +	if (abi == -1) {
> >  		/* this kernel doesn't have landlock built in */
> >  		if (errno == ENOSYS || errno == EOPNOTSUPP)
> >  			return 0;
> >  		return -1;
> >  	}
> > +	if (abi < 2)
> > +		rattr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
> > +	if (abi < 3)
> > +		rattr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
> >  
> > +	fd = landlock_create_ruleset(&rattr, sizeof(rattr), 0);
> > +	if (fd == -1)
> > +		return -1;
> > +
> >  	if (landlock_restrict_self(fd, 0)) {
> >  		saved_errno = errno;
> >  		close(fd);
> > 
> >