Messages in this thread Patch in this message | | | From | Eric Dumazet <> | Date | Mon, 30 Oct 2023 09:16:08 +0100 | Subject | Re: [PATCH v16 net-next 15/23] net/tcp: Add tcp_hash_fail() ratelimited logs |
| |
On Mon, Oct 23, 2023 at 9:22 PM Dmitry Safonov <dima@arista.com> wrote: > > Add a helper for logging connection-detailed messages for failed TCP > hash verification (both MD5 and AO). > > Co-developed-by: Francesco Ruggeri <fruggeri@arista.com> > Signed-off-by: Francesco Ruggeri <fruggeri@arista.com> > Co-developed-by: Salam Noureddine <noureddine@arista.com> > Signed-off-by: Salam Noureddine <noureddine@arista.com> > Signed-off-by: Dmitry Safonov <dima@arista.com> > Acked-by: David Ahern <dsahern@kernel.org> > --- > include/net/tcp.h | 14 ++++++++++++-- > include/net/tcp_ao.h | 29 +++++++++++++++++++++++++++++ > net/ipv4/tcp.c | 23 +++++++++++++---------- > net/ipv4/tcp_ao.c | 7 +++++++ > 4 files changed, 61 insertions(+), 12 deletions(-) > > diff --git a/include/net/tcp.h b/include/net/tcp.h > index d29c8a867f0e..c93ac6cc12c4 100644 > --- a/include/net/tcp.h > +++ b/include/net/tcp.h > @@ -2746,12 +2746,18 @@ tcp_inbound_hash(struct sock *sk, const struct request_sock *req, > int l3index; > > /* Invalid option or two times meet any of auth options */ > - if (tcp_parse_auth_options(th, &md5_location, &aoh)) > + if (tcp_parse_auth_options(th, &md5_location, &aoh)) { > + tcp_hash_fail("TCP segment has incorrect auth options set", > + family, skb, ""); > return SKB_DROP_REASON_TCP_AUTH_HDR; > + } > > if (req) { > if (tcp_rsk_used_ao(req) != !!aoh) { > NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOBAD); > + tcp_hash_fail("TCP connection can't start/end using TCP-AO", > + family, skb, "%s", > + !aoh ? "missing AO" : "AO signed"); > return SKB_DROP_REASON_TCP_AOFAILURE; > } > } > @@ -2768,10 +2774,14 @@ tcp_inbound_hash(struct sock *sk, const struct request_sock *req, > * the last key is impossible to remove, so there's > * always at least one current_key. > */ > - if (tcp_ao_required(sk, saddr, family, true)) > + if (tcp_ao_required(sk, saddr, family, true)) { > + tcp_hash_fail("AO hash is required, but not found", > + family, skb, "L3 index %d", l3index); > return SKB_DROP_REASON_TCP_AONOTFOUND; > + } > if (unlikely(tcp_md5_do_lookup(sk, l3index, saddr, family))) { > NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND); > + tcp_hash_fail("MD5 Hash not found", family, skb, ""); > return SKB_DROP_REASON_TCP_MD5NOTFOUND; > } > return SKB_NOT_DROPPED_YET; > diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h > index 0c3516d1b968..4da6e3657913 100644 > --- a/include/net/tcp_ao.h > +++ b/include/net/tcp_ao.h > @@ -118,6 +118,35 @@ struct tcp_ao_info { > struct rcu_head rcu; > }; > > +#define tcp_hash_fail(msg, family, skb, fmt, ...) \ > +do { \ > + const struct tcphdr *th = tcp_hdr(skb); \ > + char hdr_flags[5] = {}; \ > + char *f = hdr_flags; \ > + \ > + if (th->fin) \ > + *f++ = 'F'; \ > + if (th->syn) \ > + *f++ = 'S'; \ > + if (th->rst) \ > + *f++ = 'R'; \ > + if (th->ack) \ > + *f++ = 'A'; \ > + if (f != hdr_flags) \ > + *f = ' '; \
I see no null termination of hdr_flags[] if FIN+SYN+ACK+RST are all set.
I will send something like:
diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h index a375a171ef3cb37ab1d8246c72c6a3e83f5c9184..5daf96a3dbee14bd3786e19ea4972e351058e6e7 100644 --- a/include/net/tcp_ao.h +++ b/include/net/tcp_ao.h @@ -124,7 +124,7 @@ struct tcp_ao_info { #define tcp_hash_fail(msg, family, skb, fmt, ...) \ do { \ const struct tcphdr *th = tcp_hdr(skb); \ - char hdr_flags[5] = {}; \ + char hdr_flags[5]; \ char *f = hdr_flags; \ \ if (th->fin) \ @@ -135,8 +135,7 @@ do { \ *f++ = 'R'; \ if (th->ack) \ *f++ = 'A'; \ - if (f != hdr_flags) \ - *f = ' '; \ + *f = 0; \ if ((family) == AF_INET) { \ net_info_ratelimited("%s for (%pI4, %d)->(%pI4, %d) %s" fmt "\n", \ msg, &ip_hdr(skb)->saddr, ntohs(th->source), \
| |