diff mbox series

Clock-independent TCP ISN generation

Message ID 70c41960-6d14-3943-31ca-75598ad3d2d7@gmail.com
State Rejected
Delegated to: David Miller
Headers show
Series Clock-independent TCP ISN generation | expand

Commit Message

Cyrus Sh Sept. 3, 2019, 5:06 a.m. UTC
This patch addresses the privacy issue of TCP ISN generation in Linux
kernel. Currently an adversary can deanonymize a user behind an anonymity
network by inducing a load pattern on the target machine and correlating
its clock skew with the pattern. Since the kernel adds a clock-based
counter to generated ISNs, the adversary can observe SYN packets with
similar IP and port numbers to find out the clock skew of the target
machine and this can help them identify the user.  To resolve this problem
I have changed the related function to generate the initial sequence
numbers randomly and independent from the cpu clock. This feature is
controlled by a new sysctl option called "tcp_random_isn" which I've added
to the kernel. Once enabled the initial sequence numbers are guaranteed to
be generated independently from each other and from the hardware clock of
the machine. If the option is off, ISNs are generated as before.  To get
more information about this patch and its effectiveness you can refer to my
post here:
https://bitguard.wordpress.com/?p=982
and to see a discussion about the issue you can read this:
https://trac.torproject.org/projects/tor/ticket/16659

Signed-off-by: Sirus Shahini <sirus.shahini@gmail.com>
---
 include/net/tcp.h           |  1 +
 include/uapi/linux/sysctl.h |  1 +
 kernel/sysctl_binary.c      |  1 +
 net/core/secure_seq.c       | 24 +++++++++++++++++++++++-
 net/ipv4/sysctl_net_ipv4.c  |  7 +++++++
 net/ipv4/tcp_input.c        |  1 +
 6 files changed, 34 insertions(+), 1 deletion(-)

Comments

Eric Dumazet Sept. 3, 2019, 7:41 a.m. UTC | #1
On 9/3/19 7:06 AM, Cyrus Sh wrote:
> This patch addresses the privacy issue of TCP ISN generation in Linux
> kernel. Currently an adversary can deanonymize a user behind an anonymity
> network by inducing a load pattern on the target machine and correlating
> its clock skew with the pattern. Since the kernel adds a clock-based
> counter to generated ISNs, the adversary can observe SYN packets with
> similar IP and port numbers to find out the clock skew of the target
> machine and this can help them identify the user.  To resolve this problem
> I have changed the related function to generate the initial sequence
> numbers randomly and independent from the cpu clock. This feature is
> controlled by a new sysctl option called "tcp_random_isn" which I've added
> to the kernel. Once enabled the initial sequence numbers are guaranteed to
> be generated independently from each other and from the hardware clock of
> the machine. If the option is off, ISNs are generated as before.  To get
> more information about this patch and its effectiveness you can refer to my
> post here:
> https://bitguard.wordpress.com/?p=982


<quote>
Firstly it’s unlikely that this happens at all,
</quote>

Sorry this happens all the time.
Some people use very disturbing setups really, and they are not trying to be malicious.

Clock skew seems quite secondary. Some firewall rules should prevent this kind of attacks ?

> and to see a discussion about the issue you can read this:
> https://trac.torproject.org/projects/tor/ticket/16659

Four years old discussion. Does not seem urgent matter :/

> 
> Signed-off-by: Sirus Shahini <sirus.shahini@gmail.com>
> ---
>  include/net/tcp.h           |  1 +
>  include/uapi/linux/sysctl.h |  1 +
>  kernel/sysctl_binary.c      |  1 +
>  net/core/secure_seq.c       | 24 +++++++++++++++++++++++-
>  net/ipv4/sysctl_net_ipv4.c  |  7 +++++++
>  net/ipv4/tcp_input.c        |  1 +
>  6 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 81e8ade..4ad1bbf 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -241,6 +241,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
>  
>  /* sysctl variables for tcp */
>  extern int sysctl_tcp_max_orphans;
> +extern int sysctl_tcp_random_isn;

All TCP sysctls are per netns these days. (Look in include/net/netns/ipv4.h )

>  extern long sysctl_tcp_mem[3];
>  
>  #define TCP_RACK_LOSS_DETECTION  0x1 /* Use RACK to detect losses */
> diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
> index 87aa2a6..ba8927e 100644
> --- a/include/uapi/linux/sysctl.h
> +++ b/include/uapi/linux/sysctl.h
> @@ -426,6 +426,7 @@ enum
>  	NET_TCP_ALLOWED_CONG_CONTROL=123,
>  	NET_TCP_MAX_SSTHRESH=124,
>  	NET_TCP_FRTO_RESPONSE=125,
> +	NET_IPV4_TCP_RANDOM_ISN=126,

Nope, we do not add new sysctls there anymore.

Everybody should now have /proc files to tune the values.

>  };
>  
>  enum {
> diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
> index 73c1320..0faf7d4 100644
> --- a/kernel/sysctl_binary.c
> +++ b/kernel/sysctl_binary.c
> @@ -332,6 +332,7 @@ static const struct bin_table bin_net_ipv4_netfilter_table[] = {
>  };
>  
>  static const struct bin_table bin_net_ipv4_table[] = {
> +	{CTL_INT,   NET_IPV4_TCP_RANDOM_ISN     "tcp_random_isn"}

Same remark. We no longer add stuff there.

>  	{CTL_INT,	NET_IPV4_FORWARD,			"ip_forward" },
>  
>  	{ CTL_DIR,	NET_IPV4_CONF,		"conf",		bin_net_ipv4_conf_table },
> diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> index 7b6b1d2..b644bbe 100644
> --- a/net/core/secure_seq.c
> +++ b/net/core/secure_seq.c
> @@ -22,6 +22,7 @@
>  
>  static siphash_key_t net_secret __read_mostly;
>  static siphash_key_t ts_secret __read_mostly;
> +static siphash_key_t last_secret = {{0,0}} ;
>  
>  static __always_inline void net_secret_init(void)
>  {
> @@ -134,8 +135,29 @@ u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
>  		   __be16 sport, __be16 dport)
>  {
>  	u32 hash;
> -
> +	u32 temp;
> +	
>  	net_secret_init();
> +	
> +	if (sysctl_tcp_random_isn){
> +		if (!last_secret.key[0] && !last_secret.key[1]){
> +			memcpy(&last_secret,&net_secret,sizeof(last_secret));	
> +					
> +		}else{

Check your patch using scripts/checkpatch.pl

All these missing spaces should be spotted.

> +			temp = *((u32*)&(net_secret.key[0]));
> +			temp >>= 8;
> +			last_secret.key[0]+=temp;
> +			temp = *((u32*)&(net_secret.key[1]));
> +			temp >>= 8;
> +			last_secret.key[1]+=temp;

Why not simply use an official random generator, instead of these convoluted
and not SMP safe attempts ?

Check drivers/char/random.c for officially maintained generators.

> +		}
> +		hash = siphash_3u32((__force u32)saddr, (__force u32)daddr,
> +			        (__force u32)sport << 16 | (__force u32)dport,
> +			        &last_secret);
> +		return hash;
> +	}
> +	
> +	
>  	hash = siphash_3u32((__force u32)saddr, (__force u32)daddr,
>  			    (__force u32)sport << 16 | (__force u32)dport,
>  			    &net_secret);
Cyrus Sh Sept. 3, 2019, 3:39 p.m. UTC | #2
On 9/3/19 1:41 AM, Eric Dumazet wrote:
> Clock skew seems quite secondary. Some firewall rules should prevent this kind of attacks ?

Can you provide any reference to somewhere that explains these firewall rules
and how to exactly use them to prevent this specific type of attack?
Eric Dumazet Sept. 3, 2019, 3:59 p.m. UTC | #3
On 9/3/19 5:39 PM, Cyrus Sh wrote:
> 
> 
> On 9/3/19 1:41 AM, Eric Dumazet wrote:
>> Clock skew seems quite secondary. Some firewall rules should prevent this kind of attacks ?
> 
> Can you provide any reference to somewhere that explains these firewall rules
> and how to exactly use them to prevent this specific type of attack?
> 

You could add a random delay to all SYN packets, if you believe your host has clock skews.
Cyrus Sh Sept. 3, 2019, 4:06 p.m. UTC | #4
On 9/3/19 9:59 AM, Eric Dumazet wrote:
> 
> You could add a random delay to all SYN packets, if you believe your host has clock skews.

In theory yes, but again do you know any practical example with tested
applications and the list of the rules? I'm interested to see an actual example
that somebody has carried out and observed its results.
Cyrus Sh Sept. 3, 2019, 4:12 p.m. UTC | #5
On 9/3/19 9:59 AM, Eric Dumazet wrote:

> You could add a random delay to all SYN packets, if you believe your host has clock skews.

And by the way adding delays has its own performance penalties.
Eric Dumazet Sept. 3, 2019, 4:16 p.m. UTC | #6
On 9/3/19 6:12 PM, Cyrus Sh wrote:
> 
> 
> On 9/3/19 9:59 AM, Eric Dumazet wrote:
> 
>> You could add a random delay to all SYN packets, if you believe your host has clock skews.
> 
> And by the way adding delays has its own performance penalties.
> 


You understand your patch has been rejected, right ?

You will have to convince people at IETF and get a proper RFC before
I even look at the idea.

BTW, sending a patch only dealing with IPv4 is also not a good thing.
Eric Dumazet Sept. 3, 2019, 4:17 p.m. UTC | #7
On 9/3/19 6:06 PM, Cyrus Sh wrote:
> 
> 
> On 9/3/19 9:59 AM, Eric Dumazet wrote:
>>
>> You could add a random delay to all SYN packets, if you believe your host has clock skews.
> 
> In theory yes, but again do you know any practical example with tested
> applications and the list of the rules? I'm interested to see an actual example
> that somebody has carried out and observed its results.
> 

Do you have a real program showing us how this clock skew can be used practically ?
Cyrus Sh Sept. 3, 2019, 4:27 p.m. UTC | #8
On 9/3/19 10:17 AM, Eric Dumazet wrote:

> Do you have a real program showing us how this clock skew can be used practically ?
This is a well studied issue. You can take a look at this presentation as an
example:
http://caia.swin.edu.au/talks/CAIA-TALK-080728A.pdf

> You will have to convince people at IETF and get a proper RFC 
No I won't. A lot of these standards have been written at a time that anonymity
networks were not of big importance. Now that they are, we try to lessen the
negative impacts of some RFC deficiencies by improving the implementation. It's
up to you whether to want to keep using a problematic code that may endanger
users or want to do something about it since we won't insist on having a patch
accepted.
Eric Dumazet Sept. 3, 2019, 4:42 p.m. UTC | #9
On 9/3/19 6:27 PM, Cyrus Sh wrote:
> 
> 
> On 9/3/19 10:17 AM, Eric Dumazet wrote:
> 
>> Do you have a real program showing us how this clock skew can be used practically ?
> This is a well studied issue. You can take a look at this presentation as an
> example:
> http://caia.swin.edu.au/talks/CAIA-TALK-080728A.pdf

2008 ? Really ?

I do not want an example, I want a proof that current systems are
exhibiting all the needed behavior.

I do not have time to spend hours reading old stuff based
on old architectures.

> 
>> You will have to convince people at IETF and get a proper RFC 
> No I won't. A lot of these standards have been written at a time that anonymity
> networks were not of big importance. Now that they are, we try to lessen the
> negative impacts of some RFC deficiencies by improving the implementation. It's
> up to you whether to want to keep using a problematic code that may endanger
> users or want to do something about it since we won't insist on having a patch
> accepted.
> 

Then this is the end. linux wont change something as fundamental without
proper feedback from the community.
David Miller Sept. 3, 2019, 10:43 p.m. UTC | #10
From: Cyrus Sh <sirus.shahini@gmail.com>
Date: Tue, 3 Sep 2019 10:06:03 -0600

> On 9/3/19 9:59 AM, Eric Dumazet wrote:
>> 
>> You could add a random delay to all SYN packets, if you believe your host has clock skews.
> 
> In theory yes, but again do you know any practical example with tested
> applications and the list of the rules? I'm interested to see an actual example
> that somebody has carried out and observed its results.

That's ironic that you're asking for specific and "practical" examples
when you submitted a kernel patch that doesn't even compile.
David Miller Sept. 3, 2019, 10:45 p.m. UTC | #11
From: Cyrus Sh <sirus.shahini@gmail.com>
Date: Tue, 3 Sep 2019 10:27:41 -0600

> It's up to you whether to want to keep using a problematic code that
> may endanger users or want to do something about it since we won't
> insist on having a patch accepted.

At least our problematic code, unlike your patch, compiles.
Cyrus Sh Sept. 4, 2019, 12:45 a.m. UTC | #12
On 9/3/19 4:45 PM, David Miller wrote:

> At least our problematic code, unlike your patch, compiles.

I obviously compiled and tested the code before sending along and this should be
easy to understand. Even I published the results in the link that I mentioned in
the initial message. Now I'm not sure what you're doing that you're unable to
compile the code. What compilation error message do you get? This patch has
been written for kernel 5.3-rc7. 
Further the reason that I asked for a specific practical example was that I
wanted to actually test different techniques and see their effectiveness. (Again
should be easy to understand, I don't know why it's not for you and why it made
you upset)
diff mbox series

Patch

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 81e8ade..4ad1bbf 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -241,6 +241,7 @@  void tcp_time_wait(struct sock *sk, int state, int timeo);
 
 /* sysctl variables for tcp */
 extern int sysctl_tcp_max_orphans;
+extern int sysctl_tcp_random_isn;
 extern long sysctl_tcp_mem[3];
 
 #define TCP_RACK_LOSS_DETECTION  0x1 /* Use RACK to detect losses */
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 87aa2a6..ba8927e 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -426,6 +426,7 @@  enum
 	NET_TCP_ALLOWED_CONG_CONTROL=123,
 	NET_TCP_MAX_SSTHRESH=124,
 	NET_TCP_FRTO_RESPONSE=125,
+	NET_IPV4_TCP_RANDOM_ISN=126,
 };
 
 enum {
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 73c1320..0faf7d4 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -332,6 +332,7 @@  static const struct bin_table bin_net_ipv4_netfilter_table[] = {
 };
 
 static const struct bin_table bin_net_ipv4_table[] = {
+	{CTL_INT,   NET_IPV4_TCP_RANDOM_ISN     "tcp_random_isn"}
 	{CTL_INT,	NET_IPV4_FORWARD,			"ip_forward" },
 
 	{ CTL_DIR,	NET_IPV4_CONF,		"conf",		bin_net_ipv4_conf_table },
diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 7b6b1d2..b644bbe 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -22,6 +22,7 @@ 
 
 static siphash_key_t net_secret __read_mostly;
 static siphash_key_t ts_secret __read_mostly;
+static siphash_key_t last_secret = {{0,0}} ;
 
 static __always_inline void net_secret_init(void)
 {
@@ -134,8 +135,29 @@  u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
 		   __be16 sport, __be16 dport)
 {
 	u32 hash;
-
+	u32 temp;
+	
 	net_secret_init();
+	
+	if (sysctl_tcp_random_isn){
+		if (!last_secret.key[0] && !last_secret.key[1]){
+			memcpy(&last_secret,&net_secret,sizeof(last_secret));	
+					
+		}else{
+			temp = *((u32*)&(net_secret.key[0]));
+			temp >>= 8;
+			last_secret.key[0]+=temp;
+			temp = *((u32*)&(net_secret.key[1]));
+			temp >>= 8;
+			last_secret.key[1]+=temp;
+		}
+		hash = siphash_3u32((__force u32)saddr, (__force u32)daddr,
+			        (__force u32)sport << 16 | (__force u32)dport,
+			        &last_secret);
+		return hash;
+	}
+	
+	
 	hash = siphash_3u32((__force u32)saddr, (__force u32)daddr,
 			    (__force u32)sport << 16 | (__force u32)dport,
 			    &net_secret);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 0b980e8..74b2b6a 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -479,6 +479,13 @@  static int proc_fib_multipath_hash_policy(struct ctl_table *table, int write,
 
 static struct ctl_table ipv4_table[] = {
 	{
+    	.procname	= "tcp_random_isn",
+		.data		= &sysctl_tcp_random_isn,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec  
+	},
+	{
 		.procname	= "tcp_max_orphans",
 		.data		= &sysctl_tcp_max_orphans,
 		.maxlen		= sizeof(int),
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c21e8a2..c6b4ebf 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -80,6 +80,7 @@ 
 #include <linux/jump_label_ratelimit.h>
 #include <net/busy_poll.h>
 
+int sysctl_tcp_random_isn __read_mostly = 0;
 int sysctl_tcp_max_orphans __read_mostly = NR_FILE;
 
 #define FLAG_DATA		0x01 /* Incoming frame contained data.		*/