All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ksmbd: disable SMB2_GLOBAL_CAP_ENCRYPTION for SMB 3.1.1
@ 2021-12-16  0:31 Marcos Del Sol Vives
  2021-12-16  1:14 ` Namjae Jeon
  0 siblings, 1 reply; 2+ messages in thread
From: Marcos Del Sol Vives @ 2021-12-16  0:31 UTC (permalink / raw)
  To: linux-cifs; +Cc: Marcos Del Sol Vives, linux-kernel, Namjae Jeon

According to the official Microsoft MS-SMB2 document section 3.3.5.4, this
flag should be used only for 3.0 and 3.0.2 dialects. Setting it for 3.1.1
is a violation of the specification.

This causes my Windows 10 client to detect an anomaly in the negotiation,
and disable encryption entirely despite being explicitly enabled in ksmbd,
causing all data transfers to go in plain text.

Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>
Cc: linux-kernel@vger.kernel.org
Cc: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2ops.c |  3 ---
 fs/ksmbd/smb2pdu.c | 25 +++++++++++++++++++++----
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/fs/ksmbd/smb2ops.c b/fs/ksmbd/smb2ops.c
index 0a5d8450e835..02a44d28bdaf 100644
--- a/fs/ksmbd/smb2ops.c
+++ b/fs/ksmbd/smb2ops.c
@@ -271,9 +271,6 @@ int init_smb3_11_server(struct ksmbd_conn *conn)
 	if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES)
 		conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING;
 
-	if (conn->cipher_type)
-		conn->vals->capabilities |= SMB2_GLOBAL_CAP_ENCRYPTION;
-
 	if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL)
 		conn->vals->capabilities |= SMB2_GLOBAL_CAP_MULTI_CHANNEL;
 
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 49c9da37315c..6193d5a1d653 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -915,6 +915,25 @@ static void decode_encrypt_ctxt(struct ksmbd_conn *conn,
 	}
 }
 
+/**
+ * should_encrypt() - checks if connection should be encrypted
+ * @conn:	smb connection
+ *
+ * Return:	true if should be encrypted, else false
+ */
+static bool should_encrypt(struct ksmbd_conn *conn)
+{
+	if (!conn->ops->generate_encryptionkey)
+		return false;
+
+	/*
+	 * SMB 3.0 and 3.0.2 dialects use the SMB2_GLOBAL_CAP_ENCRYPTION flag.
+	 * SMB 3.1.1 uses the cipher_type field.
+	 */
+	return (conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION) ||
+	    conn->cipher_type;
+}
+
 static void decode_compress_ctxt(struct ksmbd_conn *conn,
 				 struct smb2_compression_capabilities_context *pneg_ctxt)
 {
@@ -1469,8 +1488,7 @@ static int ntlm_authenticate(struct ksmbd_work *work)
 		    (req->SecurityMode & SMB2_NEGOTIATE_SIGNING_REQUIRED))
 			sess->sign = true;
 
-		if (conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION &&
-		    conn->ops->generate_encryptionkey &&
+		if (should_encrypt(conn) &&
 		    !(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
 			rc = conn->ops->generate_encryptionkey(sess);
 			if (rc) {
@@ -1559,8 +1577,7 @@ static int krb5_authenticate(struct ksmbd_work *work)
 	    (req->SecurityMode & SMB2_NEGOTIATE_SIGNING_REQUIRED))
 		sess->sign = true;
 
-	if ((conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION) &&
-	    conn->ops->generate_encryptionkey) {
+	if (should_encrypt(conn)) {
 		retval = conn->ops->generate_encryptionkey(sess);
 		if (retval) {
 			ksmbd_debug(SMB,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ksmbd: disable SMB2_GLOBAL_CAP_ENCRYPTION for SMB 3.1.1
  2021-12-16  0:31 [PATCH] ksmbd: disable SMB2_GLOBAL_CAP_ENCRYPTION for SMB 3.1.1 Marcos Del Sol Vives
@ 2021-12-16  1:14 ` Namjae Jeon
  0 siblings, 0 replies; 2+ messages in thread
From: Namjae Jeon @ 2021-12-16  1:14 UTC (permalink / raw)
  To: Marcos Del Sol Vives; +Cc: linux-cifs, linux-kernel

2021-12-16 9:31 GMT+09:00, Marcos Del Sol Vives <marcos@orca.pet>:
> According to the official Microsoft MS-SMB2 document section 3.3.5.4, this
> flag should be used only for 3.0 and 3.0.2 dialects. Setting it for 3.1.1
> is a violation of the specification.
>
> This causes my Windows 10 client to detect an anomaly in the negotiation,
> and disable encryption entirely despite being explicitly enabled in ksmbd,
> causing all data transfers to go in plain text.
>
> Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>
> Cc: linux-kernel@vger.kernel.org
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  fs/ksmbd/smb2ops.c |  3 ---
>  fs/ksmbd/smb2pdu.c | 25 +++++++++++++++++++++----
>  2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ksmbd/smb2ops.c b/fs/ksmbd/smb2ops.c
> index 0a5d8450e835..02a44d28bdaf 100644
> --- a/fs/ksmbd/smb2ops.c
> +++ b/fs/ksmbd/smb2ops.c
> @@ -271,9 +271,6 @@ int init_smb3_11_server(struct ksmbd_conn *conn)
>  	if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES)
>  		conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING;
>
> -	if (conn->cipher_type)
> -		conn->vals->capabilities |= SMB2_GLOBAL_CAP_ENCRYPTION;
> -
>  	if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL)
>  		conn->vals->capabilities |= SMB2_GLOBAL_CAP_MULTI_CHANNEL;
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 49c9da37315c..6193d5a1d653 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -915,6 +915,25 @@ static void decode_encrypt_ctxt(struct ksmbd_conn
> *conn,
>  	}
>  }
>
> +/**
> + * should_encrypt() - checks if connection should be encrypted
> + * @conn:	smb connection
> + *
> + * Return:	true if should be encrypted, else false
> + */
> +static bool should_encrypt(struct ksmbd_conn *conn)
Can you change function name to smb3_encryption_negotiated() ?
And need to update function description also.

Thanks for your patch!
> +{
> +	if (!conn->ops->generate_encryptionkey)
> +		return false;
> +
> +	/*
> +	 * SMB 3.0 and 3.0.2 dialects use the SMB2_GLOBAL_CAP_ENCRYPTION flag.
> +	 * SMB 3.1.1 uses the cipher_type field.
> +	 */
> +	return (conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION) ||
> +	    conn->cipher_type;
> +}
> +
>  static void decode_compress_ctxt(struct ksmbd_conn *conn,
>  				 struct smb2_compression_capabilities_context *pneg_ctxt)
>  {
> @@ -1469,8 +1488,7 @@ static int ntlm_authenticate(struct ksmbd_work *work)
>  		    (req->SecurityMode & SMB2_NEGOTIATE_SIGNING_REQUIRED))
>  			sess->sign = true;
>
> -		if (conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION &&
> -		    conn->ops->generate_encryptionkey &&
> +		if (should_encrypt(conn) &&
>  		    !(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
>  			rc = conn->ops->generate_encryptionkey(sess);
>  			if (rc) {
> @@ -1559,8 +1577,7 @@ static int krb5_authenticate(struct ksmbd_work *work)
>  	    (req->SecurityMode & SMB2_NEGOTIATE_SIGNING_REQUIRED))
>  		sess->sign = true;
>
> -	if ((conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION) &&
> -	    conn->ops->generate_encryptionkey) {
> +	if (should_encrypt(conn)) {
>  		retval = conn->ops->generate_encryptionkey(sess);
>  		if (retval) {
>  			ksmbd_debug(SMB,
> --
> 2.25.1
>
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-12-16  1:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16  0:31 [PATCH] ksmbd: disable SMB2_GLOBAL_CAP_ENCRYPTION for SMB 3.1.1 Marcos Del Sol Vives
2021-12-16  1:14 ` Namjae Jeon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.