When decryption fails but parsing continues: an rxkad ticket bug

First posted 2026-04-08 · merged into mainline 2026-04-08 · rxrpc, rxkad, crypto API

This is the writeup of a small fix I made in the kernel's RxRPC implementation: a missing return-value check on crypto_skcipher_decrypt() in the rxkad ticket-decryption path. The patch itself is four lines, but the story around it — how the bug works, how old it is, and why this class of mistake keeps surviving in security code — is worth a longer post. Everything below is taken from the real commit and the real source tree; no logs or traces are invented.

rxkad in one paragraph

RxRPC is the RPC transport that lives in net/rxrpc/, best known as the protocol underneath the kernel's AFS client. It carries calls over UDP and supports pluggable security classes. The classic one is rxkad (net/rxrpc/rxkad.c), a Kerberos-IV-style handshake: a client connects to a server, the server sends a CHALLENGE packet, and the client answers with a RESPONSE packet that embeds a Kerberos ticket — a blob encrypted with the server's secret key. The server decrypts the ticket with pcbc(des), and inside it finds the session key, the client principal's name, a lifetime, and an issue timestamp. If the ticket decrypts and parses cleanly, the server trusts the session key inside it and the connection becomes secure. The ticket bytes come straight off the wire, from the client.

The bug

On the server side, when a RESPONSE packet arrives, the connection-event handler (rxrpc_input_conn_event() in net/rxrpc/conn_event.c) dispatches on the packet type; for RXRPC_PACKET_TYPE_RESPONSE it calls rxrpc_verify_response(), which invokes the security class's verify_response op — rxkad_verify_response() for rxkad. That function copies the response header out of the skb, sanity-checks the version, the key version number, and the ticket length, then copies the ticket itself into a kmalloc'd buffer and hands it to rxkad_decrypt_ticket():

	/* extract the kerberos ticket and decrypt and decode it */
	ret = -ENOMEM;
	ticket = kmalloc(ticket_len, GFP_NOFS);
	if (!ticket)
		goto temporary_error_free_resp;

	if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header) + sizeof(*response),
			  ticket, ticket_len) < 0) {
		rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
				 rxkad_abort_resp_short_tkt);
		goto protocol_error;
	}

	ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
				   &session_key, &expiry);

The length check just before this is a range check only — ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN — with no alignment requirement. Now the vulnerable code, as it existed before the fix (fe4447cd9562~1:net/rxrpc/rxkad.c):

	req = skcipher_request_alloc(server_key->payload.data[0], GFP_NOFS);
	if (!req)
		return -ENOMEM;

	sg_init_one(&sg[0], ticket, ticket_len);
	skcipher_request_set_callback(req, 0, NULL, NULL);
	skcipher_request_set_crypt(req, sg, sg, ticket_len, iv.x);
	crypto_skcipher_decrypt(req);
	skcipher_request_free(req);

	p = ticket;
	end = p + ticket_len;

The return value of crypto_skcipher_decrypt() is simply discarded. The buffer is then treated as plaintext. But a skcipher decryption can fail: the server key's cipher is pcbc(des) (allocated in rxkad_preparse_server_key()), a block mode with an 8-byte block size, and the crypto API rejects a request whose length is not a multiple of the block size — typically with -EINVAL, without touching the buffer at all.

So the failure mode is deterministic and fully attacker-controlled: send a RESPONSE whose ticket length passes the range check but is not block-aligned — say 9 bytes, or 25 — and the decrypt operation fails, leaving the buffer exactly as it arrived from the network. Execution then falls through into the ticket parser with p = ticket pointing at raw, attacker-chosen ciphertext that the cipher never touched.

What the parser does next

The parsing that follows is not trivial. The same function walks the buffer field by field, and several of the fields have real consequences downstream. Here is the actual pre-fix code (lightly trimmed for width):

	/* extract the ticket flags */
	little_endian = *p & 1;
	p++;

	/* extract the authentication name */
	name = Z(ANAME, aname);
	/* extract the principal's instance */
	name = Z(INST, inst);
	/* extract the principal's authentication domain */
	name = Z(REALM, realm);

	if (end - p < 4 + 8 + 4 + 2)
		return rxrpc_abort_conn(conn, skb, RXKADBADTICKET, -EPROTO,
					rxkad_abort_resp_tkt_short);

	/* get the IPv4 address of the entity that requested the ticket */
	memcpy(&addr, p, sizeof(addr));
	p += 4;

	/* get the session key from the ticket */
	memcpy(&key, p, sizeof(key));
	p += 8;
	memcpy(_session_key, &key, sizeof(key));

	/* get the ticket's lifetime */
	life = *p++ * 5 * 60;

	/* get the issue time of the ticket */
	if (little_endian) {
		__le32 stamp;
		memcpy(&stamp, p, 4);
		issue = rxrpc_u32_to_time64(le32_to_cpu(stamp));
	} else {
		__be32 stamp;
		memcpy(&stamp, p, 4);
		issue = rxrpc_u32_to_time64(be32_to_cpu(stamp));
	}
	p += 4;
	now = ktime_get_real_seconds();

	/* check the ticket is in date */
	if (issue > now)
		return rxrpc_abort_conn(conn, skb, RXKADNOAUTH, -EKEYREJECTED,
					rxkad_abort_resp_tkt_future);
	if (issue < now - life)
		return rxrpc_abort_conn(conn, skb, RXKADEXPIRED, -EKEYEXPIRED,
					rxkad_abort_resp_tkt_expired);

Read that as an attacker: every byte is of my choosing. The Z() macro scans for NUL-terminated printable strings (the authentication name, instance, realm, and later the service names) with per-field length caps, so those are bounded — but the attacker controls whether they pass. Then come the fields that really matter:

Does this alone hand the attacker a fully authenticated session? Not by itself — back in rxkad_verify_response(), the extracted session key is used to decrypt the response's encrypted block (rxkad_decrypt_response()), and the epoch, connection ID, security index, and checksum inside must match. But that is precisely the point: the security boundary of the whole handshake is "the ticket decrypted under the server's key", and the unchecked return value silently demotes it to "the ticket is whatever bytes the client sent". The parser was never designed to run on unauthenticated input, yet that is exactly what it was doing. A bug like this is a foothold: it gives an attacker a fully controllable buffer flowing through a privileged parser, and it erases the assumption every later check is built on.

How old is it

The Fixes: tag on the fix commit points all the way back:

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")

That commit, by David Howells, is dated 2007-04-26 — the original rxkad implementation, added when the new AF_RXRPC was introduced. And the bug is literally there in the initial version of the function:

	sg_init_one(&ssg[0], ticket, ticket_len);
	memcpy(dsg, ssg, sizeof(dsg));
	crypto_blkcipher_decrypt_iv(&desc, dsg, ssg, ticket_len);

	p = ticket;
	end = p + ticket_len;

Same shape: decrypt into the buffer, ignore the result, start parsing. The archaeology is easy to reproduce:

$ git log --oneline -S 'crypto_skcipher_decrypt(req)' -- net/rxrpc/rxkad.c
432042e25e33 net/rxrpc: Reimplement DES-PCBC using DES library
97b768514a6e net/rxrpc: Use local FCrypt-PCBC implementation
1afe593b4239 rxrpc: Use skcipher

$ git log -L '/^static int rxkad_decrypt_ticket/,/^}/:net/rxrpc/rxkad.c'

The -L trace shows the function being touched again and again over nineteen years — 1afe593b4239 ("rxrpc: Use skcipher", 2016) mechanically converted crypto_blkcipher_decrypt_iv() to crypto_skcipher_decrypt(); a263629da519 moved the SG list off the stack; ef68622da9cc reworked error handling; fb46f6ee10e7 added the abort-reason tracepoints; 10674a03c633 converted time_t to time64_t — and every single change faithfully preserved the discarded return value. The unchecked call survived three generations of crypto API (blkcipher → skcipher → the later DES-library rewrite) without anyone asking whether it could fail. Nineteen years, in a security class, in a function whose entire job is to establish trust. "This code is ancient, surely someone has checked it" is not an argument; it's often exactly backwards. Old code has mostly been ported, not audited.

The fix

The patch, in full:

@@ -958,6 +958,7 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
 	struct in_addr addr;
 	unsigned int life;
 	time64_t issue, now;
+	int ret;
 	bool little_endian;
 	u8 *p, *q, *name, *end;

@@ -977,8 +978,11 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
 	sg_init_one(&sg[0], ticket, ticket_len);
 	skcipher_request_set_callback(req, 0, NULL, NULL);
 	skcipher_request_set_crypt(req, sg, sg, ticket_len, iv.x);
-	crypto_skcipher_decrypt(req);
+	ret = crypto_skcipher_decrypt(req);
 	skcipher_request_free(req);
+	if (ret < 0)
+		return rxrpc_abort_conn(conn, skb, RXKADBADTICKET, -EPROTO,
+					rxkad_abort_resp_tkt_short);

 	p = ticket;
 	end = p + ticket_len;

Three things to note:

How it was investigated / verified

Honestly: this was code-path analysis, not a crash chase. There was no oops, no KASAN report, and I have no runtime logs to show — so there are none in this post. The chain of reasoning was:

The fix was then verified by reading it against the tree (it applies cleanly, the abort path is well-formed, no leak of req or ticket — the caller frees ticket on the error path) and by review on the mailing list.

Upstream

I sent the patch on 2026-04-08. David Howells (the RxRPC/AFS maintainer — and the author of the 2007 code being fixed) picked it up into his rxrpc series, and it was applied to net-next and merged into mainline the same day as commit fe4447cd95623b1cfacc15f280aab73a6d7340b2 ("rxrpc: reject undecryptable rxkad response tickets"), signed off by Jakub Kicinski, with a Cc to stable for backporting.

The credits from the commit message, because they matter — this was a team effort:

Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Co-developed-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Yuan Tan <yuantan098@gmail.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Yuqi Xu <xuyuqiabc@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

One postscript: shortly afterwards, commit 432042e25e33 ("net/rxrpc: Reimplement DES-PCBC using DES library") replaced the skcipher machinery in this function with a direct DES-PCBC implementation — and kept the rejection, now as an explicit ticket_len % DES_BLOCK_SIZE != 0 check with the same RXKADBADTICKET abort. The fix survived the rewrite, which is exactly what you want to see.

Takeaway

Every cryptographic operation can fail, and the failure mode is rarely the one you first imagine — here it wasn't a padding oracle or a weak cipher, just a length check inside the crypto API. The rule is simple: the return value of a crypto_* operation is part of the security boundary, not a style detail. Decide explicitly what failure means — and make sure the data does not move on until you have.