diff mbox series

[SMB3.1.1] do not fail if no encryption required when server doesn't support encryption

Message ID CAH2r5mv_0rLQF=npjc4CVJBDhsc8Eu_sJtY6xUDbBXs7aYhSzA@mail.gmail.com
State New
Headers show
Series [SMB3.1.1] do not fail if no encryption required when server doesn't support encryption | expand

Commit Message

Steve French Oct. 17, 2020, 9:03 a.m. UTC
There are cases where the server can return a cipher type of 0 and
    it not be an error. For example, if server only supported AES256_CCM
    (very unlikely) or server supported no encryption types or
    had all disabled. In those cases encryption would not be supported,
    but that can be ok if the client did not require encryption on mount.

    In the case in which mount requested encryption ("seal" on mount)
    then checks later on during tree connection will return the proper
    rc, but if seal was not requested by client, since server is allowed
    to return 0 to indicate no supported cipher, we should not fail mount.

    Reported-by: Pavel Shilovsky <pshilov@microsoft.com>
    Signed-off-by: Steve French <stfrench@microsoft.com>

Comments

Tom Talpey Oct. 17, 2020, 5:08 p.m. UTC | #1
On 10/17/2020 5:03 AM, Steve French wrote:
>      There are cases where the server can return a cipher type of 0 and
>      it not be an error. For example, if server only supported AES256_CCM
>      (very unlikely) or server supported no encryption types or

It seems me that the simpler statement is that there are
no ciphers supported in common between client and server.

>      had all disabled. In those cases encryption would not be supported,
>      but that can be ok if the client did not require encryption on mount.
> 
>      In the case in which mount requested encryption ("seal" on mount)

I'm confused. Doesn't "seal" mean signing?

Tom.

>      then checks later on during tree connection will return the proper
>      rc, but if seal was not requested by client, since server is allowed
>      to return 0 to indicate no supported cipher, we should not fail mount.
> 
>      Reported-by: Pavel Shilovsky <pshilov@microsoft.com>
>      Signed-off-by: Steve French <stfrench@microsoft.com>
> 
>
Steve French Oct. 17, 2020, 7:47 p.m. UTC | #2
To be consistent with others including Samba we used "seal" (a long
time ago it seems now) to be the mount option to mean "require
encryption for this mount"

See various references to seal (to mean encrypt) in
https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html for
example

Not sure the etymology here of "seal" but my guess is that its use to
mean "encrypt" came from the alternative meaning of "seal" not a large
aquatic mammal but instead "apply a nonporous coating to make it
impervious."

On Sat, Oct 17, 2020 at 12:08 PM Tom Talpey <tom@talpey.com> wrote:
>
> On 10/17/2020 5:03 AM, Steve French wrote:
> >      There are cases where the server can return a cipher type of 0 and
> >      it not be an error. For example, if server only supported AES256_CCM
> >      (very unlikely) or server supported no encryption types or
>
> It seems me that the simpler statement is that there are
> no ciphers supported in common between client and server.
>
> >      had all disabled. In those cases encryption would not be supported,
> >      but that can be ok if the client did not require encryption on mount.
> >
> >      In the case in which mount requested encryption ("seal" on mount)
>
> I'm confused. Doesn't "seal" mean signing?
>
> Tom.
>
> >      then checks later on during tree connection will return the proper
> >      rc, but if seal was not requested by client, since server is allowed
> >      to return 0 to indicate no supported cipher, we should not fail mount.
> >
> >      Reported-by: Pavel Shilovsky <pshilov@microsoft.com>
> >      Signed-off-by: Steve French <stfrench@microsoft.com>
> >
> >
Tom Talpey Oct. 17, 2020, 11:24 p.m. UTC | #3
Ah, ok consistency is good. I still suggest my first comment though.

Dumb followup question, in that case. Is "seal" a mount option?
It is not listed in the manpage (nor is "encrypt" etc).

Tom.

On 10/17/2020 3:47 PM, Steve French wrote:
> To be consistent with others including Samba we used "seal" (a long
> time ago it seems now) to be the mount option to mean "require
> encryption for this mount"
> 
> See various references to seal (to mean encrypt) in
> https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html for
> example
> 
> Not sure the etymology here of "seal" but my guess is that its use to
> mean "encrypt" came from the alternative meaning of "seal" not a large
> aquatic mammal but instead "apply a nonporous coating to make it
> impervious."
> 
> On Sat, Oct 17, 2020 at 12:08 PM Tom Talpey <tom@talpey.com> wrote:
>>
>> On 10/17/2020 5:03 AM, Steve French wrote:
>>>       There are cases where the server can return a cipher type of 0 and
>>>       it not be an error. For example, if server only supported AES256_CCM
>>>       (very unlikely) or server supported no encryption types or
>>
>> It seems me that the simpler statement is that there are
>> no ciphers supported in common between client and server.
>>
>>>       had all disabled. In those cases encryption would not be supported,
>>>       but that can be ok if the client did not require encryption on mount.
>>>
>>>       In the case in which mount requested encryption ("seal" on mount)
>>
>> I'm confused. Doesn't "seal" mean signing?
>>
>> Tom.
>>
>>>       then checks later on during tree connection will return the proper
>>>       rc, but if seal was not requested by client, since server is allowed
>>>       to return 0 to indicate no supported cipher, we should not fail mount.
>>>
>>>       Reported-by: Pavel Shilovsky <pshilov@microsoft.com>
>>>       Signed-off-by: Steve French <stfrench@microsoft.com>
>>>
>>>
> 
> 
>
Pavel Shilovsky Oct. 19, 2020, 6:16 p.m. UTC | #4
Thanks for fixing this!

Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
--
Best regards,
Pavel Shilovsky

сб, 17 окт. 2020 г. в 02:03, Steve French <smfrench@gmail.com>:
>
>     There are cases where the server can return a cipher type of 0 and
>     it not be an error. For example, if server only supported AES256_CCM
>     (very unlikely) or server supported no encryption types or
>     had all disabled. In those cases encryption would not be supported,
>     but that can be ok if the client did not require encryption on mount.
>
>     In the case in which mount requested encryption ("seal" on mount)
>     then checks later on during tree connection will return the proper
>     rc, but if seal was not requested by client, since server is allowed
>     to return 0 to indicate no supported cipher, we should not fail mount.
>
>     Reported-by: Pavel Shilovsky <pshilov@microsoft.com>
>     Signed-off-by: Steve French <stfrench@microsoft.com>
>
>
> --
> Thanks,
>
> Steve
diff mbox series

Patch

From c6db40cc1a46730a78dc3e79d0791e10752d6853 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Sat, 17 Oct 2020 03:54:27 -0500
Subject: [PATCH] smb3.1.1: do not fail if no encryption required but server
 doesn't support it

There are cases where the server can return a cipher type of 0 and
it not be an error. For example, if server only supported AES256_CCM
(very unlikely) or server supported no encryption types or
had all disabled. In those cases encryption would not be supported,
but that can be ok if the client did not require encryption on mount.

In the case in which mount requested encryption ("seal" on mount)
then checks later on during tree connection will return the proper
rc, but if seal was not requested by client, since server is allowed
to return 0 to indicate no supported cipher, we should not fail mount.

Reported-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index d504bc296349..025db5e8c183 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -616,9 +616,19 @@  static int decode_encrypt_ctx(struct TCP_Server_Info *server,
 			return -EOPNOTSUPP;
 		}
 	} else if (ctxt->Ciphers[0] == 0) {
-		/* e.g. if server only supported AES256_CCM (very unlikely) */
-		cifs_dbg(VFS, "Server does not support requested encryption types\n");
-		return -EOPNOTSUPP;
+		/*
+		 * e.g. if server only supported AES256_CCM (very unlikely)
+		 * or server supported no encryption types or had all disabled.
+		 * Since GLOBAL_CAP_ENCRYPTION will be not set, in the case
+		 * in which mount requested encryption ("seal") checks later
+		 * on during tree connection will return proper rc, but if
+		 * seal not requested by client, since server is allowed to
+		 * return 0 to indicate no supported cipher, we can't fail here
+		 */
+		server->cipher_type = 0;
+		server->capabilities &= ~SMB2_GLOBAL_CAP_ENCRYPTION;
+		pr_warn_once("Server does not support requested encryption types\n");
+		return 0;
 	} else if ((ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES128_CCM) &&
 		   (ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES128_GCM) &&
 		   (ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES256_GCM)) {
-- 
2.25.1