Re: [PATCH 1/9] handshake: Require admin permission for DONE command

On Fri, 2026-06-05 at 13:34 -0400, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
> 
> ACCEPT and DONE are the two downcalls of the handshake genl
> family, both intended for use by the trusted handshake agent
> (tlshd). ACCEPT already requires GENL_ADMIN_PERM; DONE has
> no privilege check at all.
> 
> The fd-lookup in handshake_nl_done_doit() only confirms that
> some pending handshake request exists for the supplied sockfd;
> it does not authenticate the sender. An unprivileged process
> that guesses or observes a valid sockfd can therefore submit
> a DONE with HANDSHAKE_A_DONE_STATUS == 0, leaving the kernel
> consumer to proceed as if the handshake succeeded. A non-zero
> status on a forged DONE tears down a legitimate in-flight
> handshake before tlshd can report its real result.
> 
> A subsequent patch teaches the DONE handler to carry session
> tags consumed for access control. That work makes closing the
> existing gap a prerequisite, but the gap itself predates tags.
> 
> Fixes: 3b3009ea8abb ("net/handshake: Create a NETLINK service for handling handshake requests")
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>  Documentation/netlink/specs/handshake.yaml | 1 +
>  net/handshake/genl.c                       | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
> index 95c3fade7a8d..24f5a0ac5920 100644
> --- a/Documentation/netlink/specs/handshake.yaml
> +++ b/Documentation/netlink/specs/handshake.yaml
> @@ -117,6 +117,7 @@ operations:
>        name: done
>        doc: Handler reports handshake completion
>        attribute-set: done
> +      flags: [admin-perm]
>        do:
>          request:
>            attributes:
> diff --git a/net/handshake/genl.c b/net/handshake/genl.c
> index 870612609491..791c45671cd6 100644
> --- a/net/handshake/genl.c
> +++ b/net/handshake/genl.c
> @@ -37,7 +37,7 @@ static const struct genl_split_ops handshake_nl_ops[] = {
>  		.doit		= handshake_nl_done_doit,
>  		.policy		= handshake_done_nl_policy,
>  		.maxattr	= HANDSHAKE_A_DONE_REMOTE_AUTH,
> -		.flags		= GENL_CMD_CAP_DO,
> +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
>  	},
>  };
>  

Seems like this ought to go in ahead of the rest of the set? tlshd
generally runs as root anyway so I don't forsee a problem just doing
this:

Reviewed-by: Jeff Layton <[email protected]>