Re: [PATCH net] tcp: make challenge acks less predictable

2016-07-09 Thread Eric Dumazet
On Fri, 2016-07-08 at 17:27 -0700, Yue Cao wrote:
> Hi Eric, 
> Thank you for the email. After rethinking the suggested patch, our
> side-channel attack might still work.
> The main idea behind the patch is to change challenge_count lifetime
> from 1s to a random value in the range [0.5s, 1.5s), which creates a
> time synchronization issue at the attacker's end. 
> In our modified attack, 
> 1. Instead of sending several packets throughout the 1s duration,
> attacker sends fewer packets in a short period (e.g. 0.1s, or even
> shorter). It is likely that this short period will be included in one
> challenge_count lifetime at the server’s end.
> 2. If this short period covers two challenge_counts’ lifetime or some
> rare case that attacker is not sure, attacker can repeat sending same
> packets after a short period (e.g. 1.5s) to confirm it. 
> 3. These packets should include one or more spoofed packets and 1005(a
> value bigger than 1001) packets to exhaust such side channel. 
> In summary, if the attacker receives less than 1000 packets from the
> server, it must be a good guess. If the attacker receives more than
> 1000 packets from the server, this short period covers two
> challenge_counts’ lifetime and the attacker has to repeat sending same
> packets after a short duration. If the attacker receives exactly 1000
> packets from the server, it is most likely a wrong guess. However, the
> attacker would better repeat sending packets to confirm it since these
> 1000 packets may be sent from two continuous challenge_counts’
> lifetime(though it’s a rare case).

OK so all we need is to vary the 1000 value a bit so that attacker can
not predict it, as Linus first did.

I will send a V2, thanks a lot !

[PATCH net] tcp: make challenge acks less predictable

2016-07-08 Thread Eric Dumazet
From: Eric Dumazet 

Yue Cao claims that current host rate limiting of challenge ACKS
(RFC 5961) could leak enough information to allow a patient attacker
to hijack TCP sessions. He will soon provide details in an academic

This patch increases the default limit from 100 to 1000, and adds
some randomization so that the attacker can no longer hijack
sessions without spending a considerable amount of probes.

Based on initial analysis and patch from Linus.

Note that we also have per socket rate limiting, so it is tempting
to remove the host limit. This might be done later.

Fixes: 282f23c6ee34 ("tcp: implement RFC 5961 3.2")
Reported-by: Yue Cao 
Signed-off-by: Eric Dumazet 
Suggested-by: Linus Torvalds 
Cc: Yuchung Cheng 
Cc: Neal Cardwell 
diff --git a/Documentation/networking/ip-sysctl.txt 
index 9ae929395b24..391ed93a8e49 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -732,7 +732,7 @@ tcp_limit_output_bytes - INTEGER
 tcp_challenge_ack_limit - INTEGER
Limits number of Challenge ACK sent per second, as recommended
in RFC 5961 (Improving TCP's Robustness to Blind In-Window Attacks)
-   Default: 100
+   Default: 1000
 UDP variables:
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d6c8f4cd0800..25f95a41090a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -87,7 +87,7 @@ int sysctl_tcp_adv_win_scale __read_mostly = 1;
 /* rfc5961 challenge ack rate limiting */
-int sysctl_tcp_challenge_ack_limit = 100;
+int sysctl_tcp_challenge_ack_limit = 1000;
 int sysctl_tcp_stdurg __read_mostly;
 int sysctl_tcp_rfc1337 __read_mostly;
@@ -3455,10 +3455,11 @@ not_rate_limited:
 static void tcp_send_challenge_ack(struct sock *sk, const struct sk_buff *skb)
/* unprotected vars, we dont care of overwrites */
-   static u32 challenge_timestamp;
+   static unsigned int challenge_window = HZ;
+   static unsigned long challenge_timestamp;
static unsigned int challenge_count;
struct tcp_sock *tp = tcp_sk(sk);
-   u32 now;
+   unsigned long now;
/* First check our per-socket dupack rate limit. */
if (tcp_oow_rate_limited(sock_net(sk), skb,
@@ -3467,9 +3468,11 @@ static void tcp_send_challenge_ack(struct sock *sk, 
const struct sk_buff *skb)
/* Then check the check host-wide RFC 5961 rate limit. */
-   now = jiffies / HZ;
-   if (now != challenge_timestamp) {
+   now = jiffies;
+   if (time_before(now, challenge_timestamp) ||
+   time_after_eq(now, challenge_timestamp + challenge_window)) {
challenge_timestamp = now;
+   challenge_window = HZ/2 + prandom_u32_max(HZ);
challenge_count = 0;
if (++challenge_count <= sysctl_tcp_challenge_ack_limit) {