[PATCH mptcp-net] mptcp: no admin perm to list endpoints

Matthieu Baerts (NGI0) posted 1 patch 2 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20241022-mptcp-pm-dump-addr-admin-v1-1-9ad328d01817@kernel.org
Documentation/netlink/specs/mptcp_pm.yaml | 1 -
net/mptcp/mptcp_pm_gen.c                  | 1 -
2 files changed, 2 deletions(-)
[PATCH mptcp-net] mptcp: no admin perm to list endpoints
Posted by Matthieu Baerts (NGI0) 2 weeks, 5 days ago
During the switch to YNL, the command to list all endpoints has been
accidentally restricted to users with admin permissions.

It looks like there are no reasons to have this restriction which makes
it harder for a user to quickly check if the endpoint list has been
correctly populated by an automated tool. Best to go back to the
previous behaviour then.

mptcp_pm_gen.c has been modified using ynl-gen-c.py:

   $ ./tools/net/ynl/ynl-gen-c.py --mode kernel \
     --spec Documentation/netlink/specs/mptcp_pm.yaml --source \
     -o net/mptcp/mptcp_pm_gen.c

The header file doesn't need to be regenerated.

Fixes: 1d0507f46843 ("net: mptcp: convert netlink from small_ops to ops")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/netlink/specs/mptcp_pm.yaml | 1 -
 net/mptcp/mptcp_pm_gen.c                  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index 30d8342cacc8704c42b84c9e03f96c906e81733e..dc190bf838fec6add28b61e5e2cac8dee601b012 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -293,7 +293,6 @@ operations:
       doc: Get endpoint information
       attribute-set: attr
       dont-validate: [ strict ]
-      flags: [ uns-admin-perm ]
       do: &get-addr-attrs
         request:
           attributes:
diff --git a/net/mptcp/mptcp_pm_gen.c b/net/mptcp/mptcp_pm_gen.c
index c30a2a90a19252dd41a74109d5762a091129269d..bfb37c5a88c4ef90740699dfda345b52e206966b 100644
--- a/net/mptcp/mptcp_pm_gen.c
+++ b/net/mptcp/mptcp_pm_gen.c
@@ -112,7 +112,6 @@ const struct genl_ops mptcp_pm_nl_ops[11] = {
 		.dumpit		= mptcp_pm_nl_get_addr_dumpit,
 		.policy		= mptcp_pm_get_addr_nl_policy,
 		.maxattr	= MPTCP_PM_ATTR_TOKEN,
-		.flags		= GENL_UNS_ADMIN_PERM,
 	},
 	{
 		.cmd		= MPTCP_PM_CMD_FLUSH_ADDRS,

---
base-commit: 66bee427d4db76bd1afbf0184a2fd1f238a4ee27
change-id: 20241022-mptcp-pm-dump-addr-admin-0b226758e9f5

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>
Re: [PATCH mptcp-net] mptcp: no admin perm to list endpoints
Posted by Matthieu Baerts 2 weeks, 4 days ago
Hi Davide, Mat,

On 22/10/2024 18:43, Matthieu Baerts (NGI0) wrote:
> During the switch to YNL, the command to list all endpoints has been
> accidentally restricted to users with admin permissions.
> 
> It looks like there are no reasons to have this restriction which makes
> it harder for a user to quickly check if the endpoint list has been
> correctly populated by an automated tool. Best to go back to the
> previous behaviour then.

Thank you for the quick review!

Now in our tree:

New patches for t/upstream-net and t/upstream:
- 983e36f915c9: mptcp: no admin perm to list endpoints
- Results: 92052b68f397..0a8a1adbb6dc (export-net)
- Results: 00ca859b77be..608ba5fe2f44 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/9050db2fb737cfee7cbea717bfce23c98a493f7d/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/e11b0188e765c92ab69435e015ab77a3e94b769c/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-net] mptcp: no admin perm to list endpoints
Posted by Mat Martineau 2 weeks, 4 days ago
On Tue, 22 Oct 2024, Matthieu Baerts (NGI0) wrote:

> During the switch to YNL, the command to list all endpoints has been
> accidentally restricted to users with admin permissions.
>
> It looks like there are no reasons to have this restriction which makes
> it harder for a user to quickly check if the endpoint list has been
> correctly populated by an automated tool. Best to go back to the
> previous behaviour then.
>
> mptcp_pm_gen.c has been modified using ynl-gen-c.py:
>
>   $ ./tools/net/ynl/ynl-gen-c.py --mode kernel \
>     --spec Documentation/netlink/specs/mptcp_pm.yaml --source \
>     -o net/mptcp/mptcp_pm_gen.c
>
> The header file doesn't need to be regenerated.
>
> Fixes: 1d0507f46843 ("net: mptcp: convert netlink from small_ops to ops")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Looks good to me:

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

> ---
> Documentation/netlink/specs/mptcp_pm.yaml | 1 -
> net/mptcp/mptcp_pm_gen.c                  | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
> index 30d8342cacc8704c42b84c9e03f96c906e81733e..dc190bf838fec6add28b61e5e2cac8dee601b012 100644
> --- a/Documentation/netlink/specs/mptcp_pm.yaml
> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> @@ -293,7 +293,6 @@ operations:
>       doc: Get endpoint information
>       attribute-set: attr
>       dont-validate: [ strict ]
> -      flags: [ uns-admin-perm ]
>       do: &get-addr-attrs
>         request:
>           attributes:
> diff --git a/net/mptcp/mptcp_pm_gen.c b/net/mptcp/mptcp_pm_gen.c
> index c30a2a90a19252dd41a74109d5762a091129269d..bfb37c5a88c4ef90740699dfda345b52e206966b 100644
> --- a/net/mptcp/mptcp_pm_gen.c
> +++ b/net/mptcp/mptcp_pm_gen.c
> @@ -112,7 +112,6 @@ const struct genl_ops mptcp_pm_nl_ops[11] = {
> 		.dumpit		= mptcp_pm_nl_get_addr_dumpit,
> 		.policy		= mptcp_pm_get_addr_nl_policy,
> 		.maxattr	= MPTCP_PM_ATTR_TOKEN,
> -		.flags		= GENL_UNS_ADMIN_PERM,
> 	},
> 	{
> 		.cmd		= MPTCP_PM_CMD_FLUSH_ADDRS,
>
> ---
> base-commit: 66bee427d4db76bd1afbf0184a2fd1f238a4ee27
> change-id: 20241022-mptcp-pm-dump-addr-admin-0b226758e9f5
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>
Re: [PATCH mptcp-net] mptcp: no admin perm to list endpoints
Posted by Davide Caratti 2 weeks, 5 days ago
hello,

On Tue, Oct 22, 2024 at 6:43 PM Matthieu Baerts (NGI0)
<matttbe@kernel.org> wrote:
>
> During the switch to YNL, the command to list all endpoints has been
> accidentally restricted to users with admin permissions.
>

right _ the permission was preserved with MPTCP_PM_CMD_GET_LIMITS but
not with MPTCP_PM_CMD_GET_ADDR.

Reviewed-by: Davide Caratti <dcaratti@redhat.com>