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

[Jérémy Jean]

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@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> > > > ---
> > > >  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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxxx/

Regards,
Jérémy