Re: [PATCH] tcp: fix authentication bypass via unsupported TCP auth options

["Jérémy Jean" <jeremy.jean@oss.cyber.gouv.fr>]

On 2026-11-07 23:23, Kuniyuki Iwashima wrote:
> On Thu, 22 Oct 2026 20:36:53 +0200, Jérémy Jean wrote:
>> On 2026-10-22 16:46, Eric Dumazet wrote:
>> > Le mer. 21 oct. 2026 à 21:32, Jérémy Jean
>> > <Jeremy.Jean@oss.cyber.gouv.fr> a écrit :
>> > >
>> > > tcp_inbound_hash() parses both TCP-MD5 and TCP-AO options when at
>> > > least one of the two features is enabled. With TCP_MD5SIG=y and
>> > > TCP_AO=n, a packet with an AO option reaches the disabled
>> > > tcp_inbound_ao_hash() stub. That stub returns success, so sockets
>> > > configured to require MD5 authentication end up accepting
>> > > unauthenticated data.
>> > >
>> > > The inverse configuration with TCP_MD5SIG=n and TCP_AO=y has the same
>> > > problem for an MD5 option on AO sockets.
>> > >
>> > > Make the disabled stubs reject options for the unsupported
>> > > authentication scheme.
>> > >
>> > > Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments")
>> > > Cc: stable@vger.kernel.org
>> > > Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> > > ---
>> > >  include/net/tcp_ao.h | 2 +-
>> > >  net/ipv4/tcp.c       | 2 +-
>> > >  2 files changed, 2 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
>> > > index 9a2333e62e994..bb249d6df8723 100644
>> > > --- a/include/net/tcp_ao.h
>> > > +++ b/include/net/tcp_ao.h
>> > > @@ -303,7 +303,7 @@ static inline enum skb_drop_reason
>> > > tcp_inbound_ao_hash(struct sock *sk,
>> > >                 const struct request_sock *req, int l3index,
>> > >                 const struct tcp_ao_hdr *aoh)
>> > >  {
>> > > -       return SKB_NOT_DROPPED_YET;
>> > > +       return SKB_DROP_REASON_TCP_AOUNEXPECTED;
>> > >  }
>> > >
>> > >  static inline struct tcp_ao_key *tcp_ao_do_lookup(const struct sock
>> > > *sk,
>> > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> > > index 87ef6d5cbfeba..7d800b6283bd2 100644
>> > > --- a/net/ipv4/tcp.c
>> > > +++ b/net/ipv4/tcp.c
>> > > @@ -4962,7 +4962,7 @@ tcp_inbound_md5_hash(const struct sock *sk,
>> > > const struct sk_buff *skb,
>> > >                      const void *saddr, const void *daddr,
>> > >                      int family, int l3index, const __u8
>> > > *hash_location)
>> > >  {
>> > > -       return SKB_NOT_DROPPED_YET;
>> > > +       return SKB_DROP_REASON_TCP_MD5UNEXPECTED;
>> > >  }
>> > >
>> > >  #endif
>> > > --
>> > > 2.47.3
>> > >
>> >
>> > Good catch, thanks.
>> >
>> > The forged AO option makes us skip the unsigned fast path, which is
>> > the only place where tcp_md5_do_lookup() can return MD5NOTFOUND.
>> > An attacker able to hit the window of an MD5-protected session (BGP...)
>> > can therefore inject RST or data with a junk AO option, which is
>> > exactly what RFC 2385 is supposed to prevent.
>> >
>> > Please mention in the changelog that TCP_AO depends on 64BIT: every
>> > 32-bit kernel with TCP_MD5SIG=y is in the vulnerable configuration.
>> >
>> > With your patch the stubs now return what the real helpers return for
>> > a socket without keys (AOUNEXPECTED / MD5UNEXPECTED), so behavior
>> > becomes consistent across configs. One visible change is that MD5=n,
>> > AO=y kernels will now drop segments carrying an MD5 option to unkeyed
>> > sockets, as MD5=y kernels already do. That is fine, but worth one
>> > sentence.
>> >
>> > Optional: bumping LINUX_MIB_TCPAOKEYNOTFOUND /
>> > LINUX_MIB_TCPMD5UNEXPECTED
>> > from the stubs would make these drops visible in nstat.
>> >
>> > This is a fix, please send v2 as [PATCH net].
>> >
>> > Also please CC TCP-AO folks from Arista
>> >
>> > Reviewed-by: Eric Dumazet <edumazet@kernel.org>
>> 
>> Hello Eric,
>> 
>> Thanks for the quick review.
>> 
>> I have prepared a v2, which only adds some bits to the commit message,
>> and leave the actual patch untouched.
>> 
>> Noted for the Cc:, I will use the 3 @arista.com addresses at the top 
>> of
>> the tcp_ao.c file.
>> 
>> For your optional remark, I was thinking of adding a second commit as
>> a 2/2 in a 2-patch series. Something like the below diff, which 
>> AFAICT,
>> would make the drops visible under TCPAOKeyNotFound and 
>> TCPMD5Unexpected
>> in nstat. Is it what you had in mind? I am not particularly familiar
>> with that subsystem; yet, while getting this kind of feedbacks in 
>> nstat
>> seems good, I am opened for advice. Do you prefer that I add this
>> in the same fix instead?
> 
> FWIW, a single fix commit sounds good to me as it didn't
> drop the packet previously.
> 
> Could you post v2 ?

Thanks for the feedback.
I have just sent the v2 here:
https://lore.kernel.org/all/20261107224733.1104094-2-Jeremy.Jean@oss.cyber.gouv.fr/

Regards,
Jérémy