"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Bryan Steele <brynet@gmail.com>
Subject:
Re: [portable] support for landlock ABI v2 and v3
To:
gameoftrees@openbsd.org
Date:
Sat, 15 Jul 2023 15:28:39 -0400

Download raw body.

Thread
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?

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