* [PATCH] MD5: don't warn when an unexpected signature is seen. @ 2008-07-29 17:36 Adam Langley 2008-07-30 8:45 ` Pekka Savola 2008-07-30 10:08 ` David Miller 0 siblings, 2 replies; 6+ messages in thread From: Adam Langley @ 2008-07-29 17:36 UTC (permalink / raw) To: davem; +Cc: netdev Currently, connecting to a listening socket with an MD5 signature option, when MD5 is not configured on the listening socket, will generate the following warning: MD5 Hash NOT expected but found This is rate limited, but too verbose given that it can be induced with an unverified SYN packet. This patch removes the warning Signed-off-by: Adam Langley <agl@imperialviolet.org> --- net/ipv4/tcp_ipv4.c | 7 +------ net/ipv6/tcp_ipv6.c | 7 ------- 2 files changed, 1 insertions(+), 13 deletions(-) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index a2b06d0..8cafa92 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1123,13 +1123,8 @@ static int tcp_v4_inbound_md5_hash(struct sock *sk, struct sk_buff *skb) return 1; } - if (!hash_expected && hash_location) { - LIMIT_NETDEBUG(KERN_INFO "MD5 Hash NOT expected but found " - "(" NIPQUAD_FMT ", %d)->(" NIPQUAD_FMT ", %d)\n", - NIPQUAD(iph->saddr), ntohs(th->source), - NIPQUAD(iph->daddr), ntohs(th->dest)); + if (!hash_expected && hash_location) return 1; - } /* Okay, so this is hash_expected and hash_location - * so we need to calculate the checksum. diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index cff778b..7bb588c 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -853,13 +853,6 @@ static int tcp_v6_inbound_md5_hash (struct sock *sk, struct sk_buff *skb) if (!hash_expected) { if (!hash_location) return 0; - if (net_ratelimit()) { - printk(KERN_INFO "MD5 Hash NOT expected but found " - "(" NIP6_FMT ", %u)->" - "(" NIP6_FMT ", %u)\n", - NIP6(ip6h->saddr), ntohs(th->source), - NIP6(ip6h->daddr), ntohs(th->dest)); - } return 1; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] MD5: don't warn when an unexpected signature is seen. 2008-07-29 17:36 [PATCH] MD5: don't warn when an unexpected signature is seen Adam Langley @ 2008-07-30 8:45 ` Pekka Savola 2008-07-30 8:59 ` David Miller 2008-07-30 10:08 ` David Miller 1 sibling, 1 reply; 6+ messages in thread From: Pekka Savola @ 2008-07-30 8:45 UTC (permalink / raw) To: Adam Langley; +Cc: davem, netdev Just wondering. If the warning is removed, should this be tracked somewhere else, e.g. 'netstat -s' TCP statistics? On Tue, 29 Jul 2008, Adam Langley wrote: > Currently, connecting to a listening socket with an MD5 signature option, when > MD5 is not configured on the listening socket, will generate the following > warning: > MD5 Hash NOT expected but found > > This is rate limited, but too verbose given that it can be induced with an > unverified SYN packet. > > This patch removes the warning > > Signed-off-by: Adam Langley <agl@imperialviolet.org> > --- > > net/ipv4/tcp_ipv4.c | 7 +------ > net/ipv6/tcp_ipv6.c | 7 ------- > 2 files changed, 1 insertions(+), 13 deletions(-) > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c > index a2b06d0..8cafa92 100644 > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -1123,13 +1123,8 @@ static int tcp_v4_inbound_md5_hash(struct sock *sk, struct sk_buff *skb) > return 1; > } > > - if (!hash_expected && hash_location) { > - LIMIT_NETDEBUG(KERN_INFO "MD5 Hash NOT expected but found " > - "(" NIPQUAD_FMT ", %d)->(" NIPQUAD_FMT ", %d)\n", > - NIPQUAD(iph->saddr), ntohs(th->source), > - NIPQUAD(iph->daddr), ntohs(th->dest)); > + if (!hash_expected && hash_location) > return 1; > - } > > /* Okay, so this is hash_expected and hash_location - > * so we need to calculate the checksum. > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c > index cff778b..7bb588c 100644 > --- a/net/ipv6/tcp_ipv6.c > +++ b/net/ipv6/tcp_ipv6.c > @@ -853,13 +853,6 @@ static int tcp_v6_inbound_md5_hash (struct sock *sk, struct sk_buff *skb) > if (!hash_expected) { > if (!hash_location) > return 0; > - if (net_ratelimit()) { > - printk(KERN_INFO "MD5 Hash NOT expected but found " > - "(" NIP6_FMT ", %u)->" > - "(" NIP6_FMT ", %u)\n", > - NIP6(ip6h->saddr), ntohs(th->source), > - NIP6(ip6h->daddr), ntohs(th->dest)); > - } > return 1; > } > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Pekka Savola "You each name yourselves king, yet the Netcore Oy kingdom bleeds." Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MD5: don't warn when an unexpected signature is seen. 2008-07-30 8:45 ` Pekka Savola @ 2008-07-30 8:59 ` David Miller 0 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2008-07-30 8:59 UTC (permalink / raw) To: pekkas; +Cc: agl, netdev From: Pekka Savola <pekkas@netcore.fi> Date: Wed, 30 Jul 2008 11:45:50 +0300 (EEST) > Just wondering. If the warning is removed, should this be tracked > somewhere else, e.g. 'netstat -s' TCP statistics? Yes, this is what should be done. I was going to mention this when I got to this patch in my backlog. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MD5: don't warn when an unexpected signature is seen. 2008-07-29 17:36 [PATCH] MD5: don't warn when an unexpected signature is seen Adam Langley 2008-07-30 8:45 ` Pekka Savola @ 2008-07-30 10:08 ` David Miller 2008-07-30 10:16 ` Ingo Oeser 1 sibling, 1 reply; 6+ messages in thread From: David Miller @ 2008-07-30 10:08 UTC (permalink / raw) To: agl; +Cc: netdev From: "Adam Langley" <agl@imperialviolet.org> Date: Tue, 29 Jul 2008 10:36:00 -0700 > Currently, connecting to a listening socket with an MD5 signature option, when > MD5 is not configured on the listening socket, will generate the following > warning: > MD5 Hash NOT expected but found > > This is rate limited, but too verbose given that it can be induced with an > unverified SYN packet. > > This patch removes the warning > > Signed-off-by: Adam Langley <agl@imperialviolet.org> As I mentioned in another reply, we should at least capture such events in a MIB counter. Thus I've checked in the following patch. tcp: MD5: Use MIB counter instead of warning for MD5 mismatch. >From a report by Matti Aarnio, and preliminary patch by Adam Langley. Signed-off-by: David S. Miller <davem@davemloft.net> --- include/linux/snmp.h | 2 ++ net/ipv4/proc.c | 2 ++ net/ipv4/tcp_ipv4.c | 10 ++-------- net/ipv6/tcp_ipv6.c | 27 ++++++++------------------- 4 files changed, 14 insertions(+), 27 deletions(-) diff --git a/include/linux/snmp.h b/include/linux/snmp.h index 5df62ef..7a6e6bb 100644 --- a/include/linux/snmp.h +++ b/include/linux/snmp.h @@ -214,6 +214,8 @@ enum LINUX_MIB_TCPDSACKIGNOREDOLD, /* TCPSACKIgnoredOld */ LINUX_MIB_TCPDSACKIGNOREDNOUNDO, /* TCPSACKIgnoredNoUndo */ LINUX_MIB_TCPSPURIOUSRTOS, /* TCPSpuriousRTOs */ + LINUX_MIB_TCPMD5NOTFOUND, /* TCPMD5NotFound */ + LINUX_MIB_TCPMD5UNEXPECTED, /* TCPMD5Unexpected */ __LINUX_MIB_MAX }; diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c index 834356e..8f5a403 100644 --- a/net/ipv4/proc.c +++ b/net/ipv4/proc.c @@ -232,6 +232,8 @@ static const struct snmp_mib snmp4_net_list[] = { SNMP_MIB_ITEM("TCPDSACKIgnoredOld", LINUX_MIB_TCPDSACKIGNOREDOLD), SNMP_MIB_ITEM("TCPDSACKIgnoredNoUndo", LINUX_MIB_TCPDSACKIGNOREDNOUNDO), SNMP_MIB_ITEM("TCPSpuriousRTOs", LINUX_MIB_TCPSPURIOUSRTOS), + SNMP_MIB_ITEM("TCPMD5NotFound", LINUX_MIB_TCPMD5NOTFOUND), + SNMP_MIB_ITEM("TCPMD5Unexpected", LINUX_MIB_TCPMD5UNEXPECTED), SNMP_MIB_SENTINEL }; diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index a2b06d0..4607ec1 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1116,18 +1116,12 @@ static int tcp_v4_inbound_md5_hash(struct sock *sk, struct sk_buff *skb) return 0; if (hash_expected && !hash_location) { - LIMIT_NETDEBUG(KERN_INFO "MD5 Hash expected but NOT found " - "(" NIPQUAD_FMT ", %d)->(" NIPQUAD_FMT ", %d)\n", - NIPQUAD(iph->saddr), ntohs(th->source), - NIPQUAD(iph->daddr), ntohs(th->dest)); + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5NOTFOUND); return 1; } if (!hash_expected && hash_location) { - LIMIT_NETDEBUG(KERN_INFO "MD5 Hash NOT expected but found " - "(" NIPQUAD_FMT ", %d)->(" NIPQUAD_FMT ", %d)\n", - NIPQUAD(iph->saddr), ntohs(th->source), - NIPQUAD(iph->daddr), ntohs(th->dest)); + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5UNEXPECTED); return 1; } diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index cff778b..0ab09c6 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -849,28 +849,17 @@ static int tcp_v6_inbound_md5_hash (struct sock *sk, struct sk_buff *skb) hash_expected = tcp_v6_md5_do_lookup(sk, &ip6h->saddr); hash_location = tcp_parse_md5sig_option(th); - /* do we have a hash as expected? */ - if (!hash_expected) { - if (!hash_location) - return 0; - if (net_ratelimit()) { - printk(KERN_INFO "MD5 Hash NOT expected but found " - "(" NIP6_FMT ", %u)->" - "(" NIP6_FMT ", %u)\n", - NIP6(ip6h->saddr), ntohs(th->source), - NIP6(ip6h->daddr), ntohs(th->dest)); - } + /* We've parsed the options - do we have a hash? */ + if (!hash_expected && !hash_location) + return 0; + + if (hash_expected && !hash_location) { + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5NOTFOUND); return 1; } - if (!hash_location) { - if (net_ratelimit()) { - printk(KERN_INFO "MD5 Hash expected but NOT found " - "(" NIP6_FMT ", %u)->" - "(" NIP6_FMT ", %u)\n", - NIP6(ip6h->saddr), ntohs(th->source), - NIP6(ip6h->daddr), ntohs(th->dest)); - } + if (!hash_expected && hash_location) { + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5UNEXPECTED); return 1; } -- 1.5.6.GIT ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] MD5: don't warn when an unexpected signature is seen. 2008-07-30 10:08 ` David Miller @ 2008-07-30 10:16 ` Ingo Oeser 2008-07-30 10:22 ` David Miller 0 siblings, 1 reply; 6+ messages in thread From: Ingo Oeser @ 2008-07-30 10:16 UTC (permalink / raw) To: David Miller; +Cc: agl, netdev Hi David, great that you did this! Counters are much better than messages here :-) But you might find this will not compile. Details below. David Miller schrieb: > diff --git a/include/linux/snmp.h b/include/linux/snmp.h > index 5df62ef..7a6e6bb 100644 > --- a/include/linux/snmp.h > +++ b/include/linux/snmp.h > @@ -214,6 +214,8 @@ enum > LINUX_MIB_TCPDSACKIGNOREDOLD, /* TCPSACKIgnoredOld */ > LINUX_MIB_TCPDSACKIGNOREDNOUNDO, /* TCPSACKIgnoredNoUndo */ > LINUX_MIB_TCPSPURIOUSRTOS, /* TCPSpuriousRTOs */ > + LINUX_MIB_TCPMD5NOTFOUND, /* TCPMD5NotFound */ > + LINUX_MIB_TCPMD5UNEXPECTED, /* TCPMD5Unexpected */ These contain "_TCP" in the name. > __LINUX_MIB_MAX > }; > > diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c > index 834356e..8f5a403 100644 > --- a/net/ipv4/proc.c > +++ b/net/ipv4/proc.c > @@ -232,6 +232,8 @@ static const struct snmp_mib snmp4_net_list[] = { > SNMP_MIB_ITEM("TCPDSACKIgnoredOld", LINUX_MIB_TCPDSACKIGNOREDOLD), > SNMP_MIB_ITEM("TCPDSACKIgnoredNoUndo", LINUX_MIB_TCPDSACKIGNOREDNOUNDO), > SNMP_MIB_ITEM("TCPSpuriousRTOs", LINUX_MIB_TCPSPURIOUSRTOS), > + SNMP_MIB_ITEM("TCPMD5NotFound", LINUX_MIB_TCPMD5NOTFOUND), > + SNMP_MIB_ITEM("TCPMD5Unexpected", LINUX_MIB_TCPMD5UNEXPECTED), These contain "_TCP" in the name, too. > SNMP_MIB_SENTINEL > }; > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c > index a2b06d0..4607ec1 100644 > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -1116,18 +1116,12 @@ static int tcp_v4_inbound_md5_hash(struct sock *sk, struct sk_buff *skb) > return 0; > > if (hash_expected && !hash_location) { > - LIMIT_NETDEBUG(KERN_INFO "MD5 Hash expected but NOT found " > - "(" NIPQUAD_FMT ", %d)->(" NIPQUAD_FMT ", %d)\n", > - NIPQUAD(iph->saddr), ntohs(th->source), > - NIPQUAD(iph->daddr), ntohs(th->dest)); > + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5NOTFOUND); This doesn't contain "_TCP" in the name. Why? > return 1; > } > > if (!hash_expected && hash_location) { > - LIMIT_NETDEBUG(KERN_INFO "MD5 Hash NOT expected but found " > - "(" NIPQUAD_FMT ", %d)->(" NIPQUAD_FMT ", %d)\n", > - NIPQUAD(iph->saddr), ntohs(th->source), > - NIPQUAD(iph->daddr), ntohs(th->dest)); > + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5UNEXPECTED); This doesn't contain "_TCP" in the name. Why? > return 1; > } > > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c > index cff778b..0ab09c6 100644 > --- a/net/ipv6/tcp_ipv6.c > +++ b/net/ipv6/tcp_ipv6.c > @@ -849,28 +849,17 @@ static int tcp_v6_inbound_md5_hash (struct sock *sk, struct sk_buff *skb) > hash_expected = tcp_v6_md5_do_lookup(sk, &ip6h->saddr); > hash_location = tcp_parse_md5sig_option(th); > > - /* do we have a hash as expected? */ > - if (!hash_expected) { > - if (!hash_location) > - return 0; > - if (net_ratelimit()) { > - printk(KERN_INFO "MD5 Hash NOT expected but found " > - "(" NIP6_FMT ", %u)->" > - "(" NIP6_FMT ", %u)\n", > - NIP6(ip6h->saddr), ntohs(th->source), > - NIP6(ip6h->daddr), ntohs(th->dest)); > - } > + /* We've parsed the options - do we have a hash? */ > + if (!hash_expected && !hash_location) > + return 0; > + > + if (hash_expected && !hash_location) { > + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5NOTFOUND); This doesn't contain "_TCP" in the name. Why? > return 1; > } > > - if (!hash_location) { > - if (net_ratelimit()) { > - printk(KERN_INFO "MD5 Hash expected but NOT found " > - "(" NIP6_FMT ", %u)->" > - "(" NIP6_FMT ", %u)\n", > - NIP6(ip6h->saddr), ntohs(th->source), > - NIP6(ip6h->daddr), ntohs(th->dest)); > - } > + if (!hash_expected && hash_location) { > + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5UNEXPECTED); This doesn't contain "_TCP" in the name. Why? > return 1; > } > Best Regards Ingo Oeser ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MD5: don't warn when an unexpected signature is seen. 2008-07-30 10:16 ` Ingo Oeser @ 2008-07-30 10:22 ` David Miller 0 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2008-07-30 10:22 UTC (permalink / raw) To: netdev; +Cc: agl, netdev From: Ingo Oeser <netdev@axxeo.de> Date: Wed, 30 Jul 2008 12:16:00 +0200 > > + if (!hash_expected && hash_location) { > > + NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_MD5UNEXPECTED); > > This doesn't contain "_TCP" in the name. Why? Because I didn't post the refreshed patch that I actually compile tested :-) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-07-30 10:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-07-29 17:36 [PATCH] MD5: don't warn when an unexpected signature is seen Adam Langley 2008-07-30 8:45 ` Pekka Savola 2008-07-30 8:59 ` David Miller 2008-07-30 10:08 ` David Miller 2008-07-30 10:16 ` Ingo Oeser 2008-07-30 10:22 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).