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

Commit Message

Matthieu Baerts (NGI0) Oct. 22, 2024, 4:43 p.m. UTC

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) <[e-mail]>
---
 Documentation/netlink/specs/mptcp_pm.yaml | 1 -
 net/mptcp/mptcp_pm_gen.c                  | 1 -
 2 files changed, 2 deletions(-)


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

Best regards,

Comments

Davide Caratti Oct. 22, 2024, 5:11 p.m. UTC | #1

hello,

On Tue, Oct 22, 2024 at 6:43 PM Matthieu Baerts (NGI0)
<[e-mail]> 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 <[e-mail]>

MPTCP CI Oct. 22, 2024, 5:53 p.m. UTC | #2

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/11464957788

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


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)

Mat Martineau Oct. 23, 2024, 12:17 a.m. UTC | #3

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) <[e-mail]>

Looks good to me:

Reviewed-by: Mat Martineau <[e-mail]>

> ---
> 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) <[e-mail]>
>
>
>

Matthieu Baerts (NGI0) Oct. 23, 2024, 10:55 a.m. UTC | #4

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

Patch

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,