Skip to contentSkip to conversation

tcp: Initial support for RFC5925 auth option

PATCH Superseded Leonard Crestez 5 participants 55 messages Nov 1, 2021 – Nov 5, 2021

Subsystems (6)

v2 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 tcp: Initial support for RFC5925 auth option

This is similar to TCP MD5 in functionality but it's sufficiently different that wire formats are incompatible. Compared to TCP-MD5 more algorithms are supported and multiple keys can be used on the same connection but there is still no negotiation mechanism.

Expected use-case is protecting long-duration BGP/LDP connections between routers using pre-shared keys. The goal of this series is to allow routers using the linux TCP stack to interoperate with vendors such as Cisco and Juniper.

Both algorithms described in RFC5926 are implemented but the code is not very easily extensible beyond that. In particular there are several code paths making stack allocations based on RFC5926 maximum, those would have to be increased.

This version implements SNE and l3mdev awareness and adds more tests. Here are some known flaws and limitations:

* Interaction with TCP-MD5 not tested in all corners
* Interaction with FASTOPEN not tested and unlikely to work because sequence number assumptions for syn/ack.
* Not clear if crypto_shash_setkey might sleep. If some implementation do that then maybe they could be excluded through alloc flags.
* Traffic key is not cached (reducing performance)
* User is responsible for ensuring keys do not overlap.
* There is no useful way to list keys, making userspace debug difficult.
* There is no prefixlen support equivalent to md5. This is used in some complex FRR configs.

Test suite was added to tools/selftests/tcp_authopt. Tests are written in python using pytest and scapy and check the API in some detail and validate packet captures. Python code is already used in linux and in kselftests but virtualenvs not very much, this particular test suite uses pip to create a private virtualenv and hide dependencies.

This actually forms the bulk of the series by raw line-count. Since there is a lot of code it was mostly split on "functional area" so most files are only affected by a single code. A lot of those tests are relevant to TCP-MD5 so perhaps it might help to split into a separate series?

Some testing support is included in nettest and fcnal-test.sh, similar to the current level of tcp-md5 testing.

SNE was tested by creating connections in a loop until a large SEQ is randomly selected and then making it rollover. The "connect in a loop" step ran into timewait overflow and connection failure on port reuse. After spending some time on this issue and my conclusion is that AO makes it impossible to kill remainders of old connections in a manner similar to unsigned or md5sig, this is because signatures are dependent on ISNs. This means that if a timewait socket is closed improperly then information required to RST the peer is lost.

The fact that AO completely breaks all connection-less RSTs is acknowledged in the RFC and the workaround of "respect timewait" seems acceptable.

Changes for frr (old): https://github.com/FRRouting/frr/pull/9442
That PR was made early for ABI feedback, it has many issues.

Changes for yabgp (old): https://github.com/cdleonard/yabgp/commits/tcp_authopt
This can be use for easy interoperability testing with cisco/juniper/etc.

Changes since PATCH v1:
* Implement Sequence Number Extension
* Implement l3index for vrf: TCP_AUTHOPT_KEY_IFINDEX as equivalent of TCP_MD5SIG_FLAG_IFINDEX
* Expand TCP-AO tests in fcnal-test.sh to near-parity with md5.
* Show addr/port on failure similar to md5
* Remove tox dependency from test suite (create venv directly)
* Switch default pytest output format to TAP (kselftest standard)
* Fix _copy_from_sockptr_tolerant stack corruption on short sockopts. This was covered in test but error was invisible without STACKPROTECTOR=y
* Fix sysctl_tcp_authopt check in tcp_get_authopt_val before memset. This was harmless because error code is checked in getsockopt anyway.
* Fix dropping md5 packets on all sockets with AO enabled
* Fix checking (key->recv_id & TCP_AUTHOPT_KEY_ADDR_BIND) instead of key->flags in tcp_authopt_key_match_exact
* Fix PATCH 1/19 not compiling due to missing "int err" declaration
* Add ratelimited message for AO and MD5 both present
* Export all symbols required by CONFIG_IPV6=m (again)
* Fix compilation with CONFIG_TCP_AUTHOPT=y CONFIG_TCP_MD5SIG=n
* Fix checkpatch issues
* Pass -rrequirements.txt to tox to avoid dependency variation.

Changes since RFCv3:
* Implement TCP_AUTHOPT handling for timewait and reset replies. Write tests to execute these paths by injecting packets with scapy
* Handle combining md5 and authopt: if both are configured use authopt.
* Fix locking issues around send_key, introduced in on of the later patches.
* Handle IPv4-mapped-IPv6 addresses: it used to be that an ipv4 SYN sent to an ipv6 socket with TCP-AO triggered WARN
* Implement un-namespaced sysctl disabled this feature by default
* Allocate new key before removing any old one in setsockopt (Dmitry)
* Remove tcp_authopt_key_info.local_id because it's no longer used (Dmitry)
* Propagate errors from TCP_AUTHOPT getsockopt (Dmitry)
* Fix no-longer-correct TCP_AUTHOPT_KEY_DEL docs (Dmitry)
* Simplify crypto allocation (Eric)
* Use kzmalloc instead of __GFP_ZERO (Eric)
* Add static_key_false tcp_authopt_needed (Eric)
* Clear authopt_info copied from oldsk in __tcp_authopt_openreq (Eric)
* Replace memcmp in ipv4 and ipv6 addr comparisons (Eric)
* Export symbols for CONFIG_IPV6=m (kernel test robot)
* Mark more functions static (kernel test robot)
* Fix build with CONFIG_PROVE_RCU_LIST=y (kernel test robot)

Changes since RFCv2:
* Removed local_id from ABI and match on send_id/recv_id/addr
* Add all relevant out-of-tree tests to tools/testing/selftests
* Return an error instead of ignoring unknown flags, hopefully this makes it easier to extend.
* Check sk_family before __tcp_authopt_info_get_or_create in tcp_set_authopt_key
* Use sock_owned_by_me instead of WARN_ON(!lockdep_sock_is_held(sk))
* Fix some intermediate build failures reported by kbuild robot
* Improve documentation

Changes since RFC:
* Split into per-topic commits for ease of review. The intermediate commits compile with a few "unused function" warnings and don't do anything useful by themselves.
* Add ABI documention including kernel-doc on uapi
* Fix lockdep warnings from crypto by creating pools with one shash for each cpu
* Accept short options to setsockopt by padding with zeros; this approach allows increasing the size of the structs in the future.
* Support for aes-128-cmac-96
* Support for binding addresses to keys in a way similar to old tcp_md5
* Add support for retrieving received keyid/rnextkeyid and controling the keyid/rnextkeyid being sent.

Leonard Crestez (25):
tcp: authopt: Initial support and key management
docs: Add user documentation for tcp_authopt
selftests: Initial tcp_authopt test module
selftests: tcp_authopt: Initial sockopt manipulation
tcp: authopt: Add crypto initialization
tcp: authopt: Compute packet signatures
tcp: Use BIT() for OPTION_* constants
tcp: authopt: Hook into tcp core
tcp: authopt: Disable via sysctl by default
selftests: tcp_authopt: Test key address binding
tcp: authopt: Implement Sequence Number Extension
tcp: ipv6: Add AO signing for tcp_v6_send_response
tcp: authopt: Add support for signing skb-less replies
tcp: ipv4: Add AO signing for skb-less replies
selftests: tcp_authopt: Implement SNE in python
selftests: tcp_authopt: Add scapy-based packet signing code
selftests: tcp_authopt: Add packet-level tests
selftests: tcp_authopt: Initial sne test
tcp: authopt: Add key selection controls
selftests: tcp_authopt: Add tests for rollover
tcp: authopt: Add initial l3index support
selftests: tcp_authopt: Initial tests for l3mdev handling
selftests: nettest: Rename md5_prefix to key_addr_prefix
selftests: nettest: Initial tcp_authopt support
selftests: net/fcnal: Initial tcp_authopt support

 Documentation/networking/index.rst            |    1 +
 Documentation/networking/ip-sysctl.rst        |    6 +
 Documentation/networking/tcp_authopt.rst      |   69 +
 include/linux/tcp.h                           |    9 +
 include/net/tcp.h                             |    1 +
 include/net/tcp_authopt.h                     |  271 +++
 include/uapi/linux/snmp.h                     |    1 +
 include/uapi/linux/tcp.h                      |  123 ++
 net/ipv4/Kconfig                              |   14 +
 net/ipv4/Makefile                             |    1 +
 net/ipv4/proc.c                               |    1 +
 net/ipv4/sysctl_net_ipv4.c                    |   10 +
 net/ipv4/tcp.c                                |   30 +
 net/ipv4/tcp_authopt.c                        | 1617 +++++++++++++++++
 net/ipv4/tcp_input.c                          |   18 +
 net/ipv4/tcp_ipv4.c                           |  104 +-
 net/ipv4/tcp_minisocks.c                      |   12 +
 net/ipv4/tcp_output.c                         |  100 +-
 net/ipv6/tcp_ipv6.c                           |   60 +-
 tools/testing/selftests/net/fcnal-test.sh     |  249 +++
 tools/testing/selftests/net/nettest.c         |  123 +-
 tools/testing/selftests/tcp_authopt/Makefile  |   10 +
 .../testing/selftests/tcp_authopt/README.rst  |   18 +
 tools/testing/selftests/tcp_authopt/config    |    6 +
 .../selftests/tcp_authopt/requirements.txt    |   46 +
 tools/testing/selftests/tcp_authopt/run.sh    |   31 +
 tools/testing/selftests/tcp_authopt/settings  |    1 +
 tools/testing/selftests/tcp_authopt/setup.cfg |   35 +
 tools/testing/selftests/tcp_authopt/setup.py  |    6 +
 .../tcp_authopt/tcp_authopt_test/__init__.py  |    0
 .../tcp_authopt/tcp_authopt_test/conftest.py  |   71 +
 .../full_tcp_sniff_session.py                 |   91 +
 .../tcp_authopt_test/linux_tcp_authopt.py     |  285 +++
 .../tcp_authopt_test/linux_tcp_md5sig.py      |  110 ++
 .../tcp_authopt_test/linux_tcp_repair.py      |   67 +
 .../tcp_authopt_test/netns_fixture.py         |   85 +
 .../tcp_authopt_test/scapy_conntrack.py       |  173 ++
 .../tcp_authopt_test/scapy_tcp_authopt.py     |  220 +++
 .../tcp_authopt_test/scapy_utils.py           |  177 ++
 .../tcp_authopt/tcp_authopt_test/server.py    |  124 ++
 .../tcp_authopt/tcp_authopt_test/sne_alg.py   |  111 ++
 .../tcp_authopt/tcp_authopt_test/sockaddr.py  |  122 ++
 .../tcp_connection_fixture.py                 |  276 +++
 .../tcp_authopt/tcp_authopt_test/test_bind.py |  155 ++
 .../tcp_authopt_test/test_rollover.py         |  181 ++
 .../tcp_authopt/tcp_authopt_test/test_sne.py  |  202 ++
 .../tcp_authopt_test/test_sne_alg.py          |   96 +
 .../tcp_authopt_test/test_sockopt.py          |  203 +++
 .../tcp_authopt_test/test_vectors.py          |  365 ++++
 .../tcp_authopt_test/test_verify_capture.py   |  559 ++++++
 .../tcp_authopt_test/test_vrf_bind.py         |  492 +++++
 .../tcp_authopt/tcp_authopt_test/utils.py     |  114 ++
 .../tcp_authopt/tcp_authopt_test/validator.py |  138 ++
 .../tcp_authopt_test/vrf_netns_fixture.py     |  127 ++
 54 files changed, 7471 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/networking/tcp_authopt.rst
 create mode 100644 include/net/tcp_authopt.h
 create mode 100644 net/ipv4/tcp_authopt.c
 create mode 100644 tools/testing/selftests/tcp_authopt/Makefile
 create mode 100644 tools/testing/selftests/tcp_authopt/README.rst
 create mode 100644 tools/testing/selftests/tcp_authopt/config
 create mode 100644 tools/testing/selftests/tcp_authopt/requirements.txt
 create mode 100755 tools/testing/selftests/tcp_authopt/run.sh
 create mode 100644 tools/testing/selftests/tcp_authopt/settings
 create mode 100644 tools/testing/selftests/tcp_authopt/setup.cfg
 create mode 100644 tools/testing/selftests/tcp_authopt/setup.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/__init__.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/conftest.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/full_tcp_sniff_session.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_authopt.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_md5sig.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_repair.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/netns_fixture.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_conntrack.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_tcp_authopt.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_utils.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/server.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/sne_alg.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/sockaddr.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/tcp_connection_fixture.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_bind.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_rollover.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sne.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sne_alg.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sockopt.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_vectors.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_verify_capture.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_vrf_bind.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/utils.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/validator.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/vrf_netns_fixture.py

base-commit: d4a07dc5ac34528f292a4f328cf3c65aba312e1b

Signature · 1 line
2.25.1
#1 lore raw

PATCH v2 1/25 tcp: authopt: Initial support and key management

This commit adds support to add and remove keys but does not use them further.

Similar to tcp md5 a single pointer to a struct tcp_authopt_info* struct is added to struct tcp_sock, this avoids increasing memory usage. The data structures related to tcp_authopt are initialized on setsockopt and only freed on socket close.

---
 include/linux/tcp.h       |   9 ++
 include/net/tcp.h         |   1 +
 include/net/tcp_authopt.h |  76 +++++++++++
 include/uapi/linux/tcp.h  |  81 ++++++++++++
 net/ipv4/Kconfig          |  14 ++
 net/ipv4/Makefile         |   1 +
 net/ipv4/tcp.c            |  30 +++++
 net/ipv4/tcp_authopt.c    | 263 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_ipv4.c       |   2 +
 9 files changed, 477 insertions(+)
 create mode 100644 include/net/tcp_authopt.h
 create mode 100644 net/ipv4/tcp_authopt.c
include/linux/tcp.h +9 -0
index 48d8a363319e..50038f35ba51 100644
@@ -140,10 +140,12 @@ struct tcp_request_sock {
 static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req)
 {
 	return (struct tcp_request_sock *)req;
 }
 
+struct tcp_authopt_info;
+
 struct tcp_sock {
 	/* inet_connection_sock has to be the first member of tcp_sock */
 	struct inet_connection_sock	inet_conn;
 	u16	tcp_header_len;	/* Bytes of tcp header to send		*/
 	u16	gso_segs;	/* Max number of segs per GSO packet	*/
@@ -403,10 +405,14 @@ struct tcp_sock {
 
 /* TCP MD5 Signature Option information */
 	struct tcp_md5sig_info	__rcu *md5sig_info;
 #endif
 
+#ifdef CONFIG_TCP_AUTHOPT
+	struct tcp_authopt_info	__rcu *authopt_info;
+#endif
+
 /* TCP fastopen related information */
 	struct tcp_fastopen_request *fastopen_req;
 	/* fastopen_rsk points to request_sock that resulted in this big
 	 * socket. Used to retransmit SYNACKs etc.
 	 */
@@ -453,10 +459,13 @@ struct tcp_timewait_sock {
 	int			  tw_ts_recent_stamp;
 	u32			  tw_tx_delay;
 #ifdef CONFIG_TCP_MD5SIG
 	struct tcp_md5sig_key	  *tw_md5_key;
 #endif
+#ifdef CONFIG_TCP_AUTHOPT
+	struct tcp_authopt_info	  *tw_authopt_info;
+#endif
 };
 
 static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk)
 {
 	return (struct tcp_timewait_sock *)sk;
include/net/tcp.h +1 -0
index 8e8c5922a7b0..620bd2c9250b 100644
@@ -184,10 +184,11 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
 #define TCPOPT_WINDOW		3	/* Window scaling */
 #define TCPOPT_SACK_PERM        4       /* SACK Permitted */
 #define TCPOPT_SACK             5       /* SACK Block */
 #define TCPOPT_TIMESTAMP	8	/* Better RTT estimations/PAWS */
 #define TCPOPT_MD5SIG		19	/* MD5 Signature (RFC2385) */
+#define TCPOPT_AUTHOPT		29	/* Auth Option (RFC5925) */
 #define TCPOPT_MPTCP		30	/* Multipath TCP (RFC6824) */
 #define TCPOPT_FASTOPEN		34	/* Fast open (RFC7413) */
 #define TCPOPT_EXP		254	/* Experimental */
 /* Magic number to be after the option value for sharing TCP
  * experimental options. See draft-ietf-tcpm-experimental-options-00.txt
include/net/tcp_authopt.h new file +76 -0

76 lines not on this page · raw message

include/uapi/linux/tcp.h +81 -0

101 lines not on this page · raw message

net/ipv4/Kconfig +14 -0
index 87983e70f03f..6459f4ea6f1d 100644
@@ -740,5 +740,19 @@ config TCP_MD5SIG
 	  RFC2385 specifies a method of giving MD5 protection to TCP sessions.
 	  Its main (only?) use is to protect BGP sessions between core routers
 	  on the Internet.
 
 	  If unsure, say N.
+
+config TCP_AUTHOPT
+	bool "TCP: Authentication Option support (RFC5925)"
+	select CRYPTO
+	select CRYPTO_SHA1
+	select CRYPTO_HMAC
+	select CRYPTO_AES
+	select CRYPTO_CMAC
+	help
+	  RFC5925 specifies a new method of giving protection to TCP sessions.
+	  Its intended use is to protect BGP sessions between core routers
+	  on the Internet. It obsoletes TCP MD5 (RFC2385) but is incompatible.
+
+	  If unsure, say N.
net/ipv4/Makefile +1 -0
index bbdd9c44f14e..d336f32ce177 100644
@@ -59,10 +59,11 @@ obj-$(CONFIG_TCP_CONG_NV) += tcp_nv.o
 obj-$(CONFIG_TCP_CONG_VENO) += tcp_veno.o
 obj-$(CONFIG_TCP_CONG_SCALABLE) += tcp_scalable.o
 obj-$(CONFIG_TCP_CONG_LP) += tcp_lp.o
 obj-$(CONFIG_TCP_CONG_YEAH) += tcp_yeah.o
 obj-$(CONFIG_TCP_CONG_ILLINOIS) += tcp_illinois.o
+obj-$(CONFIG_TCP_AUTHOPT) += tcp_authopt.o
 obj-$(CONFIG_NET_SOCK_MSG) += tcp_bpf.o
 obj-$(CONFIG_BPF_SYSCALL) += udp_bpf.o
 obj-$(CONFIG_NETLABEL) += cipso_ipv4.o
 
 obj-$(CONFIG_XFRM) += xfrm4_policy.o xfrm4_state.o xfrm4_input.o \
net/ipv4/tcp.c +30 -0
index a7b1138d619c..d303fb84802a 100644
@@ -271,10 +271,11 @@
 
 #include <net/icmp.h>
 #include <net/inet_common.h>
 #include <net/tcp.h>
 #include <net/mptcp.h>
+#include <net/tcp_authopt.h>
 #include <net/xfrm.h>
 #include <net/ip.h>
 #include <net/sock.h>
 
 #include <linux/uaccess.h>
@@ -3552,10 +3553,16 @@ static int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 	case TCP_MD5SIG:
 	case TCP_MD5SIG_EXT:
 		err = tp->af_specific->md5_parse(sk, optname, optval, optlen);
 		break;
 #endif
+	case TCP_AUTHOPT:
+		err = tcp_set_authopt(sk, optval, optlen);
+		break;
+	case TCP_AUTHOPT_KEY:
+		err = tcp_set_authopt_key(sk, optval, optlen);
+		break;
 	case TCP_USER_TIMEOUT:
 		/* Cap the max time in ms TCP will retry or probe the window
 		 * before giving up and aborting (ETIMEDOUT) a connection.
 		 */
 		if (val < 0)
@@ -4198,10 +4205,33 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
 		if (!err && copy_to_user(optval, &zc, len))
 			err = -EFAULT;
 		return err;
 	}
 #endif
+#ifdef CONFIG_TCP_AUTHOPT
+	case TCP_AUTHOPT: {
+		struct tcp_authopt info;
+		int err;
+
+		if (get_user(len, optlen))
+			return -EFAULT;
+
+		lock_sock(sk);
+		err = tcp_get_authopt_val(sk, &info);
+		release_sock(sk);
+
+		if (err)
+			return err;
+		len = min_t(unsigned int, len, sizeof(info));
+		if (put_user(len, optlen))
+			return -EFAULT;
+		if (copy_to_user(optval, &info, len))
+			return -EFAULT;
+		return 0;
+	}
+#endif
+
 	default:
 		return -ENOPROTOOPT;
 	}
 
 	if (put_user(len, optlen))
net/ipv4/tcp_authopt.c new file +263 -0

263 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +2 -0
index 13d868c43284..0c9f050fa0e8 100644
@@ -60,10 +60,11 @@
 
 #include <net/net_namespace.h>
 #include <net/icmp.h>
 #include <net/inet_hashtables.h>
 #include <net/tcp.h>
+#include <net/tcp_authopt.h>
 #include <net/transp_v6.h>
 #include <net/ipv6.h>
 #include <net/inet_common.h>
 #include <net/timewait_sock.h>
 #include <net/xfrm.h>
@@ -2276,10 +2277,11 @@ void tcp_v4_destroy_sock(struct sock *sk)
 		tcp_clear_md5_list(sk);
 		kfree_rcu(rcu_dereference_protected(tp->md5sig_info, 1), rcu);
 		tp->md5sig_info = NULL;
 	}
 #endif
+	tcp_authopt_clear(sk);
 
 	/* Clean up a referenced TCP bind bucket. */
 	if (inet_csk(sk)->icsk_bind_hash)
 		inet_put_port(sk);
 
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH v2 01/25] tcp: authopt: Initial support and key management

Quoted 17 lines · Leonard Crestez
net/ipv4/tcp_authopt.c new file +11 -0
new file mode 100644
index 000000000000..c412a712f229
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/kernel.h>
+#include <net/tcp.h>
+#include <net/tcp_authopt.h>
+#include <crypto/hash.h>
+
+/* checks that ipv4 or ipv6 addr matches. */
+static bool ipvx_addr_match(struct sockaddr_storage *a1,
+			    struct sockaddr_storage *a2)
+{

net/ipv4/tcp_authopt.c line 21

Quoted 10 lines
net/ipv4/tcp_authopt.c new file +10 -0
+	if (a1->ss_family != a2->ss_family)
+		return false;
+	if (a1->ss_family == AF_INET &&
+	    (((struct sockaddr_in *)a1)->sin_addr.s_addr !=
+	     ((struct sockaddr_in *)a2)->sin_addr.s_addr))
+		return false;
+	if (a1->ss_family == AF_INET6 &&
+	    !ipv6_addr_equal(&((struct sockaddr_in6 *)a1)->sin6_addr,
+			     &((struct sockaddr_in6 *)a2)->sin6_addr))
+		return false;

The above 2 could just be

	if (a1->ss_family == AF_INET)
		return (((struct sockaddr_in *)a1)->sin_addr.s_addr ==
			((struct sockaddr_in *)a2)->sin_addr.s_addr))
#3 lore raw

Re: [PATCH v2 01/25] tcp: authopt: Initial support and key management

Quoted 31 lines · David Ahern
Quoted 17 lines · Leonard Crestez
net/ipv4/tcp_authopt.c new file +11 -0
new file mode 100644
index 000000000000..c412a712f229
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/kernel.h>
+#include <net/tcp.h>
+#include <net/tcp_authopt.h>
+#include <crypto/hash.h>
+
+/* checks that ipv4 or ipv6 addr matches. */
+static bool ipvx_addr_match(struct sockaddr_storage *a1,
+			    struct sockaddr_storage *a2)
+{

net/ipv4/tcp_authopt.c line 21

Quoted 10 lines
net/ipv4/tcp_authopt.c new file +10 -0
+	if (a1->ss_family != a2->ss_family)
+		return false;
+	if (a1->ss_family == AF_INET &&
+	    (((struct sockaddr_in *)a1)->sin_addr.s_addr !=
+	     ((struct sockaddr_in *)a2)->sin_addr.s_addr))
+		return false;
+	if (a1->ss_family == AF_INET6 &&
+	    !ipv6_addr_equal(&((struct sockaddr_in6 *)a1)->sin6_addr,
+			     &((struct sockaddr_in6 *)a2)->sin6_addr))
+		return false;

The above 2 could just be

Quoted 3 lines
	if (a1->ss_family == AF_INET)
		return (((struct sockaddr_in *)a1)->sin_addr.s_addr ==
			((struct sockaddr_in *)a2)->sin_addr.s_addr))

OK. The function is a little weird that it has a final "return true" which is technically also reachable if AF is unexpected but that situation is prevented from higher up.

--
Regards,
Leonard

#4 lore raw

Re: [PATCH v2 01/25] tcp: authopt: Initial support and key management

In reply to Leonard Crestez

Hi Leonard,

On 11/1/21 16:34, Leonard Crestez wrote:
[..]

Quoted 20 lines
include/uapi/linux/tcp.h +20 -0
+struct tcp_authopt_key {
+	/** @flags: Combination of &enum tcp_authopt_key_flag */
+	__u32	flags;
+	/** @send_id: keyid value for send */
+	__u8	send_id;
+	/** @recv_id: keyid value for receive */
+	__u8	recv_id;
+	/** @alg: One of &enum tcp_authopt_alg */
+	__u8	alg;
+	/** @keylen: Length of the key buffer */
+	__u8	keylen;
+	/** @key: Secret key */
+	__u8	key[TCP_AUTHOPT_MAXKEYLEN];
+	/**
+	 * @addr: Key is only valid for this address
+	 *
+	 * Ignored unless TCP_AUTHOPT_KEY_ADDR_BIND flag is set
+	 */
+	struct __kernel_sockaddr_storage addr;
+};

[..]

Quoted 10 lines
net/ipv4/tcp_authopt.c new file +10 -0
+/* Free key nicely, for living sockets */
+static void tcp_authopt_key_del(struct sock *sk,
+				struct tcp_authopt_info *info,
+				struct tcp_authopt_key_info *key)
+{
+	sock_owned_by_me(sk);
+	hlist_del_rcu(&key->node);
+	atomic_sub(sizeof(*key), &sk->sk_omem_alloc);
+	kfree_rcu(key, rcu);
+}

[..]

Quoted 7 lines
net/ipv4/tcp_authopt.c new file +7 -0
+#define TCP_AUTHOPT_KEY_KNOWN_FLAGS ( \
+	TCP_AUTHOPT_KEY_DEL | \
+	TCP_AUTHOPT_KEY_EXCLUDE_OPTS | \
+	TCP_AUTHOPT_KEY_ADDR_BIND)
+
+int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
+{

[..]

net/ipv4/tcp_authopt.c line 230

Quoted 10 lines
net/ipv4/tcp_authopt.c new file +10 -0
+	/* Delete is a special case: */
+	if (opt.flags & TCP_AUTHOPT_KEY_DEL) {
+		info = rcu_dereference_check(tcp_sk(sk)->authopt_info, lockdep_sock_is_held(sk));
+		if (!info)
+			return -ENOENT;
+		key_info = tcp_authopt_key_lookup_exact(sk, info, &opt);
+		if (!key_info)
+			return -ENOENT;
+		tcp_authopt_key_del(sk, info, key_info);
+		return 0;

I remember we discussed it in RFC, that removing a key that's currently in use may result in random MKT to be used.

I think, it's possible to make this API a bit more predictable if:
- DEL command fails to remove a key that is current/receive_next;
- opt.flags has CURR/NEXT flag that has corresponding u8 current_key and u8 receive_next values. As socket lock is held - that makes current_key/receive_next change atomic with deletion of an existing key that might have been in use.

In result user may remove a key that's not in use or has to set new current/next. Which avoids the issue with random MKT being used to sign segments.

Thanks,
Dmitry

#5 lore raw

Re: [PATCH v2 01/25] tcp: authopt: Initial support and key management

Quoted 65 lines · Dmitry Safonov

Hi Leonard,

On 11/1/21 16:34, Leonard Crestez wrote:
[..]

Quoted 20 lines
include/uapi/linux/tcp.h +20 -0
+struct tcp_authopt_key {
+	/** @flags: Combination of &enum tcp_authopt_key_flag */
+	__u32	flags;
+	/** @send_id: keyid value for send */
+	__u8	send_id;
+	/** @recv_id: keyid value for receive */
+	__u8	recv_id;
+	/** @alg: One of &enum tcp_authopt_alg */
+	__u8	alg;
+	/** @keylen: Length of the key buffer */
+	__u8	keylen;
+	/** @key: Secret key */
+	__u8	key[TCP_AUTHOPT_MAXKEYLEN];
+	/**
+	 * @addr: Key is only valid for this address
+	 *
+	 * Ignored unless TCP_AUTHOPT_KEY_ADDR_BIND flag is set
+	 */
+	struct __kernel_sockaddr_storage addr;
+};

[..]

Quoted 10 lines
net/ipv4/tcp_authopt.c new file +10 -0
+/* Free key nicely, for living sockets */
+static void tcp_authopt_key_del(struct sock *sk,
+				struct tcp_authopt_info *info,
+				struct tcp_authopt_key_info *key)
+{
+	sock_owned_by_me(sk);
+	hlist_del_rcu(&key->node);
+	atomic_sub(sizeof(*key), &sk->sk_omem_alloc);
+	kfree_rcu(key, rcu);
+}

[..]

Quoted 7 lines
net/ipv4/tcp_authopt.c new file +7 -0
+#define TCP_AUTHOPT_KEY_KNOWN_FLAGS ( \
+	TCP_AUTHOPT_KEY_DEL | \
+	TCP_AUTHOPT_KEY_EXCLUDE_OPTS | \
+	TCP_AUTHOPT_KEY_ADDR_BIND)
+
+int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
+{

[..]

net/ipv4/tcp_authopt.c line 230

Quoted 10 lines
net/ipv4/tcp_authopt.c new file +10 -0
+	/* Delete is a special case: */
+	if (opt.flags & TCP_AUTHOPT_KEY_DEL) {
+		info = rcu_dereference_check(tcp_sk(sk)->authopt_info, lockdep_sock_is_held(sk));
+		if (!info)
+			return -ENOENT;
+		key_info = tcp_authopt_key_lookup_exact(sk, info, &opt);
+		if (!key_info)
+			return -ENOENT;
+		tcp_authopt_key_del(sk, info, key_info);
+		return 0;

I remember we discussed it in RFC, that removing a key that's currently in use may result in random MKT to be used.

I think, it's possible to make this API a bit more predictable if:
- DEL command fails to remove a key that is current/receive_next;
- opt.flags has CURR/NEXT flag that has corresponding u8 current_key and u8 receive_next values. As socket lock is held - that makes current_key/receive_next change atomic with deletion of an existing key that might have been in use.

Quoted 3 lines

In result user may remove a key that's not in use or has to set new current/next. Which avoids the issue with random MKT being used to sign segments.

The MKT used to sign segments is already essentially random unless the user makes a deliberate choice. This is what happens if you add two keys an call connect(). But why is this a problem?

Applications which want to deliberately control the send key can do so with TCP_AUTHOPT_FLAG_LOCK_KEYID. If that flag is not set then the key with send_id == recv_rnextkeyid is preffered as suggested by the RFC, or a random one on connect.

I think your suggestion would force additional complexity on all applications for no clear gain.

Key selection controls are only added much later in the series, this is also part of the effort to split the code into readable patches. See this patch:

https://lore.kernel.org/netdev/2dc569c0d60c80c26aafcaa201ba5b5ec53ce6bd.1635784253.git.cdleonard@gmail.com/

Removing a key while traffic is happening shouldn't cause failures in recv or send code; this takes some effort but is also required to prevent auth failures when a socket is closed and transitions to timewait. I attempted to ensure this by only doing rcu_dereference for tcp_authopt_info and tcp_authopt_key_info once per packet.

--
Regards,
Leonard

#6 lore raw

Re: [PATCH v2 01/25] tcp: authopt: Initial support and key management

Quoted 1 line · Leonard Crestez

On 11/5/21 3:22 AM, Dmitry Safonov wrote:

[..]

Quoted 10 lines
Quoted 10 lines

I remember we discussed it in RFC, that removing a key that's currently in use may result in random MKT to be used.

I think, it's possible to make this API a bit more predictable if:
- DEL command fails to remove a key that is current/receive_next;
- opt.flags has CURR/NEXT flag that has corresponding u8 current_key and u8 receive_next values. As socket lock is held - that makes current_key/receive_next change atomic with deletion of an existing key that might have been in use.

Quoted 7 lines
Quoted 3 lines

In result user may remove a key that's not in use or has to set new current/next. Which avoids the issue with random MKT being used to sign segments.

The MKT used to sign segments is already essentially random unless the user makes a deliberate choice. This is what happens if you add two keys an call connect(). But why is this a problem?

The issue is predictability and less control for a user on how the key is selected.

Let's say as a user I have two MKTs A and B. I want to use A for 6 weeks and then change to B. I want to switch to B as soon as the admin of the peer adds the key and the peer sends me (rnext_key = B.id).

With your semantics currently a random key will be used as long as I don't "lock" the id which means that rnext_key won't be respected. So there's clearly less predictability for a user to select current key in use.

Quoted 7 lines

Applications which want to deliberately control the send key can do so with TCP_AUTHOPT_FLAG_LOCK_KEYID. If that flag is not set then the key with send_id == recv_rnextkeyid is preffered as suggested by the RFC, or a random one on connect.

I think your suggestion would force additional complexity on all applications for no clear gain.

I disagree. From RFC (3.1):

"It is presumed that an MKT affecting a particular connection cannot be destroyed during an active connection -- or, equivalently, that its parameters are copied to an area local to the connection (i.e., instantiated) and so changes would affect only new connections."

which means that the user shouldn't be able to remove a key in use. So, by default you should return an error if the key in use being deleted.

The only use-case to delete a key that is in use is if it has been compromised RFC(6.1):

"Deciding when to start using a key is a performance issue. Deciding when to remove an MKT is a security issue. Invalid MKTs are expected to be removed. TCP-AO provides no mechanism to coordinate their removal, as we consider this a key management operation."

I might misread the RFC, but it seems that shouldn't happen in an ordinary usage scenario (as long as the user don't --force removal of the compromised key in an exceptional case).

So, if you allow a user to set current_key/rnext_key atomically with removal - it seems to fit this --force use-case and let user more control over which key is in use.

Quoted 5 lines

Key selection controls are only added much later in the series, this is also part of the effort to split the code into readable patches. See this patch:

https://lore.kernel.org/netdev/2dc569c0d60c80c26aafcaa201ba5b5ec53ce6bd.1635784253.git.cdleonard@gmail.com/

A separate issue with that one (if I'm not misreading) seems to be that you're going to send segments with info->send_rnextkeyid if the deleted key was TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID one.
And won't be able to verify the peer inbound segments/replies.

Quoted 5 lines

Removing a key while traffic is happening shouldn't cause failures in recv or send code; this takes some effort but is also required to prevent auth failures when a socket is closed and transitions to timewait. I attempted to ensure this by only doing rcu_dereference for tcp_authopt_info and tcp_authopt_key_info once per packet.

Thanks,
Dmitry

#7 lore raw

Re: [PATCH v2 01/25] tcp: authopt: Initial support and key management

Quoted 24 lines · Dmitry Safonov
Quoted 1 line · Leonard Crestez

On 11/5/21 3:22 AM, Dmitry Safonov wrote:

[..]

Quoted 10 lines
Quoted 10 lines

I remember we discussed it in RFC, that removing a key that's currently in use may result in random MKT to be used.

I think, it's possible to make this API a bit more predictable if:
- DEL command fails to remove a key that is current/receive_next;
- opt.flags has CURR/NEXT flag that has corresponding u8 current_key and u8 receive_next values. As socket lock is held - that makes current_key/receive_next change atomic with deletion of an existing key that might have been in use.

Quoted 7 lines
Quoted 3 lines

In result user may remove a key that's not in use or has to set new current/next. Which avoids the issue with random MKT being used to sign segments.

The MKT used to sign segments is already essentially random unless the user makes a deliberate choice. This is what happens if you add two keys an call connect(). But why is this a problem?

The issue is predictability and less control for a user on how the key is selected.

Quoted 8 lines

Let's say as a user I have two MKTs A and B. I want to use A for 6 weeks and then change to B. I want to switch to B as soon as the admin of the peer adds the key and the peer sends me (rnext_key = B.id).

With your semantics currently a random key will be used as long as I don't "lock" the id which means that rnext_key won't be respected. So there's clearly less predictability for a user to select current key in use.

RFC makes two requirements regarding keyid selection:
A) 7.2: TCP SEND [..] MUST be augmented so that the preferred outgoing MKT (current_key) can be indicated.
B) 7.5.2.e: Key must be switch to rnextkeyid if that key is available.

These requirements are in conflict so I added a flag TCP_AUTHOPT_LOCK_KEYID to determine if keyid is determined by local userspace or based on the peer's rnextkeyid.

Without a "locking" bit any key selections made from userspace would get flipped by incoming traffic. Indeed together with your suggestion of not allowing the current key to be deleted it would be possible for delete to fail repeatedly because the peer keeps sending us valid packets!

The expectation is that complex applications will use the "locking" functionality and handle the switch to recv_rnextkeyid themselves. Alternatively it's also possible for peers to only control rnextkeyid and perform key switch that way.

In your scenario the key will switch automatically as soon as the peer sends "B.send_id" in the rnextkeyid field.

Please note that key selection is only fully implemented in PATCH 19, without it the behavior is indeed more random. That patch was separated for ease of review and because detailed behavior is worth a separate discussion.

Entirely different key selections mechanisms are possible, for example each key could have a "preference" score. The most detailed discussion of key rollover I found is from Cisco:

https://www.cisco.com/c/en/us/td/docs/ios-xml/ios/iproute_pi/configuration/xe-16-12/iri-xe-16-12-book/tcp-ao.html#concept_mtn_h4n_j3b

That document describes key rollover based on lifetime intervals for each key. I believe my patches provide sufficient control to implement the same but that it is a concern for userspace.

Quoted 10 lines
Quoted 7 lines

Applications which want to deliberately control the send key can do so with TCP_AUTHOPT_FLAG_LOCK_KEYID. If that flag is not set then the key with send_id == recv_rnextkeyid is preffered as suggested by the RFC, or a random one on connect.

I think your suggestion would force additional complexity on all applications for no clear gain.

I disagree. From RFC (3.1):

Quoted 7 lines

"It is presumed that an MKT affecting a particular connection cannot be destroyed during an active connection -- or, equivalently, that its parameters are copied to an area local to the connection (i.e., instantiated) and so changes would affect only new connections."

which means that the user shouldn't be able to remove a key in use. So, by default you should return an error if the key in use being deleted.

I believe this behavior belongs in application software.

Quoted 8 lines

The only use-case to delete a key that is in use is if it has been compromised RFC(6.1):

"Deciding when to start using a key is a performance issue. Deciding when to remove an MKT is a security issue. Invalid MKTs are expected to be removed. TCP-AO provides no mechanism to coordinate their removal, as we consider this a key management operation."

Quoted 7 lines

I might misread the RFC, but it seems that shouldn't happen in an ordinary usage scenario (as long as the user don't --force removal of the compromised key in an exceptional case).

So, if you allow a user to set current_key/rnext_key atomically with removal - it seems to fit this --force use-case and let user more control over which key is in use.

Control is available: user can "lock" a different key before removing the current one.

The kernel just doesn't make this mandatory.

Quoted 10 lines
Quoted 5 lines

Key selection controls are only added much later in the series, this is also part of the effort to split the code into readable patches. See this patch:

https://lore.kernel.org/netdev/2dc569c0d60c80c26aafcaa201ba5b5ec53ce6bd.1635784253.git.cdleonard@gmail.com/

A separate issue with that one (if I'm not misreading) seems to be that you're going to send segments with info->send_rnextkeyid if the deleted key was TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID one.
And won't be able to verify the peer inbound segments/replies.

I'm not sure I understand this.

If TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID is set then the rnextkeyid byte in output packets is controlled by user directly, this behavior is kept deliberately simple.

Otherwise we send the recv_id of the current key, this attempts to ensure symmetry.

Userspace is expected to use the recv_id of a valid key with TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID, otherwise the connection will indeed break. The kernel could enforce that this value is valid but it does not attempt to prevent userspace bugs.

Quoted 5 lines
Quoted 5 lines

Removing a key while traffic is happening shouldn't cause failures in recv or send code; this takes some effort but is also required to prevent auth failures when a socket is closed and transitions to timewait. I attempted to ensure this by only doing rcu_dereference for tcp_authopt_info and tcp_authopt_key_info once per packet.

#8 lore raw

PATCH v2 2/25 docs: Add user documentation for tcp_authopt

In reply to Leonard Crestez

The .rst documentation contains a brief description of the user interface and includes kernel-doc generated from uapi header.

---
 Documentation/networking/index.rst       |  1 +
 Documentation/networking/tcp_authopt.rst | 44 ++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 Documentation/networking/tcp_authopt.rst
Documentation/networking/index.rst +1 -0
index 58bc8cd367c6..f5c324a060d8 100644
@@ -100,10 +100,11 @@ Contents:
    strparser
    switchdev
    sysfs-tagging
    tc-actions-env-rules
    tcp-thin
+   tcp_authopt
    team
    timestamping
    tipc
    tproxy
    tuntap
Documentation/networking/tcp_authopt.rst new file +44 -0

44 lines not on this page · raw message

Signature · 1 line
2.25.1
#9 lore raw

PATCH v2 3/25 selftests: Initial tcp_authopt test module

In reply to Leonard Crestez

This test suite is written as a standalone python3 package using dependencies such as scapy.

The run.sh script wrapper called from kselftest infrastructure uses "pip" to generate an isolated virtual environment just for running these tests. The run.sh wrapper can be called from anywhere and does not rely on kselftest infrastructure.

Default output is in TAP format.

---
 tools/testing/selftests/tcp_authopt/Makefile  | 10 ++++
 .../testing/selftests/tcp_authopt/README.rst  | 18 ++++++++
 tools/testing/selftests/tcp_authopt/config    |  6 +++
 .../selftests/tcp_authopt/requirements.txt    | 46 +++++++++++++++++++
 tools/testing/selftests/tcp_authopt/run.sh    | 31 +++++++++++++
 tools/testing/selftests/tcp_authopt/settings  |  1 +
 tools/testing/selftests/tcp_authopt/setup.cfg | 35 ++++++++++++++
 tools/testing/selftests/tcp_authopt/setup.py  |  6 +++
 .../tcp_authopt/tcp_authopt_test/__init__.py  |  0
 9 files changed, 153 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/Makefile
 create mode 100644 tools/testing/selftests/tcp_authopt/README.rst
 create mode 100644 tools/testing/selftests/tcp_authopt/config
 create mode 100644 tools/testing/selftests/tcp_authopt/requirements.txt
 create mode 100755 tools/testing/selftests/tcp_authopt/run.sh
 create mode 100644 tools/testing/selftests/tcp_authopt/settings
 create mode 100644 tools/testing/selftests/tcp_authopt/setup.cfg
 create mode 100644 tools/testing/selftests/tcp_authopt/setup.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/__init__.py
tools/testing/selftests/tcp_authopt/Makefile new file +10 -0
new file mode 100644
index 000000000000..256ae2c16013
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0
+include ../lib.mk
+
+TEST_PROGS += ./run.sh
+TEST_FILES := \
+	requirements.txt \
+	settings \
+	setup.cfg \
+	setup.py \
+	tcp_authopt_test
tools/testing/selftests/tcp_authopt/README.rst new file +18 -0
new file mode 100644
index 000000000000..e9548469c827
@@ -0,0 +1,18 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================================
+Tests for linux TCP Authentication Option
+=========================================
+
+Test suite is written in python3 using pytest and scapy. The test suite is
+mostly self-contained as a python package.
+
+The recommended way to run this is the included `run.sh` script as root, this
+will automatically create a virtual environment with the correct dependencies
+using `pip`. If not running under root it will automatically attempt to elevate
+using `sudo` after the virtualenv is created.
+
+An old separate version can be found here: https://github.com/cdleonard/tcp-authopt-test
+
+Integration with kselftest infrastructure is minimal: when in doubt just run
+this separately.
tools/testing/selftests/tcp_authopt/config new file +6 -0
new file mode 100644
index 000000000000..0d4e5d47fa72
@@ -0,0 +1,6 @@
+# RFC5925 TCP Authentication Option and all algorithms
+CONFIG_TCP_AUTHOPT=y
+CONFIG_CRYPTO_SHA1=M
+CONFIG_CRYPTO_HMAC=M
+CONFIG_CRYPTO_AES=M
+CONFIG_CRYPTO_CMAC=M
tools/testing/selftests/tcp_authopt/requirements.txt new file +46 -0

46 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/run.sh new file +31 -0
new file mode 100755
index 000000000000..7aeb125706a4
@@ -0,0 +1,31 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Create virtualenv using pip and run pytest
+# Accepts all args that pytest does
+#
+set -e
+cd "$(dirname "${BASH_SOURCE[0]}")"
+
+if [[ -d venv ]]; then
+	echo >&2 "Using existing $(readlink -f venv)"
+else
+	echo >&2 "Creating $(readlink -f venv)"
+	python3 -m venv venv
+	(
+		. venv/bin/activate
+		pip install wheel
+		pip install -r requirements.txt
+	)
+fi
+
+cmd=(pytest -s --log-cli-level=DEBUG --tap-stream "$@")
+if [[ $(id -u) -ne 0 ]]; then
+	echo >&2 "warning: running as non-root user, attempting sudo"
+	# sudo -E to use the virtualenv:
+	cmd=(sudo bash -c ". venv/bin/activate;$(printf " %q" "${cmd[@]}")")
+	exec "${cmd[@]}"
+else
+	. venv/bin/activate
+	exec "${cmd[@]}"
+fi
tools/testing/selftests/tcp_authopt/settings new file +1 -0
new file mode 100644
index 000000000000..6091b45d226b
@@ -0,0 +1 @@
+timeout=120
tools/testing/selftests/tcp_authopt/setup.cfg new file +35 -0
new file mode 100644
index 000000000000..452083fec64b
@@ -0,0 +1,35 @@
+[options]
+install_requires=
+    cryptography
+    nsenter
+    pytest
+    pytest-tap
+    scapy
+    waiting
+
+[options.extras_require]
+dev =
+    black
+    isort
+    mypy
+    pip-tools
+    pre-commit
+    tox
+
+[tox:tox]
+envlist = py3
+
+[testenv]
+commands = pytest {posargs}
+deps = -rrequirements.txt
+
+[metadata]
+name = tcp-authopt-test
+version = 0.1
+
+[mypy]
+ignore_missing_imports = true
+files = .
+
+[isort]
+profile = black
tools/testing/selftests/tcp_authopt/setup.py new file +6 -0
new file mode 100644
index 000000000000..055b98132e26
@@ -0,0 +1,6 @@
+#! /usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+from setuptools import setup
+
+setup()
tools/testing/selftests/tcp_authopt/tcp_authopt_test/__init__.py new file +0 -0
new file mode 100644
index 000000000000..e69de29bb2d1
Signature · 1 line
2.25.1
#10 lore raw

PATCH v2 4/25 selftests: tcp_authopt: Initial sockopt manipulation

In reply to Leonard Crestez

Add a python translation of the linux ABI for tcpao and test the behavior of TCP_AUTHOPT and TCP_AUTHOPT_KEY sockopts.

This includes several corner cases not normally covered by traffic tests.

---
 .../tcp_authopt/tcp_authopt_test/conftest.py  |  71 +++++
 .../tcp_authopt_test/linux_tcp_authopt.py     | 266 ++++++++++++++++++
 .../tcp_authopt/tcp_authopt_test/sockaddr.py  | 122 ++++++++
 .../tcp_authopt_test/test_sockopt.py          | 203 +++++++++++++
 4 files changed, 662 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/conftest.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_authopt.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/sockaddr.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sockopt.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/conftest.py new file +71 -0
new file mode 100644
index 000000000000..a06ba848669d
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0
+import logging
+import os
+from contextlib import ExitStack, nullcontext
+from typing import ContextManager
+
+import pytest
+
+from .linux_tcp_authopt import enable_sysctl_tcp_authopt, has_tcp_authopt
+
+logger = logging.getLogger(__name__)
+
+skipif_missing_tcp_authopt = pytest.mark.skipif(
+    not has_tcp_authopt(), reason="Need CONFIG_TCP_AUTHOPT"
+)
+
+
+def get_effective_capabilities():
+    for line in open("/proc/self/status", "r"):
+        if line.startswith("CapEff:"):
+            return int(line.split(":")[1], 16)
+
+
+def has_effective_capability(bit) -> bool:
+    return get_effective_capabilities() & (1 << bit) != 0
+
+
+def can_capture() -> bool:
+    return has_effective_capability(13)
+
+
+def raise_skip_no_netns():
+    if not has_effective_capability(12):
+        pytest.skip("Need CAP_NET_ADMIN for network namespaces")
+
+
+skipif_cant_capture = pytest.mark.skipif(
+    not can_capture(), reason="run as root to capture packets"
+)
+
+
[email protected]
+def exit_stack():
+    """Return a contextlib.ExitStack as a pytest fixture
+
+    This reduces indentation making code more readable
+    """
+    with ExitStack() as exit_stack:
+        yield exit_stack
+
+
+def pytest_configure():
+    # Silence messages regarding netns enter/exit:
+    logging.getLogger("nsenter").setLevel(logging.INFO)
+    if has_tcp_authopt():
+        enable_sysctl_tcp_authopt()
+
+
+def parametrize_product(**kw):
+    """Parametrize each key to each item in the value list"""
+    import itertools
+
+    return pytest.mark.parametrize(",".join(kw.keys()), itertools.product(*kw.values()))
+
+
+def raises_optional_exception(expected_exception, **kw) -> ContextManager:
+    """Like pytest.raises except accept expected_exception=None"""
+    if expected_exception is None:
+        return nullcontext()
+    else:
+        return pytest.raises(expected_exception, **kw)
tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_authopt.py new file +266 -0

266 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/sockaddr.py new file +122 -0

122 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sockopt.py new file +203 -0

203 lines not on this page · raw message

Signature · 1 line
2.25.1
#11 lore raw

PATCH v2 5/25 tcp: authopt: Add crypto initialization

In reply to Leonard Crestez

The crypto_shash API is used in order to compute packet signatures. The API comes with several unfortunate limitations:

1) Allocating a crypto_shash can sleep and must be done in user context.
2) Packet signatures must be computed in softirq context
3) Packet signatures use dynamic "traffic keys" which require exclusive access to crypto_shash for crypto_setkey.

The solution is to allocate one crypto_shash for each possible cpu for each algorithm at setsockopt time. The per-cpu tfm is then borrowed from softirq context, signatures are computed and the tfm is returned.

The pool for each algorithm is allocated on first use.

---
 include/net/tcp_authopt.h |  16 ++++
 net/ipv4/tcp_authopt.c    | 166 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+)
include/net/tcp_authopt.h +16 -0
index 42ad764e98c2..5217b6c7c900 100644
@@ -2,10 +2,24 @@
 #ifndef _LINUX_TCP_AUTHOPT_H
 #define _LINUX_TCP_AUTHOPT_H
 
 #include <uapi/linux/tcp.h>
 
+/* According to RFC5925 the length of the authentication option varies based on
+ * the signature algorithm. Linux only implements the algorithms defined in
+ * RFC5926 which have a constant length of 16.
+ *
+ * This is used in stack allocation of tcp option buffers for output. It is
+ * shorter than the length of the MD5 option.
+ *
+ * Input packets can have authentication options of different lengths but they
+ * will always be flagged as invalid (since no such algorithms are supported).
+ */
+#define TCPOLEN_AUTHOPT_OUTPUT	16
+
+struct tcp_authopt_alg_imp;
+
 /**
  * struct tcp_authopt_key_info - Representation of a Master Key Tuple as per RFC5925
  *
  * Key structure lifetime is only protected by RCU so readers needs to hold a
  * single rcu_read_lock until they're done with the key.
@@ -27,10 +41,12 @@ struct tcp_authopt_key_info {
 	u8 keylen;
 	/** @key: Same as &tcp_authopt_key.key */
 	u8 key[TCP_AUTHOPT_MAXKEYLEN];
 	/** @addr: Same as &tcp_authopt_key.addr */
 	struct sockaddr_storage addr;
+	/** @alg: Algorithm implementation matching alg_id */
+	struct tcp_authopt_alg_imp *alg;
 };
 
 /**
  * struct tcp_authopt_info - Per-socket information regarding tcp_authopt
  *
net/ipv4/tcp_authopt.c +166 -0

206 lines not on this page · raw message

Signature · 1 line
2.25.1
#12 lore raw

PATCH v2 6/25 tcp: authopt: Compute packet signatures

In reply to Leonard Crestez

Computing tcp authopt packet signatures is a two step process:

* traffic key is computed based on tcp 4-tuple, initial sequence numbers and the secret key.
* packet mac is computed based on traffic key and content of individual packets.

The traffic key could be cached for established sockets but it is not.

A single code path exists for ipv4/ipv6 and input/output. This keeps the code short but slightly slower due to lots of conditionals.

On output we read remote IP address from socket members on output, we can't use skb network header because it's computed after TCP options.

On input we read remote IP address from skb network headers, we can't use socket binding members because those are not available for SYN.

---
 net/ipv4/tcp_authopt.c | 510 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 510 insertions(+)
net/ipv4/tcp_authopt.c +510 -0
index 5455a9ecfe6b..1fd98d67ec10 100644
@@ -425,5 +425,515 @@ int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
 	memcpy(&key_info->addr, &opt.addr, sizeof(key_info->addr));
 	hlist_add_head_rcu(&key_info->node, &info->head);
 
 	return 0;
 }
+
+static int tcp_authopt_get_isn(struct sock *sk,
+			       struct sk_buff *skb,
+			       int input,
+			       __be32 *sisn,
+			       __be32 *disn)
+{
+	struct tcp_authopt_info *authopt_info;
+	struct tcphdr *th = tcp_hdr(skb);
+
+	/* special cases for SYN and SYN/ACK */
+	if (th->syn && !th->ack) {
+		*sisn = th->seq;
+		*disn = 0;
+		return 0;
+	}
+	if (th->syn && th->ack) {
+		*sisn = th->seq;
+		*disn = htonl(ntohl(th->ack_seq) - 1);
+		return 0;
+	}
+
+	/* Fetching authopt_info like this should be safe because authopt_info
+	 * is never released intil the socket is being closed
+	 *
+	 * tcp_timewait_sock is handled but not tcp_request_sock.
+	 * for the synack case sk should be the listen socket.
+	 */
+	rcu_read_lock();
+	if (unlikely(sk->sk_state == TCP_NEW_SYN_RECV)) {
+		/* should never happen, sk should be the listen socket */
+		authopt_info = NULL;
+		WARN_ONCE(1, "TCP-AO can't sign with request sock\n");
+		return -EINVAL;
+	} else if (sk->sk_state == TCP_LISTEN) {
+		/* Signature computation for non-syn packet on a listen
+		 * socket is not possible because we lack the initial
+		 * sequence numbers.
+		 *
+		 * Input segments that are not matched by any request,
+		 * established or timewait socket will get here. These
+		 * are not normally sent by peers.
+		 *
+		 * Their signature might be valid but we don't have
+		 * enough state to determine that. TCP-MD5 can attempt
+		 * to validate and reply with a signed RST because it
+		 * doesn't care about ISNs.
+		 *
+		 * Reporting an error from signature code causes the
+		 * packet to be discarded which is good.
+		 */
+		if (input) {
+			/* Assume this is an ACK to a SYN/ACK
+			 * This will incorrectly report "failed
+			 * signature" for segments without a connection.
+			 */
+			*sisn = htonl(ntohl(th->seq) - 1);
+			*disn = htonl(ntohl(th->ack_seq) - 1);
+			rcu_read_unlock();
+			return 0;
+		}
+		/* This would be an internal bug. */
+		authopt_info = NULL;
+		WARN_ONCE(1, "TCP-AO can't sign non-syn from TCP_LISTEN sock\n");
+		return -EINVAL;
+	} else 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) {
+		rcu_read_unlock();
+		return -EINVAL;
+	}
+	/* Initial sequence numbers for ESTABLISHED connections from info */
+	if (input) {
+		*sisn = htonl(authopt_info->dst_isn);
+		*disn = htonl(authopt_info->src_isn);
+	} else {
+		*sisn = htonl(authopt_info->src_isn);
+		*disn = htonl(authopt_info->dst_isn);
+	}
+	rcu_read_unlock();
+	return 0;
+}
+
+/* feed traffic key into shash */
+static int tcp_authopt_shash_traffic_key(struct shash_desc *desc,
+					 struct sock *sk,
+					 struct sk_buff *skb,
+					 bool input,
+					 bool ipv6)
+{
+	struct tcphdr *th = tcp_hdr(skb);
+	int err;
+	__be32 sisn, disn;
+	__be16 digestbits = htons(crypto_shash_digestsize(desc->tfm) * 8);
+
+	// RFC5926 section 3.1.1.1
+	err = crypto_shash_update(desc, "\x01TCP-AO", 7);
+	if (err)
+		return err;
+
+	/* Addresses from packet on input and from sk_common on output
+	 * This is because on output MAC is computed before prepending IP header
+	 */
+	if (input) {
+		if (ipv6)
+			err = crypto_shash_update(desc, (u8 *)&ipv6_hdr(skb)->saddr, 32);
+		else
+			err = crypto_shash_update(desc, (u8 *)&ip_hdr(skb)->saddr, 8);
+		if (err)
+			return err;
+	} else {
+		if (ipv6) {
+			err = crypto_shash_update(desc, (u8 *)&sk->sk_v6_rcv_saddr, 16);
+			if (err)
+				return err;
+			err = crypto_shash_update(desc, (u8 *)&sk->sk_v6_daddr, 16);
+			if (err)
+				return err;
+		} else {
+			err = crypto_shash_update(desc, (u8 *)&sk->sk_rcv_saddr, 4);
+			if (err)
+				return err;
+			err = crypto_shash_update(desc, (u8 *)&sk->sk_daddr, 4);
+			if (err)
+				return err;
+		}
+	}
+
+	/* TCP ports from header */
+	err = crypto_shash_update(desc, (u8 *)&th->source, 4);
+	if (err)
+		return err;
+	err = tcp_authopt_get_isn(sk, skb, input, &sisn, &disn);
+	if (err)
+		return err;
+	err = crypto_shash_update(desc, (u8 *)&sisn, 4);
+	if (err)
+		return err;
+	err = crypto_shash_update(desc, (u8 *)&disn, 4);
+	if (err)
+		return err;
+	err = crypto_shash_update(desc, (u8 *)&digestbits, 2);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+/* Convert a variable-length key to a 16-byte fixed-length key for AES-CMAC
+ * This is described in RFC5926 section 3.1.1.2
+ */
+static int aes_setkey_derived(struct crypto_shash *tfm, u8 *key, size_t keylen)
+{
+	static const u8 zeros[16] = {0};
+	u8 derived_key[16];
+	int err;
+
+	if (WARN_ON_ONCE(crypto_shash_digestsize(tfm) != 16))
+		return -EINVAL;
+	err = crypto_shash_setkey(tfm, zeros, sizeof(zeros));
+	if (err)
+		return err;
+	err = crypto_shash_tfm_digest(tfm, key, keylen, derived_key);
+	if (err)
+		return err;
+	return crypto_shash_setkey(tfm, derived_key, sizeof(derived_key));
+}
+
+static int tcp_authopt_setkey(struct crypto_shash *tfm, struct tcp_authopt_key_info *key)
+{
+	if (key->alg_id == TCP_AUTHOPT_ALG_AES_128_CMAC_96 && key->keylen != 16)
+		return aes_setkey_derived(tfm, key->key, key->keylen);
+	else
+		return crypto_shash_setkey(tfm, key->key, key->keylen);
+}
+
+static int tcp_authopt_get_traffic_key(struct sock *sk,
+				       struct sk_buff *skb,
+				       struct tcp_authopt_key_info *key,
+				       bool input,
+				       bool ipv6,
+				       u8 *traffic_key)
+{
+	SHASH_DESC_ON_STACK(desc, kdf_tfm);
+	struct crypto_shash *kdf_tfm;
+	int err;
+
+	kdf_tfm = tcp_authopt_get_kdf_shash(key);
+	if (IS_ERR(kdf_tfm))
+		return PTR_ERR(kdf_tfm);
+
+	err = tcp_authopt_setkey(kdf_tfm, key);
+	if (err)
+		goto out;
+
+	desc->tfm = kdf_tfm;
+	err = crypto_shash_init(desc);
+	if (err)
+		goto out;
+
+	err = tcp_authopt_shash_traffic_key(desc, sk, skb, input, ipv6);
+	if (err)
+		goto out;
+
+	err = crypto_shash_final(desc, traffic_key);
+	if (err)
+		goto out;
+
+out:
+	tcp_authopt_put_kdf_shash(key, kdf_tfm);
+	return err;
+}
+
+static int crypto_shash_update_zero(struct shash_desc *desc, int len)
+{
+	u8 zero = 0;
+	int i, err;
+
+	for (i = 0; i < len; ++i) {
+		err = crypto_shash_update(desc, &zero, 1);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int tcp_authopt_hash_tcp4_pseudoheader(struct shash_desc *desc,
+					      __be32 saddr,
+					      __be32 daddr,
+					      int nbytes)
+{
+	struct tcp4_pseudohdr phdr = {
+		.saddr = saddr,
+		.daddr = daddr,
+		.pad = 0,
+		.protocol = IPPROTO_TCP,
+		.len = htons(nbytes)
+	};
+	return crypto_shash_update(desc, (u8 *)&phdr, sizeof(phdr));
+}
+
+static int tcp_authopt_hash_tcp6_pseudoheader(struct shash_desc *desc,
+					      struct in6_addr *saddr,
+					      struct in6_addr *daddr,
+					      u32 plen)
+{
+	int err;
+	__be32 buf[2];
+
+	buf[0] = htonl(plen);
+	buf[1] = htonl(IPPROTO_TCP);
+
+	err = crypto_shash_update(desc, (u8 *)saddr, sizeof(*saddr));
+	if (err)
+		return err;
+	err = crypto_shash_update(desc, (u8 *)daddr, sizeof(*daddr));
+	if (err)
+		return err;
+	return crypto_shash_update(desc, (u8 *)&buf, sizeof(buf));
+}
+
+/* TCP authopt as found in header */
+struct tcphdr_authopt {
+	u8 num;
+	u8 len;
+	u8 keyid;
+	u8 rnextkeyid;
+	u8 mac[0];
+};
+
+/* Find TCP_AUTHOPT in header.
+ *
+ * Returns pointer to TCP_AUTHOPT or NULL if not found.
+ */
+static u8 *tcp_authopt_find_option(struct tcphdr *th)
+{
+	int length = (th->doff << 2) - sizeof(*th);
+	u8 *ptr = (u8 *)(th + 1);
+
+	while (length >= 2) {
+		int opcode = *ptr++;
+		int opsize;
+
+		switch (opcode) {
+		case TCPOPT_EOL:
+			return NULL;
+		case TCPOPT_NOP:
+			length--;
+			continue;
+		default:
+			if (length < 2)
+				return NULL;
+			opsize = *ptr++;
+			if (opsize < 2)
+				return NULL;
+			if (opsize > length)
+				return NULL;
+			if (opcode == TCPOPT_AUTHOPT)
+				return ptr - 2;
+		}
+		ptr += opsize - 2;
+		length -= opsize;
+	}
+	return NULL;
+}
+
+/** Hash tcphdr options.
+ *  If include_options is false then only the TCPOPT_AUTHOPT option itself is hashed
+ *  Maybe we could skip option parsing by assuming the AUTHOPT header is at hash_location-4?
+ */
+static int tcp_authopt_hash_opts(struct shash_desc *desc,
+				 struct tcphdr *th,
+				 bool include_options)
+{
+	int err;
+	/* start of options */
+	u8 *tcp_opts = (u8 *)(th + 1);
+	/* end of options */
+	u8 *tcp_data = ((u8 *)th) + th->doff * 4;
+	/* pointer to TCPOPT_AUTHOPT */
+	u8 *authopt_ptr = tcp_authopt_find_option(th);
+	u8 authopt_len;
+
+	if (!authopt_ptr)
+		return -EINVAL;
+	authopt_len = *(authopt_ptr + 1);
+
+	if (include_options) {
+		err = crypto_shash_update(desc, tcp_opts, authopt_ptr - tcp_opts + 4);
+		if (err)
+			return err;
+		err = crypto_shash_update_zero(desc, authopt_len - 4);
+		if (err)
+			return err;
+		err = crypto_shash_update(desc,
+					  authopt_ptr + authopt_len,
+					  tcp_data - (authopt_ptr + authopt_len));
+		if (err)
+			return err;
+	} else {
+		err = crypto_shash_update(desc, authopt_ptr, 4);
+		if (err)
+			return err;
+		err = crypto_shash_update_zero(desc, authopt_len - 4);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int skb_shash_frags(struct shash_desc *desc,
+			   struct sk_buff *skb)
+{
+	struct sk_buff *frag_iter;
+	int err, i;
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
+		u32 p_off, p_len, copied;
+		struct page *p;
+		u8 *vaddr;
+
+		skb_frag_foreach_page(f, skb_frag_off(f), skb_frag_size(f),
+				      p, p_off, p_len, copied) {
+			vaddr = kmap_atomic(p);
+			err = crypto_shash_update(desc, vaddr + p_off, p_len);
+			kunmap_atomic(vaddr);
+			if (err)
+				return err;
+		}
+	}
+
+	skb_walk_frags(skb, frag_iter) {
+		err = skb_shash_frags(desc, frag_iter);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int tcp_authopt_hash_packet(struct crypto_shash *tfm,
+				   struct sock *sk,
+				   struct sk_buff *skb,
+				   bool input,
+				   bool ipv6,
+				   bool include_options,
+				   u8 *macbuf)
+{
+	struct tcphdr *th = tcp_hdr(skb);
+	SHASH_DESC_ON_STACK(desc, tfm);
+	int err;
+
+	/* NOTE: SNE unimplemented */
+	__be32 sne = 0;
+
+	desc->tfm = tfm;
+	err = crypto_shash_init(desc);
+	if (err)
+		return err;
+
+	err = crypto_shash_update(desc, (u8 *)&sne, 4);
+	if (err)
+		return err;
+
+	if (ipv6) {
+		struct in6_addr *saddr;
+		struct in6_addr *daddr;
+
+		if (input) {
+			saddr = &ipv6_hdr(skb)->saddr;
+			daddr = &ipv6_hdr(skb)->daddr;
+		} else {
+			saddr = &sk->sk_v6_rcv_saddr;
+			daddr = &sk->sk_v6_daddr;
+		}
+		err = tcp_authopt_hash_tcp6_pseudoheader(desc, saddr, daddr, skb->len);
+		if (err)
+			return err;
+	} else {
+		__be32 saddr;
+		__be32 daddr;
+
+		if (input) {
+			saddr = ip_hdr(skb)->saddr;
+			daddr = ip_hdr(skb)->daddr;
+		} else {
+			saddr = sk->sk_rcv_saddr;
+			daddr = sk->sk_daddr;
+		}
+		err = tcp_authopt_hash_tcp4_pseudoheader(desc, saddr, daddr, skb->len);
+		if (err)
+			return err;
+	}
+
+	// TCP header with checksum set to zero
+	{
+		struct tcphdr hashed_th = *th;
+
+		hashed_th.check = 0;
+		err = crypto_shash_update(desc, (u8 *)&hashed_th, sizeof(hashed_th));
+		if (err)
+			return err;
+	}
+
+	// TCP options
+	err = tcp_authopt_hash_opts(desc, th, include_options);
+	if (err)
+		return err;
+
+	// Rest of SKB->data
+	err = crypto_shash_update(desc, (u8 *)th + th->doff * 4, skb_headlen(skb) - th->doff * 4);
+	if (err)
+		return err;
+
+	err = skb_shash_frags(desc, skb);
+	if (err)
+		return err;
+
+	return crypto_shash_final(desc, macbuf);
+}
+
+/* __tcp_authopt_calc_mac - Compute packet MAC using key
+ *
+ * The macbuf output buffer must be large enough to fit the digestsize of the
+ * underlying transform before truncation.
+ * This means TCP_AUTHOPT_MAXMACBUF, not TCP_AUTHOPT_MACLEN
+ */
+static int __tcp_authopt_calc_mac(struct sock *sk,
+				  struct sk_buff *skb,
+				  struct tcp_authopt_key_info *key,
+				  bool input,
+				  char *macbuf)
+{
+	struct crypto_shash *mac_tfm;
+	u8 traffic_key[TCP_AUTHOPT_MAX_TRAFFIC_KEY_LEN];
+	int err;
+	bool ipv6 = (sk->sk_family != AF_INET);
+
+	if (sk->sk_family != AF_INET && sk->sk_family != AF_INET6)
+		return -EINVAL;
+
+	err = tcp_authopt_get_traffic_key(sk, skb, key, input, ipv6, traffic_key);
+	if (err)
+		return err;
+
+	mac_tfm = tcp_authopt_get_mac_shash(key);
+	if (IS_ERR(mac_tfm))
+		return PTR_ERR(mac_tfm);
+	err = crypto_shash_setkey(mac_tfm, traffic_key, key->alg->traffic_key_len);
+	if (err)
+		goto out;
+
+	err = tcp_authopt_hash_packet(mac_tfm,
+				      sk,
+				      skb,
+				      input,
+				      ipv6,
+				      !(key->flags & TCP_AUTHOPT_KEY_EXCLUDE_OPTS),
+				      macbuf);
+
+out:
+	tcp_authopt_put_mac_shash(key, mac_tfm);
+	return err;
+}
Signature · 1 line
2.25.1
#13 lore raw

Re: [PATCH v2 06/25] tcp: authopt: Compute packet signatures

On 11/1/21 16:34, Leonard Crestez wrote:
[..]

Quoted 20 lines
net/ipv4/tcp_authopt.c +20 -0
+static int skb_shash_frags(struct shash_desc *desc,
+			   struct sk_buff *skb)
+{
+	struct sk_buff *frag_iter;
+	int err, i;
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
+		u32 p_off, p_len, copied;
+		struct page *p;
+		u8 *vaddr;
+
+		skb_frag_foreach_page(f, skb_frag_off(f), skb_frag_size(f),
+				      p, p_off, p_len, copied) {
+			vaddr = kmap_atomic(p);
+			err = crypto_shash_update(desc, vaddr + p_off, p_len);
+			kunmap_atomic(vaddr);
+			if (err)
+				return err;
+		}

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 814

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+	}
+
+	skb_walk_frags(skb, frag_iter) {
+		err = skb_shash_frags(desc, frag_iter);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}

This seems quite sub-optimal: IIUC, shash should only be used for small amount of hashing. That's why tcp-md5 uses ahash with scatterlists. Which drives me to the question: why not reuse tcp_md5sig_pool code?

And it seems that you can avoid TCP_AUTHOPT_ALG_* enum and just supply to crypto the string from socket option (like xfrm does).

Here is my idea:
https://lore.kernel.org/all/[email protected]/T/#u

Thanks,
Dmitry

#14 lore raw

Re: [PATCH v2 06/25] tcp: authopt: Compute packet signatures

Quoted 27 lines · Dmitry Safonov

On 11/1/21 16:34, Leonard Crestez wrote:
[..]

Quoted 25 lines
net/ipv4/tcp_authopt.c +25 -0
+static int skb_shash_frags(struct shash_desc *desc,
+			   struct sk_buff *skb)
+{
+	struct sk_buff *frag_iter;
+	int err, i;
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
+		u32 p_off, p_len, copied;
+		struct page *p;
+		u8 *vaddr;
+
+		skb_frag_foreach_page(f, skb_frag_off(f), skb_frag_size(f),
+				      p, p_off, p_len, copied) {
+			vaddr = kmap_atomic(p);
+			err = crypto_shash_update(desc, vaddr + p_off, p_len);
+			kunmap_atomic(vaddr);
+			if (err)
+				return err;
+		}
+	}
+
+	skb_walk_frags(skb, frag_iter) {
+		err = skb_shash_frags(desc, frag_iter);
+		if (err)
Quoted 8 lines

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 579

Quoted 5 lines
net/ipv4/tcp_authopt.c +5 -0
+			return err;
+	}
+
+	return 0;
+}

This seems quite sub-optimal: IIUC, shash should only be used for small amount of hashing. That's why tcp-md5 uses ahash with scatterlists.

There is indeed no good reason to prefer shash over ahash. Despite the "async" in the name it's possible to use it in atomic context.

Quoted 7 lines

Which drives me to the question: why not reuse tcp_md5sig_pool code?

And it seems that you can avoid TCP_AUTHOPT_ALG_* enum and just supply to crypto the string from socket option (like xfrm does).

Here is my idea:
https://lore.kernel.org/all/[email protected]/T/#u

Making the md5 pool more generic and reusing it can work.

This "pool" mechanism is really just a workaround for the crypto API not supporting the allocation of a hash in softirq context. It would make a lot sense for this functionality to be part of the crypto layer itself.

Looking at your generic tcp_sig_crypto there is nothing actually specific to TCP in there: it's just an ahash and a scratch buffer per-cpu.

I don't understand the interest in using arbitrary crypto algorithms beyond RFC5926, this series is already complex enough. Other than increasing the complexity of crypto allocation there are various stack allocations which would need to be up to the maximum size of a TCP options.

--
Regards,
Leonard

#15 lore raw

Re: [PATCH v2 06/25] tcp: authopt: Compute packet signatures

In reply to Leonard Crestez

On 11/1/21 16:34, Leonard Crestez wrote:
[..]

Quoted 12 lines
net/ipv4/tcp_authopt.c +12 -0
+/* Find TCP_AUTHOPT in header.
+ *
+ * Returns pointer to TCP_AUTHOPT or NULL if not found.
+ */
+static u8 *tcp_authopt_find_option(struct tcphdr *th)
+{
+	int length = (th->doff << 2) - sizeof(*th);
+	u8 *ptr = (u8 *)(th + 1);
+
+	while (length >= 2) {
+		int opcode = *ptr++;
+		int opsize;

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 725

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+
+		switch (opcode) {
+		case TCPOPT_EOL:
+			return NULL;
+		case TCPOPT_NOP:
+			length--;
+			continue;
+		default:
+			if (length < 2)
+				return NULL;

^ never true, as checked by the loop condition

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 738

Quoted 13 lines
net/ipv4/tcp_authopt.c +13 -0
+			opsize = *ptr++;
+			if (opsize < 2)
+				return NULL;
+			if (opsize > length)
+				return NULL;
+			if (opcode == TCPOPT_AUTHOPT)
+				return ptr - 2;
+		}
+		ptr += opsize - 2;
+		length -= opsize;
+	}
+	return NULL;
+}

Why copy'n'pasting tcp_parse_md5sig_option(), rather than adding a new argument to the function?

Thanks,
Dmitry

#16 lore raw

Re: [PATCH v2 06/25] tcp: authopt: Compute packet signatures

Quoted 27 lines · Dmitry Safonov

On 11/1/21 16:34, Leonard Crestez wrote:
[..]

Quoted 12 lines
net/ipv4/tcp_authopt.c +12 -0
+/* Find TCP_AUTHOPT in header.
+ *
+ * Returns pointer to TCP_AUTHOPT or NULL if not found.
+ */
+static u8 *tcp_authopt_find_option(struct tcphdr *th)
+{
+	int length = (th->doff << 2) - sizeof(*th);
+	u8 *ptr = (u8 *)(th + 1);
+
+	while (length >= 2) {
+		int opcode = *ptr++;
+		int opsize;

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 725

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+
+		switch (opcode) {
+		case TCPOPT_EOL:
+			return NULL;
+		case TCPOPT_NOP:
+			length--;
+			continue;
+		default:
+			if (length < 2)
+				return NULL;

^ never true, as checked by the loop condition

Quoted 16 lines

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 738

Quoted 13 lines
net/ipv4/tcp_authopt.c +13 -0
+			opsize = *ptr++;
+			if (opsize < 2)
+				return NULL;
+			if (opsize > length)
+				return NULL;
+			if (opcode == TCPOPT_AUTHOPT)
+				return ptr - 2;
+		}
+		ptr += opsize - 2;
+		length -= opsize;
+	}
+	return NULL;
+}

Why copy'n'pasting tcp_parse_md5sig_option(), rather than adding a new argument to the function?

No good reason.

There is a requirement in RFC5925 that packets with both AO and MD5 signatures be dropped. This currently works but the implementation is convoluted: after an AO signature is found an error is returned if MD5 is also present.

A better solution would be to do a single scan for both options up front, for example in tcp_{v4,v6}_auth_inbound_check

--
Regards,
Leonard

#17 lore raw

PATCH v2 7/25 tcp: Use BIT() for OPTION_* constants

In reply to Leonard Crestez

Extending these flags using the existing (1 << x) pattern triggers complaints from checkpatch.

Instead of ignoring checkpatch modify the existing values to use BIT(x) style in a separate commit.

---
 net/ipv4/tcp_output.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
net/ipv4/tcp_output.c +7 -7
index 6867e5db3e35..96f16386f50e 100644
@@ -406,17 +406,17 @@ static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags)
 static inline bool tcp_urg_mode(const struct tcp_sock *tp)
 {
 	return tp->snd_una != tp->snd_up;
 }
 
-#define OPTION_SACK_ADVERTISE	(1 << 0)
-#define OPTION_TS		(1 << 1)
-#define OPTION_MD5		(1 << 2)
-#define OPTION_WSCALE		(1 << 3)
-#define OPTION_FAST_OPEN_COOKIE	(1 << 8)
-#define OPTION_SMC		(1 << 9)
-#define OPTION_MPTCP		(1 << 10)
+#define OPTION_SACK_ADVERTISE	BIT(0)
+#define OPTION_TS		BIT(1)
+#define OPTION_MD5		BIT(2)
+#define OPTION_WSCALE		BIT(3)
+#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)
 {
 #if IS_ENABLED(CONFIG_SMC)
 	if (static_branch_unlikely(&tcp_have_smc)) {
Signature · 1 line
2.25.1
#18 lore raw

Re: [PATCH v2 07/25] tcp: Use BIT() for OPTION_* constants

Quoted 11 lines · Leonard Crestez

Extending these flags using the existing (1 << x) pattern triggers complaints from checkpatch.

Instead of ignoring checkpatch modify the existing values to use BIT(x) style in a separate commit.

---
 net/ipv4/tcp_output.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

This one could be sent outside of this patch set since you are not adding new values. Patch sets > 20 are generally frowned upon; sending this one separately helps get the number down.

#19 lore raw

Re: [PATCH v2 07/25] tcp: Use BIT() for OPTION_* constants

Quoted 13 lines · David Ahern
Quoted 11 lines · Leonard Crestez

Extending these flags using the existing (1 << x) pattern triggers complaints from checkpatch.

Instead of ignoring checkpatch modify the existing values to use BIT(x) style in a separate commit.

---
  net/ipv4/tcp_output.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)
Quoted 3 lines

This one could be sent outside of this patch set since you are not adding new values. Patch sets > 20 are generally frowned upon; sending this one separately helps get the number down.

In the past I've seen maintainers pick small cleanups and fixes from longer series that otherwise need further discussion.

Not sure if this practice is also common for netdev so I posted this patch separately.

--
Regards,
Leonard

#20 lore raw

PATCH v2 8/25 tcp: authopt: Hook into tcp core

In reply to Leonard Crestez

The tcp_authopt features exposes a minimal interface to the rest of the TCP stack. Only a few functions are exposed and if the feature is disabled they return neutral values, avoiding ifdefs in the rest of the code.

Add calls into tcp authopt from send, receive and accept code.

---
 include/net/tcp_authopt.h |  99 +++++++++++++
 include/uapi/linux/snmp.h |   1 +
 net/ipv4/proc.c           |   1 +
 net/ipv4/tcp_authopt.c    | 288 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_input.c      |  17 +++
 net/ipv4/tcp_ipv4.c       |  20 ++-
 net/ipv4/tcp_minisocks.c  |  12 ++
 net/ipv4/tcp_output.c     |  85 ++++++++++-
 net/ipv6/tcp_ipv6.c       |  21 ++-
 9 files changed, 538 insertions(+), 6 deletions(-)
include/net/tcp_authopt.h +99 -0
index 5217b6c7c900..8bb76128ed11 100644
@@ -65,28 +65,127 @@ struct tcp_authopt_info {
 	/** @dst_isn: Remote Initial Sequence Number */
 	u32 dst_isn;
 };
 
 #ifdef CONFIG_TCP_AUTHOPT
+DECLARE_STATIC_KEY_FALSE(tcp_authopt_needed);
+
+void tcp_authopt_free(struct sock *sk, struct tcp_authopt_info *info);
 void tcp_authopt_clear(struct sock *sk);
 int tcp_set_authopt(struct sock *sk, sockptr_t optval, unsigned int optlen);
 int tcp_get_authopt_val(struct sock *sk, struct tcp_authopt *key);
 int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen);
+struct tcp_authopt_key_info *__tcp_authopt_select_key(
+		const struct sock *sk,
+		struct tcp_authopt_info *info,
+		const struct sock *addr_sk,
+		u8 *rnextkeyid);
+static inline struct tcp_authopt_key_info *tcp_authopt_select_key(
+		const struct sock *sk,
+		const struct sock *addr_sk,
+		struct tcp_authopt_info **info,
+		u8 *rnextkeyid)
+{
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		*info = rcu_dereference(tcp_sk(sk)->authopt_info);
+
+		if (*info)
+			return __tcp_authopt_select_key(sk, *info, addr_sk, rnextkeyid);
+	}
+	return NULL;
+}
+int tcp_authopt_hash(
+		char *hash_location,
+		struct tcp_authopt_key_info *key,
+		struct tcp_authopt_info *info,
+		struct sock *sk, struct sk_buff *skb);
+int __tcp_authopt_openreq(struct sock *newsk, const struct sock *oldsk, struct request_sock *req);
+static inline int tcp_authopt_openreq(
+		struct sock *newsk,
+		const struct sock *oldsk,
+		struct request_sock *req)
+{
+	if (!rcu_dereference(tcp_sk(oldsk)->authopt_info))
+		return 0;
+	else
+		return __tcp_authopt_openreq(newsk, oldsk, req);
+}
+static inline void tcp_authopt_time_wait(
+		struct tcp_timewait_sock *tcptw,
+		struct tcp_sock *tp)
+{
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		/* Transfer ownership of authopt_info to the twsk
+		 * This requires no other users of the origin sock.
+		 */
+		sock_owned_by_me((struct sock *)tp);
+		tcptw->tw_authopt_info = tp->authopt_info;
+		tp->authopt_info = NULL;
+	} else {
+		tcptw->tw_authopt_info = NULL;
+	}
+}
+int __tcp_authopt_inbound_check(
+		struct sock *sk,
+		struct sk_buff *skb,
+		struct tcp_authopt_info *info);
+/** tcp_authopt_inbound_check - check for valid TCP-AO signature.
+ *
+ * Return negative ERRNO on error, 0 if not present and 1 if present and valid
+ * If both TCP-AO and MD5 signatures are found this is reported as an error.
+ */
+static inline int tcp_authopt_inbound_check(struct sock *sk, struct sk_buff *skb)
+{
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		struct tcp_authopt_info *info = rcu_dereference(tcp_sk(sk)->authopt_info);
+
+		if (info)
+			return __tcp_authopt_inbound_check(sk, skb, info);
+	}
+
+	return 0;
+}
 #else
 static inline int tcp_set_authopt(struct sock *sk, sockptr_t optval, unsigned int optlen)
 {
 	return -ENOPROTOOPT;
 }
 static inline int tcp_get_authopt_val(struct sock *sk, struct tcp_authopt *key)
 {
 	return -ENOPROTOOPT;
 }
+static inline void tcp_authopt_free(struct sock *sk, struct tcp_authopt_info *info)
+{
+}
 static inline void tcp_authopt_clear(struct sock *sk)
 {
 }
 static inline int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
 {
 	return -ENOPROTOOPT;
 }
+static inline int tcp_authopt_hash(
+		char *hash_location,
+		struct tcp_authopt_key_info *key,
+		struct tcp_authopt_key *info,
+		struct sock *sk, struct sk_buff *skb)
+{
+	return -EINVAL;
+}
+static inline int tcp_authopt_openreq(struct sock *newsk,
+				      const struct sock *oldsk,
+				      struct request_sock *req)
+{
+	return 0;
+}
+static inline void tcp_authopt_time_wait(
+		struct tcp_timewait_sock *tcptw,
+		struct tcp_sock *tp)
+{
+}
+static inline int tcp_authopt_inbound_check(struct sock *sk, struct sk_buff *skb)
+{
+	return 0;
+}
 #endif
 
 #endif /* _LINUX_TCP_AUTHOPT_H */
include/uapi/linux/snmp.h +1 -0
index 904909d020e2..1d96030889a1 100644
@@ -290,10 +290,11 @@ enum
 	LINUX_MIB_TCPDUPLICATEDATAREHASH,	/* TCPDuplicateDataRehash */
 	LINUX_MIB_TCPDSACKRECVSEGS,		/* TCPDSACKRecvSegs */
 	LINUX_MIB_TCPDSACKIGNOREDDUBIOUS,	/* TCPDSACKIgnoredDubious */
 	LINUX_MIB_TCPMIGRATEREQSUCCESS,		/* TCPMigrateReqSuccess */
 	LINUX_MIB_TCPMIGRATEREQFAILURE,		/* TCPMigrateReqFailure */
+	LINUX_MIB_TCPAUTHOPTFAILURE,		/* TCPAuthOptFailure */
 	__LINUX_MIB_MAX
 };
 
 /* linux Xfrm mib definitions */
 enum
net/ipv4/proc.c +1 -0
index f30273afb539..70f7a8a47045 100644
@@ -295,10 +295,11 @@ static const struct snmp_mib snmp4_net_list[] = {
 	SNMP_MIB_ITEM("TcpDuplicateDataRehash", LINUX_MIB_TCPDUPLICATEDATAREHASH),
 	SNMP_MIB_ITEM("TCPDSACKRecvSegs", LINUX_MIB_TCPDSACKRECVSEGS),
 	SNMP_MIB_ITEM("TCPDSACKIgnoredDubious", LINUX_MIB_TCPDSACKIGNOREDDUBIOUS),
 	SNMP_MIB_ITEM("TCPMigrateReqSuccess", LINUX_MIB_TCPMIGRATEREQSUCCESS),
 	SNMP_MIB_ITEM("TCPMigrateReqFailure", LINUX_MIB_TCPMIGRATEREQFAILURE),
+	SNMP_MIB_ITEM("TCPAuthOptFailure", LINUX_MIB_TCPAUTHOPTFAILURE),
 	SNMP_MIB_SENTINEL
 };
 
 static void icmpmsg_put_line(struct seq_file *seq, unsigned long *vals,
 			     unsigned short *type, int count)
net/ipv4/tcp_authopt.c +288 -0

372 lines not on this page · raw message

net/ipv4/tcp_input.c +17 -0
index 246ab7b5e857..5dcde6e74bfc 100644
@@ -70,10 +70,11 @@
 #include <linux/sysctl.h>
 #include <linux/kernel.h>
 #include <linux/prefetch.h>
 #include <net/dst.h>
 #include <net/tcp.h>
+#include <net/tcp_authopt.h>
 #include <net/inet_common.h>
 #include <linux/ipsec.h>
 #include <asm/unaligned.h>
 #include <linux/errqueue.h>
 #include <trace/events/tcp.h>
@@ -5984,18 +5985,34 @@ void tcp_init_transfer(struct sock *sk, int bpf_op, struct sk_buff *skb)
 	if (!icsk->icsk_ca_initialized)
 		tcp_init_congestion_control(sk);
 	tcp_init_buffer_space(sk);
 }
 
+static void tcp_authopt_finish_connect(struct sock *sk, struct sk_buff *skb)
+{
+#ifdef CONFIG_TCP_AUTHOPT
+	struct tcp_authopt_info *info;
+
+	info = rcu_dereference_protected(tcp_sk(sk)->authopt_info, lockdep_sock_is_held(sk));
+	if (!info)
+		return;
+
+	info->src_isn = ntohl(tcp_hdr(skb)->ack_seq) - 1;
+	info->dst_isn = ntohl(tcp_hdr(skb)->seq);
+#endif
+}
+
 void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct inet_connection_sock *icsk = inet_csk(sk);
 
 	tcp_set_state(sk, TCP_ESTABLISHED);
 	icsk->icsk_ack.lrcvtime = tcp_jiffies32;
 
+	tcp_authopt_finish_connect(sk, skb);
+
 	if (skb) {
 		icsk->icsk_af_ops->sk_rx_dst_set(sk, skb);
 		security_inet_conn_established(sk, skb);
 		sk_mark_napi_id(sk, skb);
 	}
net/ipv4/tcp_ipv4.c +18 -2
index 0c9f050fa0e8..da43567c3753 100644
@@ -1955,10 +1955,26 @@ static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
 	TCP_SKB_CB(skb)->sacked	 = 0;
 	TCP_SKB_CB(skb)->has_rxtstamp =
 			skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
 }
 
+static int tcp_v4_auth_inbound_check(struct sock *sk,
+				     struct sk_buff *skb,
+				     int dif,
+				     int sdif)
+{
+	int aoret;
+
+	aoret = tcp_authopt_inbound_check(sk, skb);
+	if (aoret < 0)
+		return aoret;
+	if (aoret > 0)
+		return 0;
+
+	return tcp_v4_inbound_md5_hash(sk, skb, dif, sdif);
+}
+
 /*
  *	From tcp_input.c
  */
 
 int tcp_v4_rcv(struct sk_buff *skb)
@@ -2012,11 +2028,11 @@ int tcp_v4_rcv(struct sk_buff *skb)
 		struct request_sock *req = inet_reqsk(sk);
 		bool req_stolen = false;
 		struct sock *nsk;
 
 		sk = req->rsk_listener;
-		if (unlikely(tcp_v4_inbound_md5_hash(sk, skb, dif, sdif))) {
+		if (unlikely(tcp_v4_auth_inbound_check(sk, skb, dif, sdif))) {
 			sk_drops_add(sk, skb);
 			reqsk_put(req);
 			goto discard_it;
 		}
 		if (tcp_checksum_complete(skb)) {
@@ -2082,11 +2098,11 @@ int tcp_v4_rcv(struct sk_buff *skb)
 	}
 
 	if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_and_relse;
 
-	if (tcp_v4_inbound_md5_hash(sk, skb, dif, sdif))
+	if (tcp_v4_auth_inbound_check(sk, skb, dif, sdif))
 		goto discard_and_relse;
 
 	nf_reset_ct(skb);
 
 	if (tcp_filter(sk, skb))
net/ipv4/tcp_minisocks.c +12 -0
index cf913a66df17..d4828cf3d6d1 100644
@@ -18,10 +18,11 @@
  *		Arnt Gulbrandsen, <[email protected]>
  *		Jorge Cwik, <[email protected]>
  */
 
 #include <net/tcp.h>
+#include <net/tcp_authopt.h>
 #include <net/xfrm.h>
 #include <net/busy_poll.h>
 
 static bool tcp_in_window(u32 seq, u32 end_seq, u32 s_win, u32 e_win)
 {
@@ -300,10 +301,11 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
 					BUG_ON(tcptw->tw_md5_key && !tcp_alloc_md5sig_pool());
 				}
 			}
 		} while (0);
 #endif
+		tcp_authopt_time_wait(tcptw, tcp_sk(sk));
 
 		/* Get the TIME_WAIT timeout firing. */
 		if (timeo < rto)
 			timeo = rto;
 
@@ -342,10 +344,19 @@ void tcp_twsk_destructor(struct sock *sk)
 
 		if (twsk->tw_md5_key)
 			kfree_rcu(twsk->tw_md5_key, rcu);
 	}
 #endif
+#ifdef CONFIG_TCP_AUTHOPT
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		struct tcp_timewait_sock *twsk = tcp_twsk(sk);
+
+		/* twsk only contains sock_common so pass NULL as sk. */
+		if (twsk->tw_authopt_info)
+			tcp_authopt_free(NULL, twsk->tw_authopt_info);
+	}
+#endif
 }
 EXPORT_SYMBOL_GPL(tcp_twsk_destructor);
 
 /* Warning : This function is called without sk_listener being locked.
  * Be sure to read socket fields once, as their value could change under us.
@@ -532,10 +543,11 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
 #ifdef CONFIG_TCP_MD5SIG
 	newtp->md5sig_info = NULL;	/*XXX*/
 	if (newtp->af_specific->md5_lookup(sk, newsk))
 		newtp->tcp_header_len += TCPOLEN_MD5SIG_ALIGNED;
 #endif
+	tcp_authopt_openreq(newsk, sk, req);
 	if (skb->len >= TCP_MSS_DEFAULT + newtp->tcp_header_len)
 		newicsk->icsk_ack.last_seg_size = skb->len - newtp->tcp_header_len;
 	newtp->rx_opt.mss_clamp = req->mss;
 	tcp_ecn_openreq_child(newtp, req);
 	newtp->fastopen_req = NULL;
net/ipv4/tcp_output.c +83 -2

228 lines not on this page · raw message

net/ipv6/tcp_ipv6.c +19 -2
index 2cc9b0e53ad1..96a29caf56c7 100644
@@ -40,10 +40,11 @@
 #include <linux/icmpv6.h>
 #include <linux/random.h>
 #include <linux/indirect_call_wrapper.h>
 
 #include <net/tcp.h>
+#include <net/tcp_authopt.h>
 #include <net/ndisc.h>
 #include <net/inet6_hashtables.h>
 #include <net/inet6_connection_sock.h>
 #include <net/ipv6.h>
 #include <net/transp_v6.h>
@@ -1619,10 +1620,26 @@ static void tcp_v6_fill_cb(struct sk_buff *skb, const struct ipv6hdr *hdr,
 	TCP_SKB_CB(skb)->sacked = 0;
 	TCP_SKB_CB(skb)->has_rxtstamp =
 			skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
 }
 
+static int tcp_v6_auth_inbound_check(struct sock *sk,
+				     struct sk_buff *skb,
+				     int dif,
+				     int sdif)
+{
+	int aoret;
+
+	aoret = tcp_authopt_inbound_check(sk, skb);
+	if (aoret < 0)
+		return aoret;
+	if (aoret > 0)
+		return 0;
+
+	return tcp_v6_inbound_md5_hash(sk, skb, dif, sdif);
+}
+
 INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 {
 	int sdif = inet6_sdif(skb);
 	int dif = inet6_iif(skb);
 	const struct tcphdr *th;
@@ -1671,11 +1688,11 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 		struct request_sock *req = inet_reqsk(sk);
 		bool req_stolen = false;
 		struct sock *nsk;
 
 		sk = req->rsk_listener;
-		if (tcp_v6_inbound_md5_hash(sk, skb, dif, sdif)) {
+		if (tcp_v6_auth_inbound_check(sk, skb, dif, sdif)) {
 			sk_drops_add(sk, skb);
 			reqsk_put(req);
 			goto discard_it;
 		}
 		if (tcp_checksum_complete(skb)) {
@@ -1738,11 +1755,11 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 	}
 
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_and_relse;
 
-	if (tcp_v6_inbound_md5_hash(sk, skb, dif, sdif))
+	if (tcp_v6_auth_inbound_check(sk, skb, dif, sdif))
 		goto discard_and_relse;
 
 	if (tcp_filter(sk, skb))
 		goto discard_and_relse;
 	th = (const struct tcphdr *)skb->data;
Signature · 1 line
2.25.1
#21 lore raw

PATCH v2 9/25 tcp: authopt: Disable via sysctl by default

In reply to Leonard Crestez

This is mainly intended to protect against local privilege escalations through a rarely used feature so it is deliberately not namespaced.

Enforcement is only at the setsockopt level, this should be enough to ensure that the tcp_authopt_needed static key never turns on.

No effort is made to handle disabling when the feature is already in use.

---
 Documentation/networking/ip-sysctl.rst |  6 ++++++
 include/net/tcp_authopt.h              |  1 +
 net/ipv4/sysctl_net_ipv4.c             | 10 ++++++++++
 net/ipv4/tcp_authopt.c                 | 13 ++++++++++++-
 4 files changed, 29 insertions(+), 1 deletion(-)
Documentation/networking/ip-sysctl.rst +6 -0
index 16b8bf72feaf..3f00681f73d7 100644
@@ -987,10 +987,16 @@ 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: 1000
 
+tcp_authopt - BOOLEAN
+	Enable the TCP Authentication Option (RFC5925), a replacement for TCP
+	MD5 Signatures (RFC2835).
+
+	Default: 0
+
 UDP variables
 =============
 
 udp_l3mdev_accept - BOOLEAN
 	Enabling this option allows a "global" bound socket to work
include/net/tcp_authopt.h +1 -0
index 8bb76128ed11..a505db1dd67b 100644
@@ -65,10 +65,11 @@ struct tcp_authopt_info {
 	/** @dst_isn: Remote Initial Sequence Number */
 	u32 dst_isn;
 };
 
 #ifdef CONFIG_TCP_AUTHOPT
+extern int sysctl_tcp_authopt;
 DECLARE_STATIC_KEY_FALSE(tcp_authopt_needed);
 
 void tcp_authopt_free(struct sock *sk, struct tcp_authopt_info *info);
 void tcp_authopt_clear(struct sock *sk);
 int tcp_set_authopt(struct sock *sk, sockptr_t optval, unsigned int optlen);
net/ipv4/sysctl_net_ipv4.c +10 -0
index 97eb54774924..cc34de6e4817 100644
@@ -17,10 +17,11 @@
 #include <net/udp.h>
 #include <net/cipso_ipv4.h>
 #include <net/ping.h>
 #include <net/protocol.h>
 #include <net/netevent.h>
+#include <net/tcp_authopt.h>
 
 static int two = 2;
 static int three __maybe_unused = 3;
 static int four = 4;
 static int thousand = 1000;
@@ -583,10 +584,19 @@ static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_douintvec_minmax,
 		.extra1		= &sysctl_fib_sync_mem_min,
 		.extra2		= &sysctl_fib_sync_mem_max,
 	},
+#ifdef CONFIG_TCP_AUTHOPT
+	{
+		.procname	= "tcp_authopt",
+		.data		= &sysctl_tcp_authopt,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
+#endif
 	{ }
 };
 
 static struct ctl_table ipv4_net_table[] = {
 	{
net/ipv4/tcp_authopt.c +12 -1
index 5e80e5e5e36e..7c49dcce7d24 100644
@@ -3,10 +3,15 @@
 #include <linux/kernel.h>
 #include <net/tcp.h>
 #include <net/tcp_authopt.h>
 #include <crypto/hash.h>
 
+/* This is mainly intended to protect against local privilege escalations through
+ * a rarely used feature so it is deliberately not namespaced.
+ */
+int sysctl_tcp_authopt;
+
 /* This is enabled when first struct tcp_authopt_info is allocated and never released */
 DEFINE_STATIC_KEY_FALSE(tcp_authopt_needed);
 EXPORT_SYMBOL(tcp_authopt_needed);
 
 /* All current algorithms have a mac length of 12 but crypto API digestsize can be larger */
@@ -360,10 +365,12 @@ int tcp_set_authopt(struct sock *sk, sockptr_t optval, unsigned int optlen)
 	struct tcp_authopt opt;
 	struct tcp_authopt_info *info;
 	int err;
 
 	sock_owned_by_me(sk);
+	if (!sysctl_tcp_authopt)
+		return -EPERM;
 
 	err = _copy_from_sockptr_tolerant((u8 *)&opt, sizeof(opt), optval, optlen);
 	if (err)
 		return err;
 
@@ -382,13 +389,15 @@ int tcp_set_authopt(struct sock *sk, sockptr_t optval, unsigned int optlen)
 int tcp_get_authopt_val(struct sock *sk, struct tcp_authopt *opt)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct tcp_authopt_info *info;
 
+	memset(opt, 0, sizeof(*opt));
 	sock_owned_by_me(sk);
+	if (!sysctl_tcp_authopt)
+		return -EPERM;
 
-	memset(opt, 0, sizeof(*opt));
 	info = rcu_dereference_check(tp->authopt_info, lockdep_sock_is_held(sk));
 	if (!info)
 		return -ENOENT;
 
 	opt->flags = info->flags & TCP_AUTHOPT_KNOWN_FLAGS;
@@ -451,10 +460,12 @@ int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
 	struct tcp_authopt_key_info *key_info, *old_key_info;
 	struct tcp_authopt_alg_imp *alg;
 	int err;
 
 	sock_owned_by_me(sk);
+	if (!sysctl_tcp_authopt)
+		return -EPERM;
 
 	err = _copy_from_sockptr_tolerant((u8 *)&opt, sizeof(opt), optval, optlen);
 	if (err)
 		return err;
 
Signature · 1 line
2.25.1
#22 lore raw

Re: [PATCH v2 09/25] tcp: authopt: Disable via sysctl by default

Quoted 19 lines · Leonard Crestez
net/ipv4/sysctl_net_ipv4.c +1 -0
index 97eb54774924..cc34de6e4817 100644
@@ -17,10 +17,11 @@
 #include <net/udp.h>
 #include <net/cipso_ipv4.h>
 #include <net/ping.h>
 #include <net/protocol.h>
 #include <net/netevent.h>
+#include <net/tcp_authopt.h>
 
 static int two = 2;
 static int three __maybe_unused = 3;
 static int four = 4;
 static int thousand = 1000;
@@ -583,10 +584,19 @@ static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_douintvec_minmax,

net/ipv4/sysctl_net_ipv4.c static struct ctl_table ipv4_table[] = { · line 595

Quoted 10 lines
net/ipv4/sysctl_net_ipv4.c +7 -0
 		.extra1		= &sysctl_fib_sync_mem_min,
 		.extra2		= &sysctl_fib_sync_mem_max,
 	},
+#ifdef CONFIG_TCP_AUTHOPT
+	{
+		.procname	= "tcp_authopt",
+		.data		= &sysctl_tcp_authopt,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,

Just add it to the namespace set, and this could be a u8 (try to plug a hole if possible) with min/max specified:

                .maxlen         = sizeof(u8),
                .mode           = 0644,
                .extra1         = SYSCTL_ZERO,
                .extra2         = SYSCTL_ONE

see icmp_echo_enable_probe as an example. And if you are not going to clean up when toggled off, you need a handler that tells the user it can not be disabled by erroring out on attempts to disable it.

#23 lore raw

Re: [PATCH v2 09/25] tcp: authopt: Disable via sysctl by default

Quoted 40 lines · David Ahern
Quoted 19 lines · Leonard Crestez
net/ipv4/sysctl_net_ipv4.c +1 -0
index 97eb54774924..cc34de6e4817 100644
@@ -17,10 +17,11 @@
  #include <net/udp.h>
  #include <net/cipso_ipv4.h>
  #include <net/ping.h>
  #include <net/protocol.h>
  #include <net/netevent.h>
+#include <net/tcp_authopt.h>
 
  static int two = 2;
  static int three __maybe_unused = 3;
  static int four = 4;
  static int thousand = 1000;
@@ -583,10 +584,19 @@ static struct ctl_table ipv4_table[] = {
  		.mode		= 0644,
  		.proc_handler	= proc_douintvec_minmax,

net/ipv4/sysctl_net_ipv4.c static struct ctl_table ipv4_table[] = { · line 595

Quoted 10 lines
net/ipv4/sysctl_net_ipv4.c +7 -0
  		.extra1		= &sysctl_fib_sync_mem_min,
  		.extra2		= &sysctl_fib_sync_mem_max,
  	},
+#ifdef CONFIG_TCP_AUTHOPT
+	{
+		.procname	= "tcp_authopt",
+		.data		= &sysctl_tcp_authopt,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,

Just add it to the namespace set, and this could be a u8 (try to plug a hole if possible) with min/max specified:

                 .maxlen         = sizeof(u8),
                 .mode           = 0644,
                 .extra1         = SYSCTL_ZERO,
                 .extra2         = SYSCTL_ONE
Quoted 3 lines

see icmp_echo_enable_probe as an example. And if you are not going to clean up when toggled off, you need a handler that tells the user it can not be disabled by erroring out on attempts to disable it.

This is deliberately per-system because the goal is to avoid possible local privilege escalations by reducing the attack surface. Even the smallest flaw could be exploited by a malicious application establishing an authenticated connection on loopback.

Applications running in containers frequently have full access to sysctls so making this per-namespace would defeat the original purpose. I can't think of any reason to prevent using this feature at the namespace level, it has no interesting effects outside TCP connections for which it is enabled.

I also believe that as similar sysctl would be useful for TCP-MD5.

You're right about adding additional prints.

--
Regards,
Leonard

#24 lore raw

Re: [PATCH v2 09/25] tcp: authopt: Disable via sysctl by default

In reply to Leonard Crestez
Quoted 11 lines · Leonard Crestez

This is mainly intended to protect against local privilege escalations through a rarely used feature so it is deliberately not namespaced.

Enforcement is only at the setsockopt level, this should be enough to ensure that the tcp_authopt_needed static key never turns on.

No effort is made to handle disabling when the feature is already in use.

---

[..]

Quoted 5 lines
net/ipv4/tcp_authopt.c +0 -0
index 5e80e5e5e36e..7c49dcce7d24 100644

net/ipv4/tcp_authopt.c line 11

Quoted 9 lines
net/ipv4/tcp_authopt.c +4 -0
@@ -3,10 +3,15 @@
 #include <linux/kernel.h>
 #include <net/tcp.h>
 #include <net/tcp_authopt.h>
 #include <crypto/hash.h>
 
+/* This is mainly intended to protect against local privilege escalations through
+ * a rarely used feature so it is deliberately not namespaced.
+ */
+int sysctl_tcp_authopt;

Could you add pr_warn_once() for setsockopt() without this set, so that it's visible in dmesg for a user that gets -EPERM.

Thanks,
Dmitry

#25 lore raw

PATCH v2 10/25 selftests: tcp_authopt: Test key address binding

In reply to Leonard Crestez

By default TCP-AO keys apply to all possible peers but it's possible to have different keys for different remote hosts.

This patch adds initial tests for the behavior behind the TCP_AUTHOPT_KEY_BIND_ADDR flag. Server rejection is tested via client timeout so this can be slightly slow.

---
 .../tcp_authopt_test/netns_fixture.py         |  85 ++++++++++
 .../tcp_authopt/tcp_authopt_test/server.py    | 124 ++++++++++++++
 .../tcp_authopt/tcp_authopt_test/test_bind.py | 155 ++++++++++++++++++
 .../tcp_authopt/tcp_authopt_test/utils.py     | 114 +++++++++++++
 4 files changed, 478 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/netns_fixture.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/server.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_bind.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/utils.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/netns_fixture.py new file +85 -0
new file mode 100644
index 000000000000..9c10fafd8694
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0
+import socket
+import subprocess
+from ipaddress import IPv4Address, IPv6Address
+
+from .conftest import raise_skip_no_netns
+
+
+class NamespaceFixture:
+    """Create a pair of namespaces connected by one veth pair
+
+    Each end of the pair has multiple addresses but everything is in the same subnet
+    """
+
+    server_netns_name = "tcp_authopt_test_server"
+    client_netns_name = "tcp_authopt_test_client"
+
+    @classmethod
+    def get_ipv4_addr(cls, ns=1, index=1) -> IPv4Address:
+        return IPv4Address("10.10.0.0") + (ns << 8) + index
+
+    @classmethod
+    def get_ipv6_addr(cls, ns=1, index=1) -> IPv6Address:
+        return IPv6Address("fd00::") + (ns << 16) + index
+
+    @classmethod
+    def get_addr(cls, address_family=socket.AF_INET, ns=1, index=1):
+        if address_family == socket.AF_INET:
+            return cls.get_ipv4_addr(ns, index)
+        elif address_family == socket.AF_INET6:
+            return cls.get_ipv6_addr(ns, index)
+        else:
+            raise ValueError(f"Bad address_family={address_family}")
+
+    # 02:* means "locally administered"
+    server_mac_addr = "02:00:00:00:00:01"
+    client_mac_addr = "02:00:00:00:00:02"
+
+    ipv4_prefix_len = 16
+    ipv6_prefix_len = 64
+
+    @classmethod
+    def get_prefix_length(cls, address_family) -> int:
+        return {
+            socket.AF_INET: cls.ipv4_prefix_len,
+            socket.AF_INET6: cls.ipv6_prefix_len,
+        }[address_family]
+
+    def __init__(self, **kw):
+        raise_skip_no_netns()
+        for k, v in kw.items():
+            setattr(self, k, v)
+
+    def __enter__(self):
+        self._del_netns()
+        script = f"""
+set -e
+ip netns add {self.server_netns_name}
+ip netns add {self.client_netns_name}
+ip link add veth0 netns {self.server_netns_name} type veth peer name veth0 netns {self.client_netns_name}
+ip netns exec {self.server_netns_name} ip link set veth0 up addr {self.server_mac_addr}
+ip netns exec {self.client_netns_name} ip link set veth0 up addr {self.client_mac_addr}
+"""
+        for index in [1, 2, 3]:
+            script += f"ip -n {self.server_netns_name} addr add {self.get_ipv4_addr(1, index)}/16 dev veth0\n"
+            script += f"ip -n {self.client_netns_name} addr add {self.get_ipv4_addr(2, index)}/16 dev veth0\n"
+            script += f"ip -n {self.server_netns_name} addr add {self.get_ipv6_addr(1, index)}/64 dev veth0 nodad\n"
+            script += f"ip -n {self.client_netns_name} addr add {self.get_ipv6_addr(2, index)}/64 dev veth0 nodad\n"
+        subprocess.run(script, shell=True, check=True)
+        return self
+
+    def _del_netns(self):
+        script = f"""\
+set -e
+if ip netns list | grep -q {self.server_netns_name}; then
+    ip netns del {self.server_netns_name}
+fi
+if ip netns list | grep -q {self.client_netns_name}; then
+    ip netns del {self.client_netns_name}
+fi
+"""
+        subprocess.run(script, shell=True, check=True)
+
+    def __exit__(self, *a):
+        self._del_netns()
tools/testing/selftests/tcp_authopt/tcp_authopt_test/server.py new file +124 -0

124 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_bind.py new file +155 -0

155 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/utils.py new file +114 -0

114 lines not on this page · raw message

Signature · 1 line
2.25.1
#26 lore raw

PATCH v2 11/25 tcp: authopt: Implement Sequence Number Extension

In reply to Leonard Crestez

Add a compute_sne function which finds the value of SNE for a certain SEQ given an already known "recent" SNE/SEQ. This is implemented using the standard tcp before/after macro and will work for SEQ values that are without 2^31 of the SEQ for which we know the SNE.

For updating we advance the value for rcv_sne at the same time as rcv_nxt and for snd_sne at the same time as snd_nxt. We could track other values (for example snd_una) but this is good enough and works very easily for timewait socket.

This implementation is different from RFC suggestions and doesn't require additional flags. It does pass tests from this draft:
https://datatracker.ietf.org/doc/draft-touch-sne/

---
 include/net/tcp_authopt.h | 36 ++++++++++++++++++
 net/ipv4/tcp_authopt.c    | 80 ++++++++++++++++++++++++++++++++++++++-
 net/ipv4/tcp_input.c      |  1 +
 net/ipv4/tcp_output.c     |  1 +
 4 files changed, 116 insertions(+), 2 deletions(-)
include/net/tcp_authopt.h +36 -0
index a505db1dd67b..7360bda20f97 100644
@@ -62,10 +62,14 @@ struct tcp_authopt_info {
 	u32 flags;
 	/** @src_isn: Local Initial Sequence Number */
 	u32 src_isn;
 	/** @dst_isn: Remote Initial Sequence Number */
 	u32 dst_isn;
+	/** @rcv_sne: Recv-side Sequence Number Extension tracking tcp_sock.rcv_nxt */
+	u32 rcv_sne;
+	/** @snd_sne: Send-side Sequence Number Extension tracking tcp_sock.snd_nxt */
+	u32 snd_sne;
 };
 
 #ifdef CONFIG_TCP_AUTHOPT
 extern int sysctl_tcp_authopt;
 DECLARE_STATIC_KEY_FALSE(tcp_authopt_needed);
@@ -143,10 +147,36 @@ static inline int tcp_authopt_inbound_check(struct sock *sk, struct sk_buff *skb
 			return __tcp_authopt_inbound_check(sk, skb, info);
 	}
 
 	return 0;
 }
+void __tcp_authopt_update_rcv_sne(struct tcp_sock *tp, struct tcp_authopt_info *info, u32 seq);
+static inline void tcp_authopt_update_rcv_sne(struct tcp_sock *tp, u32 seq)
+{
+	struct tcp_authopt_info *info;
+
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		rcu_read_lock();
+		info = rcu_dereference(tp->authopt_info);
+		if (info)
+			__tcp_authopt_update_rcv_sne(tp, info, seq);
+		rcu_read_unlock();
+	}
+}
+void __tcp_authopt_update_snd_sne(struct tcp_sock *tp, struct tcp_authopt_info *info, u32 seq);
+static inline void tcp_authopt_update_snd_sne(struct tcp_sock *tp, u32 seq)
+{
+	struct tcp_authopt_info *info;
+
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		rcu_read_lock();
+		info = rcu_dereference(tp->authopt_info);
+		if (info)
+			__tcp_authopt_update_snd_sne(tp, info, seq);
+		rcu_read_unlock();
+	}
+}
 #else
 static inline int tcp_set_authopt(struct sock *sk, sockptr_t optval, unsigned int optlen)
 {
 	return -ENOPROTOOPT;
 }
@@ -185,8 +215,14 @@ static inline void tcp_authopt_time_wait(
 }
 static inline int tcp_authopt_inbound_check(struct sock *sk, struct sk_buff *skb)
 {
 	return 0;
 }
+static inline void tcp_authopt_update_rcv_sne(struct tcp_sock *tp, u32 seq)
+{
+}
+static inline void tcp_authopt_update_snd_sne(struct tcp_sock *tp, u32 seq)
+{
+}
 #endif
 
 #endif /* _LINUX_TCP_AUTHOPT_H */
net/ipv4/tcp_authopt.c +78 -2

102 lines not on this page · raw message

net/ipv4/tcp_input.c +1 -0
index 5dcde6e74bfc..0ac74e621b4e 100644
@@ -3517,10 +3517,11 @@ static void tcp_snd_una_update(struct tcp_sock *tp, u32 ack)
 static void tcp_rcv_nxt_update(struct tcp_sock *tp, u32 seq)
 {
 	u32 delta = seq - tp->rcv_nxt;
 
 	sock_owned_by_me((struct sock *)tp);
+	tcp_authopt_update_rcv_sne(tp, seq);
 	tp->bytes_received += delta;
 	WRITE_ONCE(tp->rcv_nxt, seq);
 }
 
 /* Update our send window.
net/ipv4/tcp_output.c +1 -0
index 1e5acc5a38cf..ea53c24747b9 100644
@@ -67,10 +67,11 @@ static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 	unsigned int prior_packets = tp->packets_out;
 
+	tcp_authopt_update_snd_sne(tp, TCP_SKB_CB(skb)->end_seq);
 	WRITE_ONCE(tp->snd_nxt, TCP_SKB_CB(skb)->end_seq);
 
 	__skb_unlink(skb, &sk->sk_write_queue);
 	tcp_rbtree_insert(&sk->tcp_rtx_queue, skb);
 
Signature · 1 line
2.25.1
#27 lore raw

Re: [PATCH v2 11/25] tcp: authopt: Implement Sequence Number Extension

Quoted 5 lines
net/ipv4/tcp_authopt.c +5 -0
+/* Compute SNE for a specific packet (by seq). */
+static int compute_packet_sne(struct sock *sk, struct tcp_authopt_info *info,
+                             u32 seq, bool input, __be32 *sne)
+{
+       u32 rcv_nxt, snd_nxt;

net/ipv4/tcp_authopt.c static int skb_shash_frags(struct shash_desc *desc, · line 1023

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+
+       // We can't use normal SNE computation before reaching TCP_ESTABLISHED
+       // For TCP_SYN_SENT the dst_isn field is initialized only after we
+       // validate the remote SYN/ACK
+       // For TCP_NEW_SYN_RECV there is no tcp_authopt_info at all
+       if (sk->sk_state == TCP_SYN_SENT ||
+           sk->sk_state == TCP_NEW_SYN_RECV ||
+           sk->sk_state == TCP_LISTEN)
+               return 0;
+

In case of TCP_NEW_SYN_RECV, if our SYNACK had sequence number 0xffffffff, we will receive an ACK sequence number of 0, which should have sne = 1.

In a somewhat similar corner case, when we receive a SYNACK to our SYN in tcp_rcv_synsent_state_process, if the SYNACK has sequence number 0xffffffff, we set tp->rcv_nxt to 0, and we should set sne to 1.

There may be more similar corner cases related to a wraparound during the handshake.

Since as you pointed out all we need is "recent" valid <sne, seq> pairs as reference, rather than relying on rcv_sne being paired with tp->rcv_nxt (and similarly for snd_sne and tp->snd_nxt), would it be easier to maintain reference <sne, seq> pairs for send and receive in tcp_authopt_info, appropriately handle the different handshake cases and initialize the pairs, and only then track them in tcp_rcv_nxt_update and tcp_rcv_snd_update?

Quoted 10 lines

static void tcp_rcv_nxt_update(struct tcp_sock *tp, u32 seq)

 {
        u32 delta = seq - tp->rcv_nxt;

        sock_owned_by_me((struct sock *)tp);
+       tcp_authopt_update_rcv_sne(tp, seq);
        tp->bytes_received += delta;
        WRITE_ONCE(tp->rcv_nxt, seq);

}

Since rcv_sne and tp->rcv_nxt are not updated atomically, could there ever be a case where a reader might use the new sne with the old rcv_nxt?

Francesco

#28 lore raw

Re: [PATCH v2 11/25] tcp: authopt: Implement Sequence Number Extension

Quoted 28 lines · Francesco Ruggeri
Quoted 5 lines
net/ipv4/tcp_authopt.c +5 -0
+/* Compute SNE for a specific packet (by seq). */
+static int compute_packet_sne(struct sock *sk, struct tcp_authopt_info *info,
+                             u32 seq, bool input, __be32 *sne)
+{
+       u32 rcv_nxt, snd_nxt;

net/ipv4/tcp_authopt.c static int skb_shash_frags(struct shash_desc *desc, · line 1023

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+
+       // We can't use normal SNE computation before reaching TCP_ESTABLISHED
+       // For TCP_SYN_SENT the dst_isn field is initialized only after we
+       // validate the remote SYN/ACK
+       // For TCP_NEW_SYN_RECV there is no tcp_authopt_info at all
+       if (sk->sk_state == TCP_SYN_SENT ||
+           sk->sk_state == TCP_NEW_SYN_RECV ||
+           sk->sk_state == TCP_LISTEN)
+               return 0;
+

In case of TCP_NEW_SYN_RECV, if our SYNACK had sequence number 0xffffffff, we will receive an ACK sequence number of 0, which should have sne = 1.

In a somewhat similar corner case, when we receive a SYNACK to our SYN in tcp_rcv_synsent_state_process, if the SYNACK has sequence number 0xffffffff, we set tp->rcv_nxt to 0, and we should set sne to 1.

There may be more similar corner cases related to a wraparound during the handshake.

Quoted 7 lines

Since as you pointed out all we need is "recent" valid <sne, seq> pairs as reference, rather than relying on rcv_sne being paired with tp->rcv_nxt (and similarly for snd_sne and tp->snd_nxt), would it be easier to maintain reference <sne, seq> pairs for send and receive in tcp_authopt_info, appropriately handle the different handshake cases and initialize the pairs, and only then track them in tcp_rcv_nxt_update and tcp_rcv_snd_update?

For TCP_NEW_SYN_RECV there is no struct tcp_authopt_info, only a request minisock. I think those are deliberately kept small save resources on SYN floods so I'd rather not increase their size.

For all the handshake cases we can just rely on SNE=0 for ISN and we already need to keep track of ISNs because they're part of the signature.

I'll need to test handshake seq 0xFFFFFFFF deliberately, you're right that it can fail.

Quoted 11 lines
Quoted 10 lines
  static void tcp_rcv_nxt_update(struct tcp_sock *tp, u32 seq)
  {
         u32 delta = seq - tp->rcv_nxt;
         sock_owned_by_me((struct sock *)tp);
+       tcp_authopt_update_rcv_sne(tp, seq);
         tp->bytes_received += delta;
         WRITE_ONCE(tp->rcv_nxt, seq);

}

Quoted 3 lines

Since rcv_sne and tp->rcv_nxt are not updated atomically, could there ever be a case where a reader might use the new sne with the old rcv_nxt?

As far as I understand if all of the read and writes to SNE happen under the socket lock it should be fine. I don't know why WRITE_ONCE is used here, maybe somebody else wants to read rcv_nxt outside the socket lock? That doesn't matter for SNE.

I think the only case would be sending ipv4 RSTs outside the socket.

--
Regards,
Leonard

#29 lore raw

Re: [PATCH v2 11/25] tcp: authopt: Implement Sequence Number Extension

Quoted 38 lines · Leonard Crestez
Quoted 28 lines · Francesco Ruggeri
Quoted 5 lines
net/ipv4/tcp_authopt.c +5 -0
+/* Compute SNE for a specific packet (by seq). */
+static int compute_packet_sne(struct sock *sk, struct tcp_authopt_info *info,
+                             u32 seq, bool input, __be32 *sne)
+{
+       u32 rcv_nxt, snd_nxt;

net/ipv4/tcp_authopt.c static int skb_shash_frags(struct shash_desc *desc, · line 1023

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+
+       // We can't use normal SNE computation before reaching TCP_ESTABLISHED
+       // For TCP_SYN_SENT the dst_isn field is initialized only after we
+       // validate the remote SYN/ACK
+       // For TCP_NEW_SYN_RECV there is no tcp_authopt_info at all
+       if (sk->sk_state == TCP_SYN_SENT ||
+           sk->sk_state == TCP_NEW_SYN_RECV ||
+           sk->sk_state == TCP_LISTEN)
+               return 0;
+

In case of TCP_NEW_SYN_RECV, if our SYNACK had sequence number 0xffffffff, we will receive an ACK sequence number of 0, which should have sne = 1.

In a somewhat similar corner case, when we receive a SYNACK to our SYN in tcp_rcv_synsent_state_process, if the SYNACK has sequence number 0xffffffff, we set tp->rcv_nxt to 0, and we should set sne to 1.

There may be more similar corner cases related to a wraparound during the handshake.

Quoted 7 lines

Since as you pointed out all we need is "recent" valid <sne, seq> pairs as reference, rather than relying on rcv_sne being paired with tp->rcv_nxt (and similarly for snd_sne and tp->snd_nxt), would it be easier to maintain reference <sne, seq> pairs for send and receive in tcp_authopt_info, appropriately handle the different handshake cases and initialize the pairs, and only then track them in tcp_rcv_nxt_update and tcp_rcv_snd_update?

Quoted 7 lines

For TCP_NEW_SYN_RECV there is no struct tcp_authopt_info, only a request minisock. I think those are deliberately kept small save resources on SYN floods so I'd rather not increase their size.

For all the handshake cases we can just rely on SNE=0 for ISN and we already need to keep track of ISNs because they're part of the signature.

Exactly. But the current code, when setting rcv_sne and snd_sne, always compares the sequence number with the <info->rcv_sne, tp->rcv_nxt> (or <info->snd_sne, tp->snd_nxt>) pair, where info->rcv_sne and info->snd_sne are initialized to 0 at the time of info creation. In other words, the code assumes that rcv_sne always corresponds to tp->rcv_nxt, and snd_sne to tp->snd_nxt. But that may not be true when info is created, on account of rollovers during a handshake. So it is not just a matter of what to use for SNE before info is created and used, but also how SNEs are initialized in info. That is why I was suggesting of saving valid <sne, seq> pairs (initialized with <0, ISN>) in tcp_authopt_info rather than just SNEs, and then always compare seq to those pairs if info is available. The pairs could then be updated in tcp_rcv_nxt_update and tcp_snd_una_update.

Regards,
Francesco

#30 lore raw

Re: [PATCH v2 11/25] tcp: authopt: Implement Sequence Number Extension

Quoted 47 lines · Francesco Ruggeri
Quoted 38 lines · Leonard Crestez
Quoted 28 lines · Francesco Ruggeri
Quoted 5 lines
net/ipv4/tcp_authopt.c +5 -0
+/* Compute SNE for a specific packet (by seq). */
+static int compute_packet_sne(struct sock *sk, struct tcp_authopt_info *info,
+                             u32 seq, bool input, __be32 *sne)
+{
+       u32 rcv_nxt, snd_nxt;

net/ipv4/tcp_authopt.c static int skb_shash_frags(struct shash_desc *desc, · line 1023

Quoted 10 lines
net/ipv4/tcp_authopt.c +10 -0
+
+       // We can't use normal SNE computation before reaching TCP_ESTABLISHED
+       // For TCP_SYN_SENT the dst_isn field is initialized only after we
+       // validate the remote SYN/ACK
+       // For TCP_NEW_SYN_RECV there is no tcp_authopt_info at all
+       if (sk->sk_state == TCP_SYN_SENT ||
+           sk->sk_state == TCP_NEW_SYN_RECV ||
+           sk->sk_state == TCP_LISTEN)
+               return 0;
+

In case of TCP_NEW_SYN_RECV, if our SYNACK had sequence number 0xffffffff, we will receive an ACK sequence number of 0, which should have sne = 1.

In a somewhat similar corner case, when we receive a SYNACK to our SYN in tcp_rcv_synsent_state_process, if the SYNACK has sequence number 0xffffffff, we set tp->rcv_nxt to 0, and we should set sne to 1.

There may be more similar corner cases related to a wraparound during the handshake.

Quoted 7 lines

Since as you pointed out all we need is "recent" valid <sne, seq> pairs as reference, rather than relying on rcv_sne being paired with tp->rcv_nxt (and similarly for snd_sne and tp->snd_nxt), would it be easier to maintain reference <sne, seq> pairs for send and receive in tcp_authopt_info, appropriately handle the different handshake cases and initialize the pairs, and only then track them in tcp_rcv_nxt_update and tcp_rcv_snd_update?

Quoted 7 lines

For TCP_NEW_SYN_RECV there is no struct tcp_authopt_info, only a request minisock. I think those are deliberately kept small save resources on SYN floods so I'd rather not increase their size.

For all the handshake cases we can just rely on SNE=0 for ISN and we already need to keep track of ISNs because they're part of the signature.

Quoted 14 lines

Exactly. But the current code, when setting rcv_sne and snd_sne, always compares the sequence number with the <info->rcv_sne, tp->rcv_nxt> (or <info->snd_sne, tp->snd_nxt>) pair, where info->rcv_sne and info->snd_sne are initialized to 0 at the time of info creation. In other words, the code assumes that rcv_sne always corresponds to tp->rcv_nxt, and snd_sne to tp->snd_nxt. But that may not be true when info is created, on account of rollovers during a handshake. So it is not just a matter of what to use for SNE before info is created and used, but also how SNEs are initialized in info. That is why I was suggesting of saving valid <sne, seq> pairs (initialized with <0, ISN>) in tcp_authopt_info rather than just SNEs, and then always compare seq to those pairs if info is available. The pairs could then be updated in tcp_rcv_nxt_update and tcp_snd_una_update.

You are correct that SNE will be initialized incorrectly if a rollover happens during the handshake. I think this can be solved by initializing SNE at the same time as ISN like this:

rcv_sne = compute_sne(0, disn, rcv_nxt);
snd_sne = compute_sne(0, sisn, snd_nxt);

This relies on initial sequence numbers having an extension of zero by definition. The actual implementation is a bit more complicated but it only needs to be done when transitioning into ESTABLISHED. I think this would even work for FASTOPEN where non-zero payload is present in handshake packets.

The SYN_SEND and SYN_RECV sockets are still special but they're also special because they have to determine ISNs from the packet itself. Since those sockets only compute signatures for packets with SYN bit ON and where SEQ=ISN then SNE is again zero by definition.

I will write tests with client and server-side SEQs equal to 0xFFFFFFFF to verify because this relies on actual initialization order details.

I think snd_nxt and rcv_nxt are good choices for SNE tracking because the rest of the TCP state machine controls their advancement. In theory it's possible to use any received SEQ value but then a very old or perhaps malicious packet could cause incorrect updates to SNE.

If separate fields were used to track rcv_sne_seq and snd_sne_seq then you would still need to only advance them for SEQ values which are known to be valid. Doing so in lockstep with snd_nxt and rcv_nxt would still make sense.

--
Regards,
Leonard

#31 lore raw

Re: [PATCH v2 11/25] tcp: authopt: Implement Sequence Number Extension

In reply to Leonard Crestez
Quoted 23 lines · Leonard Crestez

Add a compute_sne function which finds the value of SNE for a certain SEQ given an already known "recent" SNE/SEQ. This is implemented using the standard tcp before/after macro and will work for SEQ values that are without 2^31 of the SEQ for which we know the SNE.

}

include/net/tcp_authopt.h +16 -0
+void __tcp_authopt_update_rcv_sne(struct tcp_sock *tp, struct tcp_authopt_info *info, u32 seq);
+static inline void tcp_authopt_update_rcv_sne(struct tcp_sock *tp, u32 seq)
+{
+	struct tcp_authopt_info *info;
+
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		rcu_read_lock();
+		info = rcu_dereference(tp->authopt_info);
+		if (info)
+			__tcp_authopt_update_rcv_sne(tp, info, seq);
+		rcu_read_unlock();
+	}
+}
+void __tcp_authopt_update_snd_sne(struct tcp_sock *tp, struct tcp_authopt_info *info, u32 seq);
+static inline void tcp_authopt_update_snd_sne(struct tcp_sock *tp, u32 seq)
+{

include/net/tcp_authopt.h static inline int tcp_authopt_inbound_check(struct sock *sk, struct sk_buff *skb · line 177

Quoted 10 lines
include/net/tcp_authopt.h +10 -0
+	struct tcp_authopt_info *info;
+
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		rcu_read_lock();
+		info = rcu_dereference(tp->authopt_info);
+		if (info)
+			__tcp_authopt_update_snd_sne(tp, info, seq);
+		rcu_read_unlock();
+	}
+}

I would think callers of these helpers own socket lock, so no rcu_read_lock()/unlock() should be needed.

Perhaps instead
rcu_dereference_protected(tp->authopt_info, lockdep_sock_is_held(sk));

#32 lore raw

Re: [PATCH v2 11/25] tcp: authopt: Implement Sequence Number Extension

Quoted 38 lines · Eric Dumazet
Quoted 23 lines · Leonard Crestez

Add a compute_sne function which finds the value of SNE for a certain SEQ given an already known "recent" SNE/SEQ. This is implemented using the standard tcp before/after macro and will work for SEQ values that are without 2^31 of the SEQ for which we know the SNE.

  }
include/net/tcp_authopt.h +16 -0
+void __tcp_authopt_update_rcv_sne(struct tcp_sock *tp, struct tcp_authopt_info *info, u32 seq);
+static inline void tcp_authopt_update_rcv_sne(struct tcp_sock *tp, u32 seq)
+{
+	struct tcp_authopt_info *info;
+
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		rcu_read_lock();
+		info = rcu_dereference(tp->authopt_info);
+		if (info)
+			__tcp_authopt_update_rcv_sne(tp, info, seq);
+		rcu_read_unlock();
+	}
+}
+void __tcp_authopt_update_snd_sne(struct tcp_sock *tp, struct tcp_authopt_info *info, u32 seq);
+static inline void tcp_authopt_update_snd_sne(struct tcp_sock *tp, u32 seq)
+{

include/net/tcp_authopt.h static inline int tcp_authopt_inbound_check(struct sock *sk, struct sk_buff *skb · line 177

Quoted 10 lines
include/net/tcp_authopt.h +10 -0
+	struct tcp_authopt_info *info;
+
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		rcu_read_lock();
+		info = rcu_dereference(tp->authopt_info);
+		if (info)
+			__tcp_authopt_update_snd_sne(tp, info, seq);
+		rcu_read_unlock();
+	}
+}

I would think callers of these helpers own socket lock, so no rcu_read_lock()/unlock() should be needed.

Quoted 2 lines

Perhaps instead
rcu_dereference_protected(tp->authopt_info, lockdep_sock_is_held(sk));

Yes, all the callers hold the socket lock and replacing rcu_read_lock doesn't trigger any RCU warnings.

--
Regards,
Leonard

#33 lore raw

PATCH v2 12/25 tcp: ipv6: Add AO signing for tcp_v6_send_response

In reply to Leonard Crestez

This is a special code path for acks and resets outside of normal connection establishment and closing.

---
 net/ipv4/tcp_authopt.c |  2 ++
 net/ipv6/tcp_ipv6.c    | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)
net/ipv4/tcp_authopt.c +2 -0
index a48b741c83e4..c9d201d8f7a7 100644
@@ -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)
net/ipv6/tcp_ipv6.c +38 -0
index 96a29caf56c7..68f9545e4347 100644
@@ -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;
Signature · 1 line
2.25.1
#34 lore raw

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

Quoted 9 lines · Leonard Crestez
net/ipv6/tcp_ipv6.c +0 -0
index 96a29caf56c7..68f9545e4347 100644
@@ -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;

net/ipv6/tcp_ipv6.c static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32 · line 915

Quoted 10 lines
net/ipv6/tcp_ipv6.c +6 -0
 	__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.

Quoted 49 lines
net/ipv6/tcp_ipv6.c +32 -0
+	/* 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;
#35 lore raw

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

Quoted 17 lines · David Ahern
Quoted 16 lines · Leonard Crestez
net/ipv6/tcp_ipv6.c +5 -0
index 96a29caf56c7..68f9545e4347 100644
@@ -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
 
Quoted 8 lines

net/ipv6/tcp_ipv6.c static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32 · line 915

Quoted 3 lines
net/ipv6/tcp_ipv6.c +1 -0
  	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.

Quoted 50 lines
Quoted 49 lines
net/ipv6/tcp_ipv6.c +32 -0
+	/* 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;
#36 lore raw

PATCH v2 13/25 tcp: authopt: Add support for signing skb-less replies

In reply to Leonard Crestez

This is required because tcp ipv4 sometimes sends replies without allocating a full skb that can be signed by tcp authopt.

Handle this with additional code in tcp authopt.

---
 include/net/tcp_authopt.h |   7 ++
 net/ipv4/tcp_authopt.c    | 147 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+)
include/net/tcp_authopt.h +7 -0
index 7360bda20f97..ae7d6a1eab8d 100644
@@ -101,10 +101,17 @@ static inline struct tcp_authopt_key_info *tcp_authopt_select_key(
 int tcp_authopt_hash(
 		char *hash_location,
 		struct tcp_authopt_key_info *key,
 		struct tcp_authopt_info *info,
 		struct sock *sk, struct sk_buff *skb);
+int tcp_v4_authopt_hash_reply(
+		char *hash_location,
+		struct tcp_authopt_info *info,
+		struct tcp_authopt_key_info *key,
+		__be32 saddr,
+		__be32 daddr,
+		struct tcphdr *th);
 int __tcp_authopt_openreq(struct sock *newsk, const struct sock *oldsk, struct request_sock *req);
 static inline int tcp_authopt_openreq(
 		struct sock *newsk,
 		const struct sock *oldsk,
 		struct request_sock *req)
net/ipv4/tcp_authopt.c +147 -0

167 lines not on this page · raw message

Signature · 1 line
2.25.1
#37 lore raw

PATCH v2 14/25 tcp: ipv4: Add AO signing for skb-less replies

In reply to Leonard Crestez

The code in tcp_v4_send_ack and tcp_v4_send_reset does not allocate a full skb so special handling is required for tcp-authopt handling.

---
 net/ipv4/tcp_ipv4.c | 82 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 3 deletions(-)
net/ipv4/tcp_ipv4.c +79 -3
index da43567c3753..21971f5fa40e 100644
@@ -644,10 +644,50 @@ void tcp_v4_send_check(struct sock *sk, struct sk_buff *skb)
 
 	__tcp_v4_send_check(skb, inet->inet_saddr, inet->inet_daddr);
 }
 EXPORT_SYMBOL(tcp_v4_send_check);
 
+#ifdef CONFIG_TCP_AUTHOPT
+/** tcp_v4_authopt_handle_reply - Insert TCPOPT_AUTHOPT if required
+ *
+ * returns number of bytes (always aligned to 4) or zero
+ */
+static int tcp_v4_authopt_handle_reply(const struct sock *sk,
+				       struct sk_buff *skb,
+				       __be32 *optptr,
+				       struct tcphdr *th)
+{
+	struct tcp_authopt_info *info;
+	struct tcp_authopt_key_info *key_info;
+	u8 rnextkeyid;
+
+	if (sk->sk_state == TCP_TIME_WAIT)
+		info = tcp_twsk(sk)->tw_authopt_info;
+	else
+		info = tcp_sk(sk)->authopt_info;
+	if (!info)
+		return 0;
+	key_info = __tcp_authopt_select_key(sk, info, sk, &rnextkeyid);
+	if (!key_info)
+		return 0;
+	*optptr = htonl((TCPOPT_AUTHOPT << 24) |
+			(TCPOLEN_AUTHOPT_OUTPUT << 16) |
+			(key_info->send_id << 8) |
+			(rnextkeyid));
+	/* must update doff before signature computation */
+	th->doff += TCPOLEN_AUTHOPT_OUTPUT / 4;
+	tcp_v4_authopt_hash_reply((char *)(optptr + 1),
+				  info,
+				  key_info,
+				  ip_hdr(skb)->daddr,
+				  ip_hdr(skb)->saddr,
+				  th);
+
+	return TCPOLEN_AUTHOPT_OUTPUT;
+}
+#endif
+
 /*
  *	This routine will send an RST to the other tcp.
  *
  *	Someone asks: why I NEVER use socket parameters (TOS, TTL etc.)
  *		      for reset.
@@ -659,10 +699,12 @@ EXPORT_SYMBOL(tcp_v4_send_check);
  *	Exception: precedence violation. We do not implement it in any case.
  */
 
 #ifdef CONFIG_TCP_MD5SIG
 #define OPTION_BYTES TCPOLEN_MD5SIG_ALIGNED
+#elif defined(OPTION_BYTES_TCP_AUTHOPT)
+#define OPTION_BYTES TCPOLEN_AUTHOPT_OUTPUT
 #else
 #define OPTION_BYTES sizeof(__be32)
 #endif
 
 static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
@@ -712,12 +754,29 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 	memset(&arg, 0, sizeof(arg));
 	arg.iov[0].iov_base = (unsigned char *)&rep;
 	arg.iov[0].iov_len  = sizeof(rep.th);
 
 	net = sk ? sock_net(sk) : dev_net(skb_dst(skb)->dev);
-#ifdef CONFIG_TCP_MD5SIG
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AUTHOPT)
 	rcu_read_lock();
+#endif
+#ifdef CONFIG_TCP_AUTHOPT
+	/* Unlike TCP-MD5 the signatures for TCP-AO depend on initial sequence
+	 * numbers so we can only handle established and time-wait sockets.
+	 */
+	if (static_branch_unlikely(&tcp_authopt_needed) && sk &&
+	    sk->sk_state != TCP_NEW_SYN_RECV &&
+	    sk->sk_state != TCP_LISTEN) {
+		int tcp_authopt_ret = tcp_v4_authopt_handle_reply(sk, skb, rep.opt, &rep.th);
+
+		if (tcp_authopt_ret) {
+			arg.iov[0].iov_len += tcp_authopt_ret;
+			goto skip_md5sig;
+		}
+	}
+#endif
+#ifdef CONFIG_TCP_MD5SIG
 	hash_location = tcp_parse_md5sig_option(th);
 	if (sk && sk_fullsock(sk)) {
 		const union tcp_md5_addr *addr;
 		int l3index;
 
@@ -755,11 +814,10 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 		addr = (union tcp_md5_addr *)&ip_hdr(skb)->saddr;
 		key = tcp_md5_do_lookup(sk1, l3index, addr, AF_INET);
 		if (!key)
 			goto out;
 
-
 		genhash = tcp_v4_md5_hash_skb(newhash, key, NULL, skb);
 		if (genhash || memcmp(hash_location, newhash, 16) != 0)
 			goto out;
 
 	}
@@ -775,10 +833,13 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 
 		tcp_v4_md5_hash_hdr((__u8 *) &rep.opt[1],
 				     key, ip_hdr(skb)->saddr,
 				     ip_hdr(skb)->daddr, &rep.th);
 	}
+#endif
+#ifdef CONFIG_TCP_AUTHOPT
+skip_md5sig:
 #endif
 	/* Can't co-exist with TCPMD5, hence check rep.opt[0] */
 	if (rep.opt[0] == 0) {
 		__be32 mrst = mptcp_reset_option(skb);
 
@@ -828,12 +889,14 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 	ctl_sk->sk_mark = 0;
 	__TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
 	__TCP_INC_STATS(net, TCP_MIB_OUTRSTS);
 	local_bh_enable();
 
-#ifdef CONFIG_TCP_MD5SIG
+#if defined(CONFIG_TCP_MD5SIG)
 out:
+#endif
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AUTHOPT)
 	rcu_read_unlock();
 #endif
 }
 
 /* The code following below sending ACKs in SYN-RECV and TIME-WAIT states
@@ -850,10 +913,12 @@ static void tcp_v4_send_ack(const struct sock *sk,
 	struct {
 		struct tcphdr th;
 		__be32 opt[(TCPOLEN_TSTAMP_ALIGNED >> 2)
 #ifdef CONFIG_TCP_MD5SIG
 			   + (TCPOLEN_MD5SIG_ALIGNED >> 2)
+#elif defined(CONFIG_TCP_AUTHOPT)
+			   + (TCPOLEN_AUTHOPT_OUTPUT >> 2)
 #endif
 			];
 	} rep;
 	struct net *net = sock_net(sk);
 	struct ip_reply_arg arg;
@@ -881,10 +946,21 @@ static void tcp_v4_send_ack(const struct sock *sk,
 	rep.th.seq     = htonl(seq);
 	rep.th.ack_seq = htonl(ack);
 	rep.th.ack     = 1;
 	rep.th.window  = htons(win);
 
+#ifdef CONFIG_TCP_AUTHOPT
+	if (static_branch_unlikely(&tcp_authopt_needed)) {
+		int aoret, offset = (tsecr) ? 3 : 0;
+
+		aoret = tcp_v4_authopt_handle_reply(sk, skb, &rep.opt[offset], &rep.th);
+		if (aoret) {
+			arg.iov[0].iov_len += aoret;
+			key = NULL;
+		}
+	}
+#endif
 #ifdef CONFIG_TCP_MD5SIG
 	if (key) {
 		int offset = (tsecr) ? 3 : 0;
 
 		rep.opt[offset++] = htonl((TCPOPT_NOP << 24) |
Signature · 1 line
2.25.1
#38 lore raw

PATCH v2 15/25 selftests: tcp_authopt: Implement SNE in python

In reply to Leonard Crestez

Add implementation and tests for Sequence Number Extension.

One implementation is based on an IETF draft:
https://datatracker.ietf.org/doc/draft-touch-sne/

The linux implementation is simpler and doesn't require additional flags, it just relies on standard before/after macros.

---
 .../tcp_authopt/tcp_authopt_test/sne_alg.py   | 111 ++++++++++++++++++
 .../tcp_authopt_test/test_sne_alg.py          |  96 +++++++++++++++
 2 files changed, 207 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/sne_alg.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sne_alg.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/sne_alg.py new file +111 -0
new file mode 100644
index 000000000000..252356dc87a4
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier: GPL-2.0
+"""Python implementation of SNE algorithms"""
+
+
+def distance(x, y):
+    if x < y:
+        return y - x
+    else:
+        return x - y
+
+
+class SequenceNumberExtender:
+    """Based on https://datatracker.ietf.org/doc/draft-touch-sne/"""
+
+    sne: int = 0
+    sne_flag: int = 1
+    prev_seq: int = 0
+
+    def calc(self, seq):
+        """Update internal state and return SNE for certain SEQ"""
+        # use current SNE to start
+        result = self.sne
+
+        # both in same SNE range?
+        if distance(seq, self.prev_seq) < 0x80000000:
+            # jumps fwd over N/2?
+            if seq >= 0x80000000 and self.prev_seq < 0x80000000:
+                self.sne_flag = 0
+            # move prev forward if needed
+            self.prev_seq = max(seq, self.prev_seq)
+        # both in diff SNE ranges?
+        else:
+            # jumps forward over zero?
+            if seq < 0x80000000:
+                # update prev
+                self.prev_seq = seq
+                # first jump over zero? (wrap)
+                if self.sne_flag == 0:
+                    # set flag so we increment once
+                    self.sne_flag = 1
+                    # increment window
+                    self.sne = self.sne + 1
+                    # use updated SNE value
+                    result = self.sne
+            # jump backward over zero?
+            else:
+                # use pre-rollover SNE value
+                result = self.sne - 1
+
+        return result
+
+
+class SequenceNumberExtenderRFC:
+    """Based on sample code in original RFC5925 document"""
+
+    sne: int = 0
+    sne_flag: int = 1
+    prev_seq: int = 0
+
+    def calc(self, seq):
+        """Update internal state and return SNE for certain SEQ"""
+        # set the flag when the SEG.SEQ first rolls over
+        if self.sne_flag == 0 and self.prev_seq > 0x7FFFFFFF and seq < 0x7FFFFFFF:
+            self.sne = self.sne + 1
+            self.sne_flag = 1
+        # decide which SNE to use after incremented
+        if self.sne_flag and seq > 0x7FFFFFFF:
+            # use the pre-increment value
+            sne = self.sne - 1
+        else:
+            # use the current value
+            sne = self.sne
+        # reset the flag in the *middle* of the window
+        if self.prev_seq < 0x7FFFFFFF and seq > 0x7FFFFFFF:
+            self.sne_flag = 0
+        # save the current SEQ for the next time through the code
+        self.prev_seq = seq
+
+        return sne
+
+
+def tcp_seq_before(a, b) -> bool:
+    return ((a - b) & 0xFFFFFFFF) > 0x80000000
+
+
+def tcp_seq_after(a, b) -> bool:
+    return tcp_seq_before(a, b)
+
+
+class SequenceNumberExtenderLinux:
+    """Based on linux implementation and with no extra flags"""
+
+    sne: int = 0
+    prev_seq: int = 0
+
+    def reset(self, seq, sne=0):
+        self.prev_seq = seq
+        self.sne = sne
+
+    def calc(self, seq, update=True):
+        sne = self.sne
+        if tcp_seq_before(seq, self.prev_seq):
+            if seq > self.prev_seq:
+                sne -= 1
+        else:
+            if seq < self.prev_seq:
+                sne += 1
+        if update and tcp_seq_before(self.prev_seq, seq):
+            self.prev_seq = seq
+            self.sne = sne
+        return sne
tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sne_alg.py new file +96 -0

96 lines not on this page · raw message

Signature · 1 line
2.25.1
#39 lore raw

PATCH v2 16/25 selftests: tcp_authopt: Add scapy-based packet signing code

In reply to Leonard Crestez

Tools like tcpdump and wireshark can parse the TCP Authentication Option but there is not yet support to verify correct signatures.

This patch implements TCP-AO signature verification using scapy and the python cryptography package.

The python code is verified itself with a subset of IETF test vectors from this page:
https://datatracker.ietf.org/doc/html/draft-touch-tcpm-ao-test-vectors-02

The code in this commit is not specific to linux

---
 .../tcp_authopt_test/scapy_tcp_authopt.py     | 220 +++++++++++
 .../tcp_authopt_test/scapy_utils.py           | 177 +++++++++
 .../tcp_authopt_test/test_vectors.py          | 365 ++++++++++++++++++
 .../tcp_authopt/tcp_authopt_test/validator.py | 138 +++++++
 4 files changed, 900 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_tcp_authopt.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_utils.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_vectors.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/validator.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_tcp_authopt.py new file +220 -0
new file mode 100644
index 000000000000..ce36321b803a
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: GPL-2.0
+"""Packet-processing utilities implementing RFC5925 and RFC2926"""
+
+import hmac
+import logging
+import struct
+
+from scapy.layers.inet import TCP
+from scapy.packet import Packet
+
+from .scapy_utils import (
+    TCPOPT_AUTHOPT,
+    IPvXAddress,
+    get_packet_ipvx_dst,
+    get_packet_ipvx_src,
+    get_tcp_doff,
+    get_tcp_pseudoheader,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _cmac_aes_digest(key: bytes, msg: bytes) -> bytes:
+    from cryptography.hazmat.backends import default_backend
+    from cryptography.hazmat.primitives import cmac
+    from cryptography.hazmat.primitives.ciphers import algorithms
+
+    backend = default_backend()
+    c = cmac.CMAC(algorithms.AES(key), backend=backend)
+    c.update(bytes(msg))
+    return c.finalize()
+
+
+class TcpAuthOptAlg:
+    @classmethod
+    def kdf(cls, master_key: bytes, context: bytes) -> bytes:
+        raise NotImplementedError()
+
+    @classmethod
+    def mac(cls, traffic_key: bytes, message: bytes) -> bytes:
+        raise NotImplementedError()
+
+    maclen = -1
+
+
+class TcpAuthOptAlg_HMAC_SHA1(TcpAuthOptAlg):
+    @classmethod
+    def kdf(cls, master_key: bytes, context: bytes) -> bytes:
+        input = b"\x01" + b"TCP-AO" + context + b"\x00\xa0"
+        return hmac.digest(master_key, input, "SHA1")
+
+    @classmethod
+    def mac(cls, traffic_key: bytes, message: bytes) -> bytes:
+        return hmac.digest(traffic_key, message, "SHA1")[:12]
+
+    maclen = 12
+
+
+class TcpAuthOptAlg_CMAC_AES(TcpAuthOptAlg):
+    @classmethod
+    def kdf(self, master_key: bytes, context: bytes) -> bytes:
+        if len(master_key) == 16:
+            key = master_key
+        else:
+            key = _cmac_aes_digest(b"\x00" * 16, master_key)
+        return _cmac_aes_digest(key, b"\x01" + b"TCP-AO" + context + b"\x00\x80")
+
+    @classmethod
+    def mac(self, traffic_key: bytes, message: bytes) -> bytes:
+        return _cmac_aes_digest(traffic_key, message)[:12]
+
+    maclen = 12
+
+
+def get_alg(name: str) -> TcpAuthOptAlg:
+    if name.upper() == "HMAC-SHA-1-96":
+        return TcpAuthOptAlg_HMAC_SHA1()
+    elif name.upper() == "AES-128-CMAC-96":
+        return TcpAuthOptAlg_CMAC_AES()
+    else:
+        raise ValueError(f"Bad TCP AuthOpt algorithms {name}")
+
+
+def build_context(
+    saddr: IPvXAddress, daddr: IPvXAddress, sport, dport, src_isn, dst_isn
+) -> bytes:
+    """Build context bytes as specified by RFC5925 section 5.2"""
+    return (
+        saddr.packed
+        + daddr.packed
+        + struct.pack(
+            "!HHII",
+            sport,
+            dport,
+            src_isn,
+            dst_isn,
+        )
+    )
+
+
+def build_context_from_packet(p: Packet, src_isn: int, dst_isn: int) -> bytes:
+    """Build context based on a scapy Packet and src/dst initial-sequence numbers"""
+    return build_context(
+        get_packet_ipvx_src(p),
+        get_packet_ipvx_dst(p),
+        p[TCP].sport,
+        p[TCP].dport,
+        src_isn,
+        dst_isn,
+    )
+
+
+def build_message_from_packet(p: Packet, include_options=True, sne=0) -> bytearray:
+    """Build message bytes as described by RFC5925 section 5.1"""
+    result = bytearray()
+    result += struct.pack("!I", sne)
+    th = p[TCP]
+
+    # ip pseudo-header:
+    result += get_tcp_pseudoheader(th)
+
+    # tcp header with checksum set to zero
+    th_bytes = bytes(p[TCP])
+    result += th_bytes[:16]
+    result += b"\x00\x00"
+    result += th_bytes[18:20]
+
+    # Even if include_options=False the TCP-AO option itself is still included
+    # with the MAC set to all-zeros. This means we need to parse TCP options.
+    pos = 20
+    tcphdr_optend = get_tcp_doff(th) * 4
+    # logger.info("th_bytes: %s", th_bytes.hex(' '))
+    assert len(th_bytes) >= tcphdr_optend
+    while pos < tcphdr_optend:
+        optnum = th_bytes[pos]
+        pos += 1
+        if optnum == 0 or optnum == 1:
+            if include_options:
+                result += bytes([optnum])
+            continue
+
+        optlen = th_bytes[pos]
+        pos += 1
+        if pos + optlen - 2 > tcphdr_optend:
+            logger.info(
+                "bad tcp option %d optlen %d beyond end-of-header", optnum, optlen
+            )
+            break
+        if optlen < 2:
+            logger.info("bad tcp option %d optlen %d less than two", optnum, optlen)
+            break
+        if optnum == TCPOPT_AUTHOPT:
+            if optlen < 4:
+                logger.info("bad tcp option %d optlen %d", optnum, optlen)
+                break
+            result += bytes([optnum, optlen])
+            result += th_bytes[pos : pos + 2]
+            result += (optlen - 4) * b"\x00"
+        elif include_options:
+            result += bytes([optnum, optlen])
+            result += th_bytes[pos : pos + optlen - 2]
+        pos += optlen - 2
+    result += bytes(p[TCP].payload)
+    return result
+
+
+def check_tcp_authopt_signature(
+    p: Packet, alg: TcpAuthOptAlg, master_key, sisn, disn, include_options=True, sne=0
+):
+    from .scapy_utils import scapy_tcp_get_authopt_val
+
+    ao = scapy_tcp_get_authopt_val(p[TCP])
+    if ao is None:
+        return None
+
+    context_bytes = build_context_from_packet(p, sisn, disn)
+    traffic_key = alg.kdf(master_key, context_bytes)
+    message_bytes = build_message_from_packet(
+        p, include_options=include_options, sne=sne
+    )
+    mac = alg.mac(traffic_key, message_bytes)
+    return mac == ao.mac
+
+
+def add_tcp_authopt_signature(
+    p: Packet,
+    alg: TcpAuthOptAlg,
+    master_key,
+    sisn,
+    disn,
+    keyid=0,
+    rnextkeyid=0,
+    include_options=True,
+    sne=0,
+):
+    """Sign a packet"""
+    th = p[TCP]
+    keyids = struct.pack("BB", keyid, rnextkeyid)
+    th.options = th.options + [(TCPOPT_AUTHOPT, keyids + alg.maclen * b"\x00")]
+
+    context_bytes = build_context_from_packet(p, sisn, disn)
+    traffic_key = alg.kdf(master_key, context_bytes)
+    message_bytes = build_message_from_packet(
+        p, include_options=include_options, sne=sne
+    )
+    mac = alg.mac(traffic_key, message_bytes)
+    th.options[-1] = (TCPOPT_AUTHOPT, keyids + mac)
+
+
+def break_tcp_authopt_signature(packet: Packet):
+    """Invalidate TCP-AO signature inside a packet
+
+    The packet must already be signed and it gets modified in-place.
+    """
+    opt = packet[TCP].options[-1]
+    if opt[0] != TCPOPT_AUTHOPT:
+        raise ValueError("TCP option list must end with TCP_AUTHOPT")
+    opt_mac = bytearray(opt[1])
+    opt_mac[-1] ^= 0xFF
+    packet[TCP].options[-1] = (opt[0], bytes(opt_mac))
tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_utils.py new file +177 -0

177 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_vectors.py new file +365 -0

365 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/validator.py new file +138 -0

138 lines not on this page · raw message

Signature · 1 line
2.25.1
#40 lore raw

PATCH v2 17/25 selftests: tcp_authopt: Add packet-level tests

In reply to Leonard Crestez

This patch validates that the TCP-AO signatures inserted by linux are correct in all algorithm permutations, using scapy.

It also tests that TCP-AO behaves correctly in a number of corner cases such as:

* reset handling
* timewait
* syn-recv
* ipv4-mapped ipv6
* interaction with tcp-md5

This reverts commit 297a301a4f1c3abe41d554a9f6df192257a017b8.

---
 .../full_tcp_sniff_session.py                 |  91 +++
 .../tcp_authopt_test/linux_tcp_md5sig.py      | 110 ++++
 .../tcp_authopt_test/scapy_conntrack.py       | 173 ++++++
 .../tcp_connection_fixture.py                 | 276 +++++++++
 .../tcp_authopt_test/test_verify_capture.py   | 559 ++++++++++++++++++
 5 files changed, 1209 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/full_tcp_sniff_session.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_md5sig.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_conntrack.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/tcp_connection_fixture.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_verify_capture.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/full_tcp_sniff_session.py new file +91 -0
new file mode 100644
index 000000000000..d37f7d947bcd
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: GPL-2.0
+import logging
+import threading
+import typing
+
+import scapy.sessions
+from scapy.packet import Packet
+
+from .scapy_conntrack import TCPConnectionInfo, TCPConnectionTracker
+
+logger = logging.getLogger(__name__)
+
+
+class FullTCPSniffSession(scapy.sessions.DefaultSession):
+    """Implementation of a scapy sniff session that can wait for a full TCP capture
+
+    Allows another thread to wait for a complete FIN handshake without polling or sleep.
+    """
+
+    #: Server port used to identify client and server
+    server_port: int
+    #: Connection tracker
+    tracker: TCPConnectionTracker
+
+    def __init__(self, server_port, **kw):
+        super().__init__(**kw)
+        self.server_port = server_port
+        self.tracker = TCPConnectionTracker()
+        self._close_event = threading.Event()
+        self._init_isn_event = threading.Event()
+        self._client_info = None
+        self._server_info = None
+
+    @property
+    def client_info(self) -> TCPConnectionInfo:
+        if not self._client_info:
+            self._client_info = self.tracker.match_one(dport=self.server_port)
+        return self._client_info
+
+    @property
+    def server_info(self) -> TCPConnectionInfo:
+        if not self._server_info:
+            self._server_info = self.tracker.match_one(sport=self.server_port)
+        return self._server_info
+
+    @property
+    def client_isn(self):
+        return self.client_info.sisn
+
+    @property
+    def server_isn(self):
+        return self.server_info.sisn
+
+    def on_packet_received(self, p: Packet):
+        super().on_packet_received(p)
+        self.tracker.handle_packet(p)
+
+        # check events:
+        if self.client_info.sisn is not None and self.client_info.disn is not None:
+            assert (
+                self.client_info.sisn == self.server_info.disn
+                and self.server_info.sisn == self.client_info.disn
+            )
+            self._init_isn_event.set()
+        if self.client_info.found_recv_finack and self.server_info.found_recv_finack:
+            self._close_event.set()
+
+    def reset(self):
+        """Reset known information about client/server"""
+        self.tracker.reset()
+        self._server_info = None
+        self._client_info = None
+        self._close_event.clear()
+        self._init_isn_event.clear()
+
+    def wait_close(self, timeout=10):
+        """Wait for a graceful close with FINs acked by both side"""
+        self._close_event.wait(timeout=timeout)
+        if not self._close_event.is_set():
+            raise TimeoutError("Timed out waiting for graceful close")
+
+    def wait_init_isn(self, timeout=10):
+        """Wait for both client_isn and server_isn to be determined"""
+        self._init_isn_event.wait(timeout=timeout)
+        if not self._init_isn_event.is_set():
+            raise TimeoutError("Timed out waiting for Initial Sequence Numbers")
+
+    def get_client_server_isn(self, timeout=10) -> typing.Tuple[int, int]:
+        """Return client/server ISN, blocking until they are captured"""
+        self.wait_init_isn(timeout=timeout)
+        return self.client_isn, self.server_isn
tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_md5sig.py new file +110 -0

110 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/scapy_conntrack.py new file +173 -0

173 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/tcp_connection_fixture.py new file +276 -0

276 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_verify_capture.py new file +559 -0

559 lines not on this page · raw message

Signature · 1 line
2.25.1
#41 lore raw

PATCH v2 18/25 selftests: tcp_authopt: Initial sne test

In reply to Leonard Crestez

In order to trigger a seq or ack rollover create many connection in a loop and check for a "high" value, then make a lot of traffic.

This relies on both TCP_REPAIR and TCP_REPAIR_AUTHOPT, making it unfit for upstream.

---
 .../tcp_authopt_test/linux_tcp_repair.py      |  67 ++++++
 .../tcp_authopt/tcp_authopt_test/test_sne.py  | 202 ++++++++++++++++++
 2 files changed, 269 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_repair.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sne.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_repair.py new file +67 -0
new file mode 100644
index 000000000000..68f111a207e6
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0
+import socket
+import struct
+from contextlib import contextmanager
+from enum import IntEnum
+
+# Extra sockopts not present in python stdlib
+TCP_REPAIR = 19
+TCP_REPAIR_QUEUE = 20
+TCP_QUEUE_SEQ = 21
+TCP_REPAIR_OPTIONS = 22
+TCP_REPAIR_WINDOW = 29
+
+
+class TCP_REPAIR_VAL(IntEnum):
+    OFF = 0
+    ON = 1
+    OFF_NO_WP = -1
+
+
+def get_tcp_repair(sock) -> TCP_REPAIR_VAL:
+    return TCP_REPAIR_VAL(sock.getsockopt(socket.SOL_TCP, TCP_REPAIR))
+
+
+def set_tcp_repair(sock, val: TCP_REPAIR_VAL) -> None:
+    return sock.setsockopt(socket.SOL_TCP, TCP_REPAIR, int(val))
+
+
+class TCP_REPAIR_QUEUE_ID(IntEnum):
+    NO_QUEUE = 0
+    RECV_QUEUE = 1
+    SEND_QUEUE = 2
+
+
+def get_tcp_repair_queue(sock) -> TCP_REPAIR_QUEUE_ID:
+    return TCP_REPAIR_QUEUE_ID(sock.getsockopt(socket.SOL_TCP, TCP_REPAIR_QUEUE))
+
+
+def set_tcp_repair_queue(sock, val: TCP_REPAIR_QUEUE_ID) -> None:
+    return sock.setsockopt(socket.SOL_TCP, TCP_REPAIR_QUEUE, int(val))
+
+
+def get_tcp_queue_seq(sock) -> int:
+    return struct.unpack("I", sock.getsockopt(socket.SOL_TCP, TCP_QUEUE_SEQ, 4))[0]
+
+
+def set_tcp_queue_seq(sock, val: int) -> None:
+    return sock.setsockopt(socket.SOL_TCP, TCP_QUEUE_SEQ, val)[0]
+
+
+@contextmanager
+def tcp_repair_toggle(sock, off_val=TCP_REPAIR_VAL.OFF_NO_WP):
+    """Set TCP_REPAIR on/off as a context"""
+    try:
+        set_tcp_repair(sock, TCP_REPAIR_VAL.ON)
+        yield
+    finally:
+        set_tcp_repair(sock, off_val)
+
+
+def get_tcp_repair_recv_send_queue_seq(sock):
+    with tcp_repair_toggle(sock):
+        set_tcp_repair_queue(sock, TCP_REPAIR_QUEUE_ID.RECV_QUEUE)
+        recv_seq = get_tcp_queue_seq(sock)
+        set_tcp_repair_queue(sock, TCP_REPAIR_QUEUE_ID.SEND_QUEUE)
+        send_seq = get_tcp_queue_seq(sock)
+        return (recv_seq, send_seq)
tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_sne.py new file +202 -0

202 lines not on this page · raw message

Signature · 1 line
2.25.1
#42 lore raw

PATCH v2 19/25 tcp: authopt: Add key selection controls

In reply to Leonard Crestez

The RFC requires that TCP can report the keyid and rnextkeyid values being sent or received, implement this via getsockopt values.

The RFC also requires that user can select the sending key and that the sending key is automatically switched based on rnextkeyid. These requirements can conflict so we implement both and add a flag which specifies if user or peer request takes priority.

Also add an option to control rnextkeyid explicitly from userspace.

---
 Documentation/networking/tcp_authopt.rst | 25 ++++++
 include/net/tcp_authopt.h                | 38 ++++++++-
 include/uapi/linux/tcp.h                 | 31 ++++++++
 net/ipv4/tcp_authopt.c                   | 98 +++++++++++++++++++++++-
 net/ipv4/tcp_ipv4.c                      |  2 +-
 net/ipv6/tcp_ipv6.c                      |  3 +-
 6 files changed, 190 insertions(+), 7 deletions(-)
Documentation/networking/tcp_authopt.rst +25 -0
index 484f66f41ad5..cded87a70d05 100644
@@ -35,10 +35,35 @@ Keys can be bound to remote addresses in a way that is similar to TCP_MD5.
 
 RFC5925 requires that key ids do not overlap when tcp identifiers (addr/port)
 overlap. This is not enforced by linux, configuring ambiguous keys will result
 in packet drops and lost connections.
 
+Key selection
+-------------
+
+On getsockopt(TCP_AUTHOPT) information is provided about keyid/rnextkeyid in
+the last send packet and about the keyid/rnextkeyd in the last valid received
+packet.
+
+By default the sending keyid is selected to match the "rnextkeyid" value sent
+by the remote side. If that keyid is not available (or for new connections) a
+random matching key is selected.
+
+If the `TCP_AUTHOPT_LOCK_KEYID` is set then the sending key is selected by the
+`tcp_authopt.send_local_id` field and rnextkeyid is ignored. If no key with
+local_id == send_local_id is configured then a random matching key is
+selected.
+
+The current sending key is cached in the socket and will not change unless
+requested by remote rnextkeyid or by setsockopt.
+
+The rnextkeyid value sent on the wire is usually the recv_id of the current
+key used for sending. If the TCP_AUTHOPT_LOCK_RNEXTKEY flag is set in
+`tcp_authopt.flags` the value of `tcp_authopt.send_rnextkeyid` is send
+instead.  This can be used to implement smooth rollover: the peer will switch
+its keyid to the received rnextkeyid when it is available.
+
 ABI Reference
 =============
 
 .. kernel-doc:: include/uapi/linux/tcp.h
    :identifiers: tcp_authopt tcp_authopt_flag tcp_authopt_key tcp_authopt_key_flag tcp_authopt_alg
include/net/tcp_authopt.h +36 -2
index ae7d6a1eab8d..9341e10ef542 100644
@@ -66,10 +66,43 @@ struct tcp_authopt_info {
 	u32 dst_isn;
 	/** @rcv_sne: Recv-side Sequence Number Extension tracking tcp_sock.rcv_nxt */
 	u32 rcv_sne;
 	/** @snd_sne: Send-side Sequence Number Extension tracking tcp_sock.snd_nxt */
 	u32 snd_sne;
+
+	/**
+	 * @send_keyid: keyid currently being sent
+	 *
+	 * This is controlled by userspace by userspace if
+	 * TCP_AUTHOPT_FLAG_LOCK_KEYID, otherwise we try to match recv_rnextkeyid
+	 */
+	u8 send_keyid;
+	/**
+	 * @send_rnextkeyid: rnextkeyid currently being sent
+	 *
+	 * This is controlled by userspace if TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID is set
+	 */
+	u8 send_rnextkeyid;
+	/**
+	 * @recv_keyid: last keyid received from remote
+	 *
+	 * This is reported to userspace but has no other special behavior attached.
+	 */
+	u8 recv_keyid;
+	/**
+	 * @recv_rnextkeyid: last rnextkeyid received from remote
+	 *
+	 * Linux tries to honor this unless TCP_AUTHOPT_FLAG_LOCK_KEYID is set
+	 */
+	u8 recv_rnextkeyid;
+
+	/**
+	 * @send_key: Current key used for sending, cached.
+	 *
+	 * Once a key is found it only changes by user or remote request.
+	 */
+	struct tcp_authopt_key_info *send_key;
 };
 
 #ifdef CONFIG_TCP_AUTHOPT
 extern int sysctl_tcp_authopt;
 DECLARE_STATIC_KEY_FALSE(tcp_authopt_needed);
@@ -81,22 +114,23 @@ int tcp_get_authopt_val(struct sock *sk, struct tcp_authopt *key);
 int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen);
 struct tcp_authopt_key_info *__tcp_authopt_select_key(
 		const struct sock *sk,
 		struct tcp_authopt_info *info,
 		const struct sock *addr_sk,
-		u8 *rnextkeyid);
+		u8 *rnextkeyid,
+		bool locked);
 static inline struct tcp_authopt_key_info *tcp_authopt_select_key(
 		const struct sock *sk,
 		const struct sock *addr_sk,
 		struct tcp_authopt_info **info,
 		u8 *rnextkeyid)
 {
 	if (static_branch_unlikely(&tcp_authopt_needed)) {
 		*info = rcu_dereference(tcp_sk(sk)->authopt_info);
 
 		if (*info)
-			return __tcp_authopt_select_key(sk, *info, addr_sk, rnextkeyid);
+			return __tcp_authopt_select_key(sk, *info, addr_sk, rnextkeyid, true);
 	}
 	return NULL;
 }
 int tcp_authopt_hash(
 		char *hash_location,
include/uapi/linux/tcp.h +31 -0
index 76d7be6b27f4..e02176390519 100644
@@ -346,10 +346,24 @@ struct tcp_diag_md5sig {
 
 /**
  * enum tcp_authopt_flag - flags for `tcp_authopt.flags`
  */
 enum tcp_authopt_flag {
+	/**
+	 * @TCP_AUTHOPT_FLAG_LOCK_KEYID: keyid controlled by sockopt
+	 *
+	 * If this is set `tcp_authopt.send_keyid` is used to determined sending
+	 * key. Otherwise a key with send_id == recv_rnextkeyid is preferred.
+	 */
+	TCP_AUTHOPT_FLAG_LOCK_KEYID = (1 << 0),
+	/**
+	 * @TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID: Override rnextkeyid from userspace
+	 *
+	 * If this is set then `tcp_authopt.send_rnextkeyid` is sent on outbound
+	 * packets. Other the recv_id of the current sending key is sent.
+	 */
+	TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID = (1 << 1),
 	/**
 	 * @TCP_AUTHOPT_FLAG_REJECT_UNEXPECTED:
 	 *	Configure behavior of segments with TCP-AO coming from hosts for which no
 	 *	key is configured. The default recommended by RFC is to silently accept
 	 *	such connections.
@@ -361,10 +375,27 @@ enum tcp_authopt_flag {
  * struct tcp_authopt - Per-socket options related to TCP Authentication Option
  */
 struct tcp_authopt {
 	/** @flags: Combination of &enum tcp_authopt_flag */
 	__u32	flags;
+	/**
+	 * @send_keyid: `tcp_authopt_key.send_id` of preferred send key
+	 *
+	 * This is only used if `TCP_AUTHOPT_FLAG_LOCK_KEYID` is set.
+	 */
+	__u8	send_keyid;
+	/**
+	 * @send_rnextkeyid: The rnextkeyid to send in packets
+	 *
+	 * This is controlled by the user iff TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID is
+	 * set. Otherwise rnextkeyid is the recv_id of the current key.
+	 */
+	__u8	send_rnextkeyid;
+	/** @recv_keyid: A recently-received keyid value. Only for getsockopt. */
+	__u8	recv_keyid;
+	/** @recv_rnextkeyid: A recently-received rnextkeyid value. Only for getsockopt. */
+	__u8	recv_rnextkeyid;
 };
 
 /**
  * enum tcp_authopt_key_flag - flags for `tcp_authopt.flags`
  *
net/ipv4/tcp_authopt.c +95 -3

182 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +1 -1
index 21971f5fa40e..2165b95ff7ed 100644
@@ -664,11 +664,11 @@ static int tcp_v4_authopt_handle_reply(const struct sock *sk,
 		info = tcp_twsk(sk)->tw_authopt_info;
 	else
 		info = tcp_sk(sk)->authopt_info;
 	if (!info)
 		return 0;
-	key_info = __tcp_authopt_select_key(sk, info, sk, &rnextkeyid);
+	key_info = __tcp_authopt_select_key(sk, info, sk, &rnextkeyid, false);
 	if (!key_info)
 		return 0;
 	*optptr = htonl((TCPOPT_AUTHOPT << 24) |
 			(TCPOLEN_AUTHOPT_OUTPUT << 16) |
 			(key_info->send_id << 8) |
net/ipv6/tcp_ipv6.c +2 -1
index 68f9545e4347..bb21f11f4246 100644
@@ -920,11 +920,12 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
 		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);
+								    &authopt_rnextkeyid,
+								    false);
 			if (authopt_key_info) {
 				tot_len += TCPOLEN_AUTHOPT_OUTPUT;
 				/* Don't use MD5 */
 				key = NULL;
 			}
Signature · 1 line
2.25.1
#43 lore raw

PATCH v2 20/25 selftests: tcp_authopt: Add tests for rollover

In reply to Leonard Crestez

RFC5925 requires that the use can examine or control the keys being used. This is implemented in linux via fields on the TCP_AUTHOPT sockopt.

Add socket-level tests for the adjusting keyids on live connections and checking the they are reflected on the peer.

Also check smooth transitions via rnextkeyid.

---
 .../tcp_authopt_test/linux_tcp_authopt.py     |  16 +-
 .../tcp_authopt_test/test_rollover.py         | 181 ++++++++++++++++++
 2 files changed, 194 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_rollover.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_authopt.py +13 -3
index b9dc9decda07..75cf5f993ccb 100644
@@ -29,10 +29,12 @@ TCP_AUTHOPT_KEY = 39
 
 TCP_AUTHOPT_MAXKEYLEN = 80
 
 
 class TCP_AUTHOPT_FLAG(IntFlag):
+    LOCK_KEYID = BIT(0)
+    LOCK_RNEXTKEYID = BIT(1)
     REJECT_UNEXPECTED = BIT(2)
 
 
 class TCP_AUTHOPT_KEY_FLAG(IntFlag):
     DEL = BIT(0)
@@ -48,24 +50,32 @@ class TCP_AUTHOPT_ALG(IntEnum):
 @dataclass
 class tcp_authopt:
     """Like linux struct tcp_authopt"""
 
     flags: int = 0
-    sizeof = 4
+    send_keyid: int = 0
+    send_rnextkeyid: int = 0
+    recv_keyid: int = 0
+    recv_rnextkeyid: int = 0
+    sizeof = 8
 
     def pack(self) -> bytes:
         return struct.pack(
-            "I",
+            "IBBBB",
             self.flags,
+            self.send_keyid,
+            self.send_rnextkeyid,
+            self.recv_keyid,
+            self.recv_rnextkeyid,
         )
 
     def __bytes__(self):
         return self.pack()
 
     @classmethod
     def unpack(cls, b: bytes):
-        tup = struct.unpack("I", b)
+        tup = struct.unpack("IBBBB", b)
         return cls(*tup)
 
 
 def set_tcp_authopt(sock, opt: tcp_authopt):
     return sock.setsockopt(socket.SOL_TCP, TCP_AUTHOPT, bytes(opt))
tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_rollover.py new file +181 -0

181 lines not on this page · raw message

Signature · 1 line
2.25.1
#44 lore raw

PATCH v2 21/25 tcp: authopt: Add initial l3index support

In reply to Leonard Crestez

This is a parallel feature to tcp_md5sig.tcpm_ifindex support and allows applications to server multiple VRFs with a single socket.

The ifindex argument must be the ifindex of a VRF device and must match exactly, keys with ifindex == 0 (outside of VRF) will not match for connections inside a VRF.

Keys without the TCP_AUTHOPT_KEY_IFINDEX will ignore ifindex and match both inside and outside VRF.

---
 include/net/tcp_authopt.h |  2 ++
 include/uapi/linux/tcp.h  | 11 ++++++
 net/ipv4/tcp_authopt.c    | 76 +++++++++++++++++++++++++++++++++++----
 3 files changed, 82 insertions(+), 7 deletions(-)
include/net/tcp_authopt.h +2 -0
index 9341e10ef542..072d5383f14b 100644
@@ -39,10 +39,12 @@ struct tcp_authopt_key_info {
 	u8 alg_id;
 	/** @keylen: Same as &tcp_authopt_key.keylen */
 	u8 keylen;
 	/** @key: Same as &tcp_authopt_key.key */
 	u8 key[TCP_AUTHOPT_MAXKEYLEN];
+	/** @l3index: Same as &tcp_authopt_key.ifindex */
+	int l3index;
 	/** @addr: Same as &tcp_authopt_key.addr */
 	struct sockaddr_storage addr;
 	/** @alg: Algorithm implementation matching alg_id */
 	struct tcp_authopt_alg_imp *alg;
 };
include/uapi/linux/tcp.h +11 -0
index e02176390519..a7f5f918ed5a 100644
@@ -400,15 +400,17 @@ struct tcp_authopt {
  * enum tcp_authopt_key_flag - flags for `tcp_authopt.flags`
  *
  * @TCP_AUTHOPT_KEY_DEL: Delete the key and ignore non-id fields
  * @TCP_AUTHOPT_KEY_EXCLUDE_OPTS: Exclude TCP options from signature
  * @TCP_AUTHOPT_KEY_ADDR_BIND: Key only valid for `tcp_authopt.addr`
+ * @TCP_AUTHOPT_KEY_IFINDEX: Key only valid for `tcp_authopt.ifindex`
  */
 enum tcp_authopt_key_flag {
 	TCP_AUTHOPT_KEY_DEL = (1 << 0),
 	TCP_AUTHOPT_KEY_EXCLUDE_OPTS = (1 << 1),
 	TCP_AUTHOPT_KEY_ADDR_BIND = (1 << 2),
+	TCP_AUTHOPT_KEY_IFINDEX = (1 << 3),
 };
 
 /**
  * enum tcp_authopt_alg - Algorithms for TCP Authentication Option
  */
@@ -450,10 +452,19 @@ struct tcp_authopt_key {
 	 * @addr: Key is only valid for this address
 	 *
 	 * Ignored unless TCP_AUTHOPT_KEY_ADDR_BIND flag is set
 	 */
 	struct __kernel_sockaddr_storage addr;
+	/**
+	 * @ifindex: ifindex of vrf (l3mdev_master) interface
+	 *
+	 * If the TCP_AUTHOPT_KEY_IFINDEX flag is set then key only applies for
+	 * connections through this interface. Interface must be an vrf master.
+	 *
+	 * This is similar to `tcp_msg5sig.tcpm_ifindex`
+	 */
+	int	ifindex;
 };
 
 /* setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) */
 
 #define TCP_RECEIVE_ZEROCOPY_FLAG_TLB_CLEAN_HINT 0x1
net/ipv4/tcp_authopt.c +69 -7

171 lines not on this page · raw message

Signature · 1 line
2.25.1
#45 lore raw

Re: [PATCH v2 21/25] tcp: authopt: Add initial l3index support

Quoted 5 lines · Leonard Crestez
net/ipv4/tcp_authopt.c +0 -0
@@ -584,10 +614,24 @@ int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
 		return -EINVAL;
 	err = tcp_authopt_alg_require(alg);
 	if (err)
 		return err;

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 627

Quoted 10 lines
net/ipv4/tcp_authopt.c +9 -0
 
+	/* check ifindex is valid (zero is always valid) */
+	if (opt.flags & TCP_AUTHOPT_KEY_IFINDEX && opt.ifindex) {
+		struct net_device *dev;
+
+		rcu_read_lock();
+		dev = dev_get_by_index_rcu(sock_net(sk), opt.ifindex);
+		if (dev && netif_is_l3_master(dev))
+			l3index = dev->ifindex;
+		rcu_read_unlock();

rcu_read_lock()... rcu_read_unlock() can be replaced with netif_index_is_l3_master(...)

#46 lore raw

Re: [PATCH v2 21/25] tcp: authopt: Add initial l3index support

Quoted 7 lines · David Ahern
Quoted 6 lines · Leonard Crestez
net/ipv4/tcp_authopt.c +0 -0
@@ -584,10 +614,24 @@ int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen)
  		return -EINVAL;
  	err = tcp_authopt_alg_require(alg);
  	if (err)
  		return err;
 
Quoted 12 lines

net/ipv4/tcp_authopt.c int tcp_set_authopt_key(struct sock *sk, sockptr_t optval, unsigned int optlen) · line 627

Quoted 9 lines
net/ipv4/tcp_authopt.c +9 -0
+	/* check ifindex is valid (zero is always valid) */
+	if (opt.flags & TCP_AUTHOPT_KEY_IFINDEX && opt.ifindex) {
+		struct net_device *dev;
+
+		rcu_read_lock();
+		dev = dev_get_by_index_rcu(sock_net(sk), opt.ifindex);
+		if (dev && netif_is_l3_master(dev))
+			l3index = dev->ifindex;
+		rcu_read_unlock();

rcu_read_lock()... rcu_read_unlock() can be replaced with netif_index_is_l3_master(...)

Yes, this makes the code shorter.

#47 lore raw

PATCH v2 22/25 selftests: tcp_authopt: Initial tests for l3mdev handling

In reply to Leonard Crestez

These tests also verify functionality in TCP-MD5 and unsigned traffic modes. They were used to find the issue fixed in commit 86f1e3a8489f ("tcp: md5: Fix overlap between vrf and non-vrf keys")

---
 .../tcp_authopt_test/linux_tcp_authopt.py     |   9 +
 .../tcp_authopt_test/test_vrf_bind.py         | 492 ++++++++++++++++++
 .../tcp_authopt_test/vrf_netns_fixture.py     | 127 +++++
 3 files changed, 628 insertions(+)
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_vrf_bind.py
 create mode 100644 tools/testing/selftests/tcp_authopt/tcp_authopt_test/vrf_netns_fixture.py
tools/testing/selftests/tcp_authopt/tcp_authopt_test/linux_tcp_authopt.py +9 -0
index 75cf5f993ccb..2a720d49cba2 100644
@@ -38,10 +38,11 @@ class TCP_AUTHOPT_FLAG(IntFlag):
 
 class TCP_AUTHOPT_KEY_FLAG(IntFlag):
     DEL = BIT(0)
     EXCLUDE_OPTS = BIT(1)
     BIND_ADDR = BIT(2)
+    IFINDEX = BIT(3)
 
 
 class TCP_AUTHOPT_ALG(IntEnum):
     HMAC_SHA_1_96 = 1
     AES_128_CMAC_96 = 2
@@ -102,25 +103,31 @@ class tcp_authopt_key:
         recv_id: int = 0,
         alg=TCP_AUTHOPT_ALG.HMAC_SHA_1_96,
         key: KeyArgType = b"",
         addr: AddrArgType = None,
         auto_flags: bool = True,
+        ifindex: typing.Optional[int] = None,
         include_options=None,
     ):
         self.flags = flags
         self.send_id = send_id
         self.recv_id = recv_id
         self.alg = alg
         self.key = key
+        self.ifindex = ifindex
         self.addr = addr
         self.auto_flags = auto_flags
         if include_options is not None:
             self.include_options = include_options
 
     def get_real_flags(self) -> TCP_AUTHOPT_KEY_FLAG:
         result = self.flags
         if self.auto_flags:
+            if self.ifindex is not None:
+                result |= TCP_AUTHOPT_KEY_FLAG.IFINDEX
+            else:
+                result &= ~TCP_AUTHOPT_KEY_FLAG.IFINDEX
             if self.addr is not None:
                 result |= TCP_AUTHOPT_KEY_FLAG.BIND_ADDR
             else:
                 result &= ~TCP_AUTHOPT_KEY_FLAG.BIND_ADDR
         return result
@@ -136,10 +143,12 @@ class tcp_authopt_key:
             self.alg,
             len(self.key),
             self.key,
         )
         data += bytes(self.addrbuf.ljust(sockaddr_storage.sizeof, b"\x00"))
+        if self.ifindex is not None:
+            data += bytes(struct.pack("I", self.ifindex))
         return data
 
     def __bytes__(self):
         return self.pack()
 
tools/testing/selftests/tcp_authopt/tcp_authopt_test/test_vrf_bind.py new file +492 -0

492 lines not on this page · raw message

tools/testing/selftests/tcp_authopt/tcp_authopt_test/vrf_netns_fixture.py new file +127 -0

127 lines not on this page · raw message

Signature · 1 line
2.25.1
#48 lore raw

PATCH v2 23/25 selftests: nettest: Rename md5_prefix to key_addr_prefix

In reply to Leonard Crestez

This is in preparation for reusing the same option for TCP-AO

---
 tools/testing/selftests/net/nettest.c | 50 +++++++++++++--------------
 1 file changed, 25 insertions(+), 25 deletions(-)
tools/testing/selftests/net/nettest.c +25 -25
index b599003eb5ba..525a439ce3b3 100644
@@ -93,17 +93,17 @@ struct sock_args {
 	const char *clientns;
 	const char *serverns;
 
 	const char *password;
 	const char *client_pw;
-	/* prefix for MD5 password */
-	const char *md5_prefix_str;
+	/* prefix for MD5/AO*/
+	const char *key_addr_prefix_str;
 	union {
 		struct sockaddr_in v4;
 		struct sockaddr_in6 v6;
-	} md5_prefix;
-	unsigned int prefix_len;
+	} key_addr;
+	unsigned int key_addr_prefix_len;
 	/* 0: default, -1: force off, +1: force on */
 	int bind_key_ifindex;
 
 	/* expected addresses and device index for connection */
 	const char *expected_dev;
@@ -263,16 +263,16 @@ static int tcp_md5sig(int sd, void *addr, socklen_t alen, struct sock_args *args
 	int rc;
 
 	md5sig.tcpm_keylen = keylen;
 	memcpy(md5sig.tcpm_key, args->password, keylen);
 
-	if (args->prefix_len) {
+	if (args->key_addr_prefix_len) {
 		opt = TCP_MD5SIG_EXT;
 		md5sig.tcpm_flags |= TCP_MD5SIG_FLAG_PREFIX;
 
-		md5sig.tcpm_prefixlen = args->prefix_len;
-		addr = &args->md5_prefix;
+		md5sig.tcpm_prefixlen = args->key_addr_prefix_len;
+		addr = &args->key_addr;
 	}
 	memcpy(&md5sig.tcpm_addr, addr, alen);
 
 	if ((args->ifindex && args->bind_key_ifindex >= 0) || args->bind_key_ifindex >= 1) {
 		opt = TCP_MD5SIG_EXT;
@@ -308,17 +308,17 @@ static int tcp_md5_remote(int sd, struct sock_args *args)
 	int alen;
 
 	switch (args->version) {
 	case AF_INET:
 		sin.sin_port = htons(args->port);
-		sin.sin_addr = args->md5_prefix.v4.sin_addr;
+		sin.sin_addr = args->key_addr.v4.sin_addr;
 		addr = &sin;
 		alen = sizeof(sin);
 		break;
 	case AF_INET6:
 		sin6.sin6_port = htons(args->port);
-		sin6.sin6_addr = args->md5_prefix.v6.sin6_addr;
+		sin6.sin6_addr = args->key_addr.v6.sin6_addr;
 		addr = &sin6;
 		alen = sizeof(sin6);
 		break;
 	default:
 		log_error("unknown address family\n");
@@ -681,11 +681,11 @@ enum addr_type {
 	ADDR_TYPE_LOCAL,
 	ADDR_TYPE_REMOTE,
 	ADDR_TYPE_MCAST,
 	ADDR_TYPE_EXPECTED_LOCAL,
 	ADDR_TYPE_EXPECTED_REMOTE,
-	ADDR_TYPE_MD5_PREFIX,
+	ADDR_TYPE_KEY_PREFIX,
 };
 
 static int convert_addr(struct sock_args *args, const char *_str,
 			enum addr_type atype)
 {
@@ -721,32 +721,32 @@ static int convert_addr(struct sock_args *args, const char *_str,
 		break;
 	case ADDR_TYPE_EXPECTED_REMOTE:
 		desc = "expected remote";
 		addr = &args->expected_raddr;
 		break;
-	case ADDR_TYPE_MD5_PREFIX:
-		desc = "md5 prefix";
+	case ADDR_TYPE_KEY_PREFIX:
+		desc = "key addr prefix";
 		if (family == AF_INET) {
-			args->md5_prefix.v4.sin_family = AF_INET;
-			addr = &args->md5_prefix.v4.sin_addr;
+			args->key_addr.v4.sin_family = AF_INET;
+			addr = &args->key_addr.v4.sin_addr;
 		} else if (family == AF_INET6) {
-			args->md5_prefix.v6.sin6_family = AF_INET6;
-			addr = &args->md5_prefix.v6.sin6_addr;
+			args->key_addr.v6.sin6_family = AF_INET6;
+			addr = &args->key_addr.v6.sin6_addr;
 		} else
 			return 1;
 
 		sep = strchr(str, '/');
 		if (sep) {
 			*sep = '\0';
 			sep++;
 			if (str_to_uint(sep, 1, pfx_len_max,
-					&args->prefix_len) != 0) {
-				fprintf(stderr, "Invalid port\n");
+					&args->key_addr_prefix_len) != 0) {
+				fprintf(stderr, "Invalid prefix\n");
 				return 1;
 			}
 		} else {
-			args->prefix_len = 0;
+			args->key_addr_prefix_len = 0;
 		}
 		break;
 	default:
 		log_error("unknown address type\n");
 		exit(1);
@@ -811,13 +811,13 @@ static int validate_addresses(struct sock_args *args)
 
 	if (args->remote_addr_str &&
 	    convert_addr(args, args->remote_addr_str, ADDR_TYPE_REMOTE) < 0)
 		return 1;
 
-	if (args->md5_prefix_str &&
-	    convert_addr(args, args->md5_prefix_str,
-			 ADDR_TYPE_MD5_PREFIX) < 0)
+	if (args->key_addr_prefix_str &&
+	    convert_addr(args, args->key_addr_prefix_str,
+			 ADDR_TYPE_KEY_PREFIX) < 0)
 		return 1;
 
 	if (args->expected_laddr_str &&
 	    convert_addr(args, args->expected_laddr_str,
 			 ADDR_TYPE_EXPECTED_LOCAL))
@@ -1992,11 +1992,11 @@ int main(int argc, char *argv[])
 			break;
 		case 'X':
 			args.client_pw = optarg;
 			break;
 		case 'm':
-			args.md5_prefix_str = optarg;
+			args.key_addr_prefix_str = optarg;
 			break;
 		case 'S':
 			args.use_setsockopt = 1;
 			break;
 		case 'C':
@@ -2048,17 +2048,17 @@ int main(int argc, char *argv[])
 			return 1;
 		}
 	}
 
 	if (args.password &&
-	    ((!args.has_remote_ip && !args.md5_prefix_str) ||
+	    ((!args.has_remote_ip && !args.key_addr_prefix_str) ||
 	      args.type != SOCK_STREAM)) {
 		log_error("MD5 passwords apply to TCP only and require a remote ip for the password\n");
 		return 1;
 	}
 
-	if (args.md5_prefix_str && !args.password) {
+	if (args.key_addr_prefix_str && !args.password) {
 		log_error("Prefix range for MD5 protection specified without a password\n");
 		return 1;
 	}
 
 	if (iter == 0) {
Signature · 1 line
2.25.1
Reviewed-by on v2 23/25
#49 lore raw

Re: [PATCH v2 23/25] selftests: nettest: Rename md5_prefix to key_addr_prefix

Quoted 7 lines · Leonard Crestez

This is in preparation for reusing the same option for TCP-AO

---
 tools/testing/selftests/net/nettest.c | 50 +++++++++++++--------------
 1 file changed, 25 insertions(+), 25 deletions(-)
#50 lore raw

PATCH v2 24/25 selftests: nettest: Initial tcp_authopt support

In reply to Leonard Crestez

Add support for configuring TCP Authentication Option. Only a single key is supported with default options.

---
 tools/testing/selftests/net/nettest.c | 75 ++++++++++++++++++++++++---
 1 file changed, 69 insertions(+), 6 deletions(-)
tools/testing/selftests/net/nettest.c +69 -6
index 525a439ce3b3..837a45921845 100644
@@ -103,10 +103,12 @@ struct sock_args {
 	} key_addr;
 	unsigned int key_addr_prefix_len;
 	/* 0: default, -1: force off, +1: force on */
 	int bind_key_ifindex;
 
+	const char *authopt_password;
+
 	/* expected addresses and device index for connection */
 	const char *expected_dev;
 	const char *expected_server_dev;
 	int expected_ifindex;
 
@@ -253,10 +255,54 @@ static int switch_ns(const char *ns)
 	close(fd);
 
 	return ret;
 }
 
+static int tcp_set_authopt(int sd, struct sock_args *args)
+{
+	struct tcp_authopt_key key;
+	int rc;
+
+	memset(&key, 0, sizeof(key));
+	strcpy((char *)key.key, args->authopt_password);
+	key.keylen = strlen(args->authopt_password);
+	key.alg = TCP_AUTHOPT_ALG_HMAC_SHA_1_96;
+
+	if (args->key_addr_prefix_str) {
+		key.flags |= TCP_AUTHOPT_KEY_ADDR_BIND;
+		switch (args->version) {
+		case AF_INET:
+			memcpy(&key.addr, &args->key_addr.v4, sizeof(args->key_addr.v4));
+			break;
+		case AF_INET6:
+			memcpy(&key.addr, &args->key_addr.v6, sizeof(args->key_addr.v6));
+			break;
+		default:
+			log_error("unknown address family\n");
+			exit(1);
+		}
+		if (args->key_addr_prefix_len) {
+			log_error("TCP_AUTHOPT does not support prefix length\n");
+			exit(1);
+		}
+	}
+
+	if ((args->ifindex && args->bind_key_ifindex >= 0) || args->bind_key_ifindex >= 1) {
+		key.flags |= TCP_AUTHOPT_KEY_IFINDEX;
+		key.ifindex = args->ifindex;
+		log_msg("TCP_AUTHOPT_KEY_IFINDEX set ifindex=%d\n", key.ifindex);
+	} else {
+		log_msg("TCP_AUTHOPT_KEY_IFINDEX off\n", key.ifindex);
+	}
+
+	rc = setsockopt(sd, IPPROTO_TCP, TCP_AUTHOPT_KEY, &key, sizeof(key));
+	if (rc < 0)
+		log_err_errno("setsockopt(TCP_AUTHOPT_KEY)");
+
+	return rc;
+}
+
 static int tcp_md5sig(int sd, void *addr, socklen_t alen, struct sock_args *args)
 {
 	int keylen = strlen(args->password);
 	struct tcp_md5sig md5sig = {};
 	int opt = TCP_MD5SIG;
@@ -1514,10 +1560,15 @@ static int do_server(struct sock_args *args, int ipc_fd)
 	if (args->password && tcp_md5_remote(lsd, args)) {
 		close(lsd);
 		goto err_exit;
 	}
 
+	if (args->authopt_password && tcp_set_authopt(lsd, args)) {
+		close(lsd);
+		goto err_exit;
+	}
+
 	ipc_write(ipc_fd, 1);
 	while (1) {
 		log_msg("waiting for client connection.\n");
 		FD_ZERO(&rfds);
 		FD_SET(lsd, &rfds);
@@ -1636,10 +1687,13 @@ static int connectsock(void *addr, socklen_t alen, struct sock_args *args)
 		goto out;
 
 	if (args->password && tcp_md5sig(sd, addr, alen, args))
 		goto err;
 
+	if (args->authopt_password && tcp_set_authopt(sd, args))
+		goto err;
+
 	if (args->bind_test_only)
 		goto out;
 
 	if (connect(sd, addr, alen) < 0) {
 		if (errno != EINPROGRESS) {
@@ -1825,11 +1879,11 @@ static int ipc_parent(int cpid, int fd, struct sock_args *args)
 
 	wait(&status);
 	return client_status;
 }
 
-#define GETOPT_STR  "sr:l:c:p:t:g:P:DRn:M:X:m:d:I:BN:O:SCi6xL:0:1:2:3:Fbq"
+#define GETOPT_STR  "sr:l:c:p:t:g:P:DRn:M:X:m:A:d:I:BN:O:SCi6xL:0:1:2:3:Fbq"
 #define OPT_FORCE_BIND_KEY_IFINDEX 1001
 #define OPT_NO_BIND_KEY_IFINDEX 1002
 
 static struct option long_opts[] = {
 	{"force-bind-key-ifindex", 0, 0, OPT_FORCE_BIND_KEY_IFINDEX},
@@ -1869,14 +1923,15 @@ static void print_usage(char *prog)
 	"    -L len        send random message of given length\n"
 	"    -n num        number of times to send message\n"
 	"\n"
 	"    -M password   use MD5 sum protection\n"
 	"    -X password   MD5 password for client mode\n"
-	"    -m prefix/len prefix and length to use for MD5 key\n"
-	"    --no-bind-key-ifindex: Force TCP_MD5SIG_FLAG_IFINDEX off\n"
-	"    --force-bind-key-ifindex: Force TCP_MD5SIG_FLAG_IFINDEX on\n"
+	"    -m prefix/len prefix and length to use for MD5/AO key\n"
+	"    --no-bind-key-ifindex: Force disable binding key to ifindex\n"
+	"    --force-bind-key-ifindex: Force enable binding key to ifindex\n"
 	"        (default: only if -I is passed)\n"
+	"    -A password   use RFC5925 TCP Authentication Option with password\n"
 	"\n"
 	"    -g grp        multicast group (e.g., 239.1.1.1)\n"
 	"    -i            interactive mode (default is echo and terminate)\n"
 	"\n"
 	"    -0 addr       Expected local address\n"
@@ -1994,10 +2049,13 @@ int main(int argc, char *argv[])
 			args.client_pw = optarg;
 			break;
 		case 'm':
 			args.key_addr_prefix_str = optarg;
 			break;
+		case 'A':
+			args.authopt_password = optarg;
+			break;
 		case 'S':
 			args.use_setsockopt = 1;
 			break;
 		case 'C':
 			args.use_cmsg = 1;
@@ -2054,12 +2112,17 @@ int main(int argc, char *argv[])
 	      args.type != SOCK_STREAM)) {
 		log_error("MD5 passwords apply to TCP only and require a remote ip for the password\n");
 		return 1;
 	}
 
-	if (args.key_addr_prefix_str && !args.password) {
-		log_error("Prefix range for MD5 protection specified without a password\n");
+	if (args.key_addr_prefix_str && !args.password && !args.authopt_password) {
+		log_error("Prefix range for authentication requires -M or -A\n");
+		return 1;
+	}
+
+	if (args.key_addr_prefix_len && args.authopt_password) {
+		log_error("TCP-AO does not support prefix match, only full address\n");
 		return 1;
 	}
 
 	if (iter == 0) {
 		fprintf(stderr, "Invalid number of messages to send\n");
Signature · 1 line
2.25.1
Reviewed-by on v2 24/25
#51 lore raw

Re: [PATCH v2 24/25] selftests: nettest: Initial tcp_authopt support

Quoted 8 lines · Leonard Crestez

Add support for configuring TCP Authentication Option. Only a single key is supported with default options.

---
 tools/testing/selftests/net/nettest.c | 75 ++++++++++++++++++++++++---
 1 file changed, 69 insertions(+), 6 deletions(-)
#52 lore raw

PATCH v2 25/25 selftests: net/fcnal: Initial tcp_authopt support

In reply to Leonard Crestez

Tests are mostly copied from tcp_md5 with minor changes.

It covers VRF support but only based on binding multiple servers: not multiple keys bound to different interfaces.

Also add a specific -t tcp_authopt to run only these tests specifically.

---
 tools/testing/selftests/net/fcnal-test.sh | 249 ++++++++++++++++++++++
 1 file changed, 249 insertions(+)
tools/testing/selftests/net/fcnal-test.sh +249 -0
index 3313566ce906..d7afd9f40848 100755
@@ -800,10 +800,252 @@ ipv4_ping()
 }
 
 ################################################################################
 # IPv4 TCP
 
+#
+# TCP Authentication Option Tests
+#
+
+# try to enable tcp_authopt sysctl
+enable_tcp_authopt()
+{
+	if [[ -e /proc/sys/net/ipv4/tcp_authopt ]]; then
+		sysctl -w net.ipv4.tcp_authopt=1
+	fi
+}
+
+# check if tcp_authopt is compiled with a client-side bind test
+has_tcp_authopt()
+{
+	run_cmd_nsb nettest -b -A ${MD5_PW} -r ${NSA_IP}
+}
+
+ipv4_tcp_authopt_novrf()
+{
+	enable_tcp_authopt
+	if ! has_tcp_authopt; then
+		echo "TCP-AO appears to be missing, skip"
+		return 0
+	fi
+
+	log_start
+	run_cmd nettest -s -A ${MD5_PW} -m ${NSB_IP} &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 0 "AO: Single address config"
+
+	log_start
+	run_cmd nettest -s  &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 2 "AO: Server no config, client uses password"
+
+	log_start
+	run_cmd nettest -s -A ${MD5_PW} -m ${NSB_IP} &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_WRONG_PW}
+	log_test $? 2 "AO: Client uses wrong password"
+
+	log_start
+	run_cmd nettest -s -A ${MD5_PW} -m ${NSB_LO_IP} &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 2 "AO: Client address does not match address configured on server"
+
+	# no prefixlen for AO yet
+}
+
+ipv6_tcp_authopt_novrf()
+{
+	enable_tcp_authopt
+	if ! has_tcp_authopt; then
+		echo "TCP-AO appears to be missing, skip"
+		return 0
+	fi
+
+	log_start
+	run_cmd nettest -6 -s -A ${MD5_PW} &
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_PW}
+	log_test $? 0 "AO: Simple correct config"
+
+	log_start
+	run_cmd nettest -6 -s
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_PW}
+	log_test $? 2 "AO: Server no config, client uses password"
+
+	log_start
+	run_cmd nettest -6 -s -A ${MD5_PW} -m ${NSB_IP6} &
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_WRONG_PW}
+	log_test $? 2 "AO: Client uses wrong password"
+
+	log_start
+	run_cmd nettest -6 -s -A ${MD5_PW} -m ${NSB_LO_IP6} &
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_PW}
+	log_test $? 2 "AO: Client address does not match address configured on server"
+
+	# no prefixlen for AO yet
+}
+
+ipv4_tcp_authopt_vrf()
+{
+	enable_tcp_authopt
+	if ! has_tcp_authopt; then
+		echo "TCP-AO appears to be missing, skip"
+		return 0
+	fi
+
+	log_start
+	run_cmd nettest -s -I ${VRF} -A ${MD5_PW} &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Simple config"
+
+	#
+	# duplicate config between default VRF and a VRF
+	#
+
+	log_start
+	run_cmd nettest -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP} &
+	run_cmd nettest -s -A ${MD5_WRONG_PW} -m ${NSB_IP} &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Servers in default-VRF and VRF, client in VRF"
+
+	log_start
+	run_cmd nettest -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP} &
+	run_cmd nettest -s -A ${MD5_WRONG_PW} -m ${NSB_IP} &
+	sleep 1
+	run_cmd_nsc nettest -r ${NSA_IP} -A ${MD5_WRONG_PW}
+	log_test $? 0 "AO: VRF: Servers in default-VRF and VRF, client in default-VRF"
+
+	log_start
+	show_hint "Should timeout since client in default VRF uses VRF password"
+	run_cmd nettest -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP} &
+	run_cmd nettest -s -A ${MD5_WRONG_PW} -m ${NSB_IP} &
+	sleep 1
+	run_cmd_nsc nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 2 "AO: VRF: Servers in default VRF and VRF, conn in default-VRF with VRF pw"
+
+	log_start
+	show_hint "Should timeout since client in VRF uses default VRF password"
+	run_cmd nettest -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP} &
+	run_cmd nettest -s -A ${MD5_WRONG_PW} -m ${NSB_IP} &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_WRONG_PW}
+	log_test $? 2 "AO: VRF: Servers in default VRF and VRF, conn in VRF with default-VRF pw"
+
+	test_ipv4_tcp_authopt_vrf__global_server__bind_ifindex0
+}
+
+test_ipv4_tcp_authopt_vrf__global_server__bind_ifindex0()
+{
+	# This particular test needs tcp_l3mdev_accept=1 for Global server to accept VRF connections
+	local old_tcp_l3mdev_accept
+	old_tcp_l3mdev_accept=$(get_sysctl net.ipv4.tcp_l3mdev_accept)
+	set_sysctl net.ipv4.tcp_l3mdev_accept=1
+
+	log_start
+	run_cmd nettest -s -A ${MD5_PW} --force-bind-key-ifindex &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 2 "AO: VRF: Global server, Key bound to ifindex=0 rejects VRF connection"
+
+	log_start
+	run_cmd nettest -s -A ${MD5_PW} --force-bind-key-ifindex &
+	sleep 1
+	run_cmd_nsc nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Global server, key bound to ifindex=0 accepts non-VRF connection"
+	log_start
+
+	run_cmd nettest -s -A ${MD5_PW} --no-bind-key-ifindex &
+	sleep 1
+	run_cmd_nsb nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Global server, key not bound to ifindex accepts VRF connection"
+
+	log_start
+	run_cmd nettest -s -A ${MD5_PW} --no-bind-key-ifindex &
+	sleep 1
+	run_cmd_nsc nettest -r ${NSA_IP} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Global server, key not bound to ifindex accepts non-VRF connection"
+
+	# restore value
+	set_sysctl net.ipv4.tcp_l3mdev_accept="$old_tcp_l3mdev_accept"
+}
+
+ipv6_tcp_authopt_vrf()
+{
+	enable_tcp_authopt
+	if ! has_tcp_authopt; then
+		echo "TCP-AO appears to be missing, skip"
+		return 0
+	fi
+
+	log_start
+	run_cmd nettest -6 -s -I ${VRF} -A ${MD5_PW} &
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Simple config"
+
+	#
+	# duplicate config between default VRF and a VRF
+	#
+
+	log_start
+	run_cmd nettest -6 -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP6} &
+	run_cmd nettest -6 -s -A ${MD5_WRONG_PW} -m ${NSB_IP6} &
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_PW}
+	log_test $? 0 "AO: VRF: Servers in default-VRF and VRF, client in VRF"
+
+	log_start
+	run_cmd nettest -6 -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP6} &
+	run_cmd nettest -6 -s -A ${MD5_WRONG_PW} -m ${NSB_IP6} &
+	sleep 1
+	run_cmd_nsc nettest -6 -r ${NSA_IP6} -A ${MD5_WRONG_PW}
+	log_test $? 0 "AO: VRF: Servers in default-VRF and VRF, client in default-VRF"
+
+	log_start
+	show_hint "Should timeout since client in default VRF uses VRF password"
+	run_cmd nettest -6 -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP6} &
+	run_cmd nettest -6 -s -A ${MD5_WRONG_PW} -m ${NSB_IP6} &
+	sleep 1
+	run_cmd_nsc nettest -6 -r ${NSA_IP6} -A ${MD5_PW}
+	log_test $? 2 "AO: VRF: Servers in default VRF and VRF, conn in default-VRF with VRF pw"
+
+	log_start
+	show_hint "Should timeout since client in VRF uses default VRF password"
+	run_cmd nettest -6 -s -I ${VRF} -A ${MD5_PW} -m ${NSB_IP6} &
+	run_cmd nettest -6 -s -A ${MD5_WRONG_PW} -m ${NSB_IP6} &
+	sleep 1
+	run_cmd_nsb nettest -6 -r ${NSA_IP6} -A ${MD5_WRONG_PW}
+	log_test $? 2 "AO: VRF: Servers in default VRF and VRF, conn in VRF with default-VRF pw"
+}
+
+only_tcp_authopt()
+{
+	log_section "TCP Authentication Option"
+
+	setup
+	set_sysctl net.ipv4.tcp_l3mdev_accept=0
+	log_subsection "TCP-AO IPv4 no VRF"
+	ipv4_tcp_authopt_novrf
+	log_subsection "TCP-AO IPv6 no VRF"
+	ipv6_tcp_authopt_novrf
+
+	setup "yes"
+	set_sysctl net.ipv4.tcp_l3mdev_accept=0
+	log_subsection "TCP-AO IPv4 VRF"
+	ipv4_tcp_authopt_vrf
+	log_subsection "TCP-AO IPv6 VRF"
+	ipv6_tcp_authopt_vrf
+}
+
 #
 # MD5 tests without VRF
 #
 ipv4_tcp_md5_novrf()
 {
@@ -1185,10 +1427,11 @@ ipv4_tcp_novrf()
 	show_hint "Should fail 'Connection refused'"
 	run_cmd nettest -d ${NSA_DEV} -r ${a}
 	log_test_addr ${a} $? 1 "No server, device client, local conn"
 
 	ipv4_tcp_md5_novrf
+	ipv4_tcp_authopt_novrf
 }
 
 ipv4_tcp_vrf()
 {
 	local a
@@ -1239,10 +1482,12 @@ ipv4_tcp_vrf()
 	run_cmd nettest -r ${a} -d ${NSA_DEV}
 	log_test_addr ${a} $? 1 "Global server, local connection"
 
 	# run MD5 tests
 	ipv4_tcp_md5
+	# run AO tests
+	ipv6_tcp_md5_vrf
 
 	#
 	# enable VRF global server
 	#
 	log_subsection "VRF Global server enabled"
@@ -2648,10 +2893,11 @@ ipv6_tcp_novrf()
 		run_cmd nettest -6 -d ${NSA_DEV} -r ${a}
 		log_test_addr ${a} $? 1 "No server, device client, local conn"
 	done
 
 	ipv6_tcp_md5_novrf
+	ipv6_tcp_authopt_novrf
 }
 
 ipv6_tcp_vrf()
 {
 	local a
@@ -2718,10 +2964,12 @@ ipv6_tcp_vrf()
 	run_cmd nettest -6 -r ${a} -d ${NSA_DEV}
 	log_test_addr ${a} $? 1 "Global server, local connection"
 
 	# run MD5 tests
 	ipv6_tcp_md5
+	# run AO tests
+	ipv6_tcp_authopt_vrf
 
 	#
 	# enable VRF global server
 	#
 	log_subsection "VRF Global server enabled"
@@ -4062,10 +4310,11 @@ do
 	ipv6_bind|bind6) ipv6_addr_bind;;
 	ipv6_runtime)    ipv6_runtime;;
 	ipv6_netfilter)  ipv6_netfilter;;
 
 	use_cases)       use_cases;;
+	tcp_authopt)     only_tcp_authopt;;
 
 	# setup namespaces and config, but do not run any tests
 	setup)		 setup; exit 0;;
 	vrf_setup)	 setup "yes"; exit 0;;
 
Signature · 1 line
2.25.1
#53 lore raw
In reply to Leonard Crestez
Quoted 53 lines · Leonard Crestez

This is similar to TCP MD5 in functionality but it's sufficiently different that wire formats are incompatible. Compared to TCP-MD5 more algorithms are supported and multiple keys can be used on the same connection but there is still no negotiation mechanism.

Expected use-case is protecting long-duration BGP/LDP connections between routers using pre-shared keys. The goal of this series is to allow routers using the linux TCP stack to interoperate with vendors such as Cisco and Juniper.

Both algorithms described in RFC5926 are implemented but the code is not very easily extensible beyond that. In particular there are several code paths making stack allocations based on RFC5926 maximum, those would have to be increased.

This version implements SNE and l3mdev awareness and adds more tests. Here are some known flaws and limitations:

* Interaction with TCP-MD5 not tested in all corners
* Interaction with FASTOPEN not tested and unlikely to work because sequence number assumptions for syn/ack.
* Not clear if crypto_shash_setkey might sleep. If some implementation do that then maybe they could be excluded through alloc flags.
* Traffic key is not cached (reducing performance)
* User is responsible for ensuring keys do not overlap.
* There is no useful way to list keys, making userspace debug difficult.
* There is no prefixlen support equivalent to md5. This is used in some complex FRR configs.

Test suite was added to tools/selftests/tcp_authopt. Tests are written in python using pytest and scapy and check the API in some detail and validate packet captures. Python code is already used in linux and in kselftests but virtualenvs not very much, this particular test suite uses pip to create a private virtualenv and hide dependencies.

This actually forms the bulk of the series by raw line-count. Since there is a lot of code it was mostly split on "functional area" so most files are only affected by a single code. A lot of those tests are relevant to TCP-MD5 so perhaps it might help to split into a separate series?

Some testing support is included in nettest and fcnal-test.sh, similar to the current level of tcp-md5 testing.

SNE was tested by creating connections in a loop until a large SEQ is randomly selected and then making it rollover. The "connect in a loop" step ran into timewait overflow and connection failure on port reuse. After spending some time on this issue and my conclusion is that AO makes it impossible to kill remainders of old connections in a manner similar to unsigned or md5sig, this is because signatures are dependent on ISNs. This means that if a timewait socket is closed improperly then information required to RST the peer is lost.

Quoted 7 lines

The fact that AO completely breaks all connection-less RSTs is acknowledged in the RFC and the workaround of "respect timewait" seems acceptable.

Changes for frr (old): https://github.com/FRRouting/frr/pull/9442
That PR was made early for ABI feedback, it has many issues.

overall looks ok to me. I did not wade through the protocol details.

I did see the comment about no prefixlen support in the tests. A lot of patches to absorb, perhaps I missed it. Does AuthOpt support for prefixes? If not, you should consider adding that as a quick follow on (within the same dev cycle). MD5 added prefix support for scalability; seems like AO should be concerned about the same.

#54 lore raw
Quoted 62 lines · David Ahern
Quoted 53 lines · Leonard Crestez

This is similar to TCP MD5 in functionality but it's sufficiently different that wire formats are incompatible. Compared to TCP-MD5 more algorithms are supported and multiple keys can be used on the same connection but there is still no negotiation mechanism.

Expected use-case is protecting long-duration BGP/LDP connections between routers using pre-shared keys. The goal of this series is to allow routers using the linux TCP stack to interoperate with vendors such as Cisco and Juniper.

Both algorithms described in RFC5926 are implemented but the code is not very easily extensible beyond that. In particular there are several code paths making stack allocations based on RFC5926 maximum, those would have to be increased.

This version implements SNE and l3mdev awareness and adds more tests. Here are some known flaws and limitations:

* Interaction with TCP-MD5 not tested in all corners
* Interaction with FASTOPEN not tested and unlikely to work because sequence number assumptions for syn/ack.
* Not clear if crypto_shash_setkey might sleep. If some implementation do that then maybe they could be excluded through alloc flags.
* Traffic key is not cached (reducing performance)
* User is responsible for ensuring keys do not overlap.
* There is no useful way to list keys, making userspace debug difficult.
* There is no prefixlen support equivalent to md5. This is used in some complex FRR configs.

Test suite was added to tools/selftests/tcp_authopt. Tests are written in python using pytest and scapy and check the API in some detail and validate packet captures. Python code is already used in linux and in kselftests but virtualenvs not very much, this particular test suite uses pip to create a private virtualenv and hide dependencies.

This actually forms the bulk of the series by raw line-count. Since there is a lot of code it was mostly split on "functional area" so most files are only affected by a single code. A lot of those tests are relevant to TCP-MD5 so perhaps it might help to split into a separate series?

Some testing support is included in nettest and fcnal-test.sh, similar to the current level of tcp-md5 testing.

SNE was tested by creating connections in a loop until a large SEQ is randomly selected and then making it rollover. The "connect in a loop" step ran into timewait overflow and connection failure on port reuse. After spending some time on this issue and my conclusion is that AO makes it impossible to kill remainders of old connections in a manner similar to unsigned or md5sig, this is because signatures are dependent on ISNs. This means that if a timewait socket is closed improperly then information required to RST the peer is lost.

Quoted 7 lines

The fact that AO completely breaks all connection-less RSTs is acknowledged in the RFC and the workaround of "respect timewait" seems acceptable.

Changes for frr (old): https://github.com/FRRouting/frr/pull/9442
That PR was made early for ABI feedback, it has many issues.

Quoted 7 lines

overall looks ok to me. I did not wade through the protocol details.

I did see the comment about no prefixlen support in the tests. A lot of patches to absorb, perhaps I missed it. Does AuthOpt support for prefixes? If not, you should consider adding that as a quick follow on (within the same dev cycle). MD5 added prefix support for scalability; seems like AO should be concerned about the same.

I just skipped it because it's not required for core functionality.

It's very straight forward so I will add it to the next version.

--
Regards,
Leonard