* [PATCH net-next] tcp: implement RFC 5961 4.2
@ 2012-07-17 11:41 Eric Dumazet
2012-07-17 14:41 ` David Miller
2012-07-17 21:02 ` Vijay Subramanian
0 siblings, 2 replies; 8+ messages in thread
From: Eric Dumazet @ 2012-07-17 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Kiran Kumar Kella
From: Eric Dumazet <edumazet@google.com>
Implement the RFC 5691 mitigation against Blind
Reset attack using SYN bit.
Section 4.2 of RFC 5961 advises to send a Challenge ACK and drop
incoming packet, instead of resetting the session.
Add a new SNMP counter to count number of challenge acks sent
in response to SYN packets.
(netstat -s | grep TCPSYNChallenge)
Remove obsolete TCPAbortOnSyn, since we no longer abort a TCP session
because of a SYN flag.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Kiran Kumar Kella <kkiran@broadcom.com>
---
include/linux/snmp.h | 2 +-
net/ipv4/proc.c | 2 +-
net/ipv4/tcp_input.c | 32 +++++++++++++++-----------------
3 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/include/linux/snmp.h b/include/linux/snmp.h
index 673e0e9..e5fcbd0 100644
--- a/include/linux/snmp.h
+++ b/include/linux/snmp.h
@@ -208,7 +208,6 @@ enum
LINUX_MIB_TCPDSACKOFOSENT, /* TCPDSACKOfoSent */
LINUX_MIB_TCPDSACKRECV, /* TCPDSACKRecv */
LINUX_MIB_TCPDSACKOFORECV, /* TCPDSACKOfoRecv */
- LINUX_MIB_TCPABORTONSYN, /* TCPAbortOnSyn */
LINUX_MIB_TCPABORTONDATA, /* TCPAbortOnData */
LINUX_MIB_TCPABORTONCLOSE, /* TCPAbortOnClose */
LINUX_MIB_TCPABORTONMEMORY, /* TCPAbortOnMemory */
@@ -238,6 +237,7 @@ enum
LINUX_MIB_TCPOFODROP, /* TCPOFODrop */
LINUX_MIB_TCPOFOMERGE, /* TCPOFOMerge */
LINUX_MIB_TCPCHALLENGEACK, /* TCPChallengeACK */
+ LINUX_MIB_TCPSYNCHALLENGE, /* TCPSYNChallenge */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 3e8e78f..2a5240b 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -232,7 +232,6 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPDSACKOfoSent", LINUX_MIB_TCPDSACKOFOSENT),
SNMP_MIB_ITEM("TCPDSACKRecv", LINUX_MIB_TCPDSACKRECV),
SNMP_MIB_ITEM("TCPDSACKOfoRecv", LINUX_MIB_TCPDSACKOFORECV),
- SNMP_MIB_ITEM("TCPAbortOnSyn", LINUX_MIB_TCPABORTONSYN),
SNMP_MIB_ITEM("TCPAbortOnData", LINUX_MIB_TCPABORTONDATA),
SNMP_MIB_ITEM("TCPAbortOnClose", LINUX_MIB_TCPABORTONCLOSE),
SNMP_MIB_ITEM("TCPAbortOnMemory", LINUX_MIB_TCPABORTONMEMORY),
@@ -262,6 +261,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPOFODrop", LINUX_MIB_TCPOFODROP),
SNMP_MIB_ITEM("TCPOFOMerge", LINUX_MIB_TCPOFOMERGE),
SNMP_MIB_ITEM("TCPChallengeACK", LINUX_MIB_TCPCHALLENGEACK),
+ SNMP_MIB_ITEM("TCPSYNChallenge", LINUX_MIB_TCPSYNCHALLENGE),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c841a89..8aaec55 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5270,8 +5270,8 @@ static void tcp_send_challenge_ack(struct sock *sk)
/* Does PAWS and seqno based validation of an incoming segment, flags will
* play significant role here.
*/
-static int tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
- const struct tcphdr *th, int syn_inerr)
+static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
+ const struct tcphdr *th, int syn_inerr)
{
const u8 *hash_location;
struct tcp_sock *tp = tcp_sk(sk);
@@ -5323,20 +5323,22 @@ static int tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
/* step 3: check security and precedence [ignored] */
- /* step 4: Check for a SYN in window. */
- if (th->syn && !before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
+ /* step 4: Check for a SYN
+ * RFC 5691 4.2 : Send a challenge ack
+ */
+ if (th->syn) {
if (syn_inerr)
TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_INERRS);
- NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPABORTONSYN);
- tcp_reset(sk);
- return -1;
+ NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
+ tcp_send_challenge_ack(sk);
+ goto discard;
}
- return 1;
+ return true;
discard:
__kfree_skb(skb);
- return 0;
+ return false;
}
/*
@@ -5366,7 +5368,6 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
const struct tcphdr *th, unsigned int len)
{
struct tcp_sock *tp = tcp_sk(sk);
- int res;
if (sk->sk_rx_dst) {
struct dst_entry *dst = sk->sk_rx_dst;
@@ -5555,9 +5556,8 @@ slow_path:
* Standard slow path.
*/
- res = tcp_validate_incoming(sk, skb, th, 1);
- if (res <= 0)
- return -res;
+ if (!tcp_validate_incoming(sk, skb, th, 1))
+ return 0;
step5:
if (th->ack && tcp_ack(sk, skb, FLAG_SLOWPATH) < 0)
@@ -5877,7 +5877,6 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
struct tcp_sock *tp = tcp_sk(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
int queued = 0;
- int res;
tp->rx_opt.saw_tstamp = 0;
@@ -5932,9 +5931,8 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
return 0;
}
- res = tcp_validate_incoming(sk, skb, th, 0);
- if (res <= 0)
- return -res;
+ if (!tcp_validate_incoming(sk, skb, th, 0))
+ return 0;
/* step 5: check the ACK field */
if (th->ack) {
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] tcp: implement RFC 5961 4.2
2012-07-17 11:41 [PATCH net-next] tcp: implement RFC 5961 4.2 Eric Dumazet
@ 2012-07-17 14:41 ` David Miller
2012-07-17 21:02 ` Vijay Subramanian
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2012-07-17 14:41 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, kkiran
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 17 Jul 2012 13:41:30 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Implement the RFC 5691 mitigation against Blind
> Reset attack using SYN bit.
>
> Section 4.2 of RFC 5961 advises to send a Challenge ACK and drop
> incoming packet, instead of resetting the session.
>
> Add a new SNMP counter to count number of challenge acks sent
> in response to SYN packets.
> (netstat -s | grep TCPSYNChallenge)
>
> Remove obsolete TCPAbortOnSyn, since we no longer abort a TCP session
> because of a SYN flag.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks good, applied, thanks Eric.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] tcp: implement RFC 5961 4.2
2012-07-17 11:41 [PATCH net-next] tcp: implement RFC 5961 4.2 Eric Dumazet
2012-07-17 14:41 ` David Miller
@ 2012-07-17 21:02 ` Vijay Subramanian
2012-07-17 21:32 ` Eric Dumazet
1 sibling, 1 reply; 8+ messages in thread
From: Vijay Subramanian @ 2012-07-17 21:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Kiran Kumar Kella
On 17 July 2012 04:41, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Implement the RFC 5691 mitigation against Blind
> Reset attack using SYN bit.
>
> Section 4.2 of RFC 5961 advises to send a Challenge ACK and drop
> incoming packet, instead of resetting the session.
Eric,
Section 4.2 has this to say:
"If the SYN bit is set, irrespective of the sequence number, TCP
MUST send an ACK (also referred to as challenge ACK) to the remote
peer:"
I believe your patch only sends challenge acks for in-window SYN packets.
After this patch, the code for out of window packets is like this:
/* Step 1: check sequence number */
if (!tcp_sequence(tp, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq)) {
/* RFC793, page 37: "In all states except SYN-SENT, all reset
* (RST) segments are validated by checking their SEQ-fields."
* And page 69: "If an incoming segment is not acceptable,
* an acknowledgment should be sent in reply (unless the RST
* bit is set, if so drop the segment and return)".
*/
if (!th->rst)
tcp_send_dupack(sk, skb);
goto discard;
}
For SYN packets that are not in window, we do end up calling
tcp_send_dupack() but not tcp_send_challenge_ack(). Will it be more
appropriate to call the latter so that
we do proper rate limiting of challenge acks and update SNMP counters correctly?
Thanks,
Vijay
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] tcp: implement RFC 5961 4.2
2012-07-17 21:02 ` Vijay Subramanian
@ 2012-07-17 21:32 ` Eric Dumazet
2012-07-17 22:10 ` Vijay Subramanian
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2012-07-17 21:32 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: David Miller, netdev, Kiran Kumar Kella
On Tue, 2012-07-17 at 14:02 -0700, Vijay Subramanian wrote:
> On 17 July 2012 04:41, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Implement the RFC 5691 mitigation against Blind
> > Reset attack using SYN bit.
> >
> > Section 4.2 of RFC 5961 advises to send a Challenge ACK and drop
> > incoming packet, instead of resetting the session.
>
> Eric,
> Section 4.2 has this to say:
> "If the SYN bit is set, irrespective of the sequence number, TCP
> MUST send an ACK (also referred to as challenge ACK) to the remote
> peer:"
>
> I believe your patch only sends challenge acks for in-window SYN packets.
> After this patch, the code for out of window packets is like this:
>
> /* Step 1: check sequence number */
> if (!tcp_sequence(tp, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq)) {
> /* RFC793, page 37: "In all states except SYN-SENT, all reset
> * (RST) segments are validated by checking their SEQ-fields."
> * And page 69: "If an incoming segment is not acceptable,
> * an acknowledgment should be sent in reply (unless the RST
> * bit is set, if so drop the segment and return)".
> */
> if (!th->rst)
> tcp_send_dupack(sk, skb);
> goto discard;
> }
>
>
> For SYN packets that are not in window, we do end up calling
> tcp_send_dupack() but not tcp_send_challenge_ack(). Will it be more
> appropriate to call the latter so that
> we do proper rate limiting of challenge acks and update SNMP counters correctly?
Well, I only wanted to avoid RST ;)
But you probably are right, we could test th->syn here as well.
Something like that ?
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8aaec55..fdd49f1 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5296,8 +5296,11 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
* an acknowledgment should be sent in reply (unless the RST
* bit is set, if so drop the segment and return)".
*/
- if (!th->rst)
+ if (!th->rst) {
+ if (th->syn)
+ goto syn_challenge;
tcp_send_dupack(sk, skb);
+ }
goto discard;
}
@@ -5327,6 +5330,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
* RFC 5691 4.2 : Send a challenge ack
*/
if (th->syn) {
+syn_challenge:
if (syn_inerr)
TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_INERRS);
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] tcp: implement RFC 5961 4.2
2012-07-17 21:32 ` Eric Dumazet
@ 2012-07-17 22:10 ` Vijay Subramanian
2012-07-17 22:21 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Vijay Subramanian @ 2012-07-17 22:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Kiran Kumar Kella
> But you probably are right, we could test th->syn here as well.
>
> Something like that ?
> - if (!th->rst)
> + if (!th->rst) {
> + if (th->syn)
> + goto syn_challenge;
> tcp_send_dupack(sk, skb);
> + }
> goto discard;
> }
>
> @@ -5327,6 +5330,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
> * RFC 5691 4.2 : Send a challenge ack
> */
> if (th->syn) {
> +syn_challenge:
> if (syn_inerr)
> TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_INERRS);
> NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
>
Yes. This is what I had in mind (along with a change to make
tcp_sequence() return bool). I am not sure if this patch is official
(or will be picked up by patchwork) but
for what its worth
Acked-by: Vijay Subramanian <subramanian.vijay@gmail.com>
I will send a separate patch to make tcp_sequence() return bool.
Thanks!
Vijay
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] tcp: implement RFC 5961 4.2
2012-07-17 22:10 ` Vijay Subramanian
@ 2012-07-17 22:21 ` Eric Dumazet
2012-07-17 22:29 ` [PATCH net-next] tcp: refine SYN handling in tcp_validate_incoming Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2012-07-17 22:21 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: David Miller, netdev, Kiran Kumar Kella
On Tue, 2012-07-17 at 15:10 -0700, Vijay Subramanian wrote:
> Yes. This is what I had in mind (along with a change to make
> tcp_sequence() return bool). I am not sure if this patch is official
> (or will be picked up by patchwork) but
> for what its worth
>
> Acked-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Well, I am going to send an official patch with all credits ASAP,
Thanks !
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next] tcp: refine SYN handling in tcp_validate_incoming
2012-07-17 22:21 ` Eric Dumazet
@ 2012-07-17 22:29 ` Eric Dumazet
2012-07-18 16:32 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2012-07-17 22:29 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: David Miller, netdev, Kiran Kumar Kella
From: Eric Dumazet <edumazet@google.com>
Followup of commit 0c24604b68fc (tcp: implement RFC 5961 4.2)
As reported by Vijay Subramanian, we should send a challenge ACK
instead of a dup ack if a SYN flag is set on a packet received out of
window.
This permits the ratelimiting to work as intended, and to increase
correct SNMP counters.
Suggested-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Cc: Kiran Kumar Kella <kkiran@broadcom.com>
---
net/ipv4/tcp_input.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8aaec55..fdd49f1 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5296,8 +5296,11 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
* an acknowledgment should be sent in reply (unless the RST
* bit is set, if so drop the segment and return)".
*/
- if (!th->rst)
+ if (!th->rst) {
+ if (th->syn)
+ goto syn_challenge;
tcp_send_dupack(sk, skb);
+ }
goto discard;
}
@@ -5327,6 +5330,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
* RFC 5691 4.2 : Send a challenge ack
*/
if (th->syn) {
+syn_challenge:
if (syn_inerr)
TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_INERRS);
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] tcp: refine SYN handling in tcp_validate_incoming
2012-07-17 22:29 ` [PATCH net-next] tcp: refine SYN handling in tcp_validate_incoming Eric Dumazet
@ 2012-07-18 16:32 ` David Miller
0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2012-07-18 16:32 UTC (permalink / raw)
To: eric.dumazet; +Cc: subramanian.vijay, netdev, kkiran
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 18 Jul 2012 00:29:30 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Followup of commit 0c24604b68fc (tcp: implement RFC 5961 4.2)
>
> As reported by Vijay Subramanian, we should send a challenge ACK
> instead of a dup ack if a SYN flag is set on a packet received out of
> window.
>
> This permits the ratelimiting to work as intended, and to increase
> correct SNMP counters.
>
> Suggested-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-07-18 16:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 11:41 [PATCH net-next] tcp: implement RFC 5961 4.2 Eric Dumazet
2012-07-17 14:41 ` David Miller
2012-07-17 21:02 ` Vijay Subramanian
2012-07-17 21:32 ` Eric Dumazet
2012-07-17 22:10 ` Vijay Subramanian
2012-07-17 22:21 ` Eric Dumazet
2012-07-17 22:29 ` [PATCH net-next] tcp: refine SYN handling in tcp_validate_incoming Eric Dumazet
2012-07-18 16:32 ` 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).