[PATCH] ksmbd: verify transform SessionId matches the decrypted header

[Dairui Zhang <zhangdairui@gmail.com>]

When an encrypted request comes in, the decryption key is picked by
the SessionId in the transform header, but the request is then
authorized under whatever session the decrypted inner header names.
Nothing ever compares the two, so on a connection with more than
one session a client can get a request decrypted with one session's
key and run it as another session. Encrypted requests are exempt
from signing too, so a valid AEAD tag is the only proof of session
identity, and it is checked against the wrong session.

MS-SMB2 says the server must check that the transform SessionId
matches the one in the decrypted SMB2 header and treat a mismatch
as a protocol error. Add that check before dispatching: the message
has to be at least one fixed SMB2 header, the first operation must
not set SMB2_FLAGS_RELATED_OPERATIONS and its SessionId must match
the transform SessionId, and every following operation in a
compound chain must either set RELATED or carry the same SessionId.
The usual ULLONG_MAX wildcard is accepted for those, same as the
existing compound session check, since it means the compound's
session and cannot be a different session. NextCommand offsets must
be 8-byte aligned and stay inside the message, and the accumulated
offset is guarded against u32 overflow.

While here I noticed there is no decompression step after
decryption, so an encrypted compression transform would be
dispatched as if it were a plain SMB2 message. ProtocolId is not
checked anywhere on that path, so a crafted transform could be made
to parse as a valid command under any session id in the payload.
Reject those for now; if encrypted compression ever comes back, the
decompression step has to be restored and the message needs the
same check after decompression.

Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v3 -> v4:
- Rebase onto ksmbd-for-next as requested.
- Guard the chain-walk offset accumulation with
  check_add_overflow() before updating it, per Namjae's review
  (an invalid NextCommand could otherwise wrap the offset and loop).
- Reject encrypted compression transforms outright: for-next has no
  decompression step after decryption, and an undecoded transform
  would otherwise be dispatched as an attacker-crafted command
  (ProtocolId is not validated downstream).
- Exempt SessionId == ULLONG_MAX in subsequent compound operations,
  matching the existing compound session-check behavior (it means
  the compound's session and cannot be a confused deputy).
---
 fs/smb/server/smb2pdu.c | 85 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index bfa8954..ec47457 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -11634,6 +11634,64 @@ bool smb3_is_transform_hdr(void *buf)
 	return trhdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM;
 }
 
+/*
+ * Validate the session binding of a decrypted message against the
+ * encryption transform SessionId, per MS-SMB2:
+ *  - the message must be at least one fixed SMB2 header;
+ *  - the first operation must not set SMB2_FLAGS_RELATED_OPERATIONS
+ *    and its SessionId must match the transform SessionId;
+ *  - each following operation in a compound chain must either set
+ *    SMB2_FLAGS_RELATED_OPERATIONS or carry the same SessionId;
+ *  - NextCommand offsets must be 8-byte aligned, and the accumulated
+ *    offset must not overflow and must stay inside the message.
+ * Returns 0 on success, -ECONNABORTED on protocol error.
+ */
+static int ksmbd_check_transform_session(struct smb2_hdr *hdr,
+				  unsigned int msg_len, __u64 tr_sess_id)
+{
+	struct smb2_hdr *in_hdr = hdr;
+	bool first = true;
+	u32 off = 0, next;
+
+	if (msg_len < sizeof(struct smb2_hdr)) {
+		pr_err_ratelimited("Decrypted message is smaller than SMB2 header\n");
+		return -ECONNABORTED;
+	}
+
+	for (;;) {
+		if (first) {
+			if (in_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS) {
+				pr_err_ratelimited("RELATED_OPERATIONS set on first operation\n");
+				return -ECONNABORTED;
+			}
+			if (le64_to_cpu(in_hdr->SessionId) != tr_sess_id) {
+				pr_err_ratelimited("SessionId mismatch between transform and inner header\n");
+				return -ECONNABORTED;
+			}
+			first = false;
+		} else if (!(in_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS) &&
+			   le64_to_cpu(in_hdr->SessionId) != ULLONG_MAX &&
+			   le64_to_cpu(in_hdr->SessionId) != tr_sess_id) {
+			pr_err_ratelimited("SessionId mismatch in compound chain\n");
+			return -ECONNABORTED;
+		}
+
+		next = le32_to_cpu(in_hdr->NextCommand);
+		if (!next)
+			return 0;
+		if (next % 8) {
+			pr_err_ratelimited("NextCommand %u is not 8-byte aligned\n", next);
+			return -ECONNABORTED;
+		}
+		if (check_add_overflow(off, next, &off) ||
+		    off + sizeof(struct smb2_hdr) > msg_len) {
+			pr_err_ratelimited("NextCommand %u is out of the message\n", next);
+			return -ECONNABORTED;
+		}
+		in_hdr = (struct smb2_hdr *)((u8 *)hdr + off);
+	}
+}
+
 int smb3_decrypt_req(struct ksmbd_work *work)
 {
 	char *buf = work->request_buf;
@@ -11641,6 +11699,7 @@ int smb3_decrypt_req(struct ksmbd_work *work)
 	struct kvec iov[2];
 	int buf_data_size = pdu_length - sizeof(struct smb2_transform_hdr);
 	struct smb2_transform_hdr *tr_hdr = smb_get_msg(buf);
+	__le32 proto;
 	int rc = 0;
 
 	if (pdu_length < sizeof(struct smb2_transform_hdr) ||
@@ -11663,6 +11722,32 @@ int smb3_decrypt_req(struct ksmbd_work *work)
 	if (rc)
 		return rc;
 
+	/*
+	 * The decryption key is selected by the transform header SessionId,
+	 * while the request is authorized under the session named in the
+	 * decrypted inner header. Per MS-SMB2 the two must match, so
+	 * validate the decrypted message before dispatching it.
+	 */
+	proto = ((struct smb2_hdr *)iov[1].iov_base)->ProtocolId;
+	if (proto == SMB2_PROTO_NUMBER) {
+		rc = ksmbd_check_transform_session(
+				(struct smb2_hdr *)iov[1].iov_base,
+				buf_data_size,
+				le64_to_cpu(tr_hdr->SessionId));
+		if (rc)
+			return rc;
+	} else if (proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+		/*
+		 * There is no decompression step after decryption, so a
+		 * compression transform would be dispatched as if it were
+		 * an SMB2 message. ProtocolId is not checked on that path,
+		 * so a crafted transform could be made to run as a valid
+		 * command under any session id in the payload.
+		 */
+		pr_err_ratelimited("Encrypted compression transform is not supported\n");
+		return -ECONNABORTED;
+	}
+
 	memmove(buf + 4, iov[1].iov_base, buf_data_size);
 	*(__be32 *)buf = cpu_to_be32(buf_data_size);
 
-- 
2.53.0