[PATCH net v10 10/14] rxrpc: Fix RxGK key parser to check enctype is supported

[netdev-bot+sashiko at kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] rxrpc_preparse_xdr_yfs_rxgk() charges the full token cost to
  prep->quotalen at net/rxrpc/key.c:209 (`prep->quotalen += datalen +…

--

rxrpc: Fix RxGK key parser to check enctype is supported

The RxGK key parser now looks up the userspace-supplied encryption type
with crypto_krb5_find_enctype() and rejects the token if the enctype is
unknown or if the supplied key length does not match the enctype.  The
RxGK-specific token handling is also made conditional on CONFIG_RXGK.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com

> diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
> index cbd26da449510..cdef609996d28 100644
> --- a/net/rxrpc/key.c
> +++ b/net/rxrpc/key.c

[ ... ]

> @@ -166,12 +167,14 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
>  				       size_t datalen,
>  				       const __be32 *xdr, unsigned int toklen)
>  {
> +	const struct krb5_enctype *enc;
>  	struct rxrpc_key_token *token, **pptoken;
>  	time64_t expiry;
>  	size_t plen;
>  	const __be32 *ticket, *key;
>  	s64 tmp;
>  	size_t raw_keylen, raw_tktlen, keylen, tktlen;
> +	int ret = -EKEYREJECTED;
>  
>  	_enter(",{%x,%x,%x,%x},%x",
>  	       ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),

[ ... ]

> @@ -229,6 +232,17 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
>  	token->rxgk->key.data	= token->rxgk->_key;
>  	token->rxgk->ticket.len = raw_tktlen;
>  
> +	/* Check the enctype is supported. */
> +	enc = crypto_krb5_find_enctype(token->rxgk->enctype);
> +	if (!enc) {
> +		ret = -ENOPKG;
> +		goto reject_token;
> +	}

[Severity: Medium]
Can this new -ENOPKG path leave prep->quotalen charged for memory that
was freed?

Earlier in rxrpc_preparse_xdr_yfs_rxgk() the whole token cost is added to
the quota before any of these checks run:

	plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
	prep->quotalen += datalen + plen;

with tktlen bounded only by AFSTOKEN_GK_TOKEN_MAX (16384).  The
reject_token label then does kfree(token->rxgk) and kfree(token) without
subtracting that amount again.

Every pre-existing reject path here returned -EKEYREJECTED, -EKEYEXPIRED
or -ENOMEM, and rxrpc_preparse_xdr() turns those into an abort:

		switch (ret2) {
		case 0:
			ret = 0;
			break;
		case -EPROTONOSUPPORT:
			break;
		case -ENOPKG:
			if (ret != 0)
				ret = -ENOPKG;
			break;
		default:
			ret = ret2;
			goto error;
		}

-ENOPKG is the soft case: if another token in the same payload (for
example an RXKAD token) parsed successfully, ret stays 0 and the key is
still instantiated, so __key_instantiate_and_link() ->
key_payload_reserve(key, prep->quotalen) commits the inflated value
against the owner's key quota:

security/keys/key.c:key_payload_reserve() {
	...
		if (delta > 0 &&
		    (key->user->qnbytes + delta > maxbytes ||
		     key->user->qnbytes + delta < key->user->qnbytes)) {
			ret = -EDQUOT;
	...
}

Since key_quota_maxbytes defaults to 20000 bytes, a single crafted
add_key("rxrpc", "afs at ...", ...) payload containing one valid RXKAD token
plus an RxGK token with an unsupported enctype appears to be able to
charge roughly 16KB that is not actually held, making later add_key()
calls for that uid fail with -EDQUOT.

Would it be better to do the enctype lookup and key length check before
the prep->quotalen += datalen + plen accounting, or to subtract
datalen + plen on the -ENOPKG path?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com