diff options
| author | Daniel Zahka <daniel.zahka@gmail.com> | 2026-09-30 16:46:47 -0700 |
|---|---|---|
| committer | Jakub Kicinski <kuba@kernel.org> | 2026-10-05 15:59:46 -0700 |
| commit | 86a19284425fb0c420e1702f8d0de891279df6b7 (patch) | |
| tree | 3ea0b0e4f03cd237ba3e4b9601ed6c34f8d172f9 /net/ipv6 | |
| parent | e404a2de7ed1a19ed356086e3b012e2227c1024d (diff) | |
net: psp: require an established connection for association setup
Check sk_state under the socket lock in both the rx-assoc and tx-assoc
handlers, and only allow association setup on sockets in
TCP_ESTABLISHED. Also, fail connect() when PSP assoc state is already
present, and remove the dead PSP MSS adjustment from
tcp_v[46]_connect().
The net effect of this commit is:
1. PSP assoc state can never exist on a listen socket.
2. The upgrade to PSP must be done while the socket is in
TCP_ESTABLISHED.
This change defeatures behavior that was previously allowed under the
PSP uapi. My justification is:
Nothing useful can be done after association setup on closed or listen
sockets today. Listen sockets could accept a PSP-encrypted TCP SYN, but
the child socket will not inherit any PSP state. On the other side,
establishing PSP state prior to connect() will result in a PSP-encrypted
TCP SYN sent to a listening peer, which in turn has the aforementioned
limitations. That implies that there cannot be any users of this
feature, so it should be safe to remove it from the PSP uapi.
In theory, the check in the tx-assoc path is more restrictive than
necessary. FIN_WAIT1/2, CLOSING, LAST_ACK and CLOSE_WAIT could be
allowed, and the peer would accept PSP-encrypted ACKs in the
post-FIN-sent states, or data in the half-close case, but it is simpler
to disallow those states because they don't fit the upgrade model.
The check in the tx-assoc path fixes a bug in commit 6b46ca260e22 ("net:
psp: add socket security association code") where an unsynchronized
write can be performed on an assoc shared with a timewait socket when
the socket is in TCP_CLOSE after shutdown. This commit is not targeted
at net because its premise of preventing listen sockets from holding
assoc state depends on net-next commit 8cc3aef0cb19 ("tcp: Do not allow
buggy transitions between ehash and lhash2.")
Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
Fixes: 6b46ca260e22 ("net: psp: add socket security association code")
Reviewed-by: Willem de Bruijn <willemb@google.com>
Link: https://patch.msgid.link/20260930-psp-defeat-v2-2-f266e7447129@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Diffstat (limited to 'net/ipv6')
| -rw-r--r-- | net/ipv6/tcp_ipv6.c | 9 |
1 files changed, 6 insertions, 3 deletions
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index afb4d8e9fa67..e006a30360ea 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -153,6 +153,9 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr, if (usin->sin6_family != AF_INET6) return -EAFNOSUPPORT; + if (psp_sk_assoc(sk)) + return -EINVAL; + fl6 = &inet_sk(sk)->cork.fl.u.ip6; memset(fl6, 0, sizeof(*fl6)); @@ -311,10 +314,10 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr, sk->sk_gso_type = SKB_GSO_TCPV6; ip6_dst_store(sk, dst, false, false); - icsk->icsk_ext_hdr_len = psp_sk_overhead(sk); + icsk->icsk_ext_hdr_len = 0; if (opt) - icsk->icsk_ext_hdr_len += opt->opt_flen + - opt->opt_nflen; + icsk->icsk_ext_hdr_len = opt->opt_flen + + opt->opt_nflen; tp->rx_opt.mss_clamp = IPV6_MIN_MTU - sizeof(struct tcphdr) - sizeof(struct ipv6hdr); |
