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

[Kuniyuki Iwashima]

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


> 
> Regards,
> Jérémy
> 
> ---
> 
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index 9a2333e62e994..xxxxxxx 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -303,6 +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)
>  {
> +     NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOKEYNOTFOUND);
>       return SKB_DROP_REASON_TCP_AOUNEXPECTED;
>  }
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 87ef6d5cbfeba..xxxxxxx 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4962,6 +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)
>  {
> +     NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
>       return SKB_DROP_REASON_TCP_MD5UNEXPECTED;
>  }