[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

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

[Jakub Kicinski]

On Mon, 22 Jun 2026 18:52:46 +0000 Eric Dumazet wrote:
> 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.

Unhappiness:

https://netdev-ctrl.bots.linux.dev/logs/vmksft/net-extra/results/702701/29-key-management-ipv4/stdout

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

[Dmitry Safonov]

Hi Eric,

On Mon, 22 Jun 2026 at 19:52, Eric Dumazet <edumazet@google.com> wrote:
>
> 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.

Yeah, at this moment, TCP-AO doesn't allow any intersection of the keys:
If you have matching VRFs, matching keyids for matching peer/masks –
then when the userspace tries to add the second key, setsockopt() is
going to return -EKEYREJECTED/-EEXIST. This is quite different from
TCP-MD5, where the most matching key is the one that's going to be
used by the kernel.

This simplification (not allowing any key intersects) is mostly from a
very permissive RFC5925, where MKT matches can be: ip-addr/mask; ip
address ranges; wildcards of addresses; tcp ports. So, this part was
intentionally simplified until there is a user who requires one of
these things. And based on their requirements, a better data structure
than a simple list could be used. Basically, the longest prefix match
is like adding power-of-two ip ranges. Also, that's another reason why
I wanted an extendable setsockopt(), where one can add new
flags/fields to uAPI without breaking the existing users.

Anyways, if you have the requirement to have intersecting keys with
bigger mask matching (imitating TCP-MD5 behaviour), we can do that,
but I think that needs a new TCP_AO_KEYF_PREFIX_MATCH (or something of
a kind). Then the keys with everything matching, but a prefix could be
added to the socket, and the longest prefix match will be used.

I think one API decision should be documented straight away (besides
the key flag) – how this flag works with multiple keys.
Say there are 4 keys on a socket, all match the peer being connected:
keyA: ip 10.0.0.0 /8 (keyid = 100)
keyB: ip 10.0.0.0 /16 (keyid = 100)
keyC: ip 10.0.0.0 /8 (keyid = 101)
keyD: ip 10.0.0.0 /16 (keyid = 102)

So, keyA and keyB obviously will have to use this new
TCP_AO_KEYF_PREFIX_MATCH. Should keyC or keyD be copied to the
established connection socket or not?
I'd think the presence of TCP_AO_KEYF_PREFIX_MATCH flag on keyC&keyD
should also affect whether they are copied or not. If the flag is not
on keyC&keyD –  they should be copied to the established socket
(together with keyB, preserving the previous behaviour).

Otherwise, if they have the flag, what should happen?
1. keyB + keyC + keyD
2. keyB + keyD
If we go with (2), then if a user wants keyC on a socket, they could
either remove TCP_AO_KEYF_PREFIX_MATCH from keyC or add keyC1 with
mask /16 and the same password as keyC – slightly inconvenient, but
quite flexible.

What do you think?

> 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
>

-- 
             Dmitry

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

[Dmitry Safonov]

Hi Eric,

Thanks for adding a selftest,

On Mon, 22 Jun 2026 at 19:52, Eric Dumazet <edumazet@google.com> wrote:
>
> 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
> ---
[..]

> +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");

Two notes here:
1. The two keys do not match, so I likely misunderstood your cover
letter. I thought you wanted to override more generic keys (longer
prefix; non-VRF-specific), but your goal is to choose between two
distinct keys, likely with different password-seeds.
2. This only changes behaviour on the initial connect() from a client,
where the client did not provide any priority of which keys to use on
SYN segment (by .set_current=1).

I think the patch is fine, yet it sounds like it's limited in
usefulness, if I didn't miss something obvious.

Thanks,
             Dmitry

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

[Eric Dumazet]

On Mon, Jun 22, 2026 at 6:13 PM Dmitry Safonov <0x7f454c46@gmail.com> wrote:
>
> Hi Eric,
>
> On Mon, 22 Jun 2026 at 19:52, Eric Dumazet <edumazet@google.com> wrote:
> >
> > 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.
>
> Yeah, at this moment, TCP-AO doesn't allow any intersection of the keys:
> If you have matching VRFs, matching keyids for matching peer/masks –
> then when the userspace tries to add the second key, setsockopt() is
> going to return -EKEYREJECTED/-EEXIST. This is quite different from
> TCP-MD5, where the most matching key is the one that's going to be
> used by the kernel.
>
> This simplification (not allowing any key intersects) is mostly from a
> very permissive RFC5925, where MKT matches can be: ip-addr/mask; ip
> address ranges; wildcards of addresses; tcp ports. So, this part was
> intentionally simplified until there is a user who requires one of
> these things. And based on their requirements, a better data structure
> than a simple list could be used. Basically, the longest prefix match
> is like adding power-of-two ip ranges. Also, that's another reason why
> I wanted an extendable setsockopt(), where one can add new
> flags/fields to uAPI without breaking the existing users.
>
> Anyways, if you have the requirement to have intersecting keys with
> bigger mask matching (imitating TCP-MD5 behaviour), we can do that,
> but I think that needs a new TCP_AO_KEYF_PREFIX_MATCH (or something of
> a kind). Then the keys with everything matching, but a prefix could be
> added to the socket, and the longest prefix match will be used.
>
> I think one API decision should be documented straight away (besides
> the key flag) – how this flag works with multiple keys.
> Say there are 4 keys on a socket, all match the peer being connected:
> keyA: ip 10.0.0.0 /8 (keyid = 100)
> keyB: ip 10.0.0.0 /16 (keyid = 100)
> keyC: ip 10.0.0.0 /8 (keyid = 101)
> keyD: ip 10.0.0.0 /16 (keyid = 102)
>
> So, keyA and keyB obviously will have to use this new
> TCP_AO_KEYF_PREFIX_MATCH. Should keyC or keyD be copied to the
> established connection socket or not?
> I'd think the presence of TCP_AO_KEYF_PREFIX_MATCH flag on keyC&keyD
> should also affect whether they are copied or not. If the flag is not
> on keyC&keyD –  they should be copied to the established socket
> (together with keyB, preserving the previous behaviour).
>
> Otherwise, if they have the flag, what should happen?
> 1. keyB + keyC + keyD
> 2. keyB + keyD
> If we go with (2), then if a user wants keyC on a socket, they could
> either remove TCP_AO_KEYF_PREFIX_MATCH from keyC or add keyC1 with
> mask /16 and the same password as keyC – slightly inconvenient, but
> quite flexible.
>
> What do you think?

If intersecting keys are not yet allowed, I think we must return an
error code at the insertion stage,
instead of hoping the user will do "the right thing".

Thanks.