diff mbox series

[v2,12/25] tcp: ipv6: Add AO signing for tcp_v6_send_response

Message ID f9ff27ecc4aabd8ed89d5dfe5195c9cda1e7dc9f.1635784253.git.cdleonard@gmail.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series [v2,01/25] tcp: authopt: Initial support and key management | expand

Commit Message

Leonard Crestez Nov. 1, 2021, 4:34 p.m. UTC 
This is a special code path for acks and resets outside of normal
connection establishment and closing.

Signed-off-by: Leonard Crestez <cdleonard@gmail.com>
---
 net/ipv4/tcp_authopt.c |  2 ++
 net/ipv6/tcp_ipv6.c    | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

Comments

David Ahern Nov. 3, 2021, 2:44 a.m. UTC | #1 
On 11/1/21 10:34 AM, Leonard Crestez wrote:
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 96a29caf56c7..68f9545e4347 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -902,13 +902,37 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>  	struct sock *ctl_sk = net->ipv6.tcp_sk;
>  	unsigned int tot_len = sizeof(struct tcphdr);
>  	__be32 mrst = 0, *topt;
>  	struct dst_entry *dst;
>  	__u32 mark = 0;
> +#ifdef CONFIG_TCP_AUTHOPT
> +	struct tcp_authopt_info *authopt_info = NULL;
> +	struct tcp_authopt_key_info *authopt_key_info = NULL;
> +	u8 authopt_rnextkeyid;
> +#endif
>  
>  	if (tsecr)
>  		tot_len += TCPOLEN_TSTAMP_ALIGNED;
> +#ifdef CONFIG_TCP_AUTHOPT

I realize MD5 is done this way, but new code can always strive to be
better. Put this and the one below in helpers such that this logic is in
the authopt.h file and the intrusion here is a one liner that either
compiles in or out based on the config setting.

> +	/* Key lookup before SKB allocation */
> +	if (static_branch_unlikely(&tcp_authopt_needed) && sk) {
> +		if (sk->sk_state == TCP_TIME_WAIT)
> +			authopt_info = tcp_twsk(sk)->tw_authopt_info;
> +		else
> +			authopt_info = rcu_dereference(tcp_sk(sk)->authopt_info);
> +
> +		if (authopt_info) {
> +			authopt_key_info = __tcp_authopt_select_key(sk, authopt_info, sk,
> +								    &authopt_rnextkeyid);
> +			if (authopt_key_info) {
> +				tot_len += TCPOLEN_AUTHOPT_OUTPUT;
> +				/* Don't use MD5 */
> +				key = NULL;
> +			}
> +		}
> +	}
> +#endif
>  #ifdef CONFIG_TCP_MD5SIG
>  	if (key)
>  		tot_len += TCPOLEN_MD5SIG_ALIGNED;
>  #endif
>  
> @@ -961,10 +985,24 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>  		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
>  				    &ipv6_hdr(skb)->saddr,
>  				    &ipv6_hdr(skb)->daddr, t1);
>  	}
>  #endif
> +#ifdef CONFIG_TCP_AUTHOPT
> +	/* Compute the TCP-AO mac. Unlike in the ipv4 case we have a real SKB */
> +	if (static_branch_unlikely(&tcp_authopt_needed) && authopt_key_info) {
> +		*topt++ = htonl((TCPOPT_AUTHOPT << 24) |
> +				(TCPOLEN_AUTHOPT_OUTPUT << 16) |
> +				(authopt_key_info->send_id << 8) |
> +				(authopt_rnextkeyid));
> +		tcp_authopt_hash((char *)topt,
> +				 authopt_key_info,
> +				 authopt_info,
> +				 (struct sock *)sk,
> +				 buff);
> +	}
> +#endif
>  
>  	memset(&fl6, 0, sizeof(fl6));
>  	fl6.daddr = ipv6_hdr(skb)->saddr;
>  	fl6.saddr = ipv6_hdr(skb)->daddr;
>  	fl6.flowlabel = label;
>
Leonard Crestez Nov. 3, 2021, 10:09 p.m. UTC | #2 
On 11/3/21 4:44 AM, David Ahern wrote:
> On 11/1/21 10:34 AM, Leonard Crestez wrote:
>> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
>> index 96a29caf56c7..68f9545e4347 100644
>> --- a/net/ipv6/tcp_ipv6.c
>> +++ b/net/ipv6/tcp_ipv6.c
>> @@ -902,13 +902,37 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>>   	struct sock *ctl_sk = net->ipv6.tcp_sk;
>>   	unsigned int tot_len = sizeof(struct tcphdr);
>>   	__be32 mrst = 0, *topt;
>>   	struct dst_entry *dst;
>>   	__u32 mark = 0;
>> +#ifdef CONFIG_TCP_AUTHOPT
>> +	struct tcp_authopt_info *authopt_info = NULL;
>> +	struct tcp_authopt_key_info *authopt_key_info = NULL;
>> +	u8 authopt_rnextkeyid;
>> +#endif
>>   
>>   	if (tsecr)
>>   		tot_len += TCPOLEN_TSTAMP_ALIGNED;
>> +#ifdef CONFIG_TCP_AUTHOPT
> 
> I realize MD5 is done this way, but new code can always strive to be
> better. Put this and the one below in helpers such that this logic is in
> the authopt.h file and the intrusion here is a one liner that either
> compiles in or out based on the config setting.

It's not very easy to separate the AO-specific parts here. Key lookup 
determines packet allocation length and whether MD5 should also be 
attempted (RFC claims adding both is invalid). The result of the key 
lookup is the used later to sign bits of the packet.

The IPv4 equivalent is even worse because no explicit reply SKB is 
allocated.

I can try to split tcp_authopt_pick_key_for_response_v6 and 
tcp_authopt_sign_response_v6.

>> +	/* Key lookup before SKB allocation */
>> +	if (static_branch_unlikely(&tcp_authopt_needed) && sk) {
>> +		if (sk->sk_state == TCP_TIME_WAIT)
>> +			authopt_info = tcp_twsk(sk)->tw_authopt_info;
>> +		else
>> +			authopt_info = rcu_dereference(tcp_sk(sk)->authopt_info);
>> +
>> +		if (authopt_info) {
>> +			authopt_key_info = __tcp_authopt_select_key(sk, authopt_info, sk,
>> +								    &authopt_rnextkeyid);
>> +			if (authopt_key_info) {
>> +				tot_len += TCPOLEN_AUTHOPT_OUTPUT;
>> +				/* Don't use MD5 */
>> +				key = NULL;
>> +			}
>> +		}
>> +	}
>> +#endif
>>   #ifdef CONFIG_TCP_MD5SIG
>>   	if (key)
>>   		tot_len += TCPOLEN_MD5SIG_ALIGNED;
>>   #endif
>>   
>> @@ -961,10 +985,24 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>>   		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
>>   				    &ipv6_hdr(skb)->saddr,
>>   				    &ipv6_hdr(skb)->daddr, t1);
>>   	}
>>   #endif
>> +#ifdef CONFIG_TCP_AUTHOPT
>> +	/* Compute the TCP-AO mac. Unlike in the ipv4 case we have a real SKB */
>> +	if (static_branch_unlikely(&tcp_authopt_needed) && authopt_key_info) {
>> +		*topt++ = htonl((TCPOPT_AUTHOPT << 24) |
>> +				(TCPOLEN_AUTHOPT_OUTPUT << 16) |
>> +				(authopt_key_info->send_id << 8) |
>> +				(authopt_rnextkeyid));
>> +		tcp_authopt_hash((char *)topt,
>> +				 authopt_key_info,
>> +				 authopt_info,
>> +				 (struct sock *)sk,
>> +				 buff);
>> +	}
>> +#endif
>>   
>>   	memset(&fl6, 0, sizeof(fl6));
>>   	fl6.daddr = ipv6_hdr(skb)->saddr;
>>   	fl6.saddr = ipv6_hdr(skb)->daddr;
>>   	fl6.flowlabel = label;
>>
>
diff mbox series

Patch

diff --git a/net/ipv4/tcp_authopt.c b/net/ipv4/tcp_authopt.c
index a48b741c83e4..c9d201d8f7a7 100644
--- a/net/ipv4/tcp_authopt.c
+++ b/net/ipv4/tcp_authopt.c
@@ -296,10 +296,11 @@  struct tcp_authopt_key_info *__tcp_authopt_select_key(const struct sock *sk,
 						      const struct sock *addr_sk,
 						      u8 *rnextkeyid)
 {
 	return tcp_authopt_lookup_send(info, addr_sk, -1);
 }
+EXPORT_SYMBOL(__tcp_authopt_select_key);
 
 static struct tcp_authopt_info *__tcp_authopt_info_get_or_create(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct tcp_authopt_info *info;
@@ -1202,10 +1203,11 @@  int tcp_authopt_hash(char *hash_location,
 	 * try to make it obvious inside the packet.
 	 */
 	memset(hash_location, 0, TCP_AUTHOPT_MACLEN);
 	return err;
 }
+EXPORT_SYMBOL(tcp_authopt_hash);
 
 static struct tcp_authopt_key_info *tcp_authopt_lookup_recv(struct sock *sk,
 							    struct sk_buff *skb,
 							    struct tcp_authopt_info *info,
 							    int recv_id)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 96a29caf56c7..68f9545e4347 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -902,13 +902,37 @@  static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
 	struct sock *ctl_sk = net->ipv6.tcp_sk;
 	unsigned int tot_len = sizeof(struct tcphdr);
 	__be32 mrst = 0, *topt;
 	struct dst_entry *dst;
 	__u32 mark = 0;
+#ifdef CONFIG_TCP_AUTHOPT
+	struct tcp_authopt_info *authopt_info = NULL;
+	struct tcp_authopt_key_info *authopt_key_info = NULL;
+	u8 authopt_rnextkeyid;
+#endif
 
 	if (tsecr)
 		tot_len += TCPOLEN_TSTAMP_ALIGNED;
+#ifdef CONFIG_TCP_AUTHOPT
+	/* Key lookup before SKB allocation */
+	if (static_branch_unlikely(&tcp_authopt_needed) && sk) {
+		if (sk->sk_state == TCP_TIME_WAIT)
+			authopt_info = tcp_twsk(sk)->tw_authopt_info;
+		else
+			authopt_info = rcu_dereference(tcp_sk(sk)->authopt_info);
+
+		if (authopt_info) {
+			authopt_key_info = __tcp_authopt_select_key(sk, authopt_info, sk,
+								    &authopt_rnextkeyid);
+			if (authopt_key_info) {
+				tot_len += TCPOLEN_AUTHOPT_OUTPUT;
+				/* Don't use MD5 */
+				key = NULL;
+			}
+		}
+	}
+#endif
 #ifdef CONFIG_TCP_MD5SIG
 	if (key)
 		tot_len += TCPOLEN_MD5SIG_ALIGNED;
 #endif
 
@@ -961,10 +985,24 @@  static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
 		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
 				    &ipv6_hdr(skb)->saddr,
 				    &ipv6_hdr(skb)->daddr, t1);
 	}
 #endif
+#ifdef CONFIG_TCP_AUTHOPT
+	/* Compute the TCP-AO mac. Unlike in the ipv4 case we have a real SKB */
+	if (static_branch_unlikely(&tcp_authopt_needed) && authopt_key_info) {
+		*topt++ = htonl((TCPOPT_AUTHOPT << 24) |
+				(TCPOLEN_AUTHOPT_OUTPUT << 16) |
+				(authopt_key_info->send_id << 8) |
+				(authopt_rnextkeyid));
+		tcp_authopt_hash((char *)topt,
+				 authopt_key_info,
+				 authopt_info,
+				 (struct sock *)sk,
+				 buff);
+	}
+#endif
 
 	memset(&fl6, 0, sizeof(fl6));
 	fl6.daddr = ipv6_hdr(skb)->saddr;
 	fl6.saddr = ipv6_hdr(skb)->daddr;
 	fl6.flowlabel = label;