diff mbox series

[v6,25/26] tcp: authopt: If no keys are valid for send report an error

Message ID 09b6b75d04cf4a439cf84a6d50dcf641c9d727bc.1658815925.git.cdleonard@gmail.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series tcp: Initial support for RFC5925 auth option | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 2 this patch: 3
netdev/cc_maintainers warning 1 maintainers not CCed: pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 7 this patch: 7
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 2 this patch: 3
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Leonard Crestez July 26, 2022, 6:15 a.m. UTC
If this is not treated specially then when all keys are removed or
expired then TCP will start sending unsigned packets which is
undesirable. Instead try to report an error on key selection and
propagate it to userspace.

The error is assigned to sk_err and propagate it as soon as possible.
In theory we could try to make the error "soft" and even let the
connection continue if userspace adds a new key but the advantages are
unclear.

Since userspace is responsible for managing keys it can also avoid
sending unsigned packets by always closing the socket before removing
the active last key.

The specific error reported is ENOKEY.

This requires changes inside TCP option write code to support aborting
the actual packet send, until this point this did not happen in any
scenario.

Signed-off-by: Leonard Crestez <cdleonard@gmail.com>
---
 net/ipv4/tcp_authopt.c |  9 +++++++--
 net/ipv4/tcp_output.c  | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/net/ipv4/tcp_authopt.c b/net/ipv4/tcp_authopt.c
index e162e5944ec5..c71f5ed5ca1d 100644
--- a/net/ipv4/tcp_authopt.c
+++ b/net/ipv4/tcp_authopt.c
@@ -439,10 +439,11 @@  struct tcp_authopt_key_info *__tcp_authopt_select_key(const struct sock *sk,
 						      u8 *rnextkeyid,
 						      bool locked)
 {
 	struct tcp_authopt_key_info *key, *new_key = NULL;
 	struct netns_tcp_authopt *net = sock_net_tcp_authopt(sk);
+	bool anykey = false;
 
 	/* Listen sockets don't refer to any specific connection so we don't try
 	 * to keep using the same key.
 	 * The rnextkeyid is stored in tcp_request_sock
 	 */
@@ -461,11 +462,13 @@  struct tcp_authopt_key_info *__tcp_authopt_select_key(const struct sock *sk,
 		else
 			send_id = rsk->recv_rnextkeyid;
 		key = tcp_authopt_lookup_send(net, addr_sk, send_id, NULL);
 		/* If no key found with specific send_id try anything else. */
 		if (!key)
-			key = tcp_authopt_lookup_send(net, addr_sk, -1, NULL);
+			key = tcp_authopt_lookup_send(net, addr_sk, -1, &anykey);
+		if (!key && anykey)
+			return ERR_PTR(-ENOKEY);
 		if (key)
 			*rnextkeyid = key->recv_id;
 		return key;
 	}
 
@@ -497,11 +500,13 @@  struct tcp_authopt_key_info *__tcp_authopt_select_key(const struct sock *sk,
 							  info->recv_rnextkeyid,
 							  NULL);
 	}
 	/* If no key found with specific send_id try anything else. */
 	if (!key && !new_key)
-		new_key = tcp_authopt_lookup_send(net, addr_sk, -1, NULL);
+		new_key = tcp_authopt_lookup_send(net, addr_sk, -1, &anykey);
+	if (!new_key && anykey)
+		return ERR_PTR(-ENOKEY);
 
 	/* Update current key only if we hold the socket lock. */
 	if (new_key && key != new_key) {
 		if (locked) {
 			if (kref_get_unless_zero(&new_key->ref)) {
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0ab3c7801f33..b8dab313af0f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -414,10 +414,11 @@  static inline bool tcp_urg_mode(const struct tcp_sock *tp)
 #define OPTION_SACK_ADVERTISE	BIT(0)
 #define OPTION_TS		BIT(1)
 #define OPTION_MD5		BIT(2)
 #define OPTION_WSCALE		BIT(3)
 #define OPTION_AUTHOPT		BIT(4)
+#define OPTION_AUTHOPT_FAIL	BIT(5)
 #define OPTION_FAST_OPEN_COOKIE	BIT(8)
 #define OPTION_SMC		BIT(9)
 #define OPTION_MPTCP		BIT(10)
 
 static void smc_options_write(__be32 *ptr, u16 *options)
@@ -786,10 +787,14 @@  static int tcp_authopt_init_options(const struct sock *sk,
 {
 #ifdef CONFIG_TCP_AUTHOPT
 	struct tcp_authopt_key_info *key;
 
 	key = tcp_authopt_select_key(sk, addr_sk, &opts->authopt_info, &opts->authopt_rnextkeyid);
+	if (IS_ERR(key)) {
+		opts->options |= OPTION_AUTHOPT_FAIL;
+		return TCPOLEN_AUTHOPT_OUTPUT;
+	}
 	if (key) {
 		opts->options |= OPTION_AUTHOPT;
 		opts->authopt_key = key;
 		return TCPOLEN_AUTHOPT_OUTPUT;
 	}
@@ -1345,10 +1350,18 @@  static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
 		 * release the following packet.
 		 */
 		if (tcp_skb_pcount(skb) > 1)
 			tcb->tcp_flags |= TCPHDR_PSH;
 	}
+#ifdef CONFIG_TCP_AUTHOPT
+	if (opts.options & OPTION_AUTHOPT_FAIL) {
+		rcu_read_unlock();
+		sk->sk_err = ENOKEY;
+		sk_error_report(sk);
+		return -ENOKEY;
+	}
+#endif
 	tcp_header_size = tcp_options_size + sizeof(struct tcphdr);
 
 	/* if no packet is in qdisc/device queue, then allow XPS to select
 	 * another queue. We can be called from tcp_tsq_handler()
 	 * which holds one reference to sk.
@@ -3655,10 +3668,17 @@  struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
 	/* bpf program will be interested in the tcp_flags */
 	TCP_SKB_CB(skb)->tcp_flags = TCPHDR_SYN | TCPHDR_ACK;
 	tcp_header_size = tcp_synack_options(sk, req, mss, skb, &opts, md5,
 					     foc, synack_type,
 					     syn_skb) + sizeof(*th);
+#ifdef CONFIG_TCP_AUTHOPT
+	if (opts.options & OPTION_AUTHOPT_FAIL) {
+		rcu_read_unlock();
+		kfree_skb(skb);
+		return NULL;
+	}
+#endif
 
 	skb_push(skb, tcp_header_size);
 	skb_reset_transport_header(skb);
 
 	th = (struct tcphdr *)skb->data;