Subject: Re: [RFC PATCH v1 5/7] samples/landlock: Add sandboxer UDP access control

From: Mickaël Salaün <[email-masked]>
Date: Fri, 4 Oct 2024 17:04:23 +0200

On Mon, Sep 16, 2024 at 02:22:28PM +0200, Matthieu Buffet wrote:
> Add environment variables to control associated access rights:
> (each one takes a list of ports separated by colons, like other
> list options)
> 
> - LL_UDP_BIND
> - LL_UDP_CONNECT
> - LL_UDP_RECVMSG
> - LL_UDP_SENDMSG
> 
> Signed-off-by: Matthieu Buffet <[email-masked]>
> ---
>  samples/landlock/sandboxer.c | 88 ++++++++++++++++++++++++++++++++----
>  1 file changed, 80 insertions(+), 8 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index 08704504dc51..dadd30dad712 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -55,6 +55,10 @@ static inline int landlock_restrict_self(const int ruleset_fd,
>  #define ENV_FS_RW_NAME "LL_FS_RW"
>  #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
>  #define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
> +#define ENV_UDP_BIND_NAME "LL_UDP_BIND"
> +#define ENV_UDP_CONNECT_NAME "LL_UDP_CONNECT"
> +#define ENV_UDP_RECVMSG_NAME "LL_UDP_RECVMSG"
> +#define ENV_UDP_SENDMSG_NAME "LL_UDP_SENDMSG"
>  #define ENV_DELIMITER ":"
>  
>  static int parse_path(char *env_path, const char ***const path_list)
> @@ -219,7 +223,7 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
>  
>  /* clang-format on */
>  
> -#define LANDLOCK_ABI_LAST 5
> +#define LANDLOCK_ABI_LAST 6
>  
>  static void print_help(const char *prog)
>  {
> @@ -247,11 +251,25 @@ static void print_help(const char *prog)
>  		"to allow nothing, e.g. %s=\"\"):\n",
>  		ENV_TCP_BIND_NAME);
>  	fprintf(stderr,
> -		"* %s: list of ports allowed to bind (server).\n",
> +		"* %s: list of TCP ports allowed to bind (server)\n",
>  		ENV_TCP_BIND_NAME);
>  	fprintf(stderr,
> -		"* %s: list of ports allowed to connect (client).\n",
> +		"* %s: list of TCP ports allowed to connect (client)\n",
>  		ENV_TCP_CONNECT_NAME);
> +	fprintf(stderr,
> +		"* %s: list of UDP ports allowed to bind (client: set as "
> +		"source port/server: listen on port)\n",
> +		ENV_UDP_BIND_NAME);
> +	fprintf(stderr,
> +		"* %s: list of UDP ports allowed to connect (client: set as "
> +		"destination port/server: only receive from one client)\n",
> +		ENV_UDP_CONNECT_NAME);
> +	fprintf(stderr,
> +		"* %s: list of UDP ports allowed to send to (client/server)\n",
> +		ENV_UDP_SENDMSG_NAME);
> +	fprintf(stderr,
> +		"* %s: list of UDP ports allowed to recv from (client/server)\n",
> +		ENV_UDP_RECVMSG_NAME);
>  	fprintf(stderr,
>  		"\n"
>  		"Example:\n"
> @@ -259,9 +277,12 @@ static void print_help(const char *prog)
>  		"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
>  		"%s=\"9418\" "
>  		"%s=\"80:443\" "
> +		"%s=\"0\" "
> +		"%s=\"53\" "
>  		"%s bash -i\n\n",
>  		ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
> -		ENV_TCP_CONNECT_NAME, prog);
> +		ENV_TCP_CONNECT_NAME, ENV_UDP_RECVMSG_NAME,
> +		ENV_UDP_SENDMSG_NAME, prog);
>  	fprintf(stderr,
>  		"This sandboxer can use Landlock features "
>  		"up to ABI version %d.\n",
> @@ -280,7 +301,11 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	struct landlock_ruleset_attr ruleset_attr = {
>  		.handled_access_fs = access_fs_rw,
>  		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> -				      LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +				      LANDLOCK_ACCESS_NET_CONNECT_TCP |
> +				      LANDLOCK_ACCESS_NET_BIND_UDP |
> +				      LANDLOCK_ACCESS_NET_CONNECT_UDP |
> +				      LANDLOCK_ACCESS_NET_RECVMSG_UDP |
> +				      LANDLOCK_ACCESS_NET_SENDMSG_UDP,
>  	};
>  
>  	if (argc < 2) {
> @@ -354,6 +379,14 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  			"provided by ABI version %d (instead of %d).\n",
>  			LANDLOCK_ABI_LAST, abi);
>  		__attribute__((fallthrough));

> +	case 5:
> +		/* Removes UDP support for ABI < 6 */
> +		ruleset_attr.handled_access_net &=
> +			~(LANDLOCK_ACCESS_NET_BIND_UDP |
> +			  LANDLOCK_ACCESS_NET_CONNECT_UDP |
> +			  LANDLOCK_ACCESS_NET_RECVMSG_UDP |
> +			  LANDLOCK_ACCESS_NET_SENDMSG_UDP);
> +		__attribute__((fallthrough));

This hunk should go just after the "scoped" field cleanup and before the
hint.  This way the hint is always printed if the current ABI is not the
last (known) one.  This hunk should then start with a fullthrough
attribute.

>  	case LANDLOCK_ABI_LAST:
>  		break;
>  	default:
> @@ -366,18 +399,42 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	access_fs_ro &= ruleset_attr.handled_access_fs;
>  	access_fs_rw &= ruleset_attr.handled_access_fs;
>  
> -	/* Removes bind access attribute if not supported by a user. */
> +	/* Removes TCP bind access attribute if not supported by a user. */

You can send a separate patch with these comment fixes.

>  	env_port_name = getenv(ENV_TCP_BIND_NAME);
>  	if (!env_port_name) {
>  		ruleset_attr.handled_access_net &=
>  			~LANDLOCK_ACCESS_NET_BIND_TCP;
>  	}
> -	/* Removes connect access attribute if not supported by a user. */
> +	/* Removes TCP connect access attribute if not supported by a user. */
>  	env_port_name = getenv(ENV_TCP_CONNECT_NAME);
>  	if (!env_port_name) {
>  		ruleset_attr.handled_access_net &=
>  			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
>  	}
> +	/* Removes UDP bind access attribute if not supported by a user. */
> +	env_port_name = getenv(ENV_UDP_BIND_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_BIND_UDP;
> +	}
> +	/* Removes UDP bind access attribute if not supported by a user. */
> +	env_port_name = getenv(ENV_UDP_CONNECT_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_CONNECT_UDP;
> +	}
> +	/* Removes UDP recv access attribute if not supported by a user. */
> +	env_port_name = getenv(ENV_UDP_RECVMSG_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_RECVMSG_UDP;
> +	}
> +	/* Removes UDP send access attribute if not supported by a user. */
> +	env_port_name = getenv(ENV_UDP_SENDMSG_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_SENDMSG_UDP;
> +	}
>  
>  	ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> @@ -392,7 +449,6 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw)) {
>  		goto err_close_ruleset;
>  	}
> -
>  	if (populate_ruleset_net(ENV_TCP_BIND_NAME, ruleset_fd,
>  				 LANDLOCK_ACCESS_NET_BIND_TCP)) {
>  		goto err_close_ruleset;
> @@ -401,6 +457,22 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  				 LANDLOCK_ACCESS_NET_CONNECT_TCP)) {
>  		goto err_close_ruleset;
>  	}
> +	if (populate_ruleset_net(ENV_UDP_BIND_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_BIND_UDP)) {
> +		goto err_close_ruleset;
> +	}
> +	if (populate_ruleset_net(ENV_UDP_CONNECT_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_CONNECT_UDP)) {
> +		goto err_close_ruleset;
> +	}
> +	if (populate_ruleset_net(ENV_UDP_RECVMSG_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_RECVMSG_UDP)) {
> +		goto err_close_ruleset;
> +	}
> +	if (populate_ruleset_net(ENV_UDP_SENDMSG_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_SENDMSG_UDP)) {
> +		goto err_close_ruleset;
> +	}
>  
>  	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
>  		perror("Failed to restrict privileges");
> -- 
> 2.39.5
> 
>