[RFC PATCH v1 0/7] landlock: Add UDP access control support

[RFC PATCH v1 0/7] landlock: Add UDP access control support

[Matthieu Buffet]

Landlocked processes can freely use UDP sockets. This may allow them to
escape their sandbox if they can reach UDP sockets of other vulnerable
processes on the same host, or allow them to send/receive to/from unwanted
hosts.

This is a first attempt to add access control around UDP usage, based on
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git
Linux 6.11-rc1 (8400291e289e).

The first two commits fix what I interpret as a bug in landlock's sample's
options parsing, in order to allow testing the actual patch contents.
These two are finished afaict and could be merged separately, but are
bundled here to have a working base-commit and allow the actual patch to
get a first round of feedback.

Add two new access rights in the same bind/connect hooks as used for
TCP, with the same semantics.

Also add two new hooks in recvmsg/sendmsg and two additional rights,
because:
- UDP allows processes to send traffic to anyone without any `bind()` nor
  `connect()` by specifying an arbitrary address in `sendmsg()`, so
  simply using existing hooks cannot prevent sending that traffic;
- UDP allows processes to receive traffic on ephemeral ports without any
  `bind()` (e.g. just `sendmsg()` to 127.0.0.1 to get a port assigned, then
  you can `recv()` on that port).

When benchmarking `iperf3 --udp` with and without sendmsg/recvmsg
sandboxing, the difference appears negligible on my laptop, which makes
me think I'm looking at a completely unrelated bottleneck somewhere else.
Advice or tests from someone with non-potato hardware and benchmarking
knowledge would be appreciated.

Selftests updated for UDP, coverage should encompass all non-critical-error
paths.

This is a first kernel patch attempt, any feedback appreciated.

Link: https://github.com/landlock-lsm/linux/issues/10

Matthieu Buffet (7):
  samples/landlock: Fix port parsing in sandboxer
  samples/landlock: Clarify option parsing behaviour
  landlock: Add UDP bind+connect access control
  landlock: Add UDP send+recv access control
  samples/landlock: Add sandboxer UDP access control
  selftests/landlock: Adapt existing tests for UDP
  selftests/landlock: Add UDP sendmsg/recvmsg tests

 include/uapi/linux/landlock.h                |  58 ++-
 samples/landlock/sandboxer.c                 | 181 +++++--
 security/landlock/limits.h                   |   2 +-
 security/landlock/net.c                      | 255 +++++++--
 security/landlock/syscalls.c                 |   2 +-
 tools/testing/selftests/landlock/base_test.c |   2 +-
 tools/testing/selftests/landlock/net_test.c  | 518 +++++++++++++++++--
 7 files changed, 886 insertions(+), 132 deletions(-)

-- 
2.39.5

[RFC PATCH v1 1/7] samples/landlock: Fix port parsing in sandboxer

[Matthieu Buffet]

Unlike LL_FS_RO and LL_FS_RW, LL_TCP_* are currently optional: either
don't specify them and these access rights won't be in handled_accesses,
or specify them and only the values passed are allowed.

If you want to specify that no port can be bind()ed, you would think
(looking at the code quickly) that setting LL_TCP_BIND="" would do it.
Due to a quirk in the parsing logic and the use of atoi() returning 0 with
no error checking for empty strings, you end up allowing bind(0) (which
means bind to any ephemeral port) without realising it. The same occurred
when leaving a trailing/leading colon (e.g. "80:").

To reproduce:
export LL_FS_RO="/" LL_FS_RW="" LL_TCP_BIND=""

---8<----- Before this patch:
./sandboxer strace -e bind nc -n -vvv -l -p 0
Executing the sandboxed command...
bind(3, {sa_family=AF_INET, sin_port=htons(0),
     sin_addr=inet_addr("0.0.0.0")}, 16) = 0
Listening on 0.0.0.0 37629

---8<----- Expected:
./sandboxer strace -e bind nc -n -vvv -l -p 0
Executing the sandboxed command...
bind(3, {sa_family=AF_INET, sin_port=htons(0),
     sin_addr=inet_addr("0.0.0.0")}, 16) = -1 EACCES (Permission denied)
nc: Permission denied

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 samples/landlock/sandboxer.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e8223c3e781a..a84ae3a15482 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -168,7 +168,18 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
 
 	env_port_name_next = env_port_name;
 	while ((strport = strsep(&env_port_name_next, ENV_DELIMITER))) {
-		net_port.port = atoi(strport);
+		char *strport_num_end = NULL;
+
+		if (strcmp(strport, "") == 0)
+			continue;
+
+		errno = 0;
+		net_port.port = strtol(strport, &strport_num_end, 0);
+		if (errno != 0 || strport_num_end == strport) {
+			fprintf(stderr,
+				"Failed to parse port at \"%s\"\n", strport);
+			goto out_free_name;
+		}
 		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
 				      &net_port, 0)) {
 			fprintf(stderr,
-- 
2.39.5

Re: [RFC PATCH v1 1/7] samples/landlock: Fix port parsing in sandboxer

[Mickaël Salaün]

Thanks for these patches, they look really good!  I'll review all of
them soon.

CCing Konstantin and Mikhail who work on network support.

On Mon, Sep 16, 2024 at 02:22:24PM +0200, Matthieu Buffet wrote:
> Unlike LL_FS_RO and LL_FS_RW, LL_TCP_* are currently optional: either
> don't specify them and these access rights won't be in handled_accesses,
> or specify them and only the values passed are allowed.
> 
> If you want to specify that no port can be bind()ed, you would think
> (looking at the code quickly) that setting LL_TCP_BIND="" would do it.
> Due to a quirk in the parsing logic and the use of atoi() returning 0 with
> no error checking for empty strings, you end up allowing bind(0) (which
> means bind to any ephemeral port) without realising it. The same occurred
> when leaving a trailing/leading colon (e.g. "80:").

Well spotted, thanks for this fix! Can you please send a standalone
patch series with this patch and the next one?  I'll merge the fixes
soon and it will shrink the UDP specific series.

> 
> To reproduce:
> export LL_FS_RO="/" LL_FS_RW="" LL_TCP_BIND=""
> 
> ---8<----- Before this patch:
> ./sandboxer strace -e bind nc -n -vvv -l -p 0
> Executing the sandboxed command...
> bind(3, {sa_family=AF_INET, sin_port=htons(0),
>      sin_addr=inet_addr("0.0.0.0")}, 16) = 0
> Listening on 0.0.0.0 37629
> 
> ---8<----- Expected:

When applying this patch, only the following text gets in the commit
message.  I guess that's because of the previous "---".

> ./sandboxer strace -e bind nc -n -vvv -l -p 0
> Executing the sandboxed command...
> bind(3, {sa_family=AF_INET, sin_port=htons(0),
>      sin_addr=inet_addr("0.0.0.0")}, 16) = -1 EACCES (Permission denied)
> nc: Permission denied
> 

You can add this tag for this fix to be backported:

Fixes: 5e990dcef12e ("samples/landlock: Support TCP restrictions")

> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>  samples/landlock/sandboxer.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index e8223c3e781a..a84ae3a15482 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -168,7 +168,18 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
>  
>  	env_port_name_next = env_port_name;
>  	while ((strport = strsep(&env_port_name_next, ENV_DELIMITER))) {
> -		net_port.port = atoi(strport);
> +		char *strport_num_end = NULL;
> +
> +		if (strcmp(strport, "") == 0)
> +			continue;
> +
> +		errno = 0;
> +		net_port.port = strtol(strport, &strport_num_end, 0);

Using strtol(3) is a good idea, for instance to check overflows.  You
can talk about that in the commit message.

> +		if (errno != 0 || strport_num_end == strport) {

I was thinking about checking the return value instead of errno, but it
looks like the strtol() API may set errno while returning an unspecified
value, so your approach looks good.

> +			fprintf(stderr,
> +				"Failed to parse port at \"%s\"\n", strport);
> +			goto out_free_name;
> +		}
>  		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
>  				      &net_port, 0)) {
>  			fprintf(stderr,
> -- 
> 2.39.5
> 
> 

Re: [RFC PATCH v1 1/7] samples/landlock: Fix port parsing in sandboxer

[Mikhail Ivanov]

On 9/16/2024 3:22 PM, Matthieu Buffet wrote:
> Unlike LL_FS_RO and LL_FS_RW, LL_TCP_* are currently optional: either
> don't specify them and these access rights won't be in handled_accesses,
> or specify them and only the values passed are allowed.
> 
> If you want to specify that no port can be bind()ed, you would think
> (looking at the code quickly) that setting LL_TCP_BIND="" would do it.
> Due to a quirk in the parsing logic and the use of atoi() returning 0 with
> no error checking for empty strings, you end up allowing bind(0) (which
> means bind to any ephemeral port) without realising it. The same occurred
> when leaving a trailing/leading colon (e.g. "80:").
> 
> To reproduce:
> export LL_FS_RO="/" LL_FS_RW="" LL_TCP_BIND=""
> 
> ---8<----- Before this patch:
> ./sandboxer strace -e bind nc -n -vvv -l -p 0
> Executing the sandboxed command...
> bind(3, {sa_family=AF_INET, sin_port=htons(0),
>       sin_addr=inet_addr("0.0.0.0")}, 16) = 0
> Listening on 0.0.0.0 37629
> 
> ---8<----- Expected:
> ./sandboxer strace -e bind nc -n -vvv -l -p 0
> Executing the sandboxed command...
> bind(3, {sa_family=AF_INET, sin_port=htons(0),
>       sin_addr=inet_addr("0.0.0.0")}, 16) = -1 EACCES (Permission denied)
> nc: Permission denied
> 
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>   samples/landlock/sandboxer.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index e8223c3e781a..a84ae3a15482 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -168,7 +168,18 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
>   
>   	env_port_name_next = env_port_name;
>   	while ((strport = strsep(&env_port_name_next, ENV_DELIMITER))) {
> -		net_port.port = atoi(strport);
> +		char *strport_num_end = NULL;
> +
> +		if (strcmp(strport, "") == 0)
> +			continue;
> +
> +		errno = 0;
> +		net_port.port = strtol(strport, &strport_num_end, 0);
> +		if (errno != 0 || strport_num_end == strport) {
> +			fprintf(stderr,
> +				"Failed to parse port at \"%s\"\n", strport);
> +			goto out_free_name;
> +		}

Probably it'll be better to make a separate function for strtol
conversion (e.g. [1])? It might be needed for the socket type
control patchset [2].

[1] 
https://lore.kernel.org/all/20240904104824.1844082-18-ivanov.mikhail1@huawei-partners.com/
[2] 
https://lore.kernel.org/all/20240904104824.1844082-19-ivanov.mikhail1@huawei-partners.com/

>   		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
>   				      &net_port, 0)) {
>   			fprintf(stderr,

[RFC PATCH v1 2/7] samples/landlock: Clarify option parsing behaviour

[Matthieu Buffet]

- Clarify which environment variables are optional, which ones are
  mandatory
- Clarify the difference between unset variables and empty ones
- Move the (larger) help message to a helper function

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 samples/landlock/sandboxer.c | 86 ++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 38 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index a84ae3a15482..08704504dc51 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -221,6 +221,53 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
 
 #define LANDLOCK_ABI_LAST 5
 
+static void print_help(const char *prog)
+{
+	fprintf(stderr,
+		"usage: %s=\"...\" %s=\"...\" [other environment variables] %s "
+		"<cmd> [args]...\n\n",
+		ENV_FS_RO_NAME, ENV_FS_RW_NAME, prog);
+	fprintf(stderr,
+		"Execute a command in a restricted environment.\n\n");
+	fprintf(stderr,
+		"Environment variables containing paths and ports "
+		"can be multi-valued, with a colon delimiter.\n"
+		"\n"
+		"Mandatory settings:\n");
+	fprintf(stderr,
+		"* %s: list of paths allowed to be used in a read-only way.\n",
+		ENV_FS_RO_NAME);
+	fprintf(stderr,
+		"* %s: list of paths allowed to be used in a read-write way.\n",
+		ENV_FS_RW_NAME);
+	fprintf(stderr,
+		"\n"
+		"Optional settings (when not set, their associated access "
+		"check is always allowed) (for lists, an empty string means "
+		"to allow nothing, e.g. %s=\"\"):\n",
+		ENV_TCP_BIND_NAME);
+	fprintf(stderr,
+		"* %s: list of ports allowed to bind (server).\n",
+		ENV_TCP_BIND_NAME);
+	fprintf(stderr,
+		"* %s: list of ports allowed to connect (client).\n",
+		ENV_TCP_CONNECT_NAME);
+	fprintf(stderr,
+		"\n"
+		"Example:\n"
+		"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
+		"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
+		"%s=\"9418\" "
+		"%s=\"80:443\" "
+		"%s bash -i\n\n",
+		ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
+		ENV_TCP_CONNECT_NAME, prog);
+	fprintf(stderr,
+		"This sandboxer can use Landlock features "
+		"up to ABI version %d.\n",
+		LANDLOCK_ABI_LAST);
+}
+
 int main(const int argc, char *const argv[], char *const *const envp)
 {
 	const char *cmd_path;
@@ -237,44 +284,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	};
 
 	if (argc < 2) {
-		fprintf(stderr,
-			"usage: %s=\"...\" %s=\"...\" %s=\"...\" %s=\"...\"%s "
-			"<cmd> [args]...\n\n",
-			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
-			ENV_TCP_CONNECT_NAME, argv[0]);
-		fprintf(stderr,
-			"Execute a command in a restricted environment.\n\n");
-		fprintf(stderr,
-			"Environment variables containing paths and ports "
-			"each separated by a colon:\n");
-		fprintf(stderr,
-			"* %s: list of paths allowed to be used in a read-only way.\n",
-			ENV_FS_RO_NAME);
-		fprintf(stderr,
-			"* %s: list of paths allowed to be used in a read-write way.\n\n",
-			ENV_FS_RW_NAME);
-		fprintf(stderr,
-			"Environment variables containing ports are optional "
-			"and could be skipped.\n");
-		fprintf(stderr,
-			"* %s: list of ports allowed to bind (server).\n",
-			ENV_TCP_BIND_NAME);
-		fprintf(stderr,
-			"* %s: list of ports allowed to connect (client).\n",
-			ENV_TCP_CONNECT_NAME);
-		fprintf(stderr,
-			"\nexample:\n"
-			"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
-			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
-			"%s=\"9418\" "
-			"%s=\"80:443\" "
-			"%s bash -i\n\n",
-			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
-			ENV_TCP_CONNECT_NAME, argv[0]);
-		fprintf(stderr,
-			"This sandboxer can use Landlock features "
-			"up to ABI version %d.\n",
-			LANDLOCK_ABI_LAST);
+		print_help(argv[0]);
 		return 1;
 	}
 
-- 
2.39.5

Re: [RFC PATCH v1 2/7] samples/landlock: Clarify option parsing behaviour

[Mickaël Salaün]

On Mon, Sep 16, 2024 at 02:22:25PM +0200, Matthieu Buffet wrote:
> - Clarify which environment variables are optional, which ones are
>   mandatory
> - Clarify the difference between unset variables and empty ones
> - Move the (larger) help message to a helper function
> 
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>  samples/landlock/sandboxer.c | 86 ++++++++++++++++++++----------------
>  1 file changed, 48 insertions(+), 38 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index a84ae3a15482..08704504dc51 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -221,6 +221,53 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
>  
>  #define LANDLOCK_ABI_LAST 5
>  
> +static void print_help(const char *prog)
> +{
> +	fprintf(stderr,
> +		"usage: %s=\"...\" %s=\"...\" [other environment variables] %s "
> +		"<cmd> [args]...\n\n",
> +		ENV_FS_RO_NAME, ENV_FS_RW_NAME, prog);
> +	fprintf(stderr,
> +		"Execute a command in a restricted environment.\n\n");
> +	fprintf(stderr,
> +		"Environment variables containing paths and ports "
> +		"can be multi-valued, with a colon delimiter.\n"
> +		"\n"
> +		"Mandatory settings:\n");
> +	fprintf(stderr,
> +		"* %s: list of paths allowed to be used in a read-only way.\n",
> +		ENV_FS_RO_NAME);
> +	fprintf(stderr,
> +		"* %s: list of paths allowed to be used in a read-write way.\n",
> +		ENV_FS_RW_NAME);
> +	fprintf(stderr,
> +		"\n"
> +		"Optional settings (when not set, their associated access "
> +		"check is always allowed) (for lists, an empty string means "
> +		"to allow nothing, e.g. %s=\"\"):\n",
> +		ENV_TCP_BIND_NAME);
> +	fprintf(stderr,
> +		"* %s: list of ports allowed to bind (server).\n",
> +		ENV_TCP_BIND_NAME);
> +	fprintf(stderr,
> +		"* %s: list of ports allowed to connect (client).\n",
> +		ENV_TCP_CONNECT_NAME);
> +	fprintf(stderr,
> +		"\n"
> +		"Example:\n"
> +		"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
> +		"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
> +		"%s=\"9418\" "
> +		"%s=\"80:443\" "
> +		"%s bash -i\n\n",
> +		ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
> +		ENV_TCP_CONNECT_NAME, prog);
> +	fprintf(stderr,
> +		"This sandboxer can use Landlock features "
> +		"up to ABI version %d.\n",
> +		LANDLOCK_ABI_LAST);
> +}
> +
>  int main(const int argc, char *const argv[], char *const *const envp)
>  {
>  	const char *cmd_path;
> @@ -237,44 +284,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	};
>  
>  	if (argc < 2) {
> -		fprintf(stderr,
> -			"usage: %s=\"...\" %s=\"...\" %s=\"...\" %s=\"...\"%s "
> -			"<cmd> [args]...\n\n",
> -			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
> -			ENV_TCP_CONNECT_NAME, argv[0]);
> -		fprintf(stderr,
> -			"Execute a command in a restricted environment.\n\n");
> -		fprintf(stderr,
> -			"Environment variables containing paths and ports "
> -			"each separated by a colon:\n");
> -		fprintf(stderr,
> -			"* %s: list of paths allowed to be used in a read-only way.\n",
> -			ENV_FS_RO_NAME);
> -		fprintf(stderr,
> -			"* %s: list of paths allowed to be used in a read-write way.\n\n",
> -			ENV_FS_RW_NAME);
> -		fprintf(stderr,
> -			"Environment variables containing ports are optional "
> -			"and could be skipped.\n");
> -		fprintf(stderr,
> -			"* %s: list of ports allowed to bind (server).\n",
> -			ENV_TCP_BIND_NAME);
> -		fprintf(stderr,
> -			"* %s: list of ports allowed to connect (client).\n",
> -			ENV_TCP_CONNECT_NAME);
> -		fprintf(stderr,
> -			"\nexample:\n"
> -			"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
> -			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
> -			"%s=\"9418\" "
> -			"%s=\"80:443\" "
> -			"%s bash -i\n\n",
> -			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
> -			ENV_TCP_CONNECT_NAME, argv[0]);
> -		fprintf(stderr,
> -			"This sandboxer can use Landlock features "
> -			"up to ABI version %d.\n",
> -			LANDLOCK_ABI_LAST);
> +		print_help(argv[0]);

Looks good, please rebase on my "next" branch with the new LL_SCOPED
variable and send it in a new series along with the previous fix:
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=next

>  		return 1;
>  	}
>  
> -- 
> 2.39.5
> 
> 

[RFC PATCH v1 3/7] landlock: Add UDP bind+connect access control

[Matthieu Buffet]

Add support for two more access rights:

- LANDLOCK_ACCESS_NET_CONNECT_UDP, to gate the possibility to connect()
  an inet SOCK_DGRAM socket. This will be used by some client applications
  (those who want to avoid specifying a destination for each datagram in
  sendmsg), and for a few servers (those creating a socket per-client, who
  want to only receive traffic from each client on these sockets)

- LANDLOCK_ACCESS_NET_BIND_UDP, to gate the possibility to bind() an
  inet SOCK_DGRAM socket. This will be required for most server
  applications (to start listening for datagrams on a non-ephemeral
  port) and can be useful for some client applications (to set the
  source port of future datagrams)

Also bump the ABI version from 5 to 6 so that userland can detect
whether these rights are supported and actually use them.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 include/uapi/linux/landlock.h | 48 +++++++++++++++++++++++--------
 security/landlock/limits.h    |  2 +-
 security/landlock/net.c       | 54 ++++++++++++++++++++++++++---------
 security/landlock/syscalls.c  |  2 +-
 4 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 2c8dbc74b955..7f9aa1cd2912 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -113,12 +113,15 @@ struct landlock_net_port_attr {
 	 *
 	 * It should be noted that port 0 passed to :manpage:`bind(2)` will bind
 	 * to an available port from the ephemeral port range.  This can be
-	 * configured with the ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl
-	 * (also used for IPv6).
+	 * configured globally with the
+	 * ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl (also used for
+	 * IPv6), and on a per-socket basis using
+	 * ``setsockopt(IP_LOCAL_PORT_RANGE)``.
 	 *
 	 * A Landlock rule with port 0 and the ``LANDLOCK_ACCESS_NET_BIND_TCP``
-	 * right means that requesting to bind on port 0 is allowed and it will
-	 * automatically translate to binding on the related port range.
+	 * or ``LANDLOCK_ACCESS_NET_BIND_UDP`` right means that requesting to
+	 * bind on port 0 is allowed and it will automatically translate to
+	 * binding on the related port range.
 	 */
 	__u64 port;
 };
@@ -261,17 +264,38 @@ struct landlock_net_port_attr {
  * Network flags
  * ~~~~~~~~~~~~~~~~
  *
- * These flags enable to restrict a sandboxed process to a set of network
- * actions. This is supported since the Landlock ABI version 4.
- *
- * The following access rights apply to TCP port numbers:
- *
- * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
- * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
- *   a remote port.
+ * These flags enable to restrict which network-related actions a sandboxed
+ * process can take. TCP support was added in Landlock ABI version 4, and UDP
+ * support in version 6.
+ *
+ * TCP access rights:
+ * - %LANDLOCK_ACCESS_NET_BIND_TCP: bind sockets to the given local port,
+ *   for servers that will listen on that port
+ * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: connect sockets to the given remote port,
+ *   to establish client connections to servers listening on that port
+ *
+ * UDP access rights:
+ * - %LANDLOCK_ACCESS_NET_BIND_UDP: bind sockets to the given local port,
+ *   either for servers that will listen on that port, or for clients wishing
+ *   to set the source port of datagrams they will send instead of using a
+ *   kernel-assigned random ephemeral port (some protocols require a specific
+ *   source port, e.g. mDNS with UDP/5353)
+ * - %LANDLOCK_ACCESS_NET_CONNECT_UDP: connect sockets to the given remote port,
+ *   either for clients that will send datagrams to that destination (and want
+ *   to send them faster, without specifying an explicit address every time),
+ *   or for servers that want to filter which client address they want to
+ *   receive datagrams from (if you create a client-specific socket for a
+ *   client-specific process, e.g. using the established-over-unconnected
+ *   method)
+ *
+ * Note that ``bind(0)`` means binding to an ephemeral kernel-assigned port,
+ * in the range configured in ``/proc/sys/net/ipv4/ip_local_port_range``
+ * globally (or on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).
  */
 /* clang-format off */
 #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
 #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
+#define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
+#define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
 /* clang-format on */
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 4eb643077a2a..182b6a8d2976 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -22,7 +22,7 @@
 #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
 
-#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_TCP
+#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_UDP
 #define LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
 
diff --git a/security/landlock/net.c b/security/landlock/net.c
index c8bcd29bde09..becc62c02cc9 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -79,8 +79,8 @@ static int current_check_access_socket(struct socket *const sock,
 	if (WARN_ON_ONCE(dom->num_layers < 1))
 		return -EACCES;
 
-	/* Checks if it's a (potential) TCP socket. */
-	if (sock->type != SOCK_STREAM)
+	/* Checks if it's a (potential) UDP or TCP socket. */
+	if (sock->type != SOCK_STREAM && sock->type != SOCK_DGRAM)
 		return 0;
 
 	/* Checks for minimal header length to safely read sa_family. */
@@ -110,17 +110,18 @@ static int current_check_access_socket(struct socket *const sock,
 	/* Specific AF_UNSPEC handling. */
 	if (address->sa_family == AF_UNSPEC) {
 		/*
-		 * Connecting to an address with AF_UNSPEC dissolves the TCP
-		 * association, which have the same effect as closing the
-		 * connection while retaining the socket object (i.e., the file
-		 * descriptor).  As for dropping privileges, closing
-		 * connections is always allowed.
-		 *
-		 * For a TCP access control system, this request is legitimate.
+		 * Connecting to an address with AF_UNSPEC dissolves the socket
+		 * association. For SOCK_STREAM, it has the same effect as closing
+		 * the connection while retaining the socket object (i.e., the
+		 * file descriptor). For SOCK_DGRAM, it removes any configured
+		 * destination address. Both cases remove accessible resources, so
+		 * they are always legitimate and allowed, like dropping any
+		 * privilege.
 		 * Let the network stack handle potential inconsistencies and
 		 * return -EINVAL if needed.
 		 */
-		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP)
+		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
+		    access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP)
 			return 0;
 
 		/*
@@ -134,7 +135,8 @@ static int current_check_access_socket(struct socket *const sock,
 		 * checks, but it is safer to return a proper error and test
 		 * consistency thanks to kselftest.
 		 */
-		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
+		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
+		    access_request == LANDLOCK_ACCESS_NET_BIND_UDP) {
 			/* addrlen has already been checked for AF_UNSPEC. */
 			const struct sockaddr_in *const sockaddr =
 				(struct sockaddr_in *)address;
@@ -175,16 +177,42 @@ static int current_check_access_socket(struct socket *const sock,
 static int hook_socket_bind(struct socket *const sock,
 			    struct sockaddr *const address, const int addrlen)
 {
+	access_mask_t access_request;
+
+	/*
+	 * Check if it's a (potential) TCP or UDP socket. These checks could
+	 * match e.g. Unix, netlink, or udplite sockets.
+	 */
+	if (sock->type == SOCK_STREAM)
+		access_request = LANDLOCK_ACCESS_NET_BIND_TCP;
+	else if (sock->type == SOCK_DGRAM)
+		access_request = LANDLOCK_ACCESS_NET_BIND_UDP;
+	else
+		return 0;
+
 	return current_check_access_socket(sock, address, addrlen,
-					   LANDLOCK_ACCESS_NET_BIND_TCP);
+					   access_request);
 }
 
 static int hook_socket_connect(struct socket *const sock,
 			       struct sockaddr *const address,
 			       const int addrlen)
 {
+	access_mask_t access_request;
+
+	/*
+	 * Check if it's a (potential) TCP or UDP socket. These checks could
+	 * match e.g. Unix, netlink, or udplite sockets.
+	 */
+	if (sock->type == SOCK_STREAM)
+		access_request = LANDLOCK_ACCESS_NET_CONNECT_TCP;
+	else if (sock->type == SOCK_DGRAM)
+		access_request = LANDLOCK_ACCESS_NET_CONNECT_UDP;
+	else
+		return 0;
+
 	return current_check_access_socket(sock, address, addrlen,
-					   LANDLOCK_ACCESS_NET_CONNECT_TCP);
+					   access_request);
 }
 
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index ccc8bc6c1584..328198e8a9f5 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -149,7 +149,7 @@ static const struct file_operations ruleset_fops = {
 	.write = fop_dummy_write,
 };
 
-#define LANDLOCK_ABI_VERSION 5
+#define LANDLOCK_ABI_VERSION 6
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
-- 
2.39.5

Re: [RFC PATCH v1 3/7] landlock: Add UDP bind+connect access control

[Mickaël Salaün]

On Mon, Sep 16, 2024 at 02:22:26PM +0200, Matthieu Buffet wrote:
> Add support for two more access rights:
> 
> - LANDLOCK_ACCESS_NET_CONNECT_UDP, to gate the possibility to connect()
>   an inet SOCK_DGRAM socket. This will be used by some client applications
>   (those who want to avoid specifying a destination for each datagram in
>   sendmsg), and for a few servers (those creating a socket per-client, who
>   want to only receive traffic from each client on these sockets)
> 
> - LANDLOCK_ACCESS_NET_BIND_UDP, to gate the possibility to bind() an
>   inet SOCK_DGRAM socket. This will be required for most server
>   applications (to start listening for datagrams on a non-ephemeral
>   port) and can be useful for some client applications (to set the
>   source port of future datagrams)
> 
> Also bump the ABI version from 5 to 6 so that userland can detect
> whether these rights are supported and actually use them.
> 

Closes: https://github.com/landlock-lsm/linux/issues/10

> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>  include/uapi/linux/landlock.h | 48 +++++++++++++++++++++++--------
>  security/landlock/limits.h    |  2 +-
>  security/landlock/net.c       | 54 ++++++++++++++++++++++++++---------
>  security/landlock/syscalls.c  |  2 +-
>  4 files changed, 79 insertions(+), 27 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 2c8dbc74b955..7f9aa1cd2912 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -113,12 +113,15 @@ struct landlock_net_port_attr {
>  	 *
>  	 * It should be noted that port 0 passed to :manpage:`bind(2)` will bind
>  	 * to an available port from the ephemeral port range.  This can be
> -	 * configured with the ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl
> -	 * (also used for IPv6).
> +	 * configured globally with the
> +	 * ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl (also used for
> +	 * IPv6), and on a per-socket basis using
> +	 * ``setsockopt(IP_LOCAL_PORT_RANGE)``.

Interesting... setsockopt(IP_LOCAL_PORT_RANGE) can always be set
independant of the sysctl, but fortunately it is only taken into account
if it fits into the sysctl range (see inet_sk_get_local_port_range),
which makes sense.

>  	 *
>  	 * A Landlock rule with port 0 and the ``LANDLOCK_ACCESS_NET_BIND_TCP``
> -	 * right means that requesting to bind on port 0 is allowed and it will
> -	 * automatically translate to binding on the related port range.
> +	 * or ``LANDLOCK_ACCESS_NET_BIND_UDP`` right means that requesting to
> +	 * bind on port 0 is allowed and it will automatically translate to
> +	 * binding on the related port range.
>  	 */
>  	__u64 port;
>  };

[RFC PATCH v1 4/7] landlock: Add UDP send+recv access control

[Matthieu Buffet]

Add support for two UDP access rights, complementing the two previous
LANDLOCK_ACCESS_NET_CONNECT_UDP and LANDLOCK_ACCESS_NET_BIND_UDP:

- LANDLOCK_ACCESS_NET_RECVMSG_UDP: to prevent a process from receiving
  datagrams. Just removing LANDLOCK_ACCESS_NET_BIND_UDP is not enough:
  it can just send a first datagram or call connect() and get an
  ephemeral port assigned, without ever calling bind(). This access right
  allows blocking a process from receiving UDP datagrams, without
  preventing them to bind() (which may be required to set source ports);

- LANDLOCK_ACCESS_NET_SENDMSG_UDP: to prevent a process from sending
  datagrams. Just removing LANDLOCK_ACCESS_NET_CONNECT_UDP is not enough:
  the process can call sendmsg() with an unconnected socket and an
  arbitrary destination address.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 include/uapi/linux/landlock.h |  18 ++-
 security/landlock/limits.h    |   2 +-
 security/landlock/net.c       | 205 +++++++++++++++++++++++++++++-----
 3 files changed, 193 insertions(+), 32 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 7f9aa1cd2912..7ea3d1adb8c3 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -287,15 +287,25 @@ struct landlock_net_port_attr {
  *   receive datagrams from (if you create a client-specific socket for a
  *   client-specific process, e.g. using the established-over-unconnected
  *   method)
- *
- * Note that ``bind(0)`` means binding to an ephemeral kernel-assigned port,
- * in the range configured in ``/proc/sys/net/ipv4/ip_local_port_range``
- * globally (or on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).
+ * - %LANDLOCK_ACCESS_NET_RECVMSG_UDP: receive datagrams on the given local port
+ *   (this is a distinct right from %LANDLOCK_ACCESS_NET_BIND_UDP, because you
+ *   may want to allow a process to set its datagrams source port using bind()
+ *   but not be able to receive datagrams)
+ * - %LANDLOCK_ACCESS_NET_SENDMSG_UDP: send datagrams to the given remote port
+ *   (this is a distinct right from %LANDLOCK_ACCESS_NET_CONNECT_UDP, because
+ *   you may want to allow a process to set which client it wants to receive
+ *   datagrams from using connect(), and not be able to send datagrams)
+ *
+ * Note that ``bind(0)`` has special semantics, meaning bind on any port in the
+ * range configured in ``/proc/sys/net/ipv4/ip_local_port_range`` globally (or
+ * on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).
  */
 /* clang-format off */
 #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
 #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
 #define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
 #define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
+#define LANDLOCK_ACCESS_NET_RECVMSG_UDP			(1ULL << 4)
+#define LANDLOCK_ACCESS_NET_SENDMSG_UDP			(1ULL << 5)
 /* clang-format on */
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 182b6a8d2976..e2697348310c 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -22,7 +22,7 @@
 #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
 
-#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_UDP
+#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_SENDMSG_UDP
 #define LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
 
diff --git a/security/landlock/net.c b/security/landlock/net.c
index becc62c02cc9..9a3c44ad3f26 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -10,6 +10,8 @@
 #include <linux/net.h>
 #include <linux/socket.h>
 #include <net/ipv6.h>
+#include <net/transp_v6.h>
+#include <net/ip.h>
 
 #include "common.h"
 #include "cred.h"
@@ -61,6 +63,45 @@ static const struct landlock_ruleset *get_current_net_domain(void)
 	return dom;
 }
 
+static int get_addr_port(const struct sockaddr *address, int addrlen,
+			 bool in_udpv6_sendmsg_ctx, __be16 *port)
+{
+	/* Checks for minimal header length to safely read sa_family. */
+	if (addrlen < offsetofend(typeof(*address), sa_family))
+		return -EINVAL;
+
+	switch (address->sa_family) {
+	case AF_UNSPEC:
+		/*
+		 * Backward compatibility games: AF_UNSPEC is mapped to AF_INET
+		 * by `bind` (v4+v6), `connect` (v4) and `sendmsg` (v4), but
+		 * interpreted as "no address" by `sendmsg` (v6). In that case
+		 * this call must succeed (even if `address` is shorter than a
+		 * `struct sockaddr_in`), and caller must check for this
+		 * condition.
+		 */
+		if (in_udpv6_sendmsg_ctx) {
+			*port = 0;
+			return 0;
+		}
+		fallthrough;
+	case AF_INET:
+		if (addrlen < sizeof(struct sockaddr_in))
+			return -EINVAL;
+		*port = ((struct sockaddr_in *)address)->sin_port;
+		return 0;
+#if IS_ENABLED(CONFIG_IPV6)
+	case AF_INET6:
+		if (addrlen < SIN6_LEN_RFC2133)
+			return -EINVAL;
+		*port = ((struct sockaddr_in6 *)address)->sin6_port;
+		return 0;
+#endif /* IS_ENABLED(CONFIG_IPV6) */
+	}
+
+	return -EAFNOSUPPORT;
+}
+
 static int current_check_access_socket(struct socket *const sock,
 				       struct sockaddr *const address,
 				       const int addrlen,
@@ -73,39 +114,18 @@ static int current_check_access_socket(struct socket *const sock,
 		.type = LANDLOCK_KEY_NET_PORT,
 	};
 	const struct landlock_ruleset *const dom = get_current_net_domain();
+	int err;
 
 	if (!dom)
 		return 0;
 	if (WARN_ON_ONCE(dom->num_layers < 1))
 		return -EACCES;
 
-	/* Checks if it's a (potential) UDP or TCP socket. */
-	if (sock->type != SOCK_STREAM && sock->type != SOCK_DGRAM)
-		return 0;
-
-	/* Checks for minimal header length to safely read sa_family. */
-	if (addrlen < offsetofend(typeof(*address), sa_family))
-		return -EINVAL;
-
-	switch (address->sa_family) {
-	case AF_UNSPEC:
-	case AF_INET:
-		if (addrlen < sizeof(struct sockaddr_in))
-			return -EINVAL;
-		port = ((struct sockaddr_in *)address)->sin_port;
-		break;
-
-#if IS_ENABLED(CONFIG_IPV6)
-	case AF_INET6:
-		if (addrlen < SIN6_LEN_RFC2133)
-			return -EINVAL;
-		port = ((struct sockaddr_in6 *)address)->sin6_port;
-		break;
-#endif /* IS_ENABLED(CONFIG_IPV6) */
-
-	default:
-		return 0;
-	}
+	err = get_addr_port(address, addrlen, false, &port);
+	if (err == -EAFNOSUPPORT)
+		return 0; // restrictions are not applicable to this socket family
+	else if (err != 0)
+		return err;
 
 	/* Specific AF_UNSPEC handling. */
 	if (address->sa_family == AF_UNSPEC) {
@@ -174,6 +194,27 @@ static int current_check_access_socket(struct socket *const sock,
 	return -EACCES;
 }
 
+static int check_access_port(const struct landlock_ruleset *const dom,
+			     access_mask_t access_request, __be16 port)
+{
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+	const struct landlock_rule *rule;
+	const struct landlock_id id = {
+		.key.data = (__force uintptr_t)port,
+		.type = LANDLOCK_KEY_NET_PORT,
+	};
+	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
+
+	rule = landlock_find_rule(dom, id);
+	access_request = landlock_init_layer_masks(
+		dom, access_request, &layer_masks, LANDLOCK_KEY_NET_PORT);
+	if (landlock_unmask_layers(rule, access_request, &layer_masks,
+				   ARRAY_SIZE(layer_masks)))
+		return 0;
+
+	return -EACCES;
+}
+
 static int hook_socket_bind(struct socket *const sock,
 			    struct sockaddr *const address, const int addrlen)
 {
@@ -215,9 +256,119 @@ static int hook_socket_connect(struct socket *const sock,
 					   access_request);
 }
 
+static int hook_socket_sendmsg(struct socket *const sock,
+			       struct msghdr *const msg, const int size)
+{
+	const struct landlock_ruleset *const dom = get_current_net_domain();
+	const struct sockaddr *address = (const struct sockaddr *)msg->msg_name;
+	int err;
+	__be16 port;
+
+	if (sock->type != SOCK_DGRAM)
+		return 0;
+	if (sock->sk->sk_protocol != IPPROTO_UDP)
+		return 0;
+	if (!dom)
+		return 0;
+	if (WARN_ON_ONCE(dom->num_layers < 1))
+		return -EACCES;
+
+	/*
+	 * Don't mimic all checks udp_sendmsg() and udpv6_sendmsg() do. Just
+	 * read what we need for access control, and fail if we can't (e.g.
+	 * because the input buffer is too short) with the same error codes as
+	 * they do. Selftests enforce that these error codes do not diverge
+	 * with the actual implementation's ones.
+	 */
+
+	/*
+	 * If there is a more specific address in the message, it will take
+	 * precedence over any connect()ed address. Base our access check on it.
+	 */
+	if (address) {
+		const bool in_udpv6_sendmsg =
+			(sock->sk->sk_prot == &udpv6_prot);
+
+		err = get_addr_port(address, msg->msg_namelen, in_udpv6_sendmsg,
+				    &port);
+		if (err != 0)
+			return err;
+
+		/*
+		 * In `udpv6_sendmsg`, AF_UNSPEC is interpreted as "no address".
+		 * In that case, the call above will succeed but without
+		 * returning a port.
+		 */
+		if (in_udpv6_sendmsg && address->sa_family == AF_UNSPEC)
+			address = NULL;
+	}
+
+	/*
+	 * Without a message-specific destination address, the socket must be
+	 * connect()ed to an address, base our access check on that one.
+	 */
+	if (!address) {
+		/*
+		 * We could let this through and count on `udp_sendmsg` and
+		 * `udpv6_sendmsg` to error out, but they could change in the
+		 * future and open a hole here without knowing. Enforce an
+		 * error, and enforce in selftests that we don't diverge in
+		 * behaviours compared to them.
+		 */
+		if (sock->sk->sk_state != TCP_ESTABLISHED)
+			return -EDESTADDRREQ;
+
+		port = inet_sk(sock->sk)->inet_dport;
+	}
+
+	return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDMSG_UDP, port);
+}
+
+static int hook_socket_recvmsg(struct socket *const sock,
+			       struct msghdr *const msg, const int size,
+			       const int flags)
+{
+	const struct landlock_ruleset *const dom = get_current_net_domain();
+	struct sock *sk = sock->sk;
+	int err;
+	__be16 port_bigendian;
+	int ephemeral_low;
+	int ephemeral_high;
+	__u16 port_hostendian;
+
+	if (sk->sk_protocol != IPPROTO_UDP)
+		return 0;
+	if (!dom)
+		return 0;
+	if (WARN_ON_ONCE(dom->num_layers < 1))
+		return -EACCES;
+
+	/* "fast" path: socket is bound to an explicitly allowed port */
+	port_bigendian = inet_sk(sk)->inet_sport;
+	err = check_access_port(dom, LANDLOCK_ACCESS_NET_RECVMSG_UDP,
+				port_bigendian);
+	if (err != -EACCES)
+		return err;
+
+	/*
+	 * Slow path: socket is bound to an ephemeral port. Need a second check
+	 * on port 0 with different semantics ("any ephemeral port").
+	 */
+	inet_sk_get_local_port_range(sk, &ephemeral_low, &ephemeral_high);
+	port_hostendian = ntohs(port_bigendian);
+	if (ephemeral_low <= port_hostendian &&
+	    port_hostendian <= ephemeral_high)
+		return check_access_port(dom, LANDLOCK_ACCESS_NET_RECVMSG_UDP,
+					 0);
+
+	return -EACCES;
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_bind, hook_socket_bind),
 	LSM_HOOK_INIT(socket_connect, hook_socket_connect),
+	LSM_HOOK_INIT(socket_sendmsg, hook_socket_sendmsg),
+	LSM_HOOK_INIT(socket_recvmsg, hook_socket_recvmsg),
 };
 
 __init void landlock_add_net_hooks(void)
-- 
2.39.5

Re: [RFC PATCH v1 4/7] landlock: Add UDP send+recv access control

[Mickaël Salaün]

On Mon, Sep 16, 2024 at 02:22:27PM +0200, Matthieu Buffet wrote:
> Add support for two UDP access rights, complementing the two previous
> LANDLOCK_ACCESS_NET_CONNECT_UDP and LANDLOCK_ACCESS_NET_BIND_UDP:
> 
> - LANDLOCK_ACCESS_NET_RECVMSG_UDP: to prevent a process from receiving

I'm wondering what would make the most sense between NET_RECVMSG_UDP and
NET_RECVFROM_UDP.  Is one more known or understood than the other?  Same
for sendmsg vs. sendto.

>   datagrams. Just removing LANDLOCK_ACCESS_NET_BIND_UDP is not enough:
>   it can just send a first datagram or call connect() and get an
>   ephemeral port assigned, without ever calling bind(). This access right
>   allows blocking a process from receiving UDP datagrams, without
>   preventing them to bind() (which may be required to set source ports);
> 
> - LANDLOCK_ACCESS_NET_SENDMSG_UDP: to prevent a process from sending
>   datagrams. Just removing LANDLOCK_ACCESS_NET_CONNECT_UDP is not enough:
>   the process can call sendmsg() with an unconnected socket and an
>   arbitrary destination address.
> 
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>  include/uapi/linux/landlock.h |  18 ++-
>  security/landlock/limits.h    |   2 +-
>  security/landlock/net.c       | 205 +++++++++++++++++++++++++++++-----
>  3 files changed, 193 insertions(+), 32 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 7f9aa1cd2912..7ea3d1adb8c3 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -287,15 +287,25 @@ struct landlock_net_port_attr {
>   *   receive datagrams from (if you create a client-specific socket for a
>   *   client-specific process, e.g. using the established-over-unconnected
>   *   method)
> - *
> - * Note that ``bind(0)`` means binding to an ephemeral kernel-assigned port,
> - * in the range configured in ``/proc/sys/net/ipv4/ip_local_port_range``
> - * globally (or on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).
> + * - %LANDLOCK_ACCESS_NET_RECVMSG_UDP: receive datagrams on the given local port
> + *   (this is a distinct right from %LANDLOCK_ACCESS_NET_BIND_UDP, because you
> + *   may want to allow a process to set its datagrams source port using bind()
> + *   but not be able to receive datagrams)
> + * - %LANDLOCK_ACCESS_NET_SENDMSG_UDP: send datagrams to the given remote port
> + *   (this is a distinct right from %LANDLOCK_ACCESS_NET_CONNECT_UDP, because
> + *   you may want to allow a process to set which client it wants to receive
> + *   datagrams from using connect(), and not be able to send datagrams)
> + *
> + * Note that ``bind(0)`` has special semantics, meaning bind on any port in the
> + * range configured in ``/proc/sys/net/ipv4/ip_local_port_range`` globally (or
> + * on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).
>   */
>  /* clang-format off */
>  #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>  #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>  #define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
>  #define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
> +#define LANDLOCK_ACCESS_NET_RECVMSG_UDP			(1ULL << 4)
> +#define LANDLOCK_ACCESS_NET_SENDMSG_UDP			(1ULL << 5)
>  /* clang-format on */
>  #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 182b6a8d2976..e2697348310c 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -22,7 +22,7 @@
>  #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
>  #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>  
> -#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_UDP
> +#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_SENDMSG_UDP
>  #define LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
>  #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
>  
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index becc62c02cc9..9a3c44ad3f26 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -10,6 +10,8 @@
>  #include <linux/net.h>
>  #include <linux/socket.h>
>  #include <net/ipv6.h>
> +#include <net/transp_v6.h>
> +#include <net/ip.h>
>  
>  #include "common.h"
>  #include "cred.h"
> @@ -61,6 +63,45 @@ static const struct landlock_ruleset *get_current_net_domain(void)
>  	return dom;
>  }
>  
> +static int get_addr_port(const struct sockaddr *address, int addrlen,
> +			 bool in_udpv6_sendmsg_ctx, __be16 *port)
> +{
> +	/* Checks for minimal header length to safely read sa_family. */
> +	if (addrlen < offsetofend(typeof(*address), sa_family))
> +		return -EINVAL;
> +
> +	switch (address->sa_family) {
> +	case AF_UNSPEC:

Please create a simple patch refactoring this code, but without any
semantic change, and then include the UDP specific part in the patch
adding support for UDP control.  This helps verify (and test) what is
the code refactoring and what is the actual change, and it could also
help for backports.  Moving this code to a standalone helper should then
be the first patch of this series.

> +		/*
> +		 * Backward compatibility games: AF_UNSPEC is mapped to AF_INET
> +		 * by `bind` (v4+v6), `connect` (v4) and `sendmsg` (v4), but

Instead of backticks, just name these syscalls as functions: bind(),
connect()...

> +		 * interpreted as "no address" by `sendmsg` (v6). In that case
> +		 * this call must succeed (even if `address` is shorter than a
> +		 * `struct sockaddr_in`), and caller must check for this
> +		 * condition.

Weird dance, but good catch.

> +		 */
> +		if (in_udpv6_sendmsg_ctx) {
> +			*port = 0;

Why set the port to zero?  In udp_sendmsg(), it looks like such a port
would return -EINVAL right?

And in this case, why ignoring the following addrlen check?

Couldn't we just remove this in_udpv6_sendmsg_ctx argument, extract the
port as long as we can, and only deal with the in_udpv6_sendmsg case in
hook_socket_sendmsg()

> +			return 0;
> +		}
> +		fallthrough;
> +	case AF_INET:
> +		if (addrlen < sizeof(struct sockaddr_in))
> +			return -EINVAL;
> +		*port = ((struct sockaddr_in *)address)->sin_port;
> +		return 0;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case AF_INET6:
> +		if (addrlen < SIN6_LEN_RFC2133)
> +			return -EINVAL;
> +		*port = ((struct sockaddr_in6 *)address)->sin6_port;
> +		return 0;
> +#endif /* IS_ENABLED(CONFIG_IPV6) */
> +	}
> +
> +	return -EAFNOSUPPORT;
> +}
> +
>  static int current_check_access_socket(struct socket *const sock,
>  				       struct sockaddr *const address,
>  				       const int addrlen,
> @@ -73,39 +114,18 @@ static int current_check_access_socket(struct socket *const sock,
>  		.type = LANDLOCK_KEY_NET_PORT,
>  	};
>  	const struct landlock_ruleset *const dom = get_current_net_domain();
> +	int err;
>  
>  	if (!dom)
>  		return 0;
>  	if (WARN_ON_ONCE(dom->num_layers < 1))
>  		return -EACCES;
>  
> -	/* Checks if it's a (potential) UDP or TCP socket. */
> -	if (sock->type != SOCK_STREAM && sock->type != SOCK_DGRAM)
> -		return 0;
> -
> -	/* Checks for minimal header length to safely read sa_family. */
> -	if (addrlen < offsetofend(typeof(*address), sa_family))
> -		return -EINVAL;
> -
> -	switch (address->sa_family) {
> -	case AF_UNSPEC:
> -	case AF_INET:
> -		if (addrlen < sizeof(struct sockaddr_in))
> -			return -EINVAL;
> -		port = ((struct sockaddr_in *)address)->sin_port;
> -		break;
> -
> -#if IS_ENABLED(CONFIG_IPV6)
> -	case AF_INET6:
> -		if (addrlen < SIN6_LEN_RFC2133)
> -			return -EINVAL;
> -		port = ((struct sockaddr_in6 *)address)->sin6_port;
> -		break;
> -#endif /* IS_ENABLED(CONFIG_IPV6) */
> -
> -	default:
> -		return 0;
> -	}
> +	err = get_addr_port(address, addrlen, false, &port);
> +	if (err == -EAFNOSUPPORT)
> +		return 0; // restrictions are not applicable to this socket family

Comments need to be /* Like this and before the commented code. */
See https://docs.kernel.org/process/maintainer-tip.html#comment-style

> +	else if (err != 0)
> +		return err;
>  
>  	/* Specific AF_UNSPEC handling. */
>  	if (address->sa_family == AF_UNSPEC) {
> @@ -174,6 +194,27 @@ static int current_check_access_socket(struct socket *const sock,
>  	return -EACCES;
>  }
>  
> +static int check_access_port(const struct landlock_ruleset *const dom,
> +			     access_mask_t access_request, __be16 port)
> +{
> +	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
> +	const struct landlock_rule *rule;
> +	const struct landlock_id id = {
> +		.key.data = (__force uintptr_t)port,
> +		.type = LANDLOCK_KEY_NET_PORT,
> +	};
> +	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
> +
> +	rule = landlock_find_rule(dom, id);
> +	access_request = landlock_init_layer_masks(
> +		dom, access_request, &layer_masks, LANDLOCK_KEY_NET_PORT);
> +	if (landlock_unmask_layers(rule, access_request, &layer_masks,
> +				   ARRAY_SIZE(layer_masks)))
> +		return 0;
> +
> +	return -EACCES;
> +}
> +
>  static int hook_socket_bind(struct socket *const sock,
>  			    struct sockaddr *const address, const int addrlen)
>  {
> @@ -215,9 +256,119 @@ static int hook_socket_connect(struct socket *const sock,
>  					   access_request);
>  }
>  
> +static int hook_socket_sendmsg(struct socket *const sock,
> +			       struct msghdr *const msg, const int size)

We can probably constify these references.

> +{
> +	const struct landlock_ruleset *const dom = get_current_net_domain();
> +	const struct sockaddr *address = (const struct sockaddr *)msg->msg_name;
> +	int err;
> +	__be16 port;
> +
> +	if (sock->type != SOCK_DGRAM)
> +		return 0;
> +	if (sock->sk->sk_protocol != IPPROTO_UDP)
> +		return 0;
> +	if (!dom)
> +		return 0;

I'd prefer this !dom check to be the first (like for most other hooks)
because it makes it clear that Landlock doesn't mess with not sandboxed
tasks.  Moreover in this case it would avoid two pointer dereferences.

> +	if (WARN_ON_ONCE(dom->num_layers < 1))
> +		return -EACCES;

This num_layers check can stay just after to the dom check though.

> +
> +	/*
> +	 * Don't mimic all checks udp_sendmsg() and udpv6_sendmsg() do. Just
> +	 * read what we need for access control, and fail if we can't (e.g.
> +	 * because the input buffer is too short) with the same error codes as
> +	 * they do. Selftests enforce that these error codes do not diverge
> +	 * with the actual implementation's ones.
> +	 */
> +
> +	/*
> +	 * If there is a more specific address in the message, it will take
> +	 * precedence over any connect()ed address. Base our access check on it.
> +	 */
> +	if (address) {
> +		const bool in_udpv6_sendmsg =
> +			(sock->sk->sk_prot == &udpv6_prot);
> +
> +		err = get_addr_port(address, msg->msg_namelen, in_udpv6_sendmsg,
> +				    &port);
> +		if (err != 0)
> +			return err;
> +
> +		/*
> +		 * In `udpv6_sendmsg`, AF_UNSPEC is interpreted as "no address".
> +		 * In that case, the call above will succeed but without
> +		 * returning a port.
> +		 */
> +		if (in_udpv6_sendmsg && address->sa_family == AF_UNSPEC)
> +			address = NULL;
> +	}
> +
> +	/*
> +	 * Without a message-specific destination address, the socket must be
> +	 * connect()ed to an address, base our access check on that one.
> +	 */
> +	if (!address) {

If the address is not specified, I think we should just allow the
request and let the network stack handle the rest.  The advantage of
this approach would be that if the socket was previously allowed to be
connected, the check is only done once and they will be almost no
performance impact when calling sendto/write/recvfrom/read on this
"connected" socket.

> +		/*
> +		 * We could let this through and count on `udp_sendmsg` and
> +		 * `udpv6_sendmsg` to error out, but they could change in the
> +		 * future and open a hole here without knowing. Enforce an
> +		 * error, and enforce in selftests that we don't diverge in
> +		 * behaviours compared to them.

This is a good approach for this patch, but if we allow connected
sockets to be freely used when the address is not specified, this check
should not be required because we would allow such action anyway and the
network stack would handle the other error cases.

> +		 */
> +		if (sock->sk->sk_state != TCP_ESTABLISHED)
> +			return -EDESTADDRREQ;
> +
> +		port = inet_sk(sock->sk)->inet_dport;
> +	}
> +
> +	return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDMSG_UDP, port);


What about something like this (with the appropriate comments)?

if (!address)
	return 0;

if (address->sa_family == AF_UNSPEC && sock->sk->sk_prot == &udpv6_prot)
	return 0;

err = get_addr_port(address, msg->msg_namelen, &port);
if (err)
	return err;

return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDMSG_UDP, port);

> +}
> +
> +static int hook_socket_recvmsg(struct socket *const sock,
> +			       struct msghdr *const msg, const int size,
> +			       const int flags)
> +{
> +	const struct landlock_ruleset *const dom = get_current_net_domain();
> +	struct sock *sk = sock->sk;
> +	int err;
> +	__be16 port_bigendian;
> +	int ephemeral_low;
> +	int ephemeral_high;
> +	__u16 port_hostendian;
> +
> +	if (sk->sk_protocol != IPPROTO_UDP)
> +		return 0;

ditto

> +	if (!dom)
> +		return 0;
> +	if (WARN_ON_ONCE(dom->num_layers < 1))
> +		return -EACCES;
> +
> +	/* "fast" path: socket is bound to an explicitly allowed port */
> +	port_bigendian = inet_sk(sk)->inet_sport;
> +	err = check_access_port(dom, LANDLOCK_ACCESS_NET_RECVMSG_UDP,
> +				port_bigendian);
> +	if (err != -EACCES)
> +		return err;

We should be able to follow the same policy for "connected" sockets.

> +
> +	/*
> +	 * Slow path: socket is bound to an ephemeral port. Need a second check
> +	 * on port 0 with different semantics ("any ephemeral port").
> +	 */
> +	inet_sk_get_local_port_range(sk, &ephemeral_low, &ephemeral_high);

Is it to handle recvmsg(with port 0)?

> +	port_hostendian = ntohs(port_bigendian);
> +	if (ephemeral_low <= port_hostendian &&
> +	    port_hostendian <= ephemeral_high)
> +		return check_access_port(dom, LANDLOCK_ACCESS_NET_RECVMSG_UDP,
> +					 0);
> +
> +	return -EACCES;
> +}
> +
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(socket_bind, hook_socket_bind),
>  	LSM_HOOK_INIT(socket_connect, hook_socket_connect),
> +	LSM_HOOK_INIT(socket_sendmsg, hook_socket_sendmsg),
> +	LSM_HOOK_INIT(socket_recvmsg, hook_socket_recvmsg),
>  };
>  
>  __init void landlock_add_net_hooks(void)
> -- 
> 2.39.5
> 
> 

Re: [RFC PATCH v1 4/7] landlock: Add UDP send+recv access control

[Matthieu Buffet]

Hi Mickaël,

I've almost finished merging your review (thanks for all the feedback), 
with the exception of this main point. Just making sure we agree on the 
limitations before I merge this into a new version.

On 9/21/2024 12:23 PM, Mickaël Salaün wrote:
 >> +	/*
 >> +	 * If there is a more specific address in the message, it will take
 >> +	 * precedence over any connect()ed address. Base our access check 
on it.
 >> +	 */
 >> +	if (address) {
 >> +		const bool in_udpv6_sendmsg =
 >> +			(sock->sk->sk_prot == &udpv6_prot);
 >> +
 >> +		err = get_addr_port(address, msg->msg_namelen, in_udpv6_sendmsg,
 >> +				    &port);
 >> +		if (err != 0)
 >> +			return err;
 >> +
 >> +		/*
 >> +		 * In `udpv6_sendmsg`, AF_UNSPEC is interpreted as "no address".
 >> +		 * In that case, the call above will succeed but without
 >> +		 * returning a port.
 >> +		 */
 >> +		if (in_udpv6_sendmsg && address->sa_family == AF_UNSPEC)
 >> +			address = NULL;
 >> +	}
 >> +
 >> +	/*
 >> +	 * Without a message-specific destination address, the socket must be
 >> +	 * connect()ed to an address, base our access check on that one.
 >> +	 */
 >> +	if (!address) {
 >
 > If the address is not specified, I think we should just allow the
 > request and let the network stack handle the rest.  The advantage of
 > this approach would be that if the socket was previously allowed to be
 > connected, the check is only done once and they will be almost no
 > performance impact when calling sendto/write/recvfrom/read on this
 > "connected" socket.
 > [...]
 > What about something like this (with the appropriate comments)?
 >
 > if (!address)
 > 	return 0;
 >
 > if (address->sa_family == AF_UNSPEC && sock->sk->sk_prot ==
 >     &udpv6_prot)
 > 	return 0;
 >
 > err = get_addr_port(address, msg->msg_namelen, &port);
 > if (err)
 > 	return err;
 >
 > return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDMSG_UDP, port);

If I understand correctly, you would like the semantics of 
LANDLOCK_ACCESS_NET_CONNECT_UDP to be {connect(), and sendmsg() without 
explicit address} and LANDLOCK_ACCESS_NET_SENDMSG_UDP to be {sendmsg() 
with explicit address}. This makes it impossible to allow a landlocked 
server to connect() in order to receive traffic only from a specific 
client while at the same time preventing it from sending traffic (that 
is, a receive-only client-specific socket ala Cloudflare's 
"established-over-unconnected"[1]).

 >> +	err = check_access_port(dom, LANDLOCK_ACCESS_NET_RECVMSG_UDP,
 >> +				port_bigendian);
 >> +	if (err != -EACCES)
 >> +		return err;
 >
 > We should be able to follow the same policy for "connected" sockets.

Again if I understand correctly, to fully merge semantics of 
LANDLOCK_ACCESS_NET_BIND_UDP and LANDLOCK_ACCESS_NET_RECVMSG_UDP (since 
if the access check is performed at bind() time, there is nothing to 
check in recvmsg() anymore). Similarly, this makes it impossible to 
allow a send-only program to bind() to set a source port without 
allowing it to recvmsg() traffic.

I do not know of real-life programs that might want to sandbox their 
network workers *that* precisely, nor how much we want to be 
future-proof and support it. If not, I can merge your feedback and:
- remove LANDLOCK_ACCESS_NET_RECVMSG_UDP and the recvmsg() hook;
- change the doc for LANDLOCK_ACCESS_NET_SENDMSG_UDP to mention that it 
is not required if the app uses connect() and then sendmsg() without 
explicit addresses;
- change the doc for LANDLOCK_ACCESS_NET_CONNECT_UDP to mention that it 
grants the right to send traffic (and similarly for 
LANDLOCK_ACCESS_NET_BIND_UDP to receive traffic), and the reason 
(performance, though I haven't managed to get a benchmark);
- rename to LANDLOCK_ACCESS_NET_CONNECT_SENDMSG_UDP, 
LANDLOCK_ACCESS_NET_SENDMSG_UDP, and 
LANDLOCK_ACCESS_NET_BIND_RECVMSG_UDP, what do you think?

If merging semantics is a problem, I mentioned socket tagging in [2] to 
reduce the performance impact (e.g. tag whether it can send traffic at 
connect() time, and tag whether it can recv at bind() time). So another 
option could be to keep precise semantics and explore that?

 >> +	/*
 >> +	 * Slow path: socket is bound to an ephemeral port. Need a second 
check
 >> +	 * on port 0 with different semantics ("any ephemeral port").
 >> +	 */
 >> +	inet_sk_get_local_port_range(sk, &ephemeral_low, &ephemeral_high);
 >
 > Is it to handle recvmsg(with port 0)?

If you mean recvmsg() on a socket that was previously bind(0), yep. This 
second rule lookup added a different meaning to rules on port 0. Without 
this, one would not be able to allow "all ephemeral ports", making the 
feature unusable for servers that bind on ephemeral ports. All this 
disappears if we remove LANDLOCK_ACCESS_NET_RECVMSG_UDP.

Matthieu

[1] 
https://blog.cloudflare.com/everything-you-ever-wanted-to-know-about-udp-sockets-but-were-afraid-to-ask-part-1/
[2] https://github.com/landlock-lsm/linux/issues/10#issuecomment-2267871144

Re: [RFC PATCH v1 4/7] landlock: Add UDP send+recv access control

[Mickaël Salaün]

On Sat, Oct 19, 2024 at 02:47:48PM +0200, Matthieu Buffet wrote:
> Hi Mickaël,
> 
> I've almost finished merging your review (thanks for all the feedback), with
> the exception of this main point. Just making sure we agree on the
> limitations before I merge this into a new version.
> 
> On 9/21/2024 12:23 PM, Mickaël Salaün wrote:
> >> +	/*
> >> +	 * If there is a more specific address in the message, it will take
> >> +	 * precedence over any connect()ed address. Base our access check on
> it.
> >> +	 */
> >> +	if (address) {
> >> +		const bool in_udpv6_sendmsg =
> >> +			(sock->sk->sk_prot == &udpv6_prot);
> >> +
> >> +		err = get_addr_port(address, msg->msg_namelen, in_udpv6_sendmsg,
> >> +				    &port);
> >> +		if (err != 0)
> >> +			return err;
> >> +
> >> +		/*
> >> +		 * In `udpv6_sendmsg`, AF_UNSPEC is interpreted as "no address".
> >> +		 * In that case, the call above will succeed but without
> >> +		 * returning a port.
> >> +		 */
> >> +		if (in_udpv6_sendmsg && address->sa_family == AF_UNSPEC)
> >> +			address = NULL;
> >> +	}
> >> +
> >> +	/*
> >> +	 * Without a message-specific destination address, the socket must be
> >> +	 * connect()ed to an address, base our access check on that one.
> >> +	 */
> >> +	if (!address) {
> >
> > If the address is not specified, I think we should just allow the
> > request and let the network stack handle the rest.  The advantage of
> > this approach would be that if the socket was previously allowed to be
> > connected, the check is only done once and they will be almost no
> > performance impact when calling sendto/write/recvfrom/read on this
> > "connected" socket.
> > [...]
> > What about something like this (with the appropriate comments)?
> >
> > if (!address)
> > 	return 0;
> >
> > if (address->sa_family == AF_UNSPEC && sock->sk->sk_prot ==
> >     &udpv6_prot)
> > 	return 0;
> >
> > err = get_addr_port(address, msg->msg_namelen, &port);
> > if (err)
> > 	return err;
> >
> > return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDMSG_UDP, port);
> 
> If I understand correctly, you would like the semantics of
> LANDLOCK_ACCESS_NET_CONNECT_UDP to be {connect(), and sendmsg() without
> explicit address} and LANDLOCK_ACCESS_NET_SENDMSG_UDP to be {sendmsg() with
> explicit address}.

Not exactly, here is the rewording with my thinking:
...the semantics of LANDLOCK_ACCESS_NET_CONNECT_UDP to be {connect()}
and LANDLOCK_ACCESS_NET_SENDMSG_UDP to be {sendmsg() with explicit
address}.

sendmsg() without explicit address should always be allowed,
whatever the Landlock policy (similarly as write(2) on a write-opened
file descriptor).  In a nutshell, sendmsg(2) without explicit address
should be handled the same as a write(2) call on a connected socket (I
guess the kernel handles such action on connected datagram sockets the
same as on connected stream sockets).

I think it is more important to first have a simple model that
enables developers to initialize a socket with connect(2) and then
sandbox the process to only be able to use this socket to communicate
with the configured peer, similar to what can be enforced with TCP
sockets.

sendmsg(2) can do two different thinks: (optionally) configure a
peer/port and write data. recvmsg(2) only reads data.  We should first
start by controlling exchange from/to peers, and maybe later controlling
data flow.

An alternative approach would be to not add a sendmsg specific access
right but only LANDLOCK_ACCESS_NET_CONNECT_UDP because connect should
be a superset of sendmsg.  This would make it impossible to specifically
deny (shared) socket's configuration change though.  I think it's
better to stick to the kernel semantic with 3 dedicated access rights,
and it should make more sense for users too.  What do you think?


> This makes it impossible to allow a landlocked server to
> connect() in order to receive traffic only from a specific client while at
> the same time preventing it from sending traffic (that is, a receive-only
> client-specific socket ala Cloudflare's "established-over-unconnected"[1]).

My proposal would indeed makes this use case impossible to enforce only
with the proposed access rights, and we would need to restrict write(2)
too BTW.  However, I think it would make sense to add complementary
access rights to restrict reading or writing to a socket.  I guess this
semantic would be useful for non-UDP protocols too with
LANDLOCK_ACCESS_NET_{READ,WRITE}_{TCP,UDP} access rights set at
socket-creation time and stored in the socket object (instead of looking
at a Landlock domain for each read/write call, similarly to the truncate
and ioctl_dev access rights).  What do you think?

I wonder what happens if we call recvmsg(2) on a newly created (and then
unconfigured) socket.  Without prior call to bind(2) I would guess that
the recvmsg(2) call fail but I'm not so sure with a datagram socket.  We
should check that.

> 
> >> +	err = check_access_port(dom, LANDLOCK_ACCESS_NET_RECVMSG_UDP,
> >> +				port_bigendian);
> >> +	if (err != -EACCES)
> >> +		return err;
> >
> > We should be able to follow the same policy for "connected" sockets.
> 
> Again if I understand correctly, to fully merge semantics of
> LANDLOCK_ACCESS_NET_BIND_UDP and LANDLOCK_ACCESS_NET_RECVMSG_UDP (since if
> the access check is performed at bind() time, there is nothing to check in
> recvmsg() anymore).

Correct (I was a bit confused with recvmsg, but it doesn't set the
receiving address/port).

> Similarly, this makes it impossible to allow a send-only
> program to bind() to set a source port without allowing it to recvmsg()
> traffic.

Correct with the current access rights.  We would need a
LANDLOCK_ACCESS_NET_READ_UDP.

> 
> I do not know of real-life programs that might want to sandbox their network
> workers *that* precisely, nor how much we want to be future-proof and
> support it. If not, I can merge your feedback and:
> - remove LANDLOCK_ACCESS_NET_RECVMSG_UDP and the recvmsg() hook;

Yes

> - change the doc for LANDLOCK_ACCESS_NET_SENDMSG_UDP to mention that it is
> not required if the app uses connect() and then sendmsg() without explicit
> addresses;

Yes

> - change the doc for LANDLOCK_ACCESS_NET_CONNECT_UDP to mention that it
> grants the right to send traffic (and similarly for
> LANDLOCK_ACCESS_NET_BIND_UDP to receive traffic), and the reason

The documentation should highlight that these flags grants the right to
configure a socket, but indeed, no restrictions are enforced on reading
or writing on sockets.

> (performance, though I haven't managed to get a benchmark);
> - rename to LANDLOCK_ACCESS_NET_CONNECT_SENDMSG_UDP,
> LANDLOCK_ACCESS_NET_SENDMSG_UDP, and LANDLOCK_ACCESS_NET_BIND_RECVMSG_UDP,
> what do you think?

The first and third rights are confusing.  I prefer simple names such as
LANDLOCK_ACCESS_NET_CONNECT_UDP and LANDLOCK_ACCESS_NET_BIND_UDP.
Moreover, LANDLOCK_ACCESS_NET_CONNECT_UDP would not impact sendmsg(2)
calls at all.

> 
> If merging semantics is a problem, I mentioned socket tagging in [2] to
> reduce the performance impact (e.g. tag whether it can send traffic at
> connect() time, and tag whether it can recv at bind() time). So another
> option could be to keep precise semantics and explore that?

This tagging mechanism looks like a good idea to implement
LANDLOCK_ACCESS_NET_{READ,WRITE}_{TCP,UDP}, but that should be a
future separate patch series.

> 
> >> +	/*
> >> +	 * Slow path: socket is bound to an ephemeral port. Need a second check
> >> +	 * on port 0 with different semantics ("any ephemeral port").
> >> +	 */
> >> +	inet_sk_get_local_port_range(sk, &ephemeral_low, &ephemeral_high);
> >
> > Is it to handle recvmsg(with port 0)?
> 
> If you mean recvmsg() on a socket that was previously bind(0), yep. This
> second rule lookup added a different meaning to rules on port 0. Without
> this, one would not be able to allow "all ephemeral ports", making the
> feature unusable for servers that bind on ephemeral ports. All this
> disappears if we remove LANDLOCK_ACCESS_NET_RECVMSG_UDP.

OK

> 
> Matthieu
> 
> [1] https://blog.cloudflare.com/everything-you-ever-wanted-to-know-about-udp-sockets-but-were-afraid-to-ask-part-1/
> [2] https://github.com/landlock-lsm/linux/issues/10#issuecomment-2267871144
> 

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

[Matthieu Buffet]

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 <matthieu@buffet.re>
---
 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));
 	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. */
 	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

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

[Mickaël Salaün]

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 <matthieu@buffet.re>
> ---
>  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
> 
> 

[RFC PATCH v1 6/7] selftests/landlock: Adapt existing tests for UDP

[Matthieu Buffet]

Make basic changes to the existing bind()/connect() test suite to also
encompass testing UDP access control.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 tools/testing/selftests/landlock/base_test.c |   2 +-
 tools/testing/selftests/landlock/net_test.c  | 145 ++++++++++++++-----
 2 files changed, 111 insertions(+), 36 deletions(-)

diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 3b26bf3cf5b9..1bc16fde2e8a 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,7 +76,7 @@ TEST(abi_version)
 	const struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
-	ASSERT_EQ(5, landlock_create_ruleset(NULL, 0,
+	ASSERT_EQ(6, landlock_create_ruleset(NULL, 0,
 					     LANDLOCK_CREATE_RULESET_VERSION));
 
 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index f21cfbbc3638..883e6648e79a 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -34,6 +34,7 @@ enum sandbox_type {
 	NO_SANDBOX,
 	/* This may be used to test rules that allow *and* deny accesses. */
 	TCP_SANDBOX,
+	UDP_SANDBOX,
 };
 
 struct protocol_variant {
@@ -123,6 +124,8 @@ static bool is_restricted(const struct protocol_variant *const prot,
 		switch (prot->type) {
 		case SOCK_STREAM:
 			return sandbox == TCP_SANDBOX;
+		case SOCK_DGRAM:
+			return sandbox == UDP_SANDBOX;
 		}
 		break;
 	}
@@ -438,6 +441,46 @@ FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_unix_datagram) {
 	},
 };
 
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_udp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_tcp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_udp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET6,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_tcp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET6,
+		.type = SOCK_STREAM,
+	},
+};
+
 static void test_bind_and_connect(struct __test_metadata *const _metadata,
 				  const struct service_fixture *const srv,
 				  const bool deny_bind, const bool deny_connect)
@@ -530,7 +573,7 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
 		ret = connect_variant(connect_fd, srv);
 		if (deny_connect) {
 			EXPECT_EQ(-EACCES, ret);
-		} else if (deny_bind) {
+		} else if (deny_bind && srv->protocol.type == SOCK_STREAM) {
 			/* No listening server. */
 			EXPECT_EQ(-ECONNREFUSED, ret);
 		} else {
@@ -569,18 +612,32 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
 
 TEST_F(protocol, bind)
 {
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		__u64 bind_access = (variant->sandbox == UDP_SANDBOX ?
+			LANDLOCK_ACCESS_NET_BIND_UDP :
+			LANDLOCK_ACCESS_NET_BIND_TCP);
+		__u64 connect_access = (variant->sandbox == UDP_SANDBOX ?
+			LANDLOCK_ACCESS_NET_CONNECT_UDP :
+			LANDLOCK_ACCESS_NET_CONNECT_TCP);
+		/*
+		 * Rights required to send/recv in addition to bind/connect,
+		 * just to confirm that bind/connect indeed worked.
+		 */
+		if (variant->sandbox == UDP_SANDBOX) {
+			bind_access |= LANDLOCK_ACCESS_NET_RECVMSG_UDP |
+				LANDLOCK_ACCESS_NET_SENDMSG_UDP;
+			connect_access |= LANDLOCK_ACCESS_NET_RECVMSG_UDP |
+				LANDLOCK_ACCESS_NET_SENDMSG_UDP;
+		}
 		const struct landlock_ruleset_attr ruleset_attr = {
-			.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
-					      LANDLOCK_ACCESS_NET_CONNECT_TCP,
+			.handled_access_net = bind_access | connect_access,
 		};
-		const struct landlock_net_port_attr tcp_bind_connect_p0 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
-					  LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		const struct landlock_net_port_attr bind_connect_p0 = {
+			.allowed_access = bind_access | connect_access,
 			.port = self->srv0.port,
 		};
-		const struct landlock_net_port_attr tcp_connect_p1 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		const struct landlock_net_port_attr connect_p1 = {
+			.allowed_access = connect_access,
 			.port = self->srv1.port,
 		};
 		int ruleset_fd;
@@ -589,15 +646,15 @@ TEST_F(protocol, bind)
 						     sizeof(ruleset_attr), 0);
 		ASSERT_LE(0, ruleset_fd);
 
-		/* Allows connect and bind for the first port.  */
+		/* Allows client and server behaviours for the first port */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind_connect_p0, 0));
+					    &bind_connect_p0, 0));
 
-		/* Allows connect and denies bind for the second port. */
+		/* Allows client and deny server behaviour for the second one */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_connect_p1, 0));
+					    &connect_p1, 0));
 
 		enforce_ruleset(_metadata, ruleset_fd);
 		EXPECT_EQ(0, close(ruleset_fd));
@@ -619,18 +676,22 @@ TEST_F(protocol, bind)
 
 TEST_F(protocol, connect)
 {
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		__u64 bind_access = (variant->sandbox == UDP_SANDBOX ?
+			LANDLOCK_ACCESS_NET_BIND_UDP :
+			LANDLOCK_ACCESS_NET_BIND_TCP);
+		__u64 connect_access = (variant->sandbox == UDP_SANDBOX ?
+			LANDLOCK_ACCESS_NET_CONNECT_UDP :
+			LANDLOCK_ACCESS_NET_CONNECT_TCP);
 		const struct landlock_ruleset_attr ruleset_attr = {
-			.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
-					      LANDLOCK_ACCESS_NET_CONNECT_TCP,
+			.handled_access_net = bind_access | connect_access,
 		};
-		const struct landlock_net_port_attr tcp_bind_connect_p0 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
-					  LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		const struct landlock_net_port_attr bind_connect_p0 = {
+			.allowed_access = bind_access | connect_access,
 			.port = self->srv0.port,
 		};
-		const struct landlock_net_port_attr tcp_bind_p1 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
+		const struct landlock_net_port_attr bind_p1 = {
+			.allowed_access = bind_access,
 			.port = self->srv1.port,
 		};
 		int ruleset_fd;
@@ -642,12 +703,12 @@ TEST_F(protocol, connect)
 		/* Allows connect and bind for the first port. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind_connect_p0, 0));
+					    &bind_connect_p0, 0));
 
 		/* Allows bind and denies connect for the second port. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind_p1, 0));
+					    &bind_p1, 0));
 
 		enforce_ruleset(_metadata, ruleset_fd);
 		EXPECT_EQ(0, close(ruleset_fd));
@@ -665,16 +726,21 @@ TEST_F(protocol, connect)
 
 TEST_F(protocol, bind_unspec)
 {
-	const struct landlock_ruleset_attr ruleset_attr = {
-		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP,
-	};
-	const struct landlock_net_port_attr tcp_bind = {
-		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
-		.port = self->srv0.port,
-	};
 	int bind_fd, ret;
 
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		const struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_net = (variant->sandbox == TCP_SANDBOX ?
+				LANDLOCK_ACCESS_NET_BIND_TCP :
+				LANDLOCK_ACCESS_NET_BIND_UDP),
+		};
+		const struct landlock_net_port_attr bind = {
+			.allowed_access = (variant->sandbox == TCP_SANDBOX ?
+				LANDLOCK_ACCESS_NET_BIND_TCP :
+				LANDLOCK_ACCESS_NET_BIND_UDP),
+			.port = self->srv0.port,
+		};
+
 		const int ruleset_fd = landlock_create_ruleset(
 			&ruleset_attr, sizeof(ruleset_attr), 0);
 		ASSERT_LE(0, ruleset_fd);
@@ -682,7 +748,7 @@ TEST_F(protocol, bind_unspec)
 		/* Allows bind. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind, 0));
+					    &bind, 0));
 		enforce_ruleset(_metadata, ruleset_fd);
 		EXPECT_EQ(0, close(ruleset_fd));
 	}
@@ -703,7 +769,12 @@ TEST_F(protocol, bind_unspec)
 	}
 	EXPECT_EQ(0, close(bind_fd));
 
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		const struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_net = (variant->sandbox == TCP_SANDBOX ?
+				LANDLOCK_ACCESS_NET_BIND_TCP :
+				LANDLOCK_ACCESS_NET_BIND_UDP),
+		};
 		const int ruleset_fd = landlock_create_ruleset(
 			&ruleset_attr, sizeof(ruleset_attr), 0);
 		ASSERT_LE(0, ruleset_fd);
@@ -1232,11 +1303,15 @@ FIXTURE_TEARDOWN(mini)
 
 /* clang-format off */
 
-#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_TCP
+#define ACCESS_LAST LANDLOCK_ACCESS_NET_SENDMSG_UDP
 
 #define ACCESS_ALL ( \
 	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)
 
 /* clang-format on */
 
-- 
2.39.5

[RFC PATCH v1 7/7] selftests/landlock: Add UDP sendmsg/recvmsg tests

[Matthieu Buffet]

Add tests specific to send/recv, orthogonal to whether the process
is allowed to bind/connect.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 tools/testing/selftests/landlock/net_test.c | 373 ++++++++++++++++++++
 1 file changed, 373 insertions(+)

diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 883e6648e79a..a02307ba069c 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -285,6 +285,29 @@ static int connect_variant(const int sock_fd,
 	return connect_variant_addrlen(sock_fd, srv, get_addrlen(srv, false));
 }
 
+static int get_msg_addr_variant(struct msghdr *msg,
+				const struct service_fixture *const srv)
+{
+	switch (srv->protocol.domain) {
+	case AF_UNSPEC:
+	case AF_INET:
+		msg->msg_name = (void *)&srv->ipv4_addr;
+		break;
+	case AF_INET6:
+		msg->msg_name = (void *)&srv->ipv6_addr;
+		break;
+	case AF_UNIX:
+		msg->msg_name = (void *)&srv->unix_addr;
+		break;
+	default:
+		errno = -EAFNOSUPPORT;
+		return -errno;
+	}
+
+	msg->msg_namelen = get_addrlen(srv, false);
+	return 0;
+}
+
 FIXTURE(protocol)
 {
 	struct service_fixture srv0, srv1, srv2, unspec_any0, unspec_srv0;
@@ -927,6 +950,356 @@ TEST_F(protocol, connect_unspec)
 	EXPECT_EQ(0, close(bind_fd));
 }
 
+FIXTURE(udp_send_recv)
+{
+	struct service_fixture srv_allowed;
+	struct service_fixture srv_denied;
+	struct service_fixture srv_ephemeral;
+	struct service_fixture addr_unspec0, addr_unspec1;
+	int srv_allowed_fd, srv_denied_fd, srv_ephemeral_fd, client_fd;
+	char read_buf[1];
+	struct iovec testmsg_contents;
+	struct msghdr testmsg;
+};
+
+FIXTURE_VARIANT(udp_send_recv)
+{
+	const struct protocol_variant prot;
+};
+
+FIXTURE_SETUP(udp_send_recv)
+{
+	const struct timeval read_timeout = {
+		.tv_sec = 0,
+		.tv_usec = 100000,
+	};
+	const struct protocol_variant prot_unspec = {
+		.domain = AF_UNSPEC,
+		.type = SOCK_STREAM,
+	};
+
+	disable_caps(_metadata);
+
+	ASSERT_EQ(0, set_service(&self->addr_unspec0, prot_unspec, 0));
+	ASSERT_EQ(0, set_service(&self->addr_unspec1, prot_unspec, 1));
+
+	/* Prepare one server socket to be denied */
+	ASSERT_EQ(0, set_service(&self->srv_denied, variant->prot, 0));
+	self->srv_denied_fd = socket_variant(&self->srv_denied);
+	ASSERT_LE(0, self->srv_denied_fd);
+	ASSERT_EQ(0, bind_variant(self->srv_denied_fd, &self->srv_denied));
+
+	/* Prepare one server socket on specific port to be allowed */
+	ASSERT_EQ(0, set_service(&self->srv_allowed, variant->prot, 1));
+	self->srv_allowed_fd = socket_variant(&self->srv_allowed);
+	ASSERT_LE(0, self->srv_allowed_fd);
+	ASSERT_EQ(0, bind_variant(self->srv_allowed_fd, &self->srv_allowed));
+
+	/* Prepare one server socket on ephemeral port to be allowed */
+	ASSERT_EQ(0, set_service(&self->srv_ephemeral, variant->prot, 0));
+	if (variant->prot.domain == AF_INET)
+		self->srv_ephemeral.ipv4_addr.sin_port = 0;
+	else if (variant->prot.domain == AF_INET6)
+		self->srv_ephemeral.ipv6_addr.sin6_port = 0;
+	self->srv_ephemeral_fd  = socket_variant(&self->srv_ephemeral);
+	ASSERT_LE(0, self->srv_ephemeral_fd);
+	ASSERT_EQ(0, bind_variant(self->srv_ephemeral_fd,
+				  &self->srv_ephemeral));
+	self->srv_ephemeral.port = get_binded_port(self->srv_ephemeral_fd,
+						   &variant->prot);
+	ASSERT_NE(0, self->srv_ephemeral.port);
+	if (variant->prot.domain == AF_INET)
+		self->srv_ephemeral.ipv4_addr.sin_port = htons(self->srv_ephemeral.port);
+	else if (variant->prot.domain == AF_INET6)
+		self->srv_ephemeral.ipv6_addr.sin6_port = htons(self->srv_ephemeral.port);
+
+	/* We must absolutely avoid blocking other tests indefinitely */
+	EXPECT_EQ(0, setsockopt(self->srv_allowed_fd, SOL_SOCKET,
+				SO_RCVTIMEO,
+				&read_timeout, sizeof(read_timeout)));
+	EXPECT_EQ(0, setsockopt(self->srv_denied_fd, SOL_SOCKET,
+				SO_RCVTIMEO,
+				&read_timeout, sizeof(read_timeout)));
+	EXPECT_EQ(0, setsockopt(self->srv_ephemeral_fd, SOL_SOCKET,
+				SO_RCVTIMEO,
+				&read_timeout, sizeof(read_timeout)));
+
+	self->client_fd = socket_variant(&self->srv_denied);
+	ASSERT_LE(0, self->client_fd);
+
+	self->read_buf[0] = 0;
+
+	self->testmsg_contents.iov_len = 1;
+	memset(&self->testmsg, 0, sizeof(self->testmsg));
+	self->testmsg.msg_iov = &self->testmsg_contents;
+	self->testmsg.msg_iovlen = 1;
+}
+
+FIXTURE_TEARDOWN(udp_send_recv)
+{
+	EXPECT_EQ(0, close(self->srv_allowed_fd));
+	EXPECT_EQ(0, close(self->srv_denied_fd));
+	EXPECT_EQ(0, close(self->client_fd));
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(udp_send_recv, ipv4) {
+	.prot = {
+		.domain = AF_INET,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(udp_send_recv, ipv6) {
+	.prot = {
+		.domain = AF_INET6,
+		.type = SOCK_DGRAM,
+	},
+};
+
+TEST_F(udp_send_recv, sendmsg)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_net = LANDLOCK_ACCESS_NET_SENDMSG_UDP,
+	};
+	const struct landlock_net_port_attr allow_one_server = {
+		.allowed_access = LANDLOCK_ACCESS_NET_SENDMSG_UDP,
+		.port = self->srv_allowed.port,
+	};
+	const int ruleset_fd = landlock_create_ruleset(
+		&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd,
+				       LANDLOCK_RULE_NET_PORT,
+				       &allow_one_server, 0));
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	/* Send without bind nor explicit address */
+	EXPECT_EQ(-1, write(self->client_fd, "A", 1));
+	EXPECT_EQ(EDESTADDRREQ, errno);
+
+	/* Send with an explicit denied address */
+	self->testmsg_contents.iov_base = "B";
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->srv_denied));
+	EXPECT_EQ(-1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(EACCES, errno);
+	EXPECT_EQ(-1, read(self->srv_denied_fd, self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(EAGAIN, errno);
+
+	/* Send with an explicit allowed address */
+	self->testmsg_contents.iov_base = "C";
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->srv_allowed));
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0))
+	{
+		TH_LOG("sendmsg failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(1, read(self->srv_allowed_fd, self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(self->read_buf[0], 'C');
+
+	/*
+	 * Sending to (AF_UNSPEC, port) should be equivalent to not specifying
+	 * a destination in IPv6, and equivalent to (AF_INET, port) in IPv4.
+	 */
+	if (variant->prot.domain == AF_INET) {
+		self->testmsg_contents.iov_base = "D";
+		EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->addr_unspec0));
+		EXPECT_EQ(-1, sendmsg(self->client_fd, &self->testmsg, 0));
+		EXPECT_EQ(EACCES, errno);
+
+		self->testmsg_contents.iov_base = "E";
+		EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->addr_unspec1));
+		EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+		EXPECT_EQ(1, read(self->srv_allowed_fd, self->read_buf,
+				  sizeof(self->read_buf)));
+		EXPECT_EQ(self->read_buf[0], 'E');
+	} else if (variant->prot.domain == AF_INET6) {
+		EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->addr_unspec0));
+		EXPECT_EQ(-1, sendmsg(self->client_fd, &self->testmsg, 0));
+		EXPECT_EQ(EDESTADDRREQ, errno);
+	}
+
+	/* Send without an address, after connect()ing to an allowed address */
+	self->testmsg.msg_name = NULL;
+	self->testmsg.msg_namelen = 0;
+	self->testmsg_contents.iov_base = "F";
+	ASSERT_EQ(0, connect_variant(self->client_fd, &self->srv_allowed));
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0))
+	{
+		TH_LOG("sendmsg failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(1, read(self->srv_allowed_fd, self->read_buf,
+		  sizeof(self->read_buf)));
+	EXPECT_EQ(self->read_buf[0], 'F');
+
+	/*
+	 * Sending to AF_UNSPEC should be equivalent to not specifying an
+	 * address (in IPv6 only) and falling back to the allowed address.
+	 */
+	if (variant->prot.domain == AF_INET6) {
+		self->testmsg_contents.iov_base = "G";
+		EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->addr_unspec0));
+		EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0)) {
+			TH_LOG("sendmsg failed: %s", strerror(errno));
+		}
+		EXPECT_EQ(1, read(self->srv_allowed_fd, self->read_buf,
+			sizeof(self->read_buf)));
+		EXPECT_EQ(self->read_buf[0], 'G');
+	}
+
+	/* Send without an address, after connect()ing to a denied address */
+	self->testmsg_contents.iov_base = "H";
+	ASSERT_EQ(0, connect_variant(self->client_fd, &self->srv_denied));
+	EXPECT_EQ(-1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(EACCES, errno);
+	EXPECT_EQ(-1, read(self->srv_denied_fd, self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(EAGAIN, errno);
+
+	/*
+	 * Sending to AF_UNSPEC should be equivalent to not specifying an
+	 * address (in IPv6 only) and falling back to the denied address.
+	 */
+	if (variant->prot.domain == AF_INET6) {
+		self->testmsg_contents.iov_base = "I";
+		EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->addr_unspec0));
+		EXPECT_EQ(-1, sendmsg(self->client_fd, &self->testmsg, 0));
+		EXPECT_EQ(EACCES, errno);
+	}
+
+	/* Send with an explicit allowed address, should overrule connect() */
+	self->testmsg_contents.iov_base = "J";
+	ASSERT_EQ(0, connect_variant(self->client_fd, &self->srv_denied));
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->srv_allowed));
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0))
+	{
+		TH_LOG("sendmsg failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(1, read(self->srv_allowed_fd, self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(self->read_buf[0], 'J');
+
+	/* Send with an explicit denied address, should overrule connect() */
+	self->testmsg_contents.iov_base = "K";
+	ASSERT_EQ(0, connect_variant(self->client_fd, &self->srv_allowed));
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->srv_denied));
+	EXPECT_EQ(-1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(EACCES, errno);
+	EXPECT_EQ(-1, read(self->srv_allowed_fd,
+			   self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(EAGAIN, errno);
+	EXPECT_EQ(-1, read(self->srv_denied_fd, self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(EAGAIN, errno);
+}
+
+TEST_F(udp_send_recv, recvmsg_on_fixed_port)
+{
+	struct sockaddr_storage from = {0};
+	socklen_t from_len = sizeof(from);
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_net = LANDLOCK_ACCESS_NET_RECVMSG_UDP,
+	};
+	const struct landlock_net_port_attr allow_one_server = {
+		.allowed_access = LANDLOCK_ACCESS_NET_RECVMSG_UDP,
+		.port = self->srv_allowed.port,
+	};
+	const int ruleset_fd = landlock_create_ruleset(
+		&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd,
+					LANDLOCK_RULE_NET_PORT,
+					&allow_one_server, 0));
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	/* Receive on an allowed port with read() */
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->srv_allowed));
+	self->testmsg_contents.iov_base = "A";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(1, read(self->srv_allowed_fd, self->read_buf,
+			  sizeof(self->read_buf))) {
+		TH_LOG("read failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(self->read_buf[0], 'A');
+
+	/* Receive on an allowed port with recv() */
+	self->testmsg_contents.iov_base = "B";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(1, recv(self->srv_allowed_fd,
+			  self->read_buf, sizeof(self->read_buf), 0)) {
+		TH_LOG("recv failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(self->read_buf[0], 'B');
+
+	/* Receive on an allowed port with recvfrom() */
+	self->testmsg_contents.iov_base = "C";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(1, recvfrom(self->srv_allowed_fd,
+			      self->read_buf, sizeof(self->read_buf), 0,
+			      (struct sockaddr *)&from, &from_len)) {
+		TH_LOG("recv failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(self->read_buf[0], 'C');
+
+	/* Receive on a denied port with read() */
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg, &self->srv_allowed));
+	self->testmsg_contents.iov_base = "D";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(-1, read(self->srv_denied_fd, self->read_buf, sizeof(self->read_buf)));
+	EXPECT_EQ(EACCES, errno);
+	EXPECT_EQ(self->read_buf[0], 'C');
+
+	/* Receive on an denied port with recv() */
+	self->testmsg_contents.iov_base = "B";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(-1, recv(self->srv_denied_fd, self->read_buf,
+			   sizeof(self->read_buf), 0));
+	EXPECT_EQ(EACCES, errno);
+	EXPECT_EQ(self->read_buf[0], 'C');
+
+	/* Receive on an denied port with recvfrom() */
+	self->testmsg_contents.iov_base = "C";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(-1, recvfrom(self->srv_denied_fd, self->read_buf,
+			       sizeof(self->read_buf), 0,
+			       (struct sockaddr *)&from, &from_len));
+	EXPECT_EQ(EACCES, errno);
+	EXPECT_EQ(self->read_buf[0], 'C');
+}
+
+TEST_F(udp_send_recv, recvmsg_on_ephemeral_port)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_net = LANDLOCK_ACCESS_NET_RECVMSG_UDP,
+	};
+	const struct landlock_net_port_attr allow_one_server = {
+		.allowed_access = LANDLOCK_ACCESS_NET_RECVMSG_UDP,
+		.port = 0,
+	};
+	const int ruleset_fd = landlock_create_ruleset(
+		&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd,
+				       LANDLOCK_RULE_NET_PORT,
+				       &allow_one_server, 0));
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	EXPECT_EQ(0, get_msg_addr_variant(&self->testmsg,
+					  &self->srv_ephemeral));
+
+	self->testmsg_contents.iov_base = "A";
+	EXPECT_EQ(1, sendmsg(self->client_fd, &self->testmsg, 0));
+	EXPECT_EQ(1, read(self->srv_ephemeral_fd, self->read_buf,
+			  sizeof(self->read_buf))) {
+		TH_LOG("read failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(self->read_buf[0], 'A');
+}
+
 FIXTURE(ipv4)
 {
 	struct service_fixture srv0, srv1;
-- 
2.39.5