[PATCH] SUNRPC: restrict integrity replies to authenticated payload

Jérémy Jean posted 1 patch 4 days, 23 hours ago
There is a newer version of this series
net/sunrpc/auth_gss/auth_gss.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
[PATCH] SUNRPC: restrict integrity replies to authenticated payload
Posted by Jérémy Jean 4 days, 23 hours ago
gss_unwrap_resp_integ() authenticates only databody_integ. The checksum
object and any bytes after it are not covered by the integrity check, but
remain visible to the XDR decoder.

This makes RPCSEC_GSS reply payloads malleable by bypassing crypto
integrity. A modified reply can contain only the RPCSEC_GSS sequence
number in databody_integ, reuse the reply verifier MIC as the body MIC,
and append bytes that the decoder consumes without invalidating the MIC.

Decode the checksum length into mic.len and truncate the decode stream
after MIC verification so only the authenticated payload remains visible.
Reject bodies shorter than the mandatory sequence number before using the
body length.

Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/sunrpc/auth_gss/auth_gss.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 8ddc65e894da..bd0457abc5fe 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -2001,7 +2001,7 @@ gss_unwrap_resp_integ(struct rpc_task *task, struct rpc_cred *cred,
 	/* opaque databody_integ<>; */
 	if (xdr_stream_decode_u32(xdr, &len))
 		goto unwrap_failed;
-	if (len & 3)
+	if (len < XDR_UNIT || len & 3)
 		goto unwrap_failed;
 	offset = rcv_buf->len - xdr_stream_remaining(xdr);
 	if (xdr_stream_decode_u32(xdr, &seqno))
@@ -2021,13 +2021,12 @@ gss_unwrap_resp_integ(struct rpc_task *task, struct rpc_cred *cred,
 
 	/* opaque checksum<>; */
 	offset += len;
-	if (xdr_decode_word(rcv_buf, offset, &len))
+	if (xdr_decode_word(rcv_buf, offset, &mic.len))
 		goto unwrap_failed;
 	offset += sizeof(__be32);
-	if (offset + len > rcv_buf->len)
+	if (offset + mic.len > rcv_buf->len)
 		goto unwrap_failed;
-	mic.len = len;
-	mic.data = kmalloc(len, GFP_KERNEL);
+	mic.data = kmalloc(mic.len, GFP_KERNEL);
 	if (ZERO_OR_NULL_PTR(mic.data))
 		goto unwrap_failed;
 	if (read_bytes_from_xdr_buf(rcv_buf, offset, mic.data, mic.len))
@@ -2038,6 +2037,12 @@ gss_unwrap_resp_integ(struct rpc_task *task, struct rpc_cred *cred,
 		clear_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags);
 	if (maj_stat != GSS_S_COMPLETE)
 		goto bad_mic;
+	if (xdr_stream_remaining(xdr) < len - XDR_UNIT)
+		goto unwrap_failed;
+
+	/* Expose only the authenticated payload to the decoder. */
+	xdr_truncate_decode(xdr, xdr_stream_remaining(xdr) -
+			    (len - XDR_UNIT));
 
 	gss_update_rslack(task, cred, 2, 2 + 1 + XDR_QUADLEN(mic.len));
 	ret = 0;
-- 
2.47.3

Re: [PATCH] SUNRPC: restrict integrity replies to authenticated payload
Posted by Chuck Lever 4 days, 2 hours ago
On 9/19/26 5:20 PM, Jérémy Jean wrote:
> Decode the checksum length into mic.len and truncate the decode
> stream after MIC verification so only the authenticated payload
> remains visible.

Your analysis holds, and I don't see a relevant fix already queued
up.


> +	if (xdr_stream_remaining(xdr) < len - XDR_UNIT)
> +		goto unwrap_failed;

This branch cannot be taken. xdr_buf_subsegment() returns -1 unless
offset + len <= rcv_buf->len, and @offset was computed as
rcv_buf->len - xdr_stream_remaining(xdr) just above. Decoding the
sequence number then drops xdr_stream_remaining() by XDR_UNIT, so
here it is at least len - XDR_UNIT. Nothing in between disturbs
that: xdr_decode_word(), read_bytes_from_xdr_buf(), and
gss_verify_mic() only read. The new "len < XDR_UNIT" test already
keeps the subtraction from wrapping, which is the other input this
check could have caught.

One observation that needs no change: gss_krb5_verify_mic_v2()
covers only cksum_len bytes of the token, so an oversized checksum
length word verifies too, and mic.len still feeds
gss_update_rslack(). rcv_buf->len bounds the value, so an attacker
gains nothing but a poor slack estimate.

The rest LGTM.

net/sunrpc/auth_gss/auth_gss.c is NFS client code, so this one goes
through Trond and Anna rather than the NFSD tree.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)