[PATCH mptcp-next v2 0/2] tcp: ulp: diag: remove net admin restriction

[PATCH mptcp-next v2 0/2] tcp: ulp: diag: remove net admin restriction

[Matthieu Baerts (NGI0)]

Since its introduction in commit 61723b393292 ("tcp: ulp: add functions
to dump ulp-specific information"), the ULP diag info have been exported
only if the requester had CAP_NET_ADMIN.

Not everything is sensitive, and some info can be exported to all users
in order to ease the debugging from the userspace side without requiring
additional capabilities.

First, the ULP name can be easily exported. Then more depending on each
layer.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v2:
- Do not export MPTCP-related sequence numbers per subflow (Mat).
- The single patch has then been split in two to ease the reviews.
- Link to v1: https://lore.kernel.org/r/20250226-mptcp-tcp-ulp-diag-cap-v1-1-e1a003ad0606@kernel.org

---
Matthieu Baerts (NGI0) (2):
      tcp: ulp: diag: always print the name if any
      tcp: ulp: diag: more info without CAP_NET_ADMIN

 include/net/tcp.h   |  4 ++--
 net/ipv4/tcp_diag.c | 21 ++++++++++-----------
 net/mptcp/diag.c    | 42 ++++++++++++++++++++++++++----------------
 net/tls/tls_main.c  |  4 ++--
 4 files changed, 40 insertions(+), 31 deletions(-)
---
base-commit: 62dab9e8d979a45d72cebb74030b937dc5ba9452
change-id: 20250226-mptcp-tcp-ulp-diag-cap-a4d9b7cd91ec

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next v2 1/2] tcp: ulp: diag: always print the name if any

[Matthieu Baerts (NGI0)]

Since its introduction in commit 61723b393292 ("tcp: ulp: add functions
to dump ulp-specific information"), the ULP diag info have been exported
only if the requester had CAP_NET_ADMIN.

At least the ULP name can be exported without CAP_NET_ADMIN. This will
already help identifying which layer is being used, e.g. which TCP
connections are in fact MPTCP subflow.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/ipv4/tcp_diag.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
index f428ecf9120f2f596e1d67db2b2a0d0d0e211905..d8bba37dbffd8c6cc7fab2328a88b6ce6ea3e9f4 100644
--- a/net/ipv4/tcp_diag.c
+++ b/net/ipv4/tcp_diag.c
@@ -83,7 +83,7 @@ static int tcp_diag_put_md5sig(struct sk_buff *skb,
 #endif
 
 static int tcp_diag_put_ulp(struct sk_buff *skb, struct sock *sk,
-			    const struct tcp_ulp_ops *ulp_ops)
+			    const struct tcp_ulp_ops *ulp_ops, bool net_admin)
 {
 	struct nlattr *nest;
 	int err;
@@ -96,7 +96,7 @@ static int tcp_diag_put_ulp(struct sk_buff *skb, struct sock *sk,
 	if (err)
 		goto nla_failure;
 
-	if (ulp_ops->get_info)
+	if (net_admin && ulp_ops->get_info)
 		err = ulp_ops->get_info(sk, skb);
 	if (err)
 		goto nla_failure;
@@ -113,6 +113,7 @@ static int tcp_diag_get_aux(struct sock *sk, bool net_admin,
 			    struct sk_buff *skb)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
+	const struct tcp_ulp_ops *ulp_ops;
 	int err = 0;
 
 #ifdef CONFIG_TCP_MD5SIG
@@ -129,15 +130,13 @@ static int tcp_diag_get_aux(struct sock *sk, bool net_admin,
 	}
 #endif
 
-	if (net_admin) {
-		const struct tcp_ulp_ops *ulp_ops;
-
-		ulp_ops = icsk->icsk_ulp_ops;
-		if (ulp_ops)
-			err = tcp_diag_put_ulp(skb, sk, ulp_ops);
-		if (err)
+	ulp_ops = icsk->icsk_ulp_ops;
+	if (ulp_ops) {
+		err = tcp_diag_put_ulp(skb, sk, ulp_ops, net_admin);
+		if (err < 0)
 			return err;
 	}
+
 	return 0;
 }
 
@@ -164,14 +163,14 @@ static size_t tcp_diag_get_aux_size(struct sock *sk, bool net_admin)
 	}
 #endif
 
-	if (net_admin && sk_fullsock(sk)) {
+	if (sk_fullsock(sk)) {
 		const struct tcp_ulp_ops *ulp_ops;
 
 		ulp_ops = icsk->icsk_ulp_ops;
 		if (ulp_ops) {
 			size += nla_total_size(0) +
 				nla_total_size(TCP_ULP_NAME_MAX);
-			if (ulp_ops->get_info_size)
+			if (net_admin && ulp_ops->get_info_size)
 				size += ulp_ops->get_info_size(sk);
 		}
 	}

-- 
2.47.1

[PATCH mptcp-next v2 2/2] tcp: ulp: diag: more info without CAP_NET_ADMIN

[Matthieu Baerts (NGI0)]

When introduced in commit 61723b393292 ("tcp: ulp: add functions to dump
ulp-specific information"), the whole ULP diag info has been exported
only if the requester had CAP_NET_ADMIN.

It looks like not everything is sensitive, and some info can be exported
to all users in order to ease the debugging from the userspace side
without requiring additional capabilities. Each layer should then decide
what can be exposed to everybody. The 'net_admin' boolean is then passed
to the different layers.

On kTLS side, it looks like there is nothing sensitive there, only some
metadata about the configuration, no cryptographic information. Then,
everything can be exported to all users.

On MPTCP side, that's different. The MPTCP-related sequence numbers per
subflow should certainly not be exposed to everybody. For example, the
DSS mapping and ssn_offset would give all users on the system access to
narrow ranges of values for the subflow TCP sequence numbers and
MPTCP-level DSNs, and then ease packet injection. The TCP diag interface
doesn't expose the TCP sequence numbers for TCP sockets, so best to do
the same here.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/net/tcp.h   |  4 ++--
 net/ipv4/tcp_diag.c |  8 ++++----
 net/mptcp/diag.c    | 42 ++++++++++++++++++++++++++----------------
 net/tls/tls_main.c  |  4 ++--
 4 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index a9bc959fb102fc6697b4a664b3773b47b3309f13..7207c52b1fc9ce3cd9cf2a8580310d0e629f82d6 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2598,8 +2598,8 @@ struct tcp_ulp_ops {
 	/* cleanup ulp */
 	void (*release)(struct sock *sk);
 	/* diagnostic */
-	int (*get_info)(struct sock *sk, struct sk_buff *skb);
-	size_t (*get_info_size)(const struct sock *sk);
+	int (*get_info)(struct sock *sk, struct sk_buff *skb, bool net_admin);
+	size_t (*get_info_size)(const struct sock *sk, bool net_admin);
 	/* clone ulp */
 	void (*clone)(const struct request_sock *req, struct sock *newsk,
 		      const gfp_t priority);
diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
index d8bba37dbffd8c6cc7fab2328a88b6ce6ea3e9f4..45e174b8cd22173b6b8eeffe71df334c45498b15 100644
--- a/net/ipv4/tcp_diag.c
+++ b/net/ipv4/tcp_diag.c
@@ -96,8 +96,8 @@ static int tcp_diag_put_ulp(struct sk_buff *skb, struct sock *sk,
 	if (err)
 		goto nla_failure;
 
-	if (net_admin && ulp_ops->get_info)
-		err = ulp_ops->get_info(sk, skb);
+	if (ulp_ops->get_info)
+		err = ulp_ops->get_info(sk, skb, net_admin);
 	if (err)
 		goto nla_failure;
 
@@ -170,8 +170,8 @@ static size_t tcp_diag_get_aux_size(struct sock *sk, bool net_admin)
 		if (ulp_ops) {
 			size += nla_total_size(0) +
 				nla_total_size(TCP_ULP_NAME_MAX);
-			if (net_admin && ulp_ops->get_info_size)
-				size += ulp_ops->get_info_size(sk);
+			if (ulp_ops->get_info_size)
+				size += ulp_ops->get_info_size(sk, net_admin);
 		}
 	}
 	return size;
diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
index 02205f7994d752cc505991efdf7aa0bbbfd830db..70cf9ebce8338bde3b0bb10fc8620905b15f5190 100644
--- a/net/mptcp/diag.c
+++ b/net/mptcp/diag.c
@@ -12,7 +12,7 @@
 #include <net/netlink.h>
 #include "protocol.h"
 
-static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
+static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin)
 {
 	struct mptcp_subflow_context *sf;
 	struct nlattr *start;
@@ -56,15 +56,6 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
 
 	if (nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_TOKEN_REM, sf->remote_token) ||
 	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_TOKEN_LOC, sf->token) ||
-	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ,
-			sf->rel_write_seq) ||
-	    nla_put_u64_64bit(skb, MPTCP_SUBFLOW_ATTR_MAP_SEQ, sf->map_seq,
-			      MPTCP_SUBFLOW_ATTR_PAD) ||
-	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_MAP_SFSEQ,
-			sf->map_subflow_seq) ||
-	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_SSN_OFFSET, sf->ssn_offset) ||
-	    nla_put_u16(skb, MPTCP_SUBFLOW_ATTR_MAP_DATALEN,
-			sf->map_data_len) ||
 	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_FLAGS, flags) ||
 	    nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_REM, sf->remote_id) ||
 	    nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_LOC, subflow_get_local_id(sf))) {
@@ -72,6 +63,21 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
 		goto nla_failure;
 	}
 
+	/* Only export seq related counters to user with CAP_NET_ADMIN */
+	if (net_admin &&
+	    (nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ,
+			 sf->rel_write_seq) ||
+	     nla_put_u64_64bit(skb, MPTCP_SUBFLOW_ATTR_MAP_SEQ, sf->map_seq,
+			       MPTCP_SUBFLOW_ATTR_PAD) ||
+	     nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_MAP_SFSEQ,
+			 sf->map_subflow_seq) ||
+	     nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_SSN_OFFSET, sf->ssn_offset) ||
+	     nla_put_u16(skb, MPTCP_SUBFLOW_ATTR_MAP_DATALEN,
+			 sf->map_data_len))) {
+		err = -EMSGSIZE;
+		goto nla_failure;
+	}
+
 	rcu_read_unlock();
 	unlock_sock_fast(sk, slow);
 	nla_nest_end(skb, start);
@@ -84,22 +90,26 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
 	return err;
 }
 
-static size_t subflow_get_info_size(const struct sock *sk)
+static size_t subflow_get_info_size(const struct sock *sk, bool net_admin)
 {
 	size_t size = 0;
 
 	size += nla_total_size(0) +	/* INET_ULP_INFO_MPTCP */
 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_TOKEN_REM */
 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_TOKEN_LOC */
-		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ */
-		nla_total_size_64bit(8) +	/* MPTCP_SUBFLOW_ATTR_MAP_SEQ */
-		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_MAP_SFSEQ */
-		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_SSN_OFFSET */
-		nla_total_size(2) +	/* MPTCP_SUBFLOW_ATTR_MAP_DATALEN */
 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_FLAGS */
 		nla_total_size(1) +	/* MPTCP_SUBFLOW_ATTR_ID_REM */
 		nla_total_size(1) +	/* MPTCP_SUBFLOW_ATTR_ID_LOC */
 		0;
+
+	if (net_admin)
+		size += nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ */
+			nla_total_size_64bit(8) +	/* MPTCP_SUBFLOW_ATTR_MAP_SEQ */
+			nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_MAP_SFSEQ */
+			nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_SSN_OFFSET */
+			nla_total_size(2) +	/* MPTCP_SUBFLOW_ATTR_MAP_DATALEN */
+			0;
+
 	return size;
 }
 
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 99ca4465f70216c5a44e4ca7477df0e93df6b76d..cb86b0bf9a53e1ff060d8e69eddbd6acfbee5194 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -1057,7 +1057,7 @@ static u16 tls_user_config(struct tls_context *ctx, bool tx)
 	return 0;
 }
 
-static int tls_get_info(struct sock *sk, struct sk_buff *skb)
+static int tls_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin)
 {
 	u16 version, cipher_type;
 	struct tls_context *ctx;
@@ -1115,7 +1115,7 @@ static int tls_get_info(struct sock *sk, struct sk_buff *skb)
 	return err;
 }
 
-static size_t tls_get_info_size(const struct sock *sk)
+static size_t tls_get_info_size(const struct sock *sk, bool net_admin)
 {
 	size_t size = 0;
 

-- 
2.47.1

Re: [PATCH mptcp-next v2 0/2] tcp: ulp: diag: remove net admin restriction

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/13683325743

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1ef9eed1fd7c
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=940679


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next v2 1/2] tcp: ulp: diag: always print the name if any

[Mat Martineau]

On Wed, 5 Mar 2025, Matthieu Baerts (NGI0) wrote:

> Since its introduction in commit 61723b393292 ("tcp: ulp: add functions
> to dump ulp-specific information"), the ULP diag info have been exported
> only if the requester had CAP_NET_ADMIN.
>
> At least the ULP name can be exported without CAP_NET_ADMIN. This will
> already help identifying which layer is being used, e.g. which TCP
> connections are in fact MPTCP subflow.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> net/ipv4/tcp_diag.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)

Hi Matthieu -

Thanks for the v2, and also for splitting up the series. These TCP changes 
look good to me:

Acked-by: Mat Martineau <martineau@kernel.org>


>
> diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
> index f428ecf9120f2f596e1d67db2b2a0d0d0e211905..d8bba37dbffd8c6cc7fab2328a88b6ce6ea3e9f4 100644
> --- a/net/ipv4/tcp_diag.c
> +++ b/net/ipv4/tcp_diag.c
> @@ -83,7 +83,7 @@ static int tcp_diag_put_md5sig(struct sk_buff *skb,
> #endif
>
> static int tcp_diag_put_ulp(struct sk_buff *skb, struct sock *sk,
> -			    const struct tcp_ulp_ops *ulp_ops)
> +			    const struct tcp_ulp_ops *ulp_ops, bool net_admin)
> {
> 	struct nlattr *nest;
> 	int err;
> @@ -96,7 +96,7 @@ static int tcp_diag_put_ulp(struct sk_buff *skb, struct sock *sk,
> 	if (err)
> 		goto nla_failure;
>
> -	if (ulp_ops->get_info)
> +	if (net_admin && ulp_ops->get_info)
> 		err = ulp_ops->get_info(sk, skb);
> 	if (err)
> 		goto nla_failure;
> @@ -113,6 +113,7 @@ static int tcp_diag_get_aux(struct sock *sk, bool net_admin,
> 			    struct sk_buff *skb)
> {
> 	struct inet_connection_sock *icsk = inet_csk(sk);
> +	const struct tcp_ulp_ops *ulp_ops;
> 	int err = 0;
>
> #ifdef CONFIG_TCP_MD5SIG
> @@ -129,15 +130,13 @@ static int tcp_diag_get_aux(struct sock *sk, bool net_admin,
> 	}
> #endif
>
> -	if (net_admin) {
> -		const struct tcp_ulp_ops *ulp_ops;
> -
> -		ulp_ops = icsk->icsk_ulp_ops;
> -		if (ulp_ops)
> -			err = tcp_diag_put_ulp(skb, sk, ulp_ops);
> -		if (err)
> +	ulp_ops = icsk->icsk_ulp_ops;
> +	if (ulp_ops) {
> +		err = tcp_diag_put_ulp(skb, sk, ulp_ops, net_admin);
> +		if (err < 0)
> 			return err;
> 	}
> +
> 	return 0;
> }
>
> @@ -164,14 +163,14 @@ static size_t tcp_diag_get_aux_size(struct sock *sk, bool net_admin)
> 	}
> #endif
>
> -	if (net_admin && sk_fullsock(sk)) {
> +	if (sk_fullsock(sk)) {
> 		const struct tcp_ulp_ops *ulp_ops;
>
> 		ulp_ops = icsk->icsk_ulp_ops;
> 		if (ulp_ops) {
> 			size += nla_total_size(0) +
> 				nla_total_size(TCP_ULP_NAME_MAX);
> -			if (ulp_ops->get_info_size)
> +			if (net_admin && ulp_ops->get_info_size)
> 				size += ulp_ops->get_info_size(sk);
> 		}
> 	}
>
> -- 
> 2.47.1
>
>

Re: [PATCH mptcp-next v2 2/2] tcp: ulp: diag: more info without CAP_NET_ADMIN

[Mat Martineau]

On Wed, 5 Mar 2025, Matthieu Baerts (NGI0) wrote:

> When introduced in commit 61723b393292 ("tcp: ulp: add functions to dump
> ulp-specific information"), the whole ULP diag info has been exported
> only if the requester had CAP_NET_ADMIN.
>
> It looks like not everything is sensitive, and some info can be exported
> to all users in order to ease the debugging from the userspace side
> without requiring additional capabilities. Each layer should then decide
> what can be exposed to everybody. The 'net_admin' boolean is then passed
> to the different layers.
>
> On kTLS side, it looks like there is nothing sensitive there, only some
> metadata about the configuration, no cryptographic information. Then,
> everything can be exported to all users.
>
> On MPTCP side, that's different. The MPTCP-related sequence numbers per
> subflow should certainly not be exposed to everybody. For example, the
> DSS mapping and ssn_offset would give all users on the system access to
> narrow ranges of values for the subflow TCP sequence numbers and
> MPTCP-level DSNs, and then ease packet injection. The TCP diag interface
> doesn't expose the TCP sequence numbers for TCP sockets, so best to do
> the same here.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> include/net/tcp.h   |  4 ++--
> net/ipv4/tcp_diag.c |  8 ++++----
> net/mptcp/diag.c    | 42 ++++++++++++++++++++++++++----------------
> net/tls/tls_main.c  |  4 ++--
> 4 files changed, 34 insertions(+), 24 deletions(-)

Matthieu -

This patch also LGTM:

Acked-by: Mat Martineau <martineau@kernel.org>

>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index a9bc959fb102fc6697b4a664b3773b47b3309f13..7207c52b1fc9ce3cd9cf2a8580310d0e629f82d6 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2598,8 +2598,8 @@ struct tcp_ulp_ops {
> 	/* cleanup ulp */
> 	void (*release)(struct sock *sk);
> 	/* diagnostic */
> -	int (*get_info)(struct sock *sk, struct sk_buff *skb);
> -	size_t (*get_info_size)(const struct sock *sk);
> +	int (*get_info)(struct sock *sk, struct sk_buff *skb, bool net_admin);
> +	size_t (*get_info_size)(const struct sock *sk, bool net_admin);
> 	/* clone ulp */
> 	void (*clone)(const struct request_sock *req, struct sock *newsk,
> 		      const gfp_t priority);
> diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
> index d8bba37dbffd8c6cc7fab2328a88b6ce6ea3e9f4..45e174b8cd22173b6b8eeffe71df334c45498b15 100644
> --- a/net/ipv4/tcp_diag.c
> +++ b/net/ipv4/tcp_diag.c
> @@ -96,8 +96,8 @@ static int tcp_diag_put_ulp(struct sk_buff *skb, struct sock *sk,
> 	if (err)
> 		goto nla_failure;
>
> -	if (net_admin && ulp_ops->get_info)
> -		err = ulp_ops->get_info(sk, skb);
> +	if (ulp_ops->get_info)
> +		err = ulp_ops->get_info(sk, skb, net_admin);
> 	if (err)
> 		goto nla_failure;
>
> @@ -170,8 +170,8 @@ static size_t tcp_diag_get_aux_size(struct sock *sk, bool net_admin)
> 		if (ulp_ops) {
> 			size += nla_total_size(0) +
> 				nla_total_size(TCP_ULP_NAME_MAX);
> -			if (net_admin && ulp_ops->get_info_size)
> -				size += ulp_ops->get_info_size(sk);
> +			if (ulp_ops->get_info_size)
> +				size += ulp_ops->get_info_size(sk, net_admin);
> 		}
> 	}
> 	return size;
> diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
> index 02205f7994d752cc505991efdf7aa0bbbfd830db..70cf9ebce8338bde3b0bb10fc8620905b15f5190 100644
> --- a/net/mptcp/diag.c
> +++ b/net/mptcp/diag.c
> @@ -12,7 +12,7 @@
> #include <net/netlink.h>
> #include "protocol.h"
>
> -static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
> +static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin)
> {
> 	struct mptcp_subflow_context *sf;
> 	struct nlattr *start;
> @@ -56,15 +56,6 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
>
> 	if (nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_TOKEN_REM, sf->remote_token) ||
> 	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_TOKEN_LOC, sf->token) ||
> -	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ,
> -			sf->rel_write_seq) ||
> -	    nla_put_u64_64bit(skb, MPTCP_SUBFLOW_ATTR_MAP_SEQ, sf->map_seq,
> -			      MPTCP_SUBFLOW_ATTR_PAD) ||
> -	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_MAP_SFSEQ,
> -			sf->map_subflow_seq) ||
> -	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_SSN_OFFSET, sf->ssn_offset) ||
> -	    nla_put_u16(skb, MPTCP_SUBFLOW_ATTR_MAP_DATALEN,
> -			sf->map_data_len) ||
> 	    nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_FLAGS, flags) ||
> 	    nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_REM, sf->remote_id) ||
> 	    nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_LOC, subflow_get_local_id(sf))) {
> @@ -72,6 +63,21 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
> 		goto nla_failure;
> 	}
>
> +	/* Only export seq related counters to user with CAP_NET_ADMIN */
> +	if (net_admin &&
> +	    (nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ,
> +			 sf->rel_write_seq) ||
> +	     nla_put_u64_64bit(skb, MPTCP_SUBFLOW_ATTR_MAP_SEQ, sf->map_seq,
> +			       MPTCP_SUBFLOW_ATTR_PAD) ||
> +	     nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_MAP_SFSEQ,
> +			 sf->map_subflow_seq) ||
> +	     nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_SSN_OFFSET, sf->ssn_offset) ||
> +	     nla_put_u16(skb, MPTCP_SUBFLOW_ATTR_MAP_DATALEN,
> +			 sf->map_data_len))) {
> +		err = -EMSGSIZE;
> +		goto nla_failure;
> +	}
> +
> 	rcu_read_unlock();
> 	unlock_sock_fast(sk, slow);
> 	nla_nest_end(skb, start);
> @@ -84,22 +90,26 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb)
> 	return err;
> }
>
> -static size_t subflow_get_info_size(const struct sock *sk)
> +static size_t subflow_get_info_size(const struct sock *sk, bool net_admin)
> {
> 	size_t size = 0;
>
> 	size += nla_total_size(0) +	/* INET_ULP_INFO_MPTCP */
> 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_TOKEN_REM */
> 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_TOKEN_LOC */
> -		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ */
> -		nla_total_size_64bit(8) +	/* MPTCP_SUBFLOW_ATTR_MAP_SEQ */
> -		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_MAP_SFSEQ */
> -		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_SSN_OFFSET */
> -		nla_total_size(2) +	/* MPTCP_SUBFLOW_ATTR_MAP_DATALEN */
> 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_FLAGS */
> 		nla_total_size(1) +	/* MPTCP_SUBFLOW_ATTR_ID_REM */
> 		nla_total_size(1) +	/* MPTCP_SUBFLOW_ATTR_ID_LOC */
> 		0;
> +
> +	if (net_admin)
> +		size += nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ */
> +			nla_total_size_64bit(8) +	/* MPTCP_SUBFLOW_ATTR_MAP_SEQ */
> +			nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_MAP_SFSEQ */
> +			nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_SSN_OFFSET */
> +			nla_total_size(2) +	/* MPTCP_SUBFLOW_ATTR_MAP_DATALEN */
> +			0;
> +
> 	return size;
> }
>
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 99ca4465f70216c5a44e4ca7477df0e93df6b76d..cb86b0bf9a53e1ff060d8e69eddbd6acfbee5194 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -1057,7 +1057,7 @@ static u16 tls_user_config(struct tls_context *ctx, bool tx)
> 	return 0;
> }
>
> -static int tls_get_info(struct sock *sk, struct sk_buff *skb)
> +static int tls_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin)
> {
> 	u16 version, cipher_type;
> 	struct tls_context *ctx;
> @@ -1115,7 +1115,7 @@ static int tls_get_info(struct sock *sk, struct sk_buff *skb)
> 	return err;
> }
>
> -static size_t tls_get_info_size(const struct sock *sk)
> +static size_t tls_get_info_size(const struct sock *sk, bool net_admin)
> {
> 	size_t size = 0;
>
>
> -- 
> 2.47.1
>
>

Re: [PATCH mptcp-next v2 0/2] tcp: ulp: diag: remove net admin restriction

[Matthieu Baerts]

Hi Mat,

On 05/03/2025 19:34, Matthieu Baerts (NGI0) wrote:
> Since its introduction in commit 61723b393292 ("tcp: ulp: add functions
> to dump ulp-specific information"), the ULP diag info have been exported
> only if the requester had CAP_NET_ADMIN.
> 
> Not everything is sensitive, and some info can be exported to all users
> in order to ease the debugging from the userspace side without requiring
> additional capabilities.
> 
> First, the ULP name can be easily exported. Then more depending on each
> layer.

Thank you for the review!

Now in our tree (feat. for net-next):

New patches for t/upstream:
- 6040da37f014: tcp: ulp: diag: always print the name if any
- 61da849b8936: tcp: ulp: diag: more info without CAP_NET_ADMIN
- Results: ccf37aa90567..8d69c228371f (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/c396630c43dca77f3d618bf9a46ac4040901c4aa/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.