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

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

[Dairui Zhang]

The decryption key for an encrypted request is selected by the
SessionId in the encryption transform header, but the request is
then authorized under the session named in the decrypted inner SMB2
header. Nothing compares the two, so on a connection carrying more
than one session a client can have a request decrypted with one
session's key and executed under another session's identity. Since
encrypted requests are also exempt from the signing requirement, the
AEAD tag is the only proof of session identity, and it is checked
against the wrong session.

Per MS-SMB2 the server must verify that the SessionId in the
transform header matches the one in the decrypted SMB2 header and
treat a mismatch as a protocol error. Compare them after decryption
and drop the connection on mismatch.

The check only applies to plain SMB2 payloads; a compression
transform payload carries the session id only after decompression
and is left as-is for now.

Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
 fs/smb/server/smb2pdu.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 4cf7083..e4cfbe6 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -10890,6 +10890,22 @@ 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; verify
+	 * that here and drop the connection on mismatch. A compression
+	 * transform payload carries the session id only after
+	 * decompression, so it is not covered by this check.
+	 */
+	if (((struct smb2_hdr *)iov[1].iov_base)->ProtocolId ==
+	    SMB2_PROTO_NUMBER &&
+	    le64_to_cpu(tr_hdr->SessionId) !=
+	    le64_to_cpu(((struct smb2_hdr *)iov[1].iov_base)->SessionId)) {
+		pr_err_ratelimited("SessionId mismatch between transform and inner header\n");
+		return -ECONNABORTED;
+	}
+
 	/* Drop the AEAD authentication tag from the inner RFC1002 frame. */
 	memmove(buf + 4, iov[1].iov_base, original_msg_size);
 	*(__be32 *)buf = cpu_to_be32(original_msg_size);
-- 
2.53.0

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

[Namjae Jeon]

> @@ -10890,6 +10890,22 @@ 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; verify
> +        * that here and drop the connection on mismatch. A compression
> +        * transform payload carries the session id only after
> +        * decompression, so it is not covered by this check.
> +        */
> +       if (((struct smb2_hdr *)iov[1].iov_base)->ProtocolId ==
> +           SMB2_PROTO_NUMBER &&
If an encrypted request contains a compression transform, this check
is skipped because the decrypted ProtocolId is
SMB2_COMPRESSION_TRANSFORM_ID. After decompression, the smb2 header’s
SessionId must be checked against the transform header’s SessionId.

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

[Dairui Zhang]

The decryption key for an encrypted request is selected by the
SessionId in the encryption transform header, but the request is
then authorized under the session named in the decrypted inner SMB2
header. Nothing compares the two, so on a connection carrying more
than one session a client can have a request decrypted with one
session's key and executed under another session's identity. Since
encrypted requests are also exempt from the signing requirement, the
AEAD tag is the only proof of session identity, and it is checked
against the wrong session.

Per MS-SMB2 the server must verify that the SessionId in the
transform header matches the one in the decrypted SMB2 header and
treat a mismatch as a protocol error. Compare them after decryption
and drop the connection on mismatch. When the encrypted payload is a
compression transform, the transform SessionId is saved and verified
against the decompressed SMB2 header instead, since a compression
transform header carries no SessionId.

Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v1 -> v2:
- Also cover the compression-transform case, per Namjae's review:
  save the transform SessionId during decryption and check it
  against the decompressed SMB2 header.
- Reject a decrypted SMB2 message smaller than the fixed header
  before reading its SessionId (a crafted short OriginalMessageSize
  would otherwise make the check itself read out of bounds).
- v1: https://lore.kernel.org/linux-cifs/20260926080224.1671214-1-zhangdairui@gmail.com/
---
 fs/smb/server/compress.c   | 14 ++++++++++++++
 fs/smb/server/ksmbd_work.h |  4 ++++
 fs/smb/server/smb2pdu.c    | 25 +++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/fs/smb/server/compress.c b/fs/smb/server/compress.c
index 5162fb8..1c5a070 100644
--- a/fs/smb/server/compress.c
+++ b/fs/smb/server/compress.c
@@ -125,6 +125,20 @@ int ksmbd_decompress_work_request(struct ksmbd_work *work)
 	if (rc)
 		return rc;
 
+	/*
+	 * The SessionId of the encryption transform header was saved
+	 * before the compression transform was parsed; the decompressed
+	 * SMB2 header must carry the same one. Per MS-SMB2 a mismatch is
+	 * a protocol error.
+	 */
+	if (work->tr_sess_id &&
+	    le64_to_cpu(((struct smb2_hdr *)smb_get_msg(out_buf))->SessionId) !=
+	    work->tr_sess_id) {
+		pr_err_ratelimited("SessionId mismatch between transform and decompressed header\n");
+		kvfree(out_buf);
+		return -ECONNABORTED;
+	}
+
 	kvfree(work->request_buf);
 	work->request_buf = out_buf;
 	return 0;
diff --git a/fs/smb/server/ksmbd_work.h b/fs/smb/server/ksmbd_work.h
index 5f1d3eb..3e8bf2a 100644
--- a/fs/smb/server/ksmbd_work.h
+++ b/fs/smb/server/ksmbd_work.h
@@ -82,6 +82,10 @@ struct ksmbd_work {
 	/* Contiguous SMB2 compression transform owned by this work item. */
 	void				*compress_buf;
 
+	/* SessionId from the encryption transform header, for the
+	 * post-decompression SessionId check. Zero when unset. */
+	__u64				tr_sess_id;
+
 	unsigned char			state;
 	/* No response for cancelled request */
 	bool                            send_no_response:1;
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 4cf7083..9eb13a8 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -10860,6 +10860,7 @@ int smb3_decrypt_req(struct ksmbd_work *work)
 	unsigned int buf_data_size;
 	struct smb2_transform_hdr *tr_hdr = smb_get_msg(buf);
 	unsigned int original_msg_size;
+	__le32 proto;
 	int rc = 0;
 
 	if (pdu_length < sizeof(struct smb2_transform_hdr)) {
@@ -10890,6 +10891,30 @@ 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; verify
+	 * that here and drop the connection on mismatch. A compression
+	 * transform payload carries the session id only after
+	 * decompression, so save the transform SessionId for the check
+	 * after decompression instead.
+	 */
+	proto = ((struct smb2_hdr *)iov[1].iov_base)->ProtocolId;
+	if (proto == SMB2_PROTO_NUMBER) {
+		if (original_msg_size < sizeof(struct smb2_hdr)) {
+			pr_err_ratelimited("Decrypted SMB2 message is too small\n");
+			return -ECONNABORTED;
+		}
+		if (le64_to_cpu(tr_hdr->SessionId) !=
+		    le64_to_cpu(((struct smb2_hdr *)iov[1].iov_base)->SessionId)) {
+			pr_err_ratelimited("SessionId mismatch between transform and inner header\n");
+			return -ECONNABORTED;
+		}
+	} else if (proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+		work->tr_sess_id = le64_to_cpu(tr_hdr->SessionId);
+	}
+
 	/* Drop the AEAD authentication tag from the inner RFC1002 frame. */
 	memmove(buf + 4, iov[1].iov_base, original_msg_size);
 	*(__be32 *)buf = cpu_to_be32(original_msg_size);
-- 
2.53.0

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

[Namjae Jeon]

On Sun, Sep 27, 2026 at 3:16 PM Dairui Zhang <zhangdairui@gmail.com> wrote:
>
> The decryption key for an encrypted request is selected by the
> SessionId in the encryption transform header, but the request is
> then authorized under the session named in the decrypted inner SMB2
> header. Nothing compares the two, so on a connection carrying more
> than one session a client can have a request decrypted with one
> session's key and executed under another session's identity. Since
> encrypted requests are also exempt from the signing requirement, the
> AEAD tag is the only proof of session identity, and it is checked
> against the wrong session.
>
> Per MS-SMB2 the server must verify that the SessionId in the
> transform header matches the one in the decrypted SMB2 header and
> treat a mismatch as a protocol error. Compare them after decryption
> and drop the connection on mismatch. When the encrypted payload is a
> compression transform, the transform SessionId is saved and verified
> against the decompressed SMB2 header instead, since a compression
> transform header carries no SessionId.

Could you extend this patch to validate the decrypted SMB2 message
against the requirements below before dispatching any command? The
current implementation checks the SessionId only in the first SMB2
header. It does not validate subsequent operations in the compound
chain.

For a singleton request and the first operation of a compounded request,
 The size of the decrypted message is less than the size of the SMB2 Header
 SMB2_FLAGS_RELATED_OPERATIONS is set in the Flags field of the SMB2 header of
the request
 The SessionId field in the SMB2 header of the request is not equal to
Request.TransformSessionId.
 In a compounded request, for each operation in the compounded chain
except the first
one, SMB2_FLAGS_RELATED_OPERATIONS is not set in the Flags field of the SMB2
header of the operation and SessionId in the SMB2 header of the
operation is not equal
to Request.TransformSessionId.
 In a compounded request, each response in a compounded chain, except
the first one,
does not start at an 8-byte aligned boundary.

Thanks!

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

[Dairui Zhang]

The decryption key for an encrypted request is selected by the
SessionId in the encryption transform header, but the request is
then authorized under the session named in the decrypted inner SMB2
header. Nothing compares the two, so on a connection carrying more
than one session a client can have a request decrypted with one
session's key and executed under another session's identity. Since
encrypted requests are also exempt from the signing requirement, the
AEAD tag is the only proof of session identity, and it is checked
against the wrong session.

Per MS-SMB2 the server must verify that the SessionId in the
transform header matches the one in the decrypted SMB2 header and
treat a mismatch as a protocol error. Validate the whole decrypted
message before dispatching it: 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 subsequent operation in a compound chain
must either set SMB2_FLAGS_RELATED_OPERATIONS or carry the same
SessionId; and NextCommand offsets must be 8-byte aligned and within
the message. When the encrypted payload is a compression transform,
the transform SessionId is saved during decryption and the same
validation runs on the decompressed message. (The 8-byte alignment
of compound responses is already handled by the existing
chained-response padding.)

Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v2 -> v3:
- Per review, extend the check to the whole decrypted message before
  dispatch: first-op size/RELATED_OPERATIONS/SessionId rules and
  per-op compound-chain SessionId and NextCommand validation, shared
  between the plain and compression paths via a common helper.
v1 -> v2:
- Cover the compression-transform case.
- Reject a decrypted SMB2 message smaller than the fixed header
  before reading its SessionId.
---
 fs/smb/server/compress.c   | 17 +++++++++
 fs/smb/server/ksmbd_work.h |  4 ++
 fs/smb/server/smb2pdu.c    | 78 ++++++++++++++++++++++++++++++++++++++
 fs/smb/server/smb2pdu.h    |  2 +
 4 files changed, 101 insertions(+)

diff --git a/fs/smb/server/compress.c b/fs/smb/server/compress.c
index 5162fb8..b07b140 100644
--- a/fs/smb/server/compress.c
+++ b/fs/smb/server/compress.c
@@ -10,6 +10,7 @@
 
 #include "compress.h"
 #include "smb_common.h"
+#include "smb2pdu.h"
 #include "../common/compress/lz77.h"
 
 #define SMB_COMPRESS_MIN_LEN	PAGE_SIZE
@@ -125,6 +126,22 @@ int ksmbd_decompress_work_request(struct ksmbd_work *work)
 	if (rc)
 		return rc;
 
+	/*
+	 * The encryption transform SessionId was saved before the
+	 * compression transform was parsed; the decompressed message must
+	 * pass the same session binding validation as a plain decrypted
+	 * message.
+	 */
+	if (work->tr_sess_id) {
+		rc = ksmbd_check_transform_session(
+				(struct smb2_hdr *)smb_get_msg(out_buf),
+				get_rfc1002_len(out_buf), work->tr_sess_id);
+		if (rc) {
+			kvfree(out_buf);
+			return rc;
+		}
+	}
+
 	kvfree(work->request_buf);
 	work->request_buf = out_buf;
 	return 0;
diff --git a/fs/smb/server/ksmbd_work.h b/fs/smb/server/ksmbd_work.h
index 5f1d3eb..3e8bf2a 100644
--- a/fs/smb/server/ksmbd_work.h
+++ b/fs/smb/server/ksmbd_work.h
@@ -82,6 +82,10 @@ struct ksmbd_work {
 	/* Contiguous SMB2 compression transform owned by this work item. */
 	void				*compress_buf;
 
+	/* SessionId from the encryption transform header, for the
+	 * post-decompression SessionId check. Zero when unset. */
+	__u64				tr_sess_id;
+
 	unsigned char			state;
 	/* No response for cancelled request */
 	bool                            send_no_response:1;
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 4cf7083..dbb7dcb 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -10852,6 +10852,62 @@ 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 inside the message.
+ * Returns 0 on success, -ECONNABORTED on protocol error.
+ */
+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) != 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;
+		}
+		off += next;
+		if (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;
@@ -10860,6 +10916,7 @@ int smb3_decrypt_req(struct ksmbd_work *work)
 	unsigned int buf_data_size;
 	struct smb2_transform_hdr *tr_hdr = smb_get_msg(buf);
 	unsigned int original_msg_size;
+	__le32 proto;
 	int rc = 0;
 
 	if (pdu_length < sizeof(struct smb2_transform_hdr)) {
@@ -10890,6 +10947,27 @@ 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 whole decrypted message before dispatching it. A
+	 * compression transform payload carries the session id only after
+	 * decompression, so save the transform SessionId for that check
+	 * instead.
+	 */
+	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,
+				original_msg_size,
+				le64_to_cpu(tr_hdr->SessionId));
+		if (rc)
+			return rc;
+	} else if (proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+		work->tr_sess_id = le64_to_cpu(tr_hdr->SessionId);
+	}
+
 	/* Drop the AEAD authentication tag from the inner RFC1002 frame. */
 	memmove(buf + 4, iov[1].iov_base, original_msg_size);
 	*(__be32 *)buf = cpu_to_be32(original_msg_size);
diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
index 1836259..2cdeb56 100644
--- a/fs/smb/server/smb2pdu.h
+++ b/fs/smb/server/smb2pdu.h
@@ -399,6 +399,8 @@ struct channel *lookup_chann_list(struct ksmbd_session *sess,
 void smb3_preauth_hash_rsp(struct ksmbd_work *work);
 bool smb3_is_transform_hdr(void *buf);
 int smb3_decrypt_req(struct ksmbd_work *work);
+int ksmbd_check_transform_session(struct smb2_hdr *hdr,
+				  unsigned int msg_len, __u64 tr_sess_id);
 int smb3_encrypt_resp(struct ksmbd_work *work);
 bool smb3_11_final_sess_setup_resp(struct ksmbd_work *work);
 int smb2_set_rsp_credits(struct ksmbd_work *work);
-- 
2.53.0

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

[Greg KH]

On Mon, Sep 28, 2026 at 11:13:00AM +0800, Dairui Zhang wrote:
> The decryption key for an encrypted request is selected by the
> SessionId in the encryption transform header, but the request is
> then authorized under the session named in the decrypted inner SMB2
> header. Nothing compares the two, so on a connection carrying more
> than one session a client can have a request decrypted with one
> session's key and executed under another session's identity. Since
> encrypted requests are also exempt from the signing requirement, the
> AEAD tag is the only proof of session identity, and it is checked
> against the wrong session.
> 
> Per MS-SMB2 the server must verify that the SessionId in the
> transform header matches the one in the decrypted SMB2 header and
> treat a mismatch as a protocol error. Validate the whole decrypted
> message before dispatching it: 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 subsequent operation in a compound chain
> must either set SMB2_FLAGS_RELATED_OPERATIONS or carry the same
> SessionId; and NextCommand offsets must be 8-byte aligned and within
> the message. When the encrypted payload is a compression transform,
> the transform SessionId is saved during decryption and the same
> validation runs on the decompressed message. (The 8-byte alignment
> of compound responses is already handled by the existing
> chained-response padding.)
> 
> Reported-by: Dairui Zhang <zhangdairui@gmail.com>
> Assisted-by: LLM
> Cc: stable@vger.kernel.org
> Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
> ---
> v2 -> v3:
> - Per review, extend the check to the whole decrypted message before
>   dispatch: first-op size/RELATED_OPERATIONS/SessionId rules and
>   per-op compound-chain SessionId and NextCommand validation, shared
>   between the plain and compression paths via a common helper.

The "v3" isn't up in the Subject line for some reason :(

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

[Dairui Zhang]

Greg KH wrote:
> The "v3" isn't up in the Subject line for some reason :(

You're right, my mistake - I forgot the version marker when
respinning (all three went out as bare [PATCH] with the same
subject). Resending as [PATCH v3] now; no code changes.

Thanks,
Dairui Zhang

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

[Dairui Zhang]

The decryption key for an encrypted request is selected by the
SessionId in the encryption transform header, but the request is
then authorized under the session named in the decrypted inner SMB2
header. Nothing compares the two, so on a connection carrying more
than one session a client can have a request decrypted with one
session's key and executed under another session's identity. Since
encrypted requests are also exempt from the signing requirement, the
AEAD tag is the only proof of session identity, and it is checked
against the wrong session.

Per MS-SMB2 the server must verify that the SessionId in the
transform header matches the one in the decrypted SMB2 header and
treat a mismatch as a protocol error. Validate the whole decrypted
message before dispatching it: 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 subsequent operation in a compound chain
must either set SMB2_FLAGS_RELATED_OPERATIONS or carry the same
SessionId; and NextCommand offsets must be 8-byte aligned and within
the message. When the encrypted payload is a compression transform,
the transform SessionId is saved during decryption and the same
validation runs on the decompressed message. (The 8-byte alignment
of compound responses is already handled by the existing
chained-response padding.)

Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v2 -> v3:
- Per review, extend the check to the whole decrypted message before
  dispatch: first-op size/RELATED_OPERATIONS/SessionId rules and
  per-op compound-chain SessionId and NextCommand validation, shared
  between the plain and compression paths via a common helper.
v1 -> v2:
- Cover the compression-transform case.
- Reject a decrypted SMB2 message smaller than the fixed header
  before reading its SessionId.
---
Resent with the [PATCH v3] subject marker Greg pointed out was
missing; no code changes from the previous submission.
---
 fs/smb/server/compress.c   | 17 +++++++++
 fs/smb/server/ksmbd_work.h |  4 ++
 fs/smb/server/smb2pdu.c    | 78 ++++++++++++++++++++++++++++++++++++++
 fs/smb/server/smb2pdu.h    |  2 +
 4 files changed, 101 insertions(+)

diff --git a/fs/smb/server/compress.c b/fs/smb/server/compress.c
index 5162fb8..b07b140 100644
--- a/fs/smb/server/compress.c
+++ b/fs/smb/server/compress.c
@@ -10,6 +10,7 @@
 
 #include "compress.h"
 #include "smb_common.h"
+#include "smb2pdu.h"
 #include "../common/compress/lz77.h"
 
 #define SMB_COMPRESS_MIN_LEN	PAGE_SIZE
@@ -125,6 +126,22 @@ int ksmbd_decompress_work_request(struct ksmbd_work *work)
 	if (rc)
 		return rc;
 
+	/*
+	 * The encryption transform SessionId was saved before the
+	 * compression transform was parsed; the decompressed message must
+	 * pass the same session binding validation as a plain decrypted
+	 * message.
+	 */
+	if (work->tr_sess_id) {
+		rc = ksmbd_check_transform_session(
+				(struct smb2_hdr *)smb_get_msg(out_buf),
+				get_rfc1002_len(out_buf), work->tr_sess_id);
+		if (rc) {
+			kvfree(out_buf);
+			return rc;
+		}
+	}
+
 	kvfree(work->request_buf);
 	work->request_buf = out_buf;
 	return 0;
diff --git a/fs/smb/server/ksmbd_work.h b/fs/smb/server/ksmbd_work.h
index 5f1d3eb..3e8bf2a 100644
--- a/fs/smb/server/ksmbd_work.h
+++ b/fs/smb/server/ksmbd_work.h
@@ -82,6 +82,10 @@ struct ksmbd_work {
 	/* Contiguous SMB2 compression transform owned by this work item. */
 	void				*compress_buf;
 
+	/* SessionId from the encryption transform header, for the
+	 * post-decompression SessionId check. Zero when unset. */
+	__u64				tr_sess_id;
+
 	unsigned char			state;
 	/* No response for cancelled request */
 	bool                            send_no_response:1;
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 4cf7083..dbb7dcb 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -10852,6 +10852,62 @@ 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 inside the message.
+ * Returns 0 on success, -ECONNABORTED on protocol error.
+ */
+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) != 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;
+		}
+		off += next;
+		if (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;
@@ -10860,6 +10916,7 @@ int smb3_decrypt_req(struct ksmbd_work *work)
 	unsigned int buf_data_size;
 	struct smb2_transform_hdr *tr_hdr = smb_get_msg(buf);
 	unsigned int original_msg_size;
+	__le32 proto;
 	int rc = 0;
 
 	if (pdu_length < sizeof(struct smb2_transform_hdr)) {
@@ -10890,6 +10947,27 @@ 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 whole decrypted message before dispatching it. A
+	 * compression transform payload carries the session id only after
+	 * decompression, so save the transform SessionId for that check
+	 * instead.
+	 */
+	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,
+				original_msg_size,
+				le64_to_cpu(tr_hdr->SessionId));
+		if (rc)
+			return rc;
+	} else if (proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+		work->tr_sess_id = le64_to_cpu(tr_hdr->SessionId);
+	}
+
 	/* Drop the AEAD authentication tag from the inner RFC1002 frame. */
 	memmove(buf + 4, iov[1].iov_base, original_msg_size);
 	*(__be32 *)buf = cpu_to_be32(original_msg_size);
diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
index 1836259..2cdeb56 100644
--- a/fs/smb/server/smb2pdu.h
+++ b/fs/smb/server/smb2pdu.h
@@ -399,6 +399,8 @@ struct channel *lookup_chann_list(struct ksmbd_session *sess,
 void smb3_preauth_hash_rsp(struct ksmbd_work *work);
 bool smb3_is_transform_hdr(void *buf);
 int smb3_decrypt_req(struct ksmbd_work *work);
+int ksmbd_check_transform_session(struct smb2_hdr *hdr,
+				  unsigned int msg_len, __u64 tr_sess_id);
 int smb3_encrypt_resp(struct ksmbd_work *work);
 bool smb3_11_final_sess_setup_resp(struct ksmbd_work *work);
 int smb2_set_rsp_credits(struct ksmbd_work *work);
-- 
2.53.0

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

[Namjae Jeon]

> +       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) != 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;
> +               }
> +               off += next;
Please check for overflow before updating off variable. Invalid
NextCommand can wrap the offset and cause an infinite loop.

And this patch does not apply cleanly to #ksmbd-for-next branch.
Please rebase it on the latest kernel(7.3-rc5) or #ksmbd-for-next and
resend it.

Thanks.

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

[Dairui Zhang]

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

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

[Namjae Jeon]

> @@ -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;
__handle_ksmbd_work() decompresses a compression transform after
decryption via ksmbd_decompress_work_request(). It would regress that
behavior...