[PATCH net 0/2] tcp: make TCP-AO lookups more predictable

[PATCH net 0/2] tcp: make TCP-AO lookups more predictable

[Eric Dumazet]

This series fixes a TCP-AO key lookup precedence bug.

TCP-AO stores MKTs in an unsorted list and returns the first match. This
allows newer, less-specific keys (wildcard VRF or shorter prefixes) to
shadow older, more-specific keys if inserted later.

Fix this by implementing sorted insertion in tcp_ao_link_mkt() based on
key specificity (VRF binding, then prefix length). This keeps the RX
lookup path fast while ensuring correctness.

The second patch adds a selftest to verify this behavior.

Eric Dumazet (2):
  tcp: fix TCP-AO key lookup precedence (shadowing)
  selftests/net: Add TCP-AO key shadowing test

 net/ipv4/tcp_ao.c                             | 27 +++++-
 tools/testing/selftests/net/tcp_ao/Makefile   |  1 +
 .../testing/selftests/net/tcp_ao/shadowing.c  | 93 +++++++++++++++++++
 3 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/tcp_ao/shadowing.c

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH net 1/2] tcp: fix TCP-AO key lookup precedence (shadowing)

[Eric Dumazet]

TCP-AO implementation stores Master Key Tuples (MKTs) in an unsorted
doubly-linked list (ao_info->head) and inserts new keys at the head.
When looking up a key, __tcp_ao_do_lookup() walks this list and returns
the first match it finds.

Because the list is unsorted, a newer, less-specific key can shadow an
older, more-specific key if it happens to be inserted later. This leads
to incorrect key selection in two scenarios:

1. VRF Shadowing: A wildcard VRF key (not bound to an interface) added
   after a VRF-specific key will shadow the VRF-specific key for traffic
   arriving on that VRF.
2. Prefix Shadowing: A less-specific prefix key (e.g., /24) added after
   a more-specific prefix key (e.g., /32) will shadow the more-specific
   key during outbound connection establishment.

Unlike TCP MD5, which walks the entire list and evaluates the "best
match" using better_md5_match(), TCP-AO expects the list order to
determine precedence.

Fix this by implementing sorted insertion in tcp_ao_link_mkt(). Keys
are inserted in descending order of specificity:
  - VRF-bound keys take precedence over unbound keys.
  - Longer prefix matches (LPM) take precedence over shorter ones.

This preserves the performance of the lockless RX lookup path (early
return on first match) while ensuring correct precedence.

Fixes: 4954f17ddefc ("net/tcp: Introduce TCP_AO setsockopt()s")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Assisted-by: Gemini:gemini-3.1-pro
---
 net/ipv4/tcp_ao.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index 2f69bcecae78a677f33033a2d30e09a8ff858ad8..2d10fb1dd4cf87cc79ef5b5ead80eb3048218250 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -341,9 +341,34 @@ static struct tcp_ao_info *tcp_ao_alloc_info(gfp_t flags)
 	return ao;
 }
 
+static bool tcp_ao_key_is_more_specific(const struct tcp_ao_key *a,
+					const struct tcp_ao_key *b)
+{
+	bool a_vrf = !!(a->keyflags & TCP_AO_KEYF_IFINDEX);
+	bool b_vrf = !!(b->keyflags & TCP_AO_KEYF_IFINDEX);
+
+	if (a_vrf != b_vrf)
+		return a_vrf; /* VRF-bound is more specific */
+
+	return a->prefixlen > b->prefixlen; /* Longer prefix is more specific */
+}
+
 static void tcp_ao_link_mkt(struct tcp_ao_info *ao, struct tcp_ao_key *mkt)
 {
-	hlist_add_head_rcu(&mkt->node, &ao->head);
+	struct tcp_ao_key *pos;
+	struct hlist_node *last = NULL;
+
+	hlist_for_each_entry(pos, &ao->head, node) {
+		if (tcp_ao_key_is_more_specific(mkt, pos)) {
+			hlist_add_before_rcu(&mkt->node, &pos->node);
+			return;
+		}
+		last = &pos->node;
+	}
+	if (last)
+		hlist_add_behind_rcu(&mkt->node, last);
+	else
+		hlist_add_head_rcu(&mkt->node, &ao->head);
 }
 
 static struct tcp_ao_key *tcp_ao_copy_key(struct sock *sk,
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH net 2/2] selftests/net: Add TCP-AO key shadowing test

[Eric Dumazet]

Add a new selftest shadowing.c to tools/testing/selftests/net/tcp_ao
to verify that more specific keys are correctly preferred over less
specific ones (shadowing prevention), regardless of their insertion order.

The test configures a server with a specific host key, and a client with
both a specific host key and a wildcard subnet key, inserted in the
"wrong" order (wildcard last, which would shadow the specific one under
the bug). It then verifies that the client can still successfully
connect to the server, which only succeeds if the client correctly
selects the more specific key for the outbound connection.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Assisted-by: Gemini:gemini-3.1-pro
---
 tools/testing/selftests/net/tcp_ao/Makefile   |  1 +
 .../testing/selftests/net/tcp_ao/shadowing.c  | 93 +++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 tools/testing/selftests/net/tcp_ao/shadowing.c

diff --git a/tools/testing/selftests/net/tcp_ao/Makefile b/tools/testing/selftests/net/tcp_ao/Makefile
index 5b0205c70c3983815315048c0ec1275525b7a29a..0c601d7049320be2310f9ff32988ae229584222e 100644
--- a/tools/testing/selftests/net/tcp_ao/Makefile
+++ b/tools/testing/selftests/net/tcp_ao/Makefile
@@ -2,6 +2,7 @@
 TEST_BOTH_AF := bench-lookups
 TEST_BOTH_AF += connect
 TEST_BOTH_AF += connect-deny
+TEST_BOTH_AF += shadowing
 TEST_BOTH_AF += icmps-accept icmps-discard
 TEST_BOTH_AF += key-management
 TEST_BOTH_AF += restore
diff --git a/tools/testing/selftests/net/tcp_ao/shadowing.c b/tools/testing/selftests/net/tcp_ao/shadowing.c
new file mode 100644
index 0000000000000000000000000000000000000000..da14b13e032d5a0632f398b7eaa72b8045e61ffe
--- /dev/null
+++ b/tools/testing/selftests/net/tcp_ao/shadowing.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <inttypes.h>
+#include "aolib.h"
+
+static void *server_fn(void *arg)
+{
+	int sk, lsk;
+	ssize_t bytes;
+
+	lsk = test_listen_socket(this_ip_addr, test_server_port, 1);
+
+	/* Server only has the specific key for the client.
+	 * It expects KeyID 100, signed with "pass_specific".
+	 */
+	if (test_add_key(lsk, "pass_specific", this_ip_dest, -1, 100, 100))
+		test_error("setsockopt(TCP_AO_ADD_KEY)");
+
+	synchronize_threads(); /* 1: Server ready and key added */
+
+	if (test_wait_fd(lsk, TEST_TIMEOUT_SEC, 0))
+		test_error("test_wait_fd()");
+
+	sk = accept(lsk, NULL, NULL);
+	if (sk < 0)
+		test_error("accept()");
+
+	synchronize_threads(); /* 2: Connection accepted */
+
+	/* Verify we can receive data from the client */
+	bytes = test_server_run(sk, 0, 0);
+	if (bytes < 0) {
+		test_fail("server: failed to receive data");
+	} else {
+		test_ok("server: connection authenticated successfully");
+	}
+
+	close(sk);
+	close(lsk);
+	return NULL;
+}
+
+static void *client_fn(void *arg)
+{
+	int sk = socket(test_family, SOCK_STREAM, IPPROTO_TCP);
+	union tcp_addr wildcard_addr = {};
+
+	if (sk < 0)
+		test_error("socket()");
+
+	/* Client adds keys in the "wrong" order (wildcard last) to trigger shadowing.
+	 * 1. Specific key (Key B, ID 100)
+	 * 2. Wildcard key (Key A, ID 101)
+	 *
+	 * Without the fix, the wildcard key will be at the head of the list
+	 * and will shadow the specific key during outbound lookup, causing
+	 * the client to send a SYN with KeyID 101 (which the server doesn't have).
+	 */
+
+	/* 1. Add specific key */
+	if (test_add_key(sk, "pass_specific", this_ip_dest, -1, 100, 100))
+		test_error("setsockopt(TCP_AO_ADD_KEY) specific");
+
+	/* 2. Add wildcard key (any address, prefix 0) */
+	if (test_add_key(sk, "pass_wildcard", wildcard_addr, 0, 101, 101))
+		test_error("setsockopt(TCP_AO_ADD_KEY) wildcard");
+
+	synchronize_threads(); /* 1: Client ready and keys added => connect() */
+
+	if (test_connect_socket(sk, this_ip_dest, test_server_port) <= 0) {
+		test_fail("client: failed to connect (shadowing bug present?)");
+		close(sk);
+		return NULL;
+	}
+
+	synchronize_threads(); /* 2: Connection established */
+
+	/* Send some data to verify the connection works */
+	if (test_client_verify(sk, 100, 20)) {
+		test_fail("client: verify failed");
+	} else {
+		test_ok("client: connection established and verified (precedence correct)");
+	}
+
+	close(sk);
+	return NULL;
+}
+
+int main(int argc, char *argv[])
+{
+	/* We expect 2 test results: 1 from server, 1 from client */
+	test_init(2, server_fn, client_fn);
+	return 0;
+}
-- 
2.55.0.rc0.799.gd6f94ed593-goog