[PATCH net] tcp md5sig: Use skb's saddr when replying to an incoming segment

[PATCH net] tcp md5sig: Use skb's saddr when replying to an incoming segment

[Christoph Paasch]

The MD5-key that belongs to a connection is identified by the peer's
IP-address. When we are in tcp_v4(6)_reqsk_send_ack(), we are replying
to an incoming segment from tcp_check_req() that failed the seq-number
checks.

Thus, to find the correct key, we need to use the skb's saddr and not
the daddr.

This bug seems to have been there since quite a while, but probably got
unnoticed because the consequences are not catastrophic. We will call
tcp_v4_reqsk_send_ack only to send a challenge-ACK back to the peer,
thus the connection doesn't really fail.

Fixes: 9501f9722922 ("tcp md5sig: Let the caller pass appropriate key for tcp_v{4,6}_do_calc_md5_hash().")
Signed-off-by: Christoph Paasch <cpaasch@apple.com>
---
 net/ipv4/tcp_ipv4.c | 2 +-
 net/ipv6/tcp_ipv6.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 77ea45da0fe9..94e28350f420 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -848,7 +848,7 @@ static void tcp_v4_reqsk_send_ack(const struct sock *sk, struct sk_buff *skb,
 			tcp_time_stamp_raw() + tcp_rsk(req)->ts_off,
 			req->ts_recent,
 			0,
-			tcp_md5_do_lookup(sk, (union tcp_md5_addr *)&ip_hdr(skb)->daddr,
+			tcp_md5_do_lookup(sk, (union tcp_md5_addr *)&ip_hdr(skb)->saddr,
 					  AF_INET),
 			inet_rsk(req)->no_srccheck ? IP_REPLY_ARG_NOSRCCHECK : 0,
 			ip_hdr(skb)->tos);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 1f04ec0e4a7a..7178476b3d2f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -994,7 +994,7 @@ static void tcp_v6_reqsk_send_ack(const struct sock *sk, struct sk_buff *skb,
 			req->rsk_rcv_wnd >> inet_rsk(req)->rcv_wscale,
 			tcp_time_stamp_raw() + tcp_rsk(req)->ts_off,
 			req->ts_recent, sk->sk_bound_dev_if,
-			tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->daddr),
+			tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->saddr),
 			0, 0);
 }
 
-- 
2.15.0

Re: [PATCH net] tcp md5sig: Use skb's saddr when replying to an incoming segment

[Eric Dumazet]

On Mon, 2017-12-11 at 00:05 -0800, Christoph Paasch wrote:
> The MD5-key that belongs to a connection is identified by the peer's
> IP-address. When we are in tcp_v4(6)_reqsk_send_ack(), we are
> replying
> to an incoming segment from tcp_check_req() that failed the seq-
> number
> checks.
> 
> Thus, to find the correct key, we need to use the skb's saddr and not
> the daddr.
> 
> This bug seems to have been there since quite a while, but probably
> got
> unnoticed because the consequences are not catastrophic. We will call
> tcp_v4_reqsk_send_ack only to send a challenge-ACK back to the peer,
> thus the connection doesn't really fail.
> 
> Fixes: 9501f9722922 ("tcp md5sig: Let the caller pass appropriate key
> for tcp_v{4,6}_do_calc_md5_hash().")
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>
> ---
>  net/ipv4/tcp_ipv4.c | 2 +-
>  net/ipv6/tcp_ipv6.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !

Re: [PATCH net] tcp md5sig: Use skb's saddr when replying to an incoming segment

[David Miller]

From: Christoph Paasch <cpaasch@apple.com>
Date: Mon, 11 Dec 2017 00:05:46 -0800

> The MD5-key that belongs to a connection is identified by the peer's
> IP-address. When we are in tcp_v4(6)_reqsk_send_ack(), we are replying
> to an incoming segment from tcp_check_req() that failed the seq-number
> checks.
> 
> Thus, to find the correct key, we need to use the skb's saddr and not
> the daddr.
> 
> This bug seems to have been there since quite a while, but probably got
> unnoticed because the consequences are not catastrophic. We will call
> tcp_v4_reqsk_send_ack only to send a challenge-ACK back to the peer,
> thus the connection doesn't really fail.
> 
> Fixes: 9501f9722922 ("tcp md5sig: Let the caller pass appropriate key for tcp_v{4,6}_do_calc_md5_hash().")
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>

Applied and queued up for -stable, thanks.