[PATCH net] rxrpc: Fix rxkad to use the key's rxkad token, not the first token

[Daniel Wade <danjwade95@gmail.com>]

rxkad takes conn->key->payload.data[0] as its token, but
rxrpc_init_client_call_security() may have chosen rxkad because of a
token that is not the first in the key.  Since YFS-RxGK support was added
the parser accepts rxgk tokens whatever the configuration, while the rxgk
class is only registered under CONFIG_RXGK=y.  So with CONFIG_RXGK=n a
key holding [YFS-RxGK, rxkad] gets rxkad security and rxkad reads the
struct rxgk_key as a struct rxkad_key.

rxkad_respond_to_challenge() then takes the ticket length from bits 16-31
of the rxgk token's lifetime, which the key's creator sets freely, and
copies up to 65535 bytes from offset 28 of an object that is only 72 to
136 bytes.  It sends what it read, including the kernel pointers in the
rxgk_key, to the peer in the RESPONSE.  Neither making such a key nor
placing a call needs privilege.

Add a helper that finds the rxkad token and use it in
rxkad_prime_packet_security(), rxkad_secure_packet_encrypt(),
rxkad_verify_packet_2() and rxkad_respond_to_challenge(), aborting in the
last if there is none.  A key's token list does not change once
instantiated, so the token stays valid as long as conn->key is held.

Discovered by an AI code review agent, reproduced on unmodified mainline
with CONFIG_RXGK=n: with [YFS-RxGK, rxkad] the server aborted with
RXKADBADTICKET, and a KASAN kernel whose compiler instruments memcpy()
reported a 64-byte slab-out-of-bounds read 28 bytes into an 88-byte
kmalloc-96 region.  With this patch both orders succeed and KASAN is
silent.

Fixes: 0ca100ff4df6 ("rxrpc: Add YFS RxGK (GSSAPI) security class")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Wade <danjwade95@gmail.com>
---
Tested on mainline b3dfbde62cf1 in a 2-vCPU KVM guest with KASAN and
PROVE_LOCKING, AF_RXRPC and RXKAD built in, CONFIG_RXGK not set.  The
test runs as an unprivileged user against an AF_RXRPC server of its own
at security level 2, and makes the rxkad ticket with AF_ALG ecb(des), so
crypto.af_alg_restrict was set to 0 in the guest.  The rxgk token's
lifetime was chosen to give a ticket length of 64; any value up to 65535
is accepted.

Unmodified, [rxkad, rxgk] succeeded and [rxgk, rxkad] aborted with
RXKADBADTICKET, the RESPONSE copying 64 bytes from 28 bytes into the
88-byte rxgk_key.  With this patch both orders succeed.

On the same source built with GCC 13.3.0, where
CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX makes KASAN check mem*() ranges,
the unmodified kernel reports:

  BUG: KASAN: slab-out-of-bounds in skb_store_bits+0x3de/0x840
  Read of size 64 at addr ffff888009d2121c by task kworker/u9:1/229
  which belongs to the cache kmalloc-96 of size 96
  allocated 88-byte region [ffff888009d21200, ffff888009d21258)

and nothing with this patch.  A KASAN kernel built with GCC 11 does not
report it: that compiler cannot emit __asan_memcpy(), and on x86
mm/kasan/shadow.c then provides no checking memcpy() at all.

The NULL check in rxkad_respond_to_challenge() cannot trigger today, as
both client and service connections reach it with a key that holds an
rxkad token.  It is there because the helper can return NULL and the
function's output goes on the wire.  At level 2 the encrypt and decrypt
paths now walk the token list per packet: one step for an ordinary key,
at most AFSTOKEN_MAX (8).

Stable: v6.16 and later are affected; 6.12.y is not, as its key parser
stores only rxkad tokens.  Between 6.16 and 7.1 the prime, encrypt and
decrypt hunks need their context adjusted; the change is the same.

Not tested: CONFIG_RXGK=y, security levels 0 and 1, the AF_RXRPC KUnit
test, interoperation with AFS servers or clients.

The test program is not included here; it is available on request.

 net/rxrpc/rxkad.c |   31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index ca9f0e82cb9..dbfdaf78d90 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -175,6 +175,22 @@ static struct rxrpc_txbuf *rxkad_alloc_txbuf(struct rxrpc_call *call, size_t rem
 	return txb;
 }
 
+/*
+ * Find the rxkad token in a key.  rxkad may have been chosen because of a
+ * token that isn't the first (see rxrpc_init_client_call_security()), so
+ * payload.data[0] mustn't be assumed to be one.
+ */
+static const struct rxrpc_key_token *rxkad_find_token(const struct key *key)
+{
+	const struct rxrpc_key_token *token;
+
+	for (token = key->payload.data[0]; token; token = token->next) {
+		if (token->security_index == RXRPC_SECURITY_RXKAD)
+			return token;
+	}
+	return NULL;
+}
+
 /*
  * prime the encryption state with the invariant parts of a connection's
  * description
@@ -182,14 +198,14 @@ static struct rxrpc_txbuf *rxkad_alloc_txbuf(struct rxrpc_call *call, size_t rem
 static void rxkad_prime_packet_security(struct rxrpc_connection *conn,
 					const struct fcrypt_key *cipher)
 {
-	struct rxrpc_key_token *token;
+	const struct rxrpc_key_token *token;
 	__be32 tmpbuf[4];
 
 	_enter("");
 
 	if (!conn->key)
 		return;
-	token = conn->key->payload.data[0];
+	token = rxkad_find_token(conn->key);
 
 	tmpbuf[0] = htonl(conn->proto.epoch);
 	tmpbuf[1] = htonl(conn->proto.cid);
@@ -266,7 +282,7 @@ static void rxkad_secure_packet_encrypt(const struct rxrpc_call *call,
 	/* Now txb->pkt_len % FCRYPT_BSIZE == 0. */
 
 	/* encrypt from the session key */
-	token = call->conn->key->payload.data[0];
+	token = rxkad_find_token(call->conn->key);
 	fcrypt_pcbc_encrypt(call->conn->rxkad.cipher, token->kad->session_key,
 			    rxkhdr, rxkhdr, txb->pkt_len / FCRYPT_BSIZE);
 	_leave("");
@@ -412,7 +428,7 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb,
 	len = round_down(len, 8);
 
 	/* decrypt from the session key */
-	token = call->conn->key->payload.data[0];
+	token = rxkad_find_token(call->conn->key);
 	fcrypt_pcbc_decrypt(call->conn->rxkad.cipher, token->kad->session_key,
 			    data, data, len / FCRYPT_BSIZE);
 
@@ -710,7 +726,10 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn,
 		return rxrpc_abort_conn(conn, challenge, RXKADEXPIRED, ret,
 					rxkad_abort_chall_key_expired);
 
-	token = conn->key->payload.data[0];
+	token = rxkad_find_token(conn->key);
+	if (!token)
+		return rxrpc_abort_conn(conn, challenge, RX_PROTOCOL_ERROR,
+					-EPROTO, rxkad_abort_chall_no_key);
 
 	/* build the response packet */
 	len = sizeof(struct rxrpc_wire_header) +