diff mbox

SMB3.11 security fixes

Message ID CAH2r5muD2oTRksmnPkNqCTwrXyX5knuA_ph2beLxvWDVyiDUgA@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Steve French March 3, 2018, 10:12 p.m. UTC
Proposed fix for the SMB3.11 (non-mandatory signing) case.

See MS-SMB2 3.2.4.1.1

On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com> wrote:
> SMB3.11 signing now works, thanks to Aurelien's patches (it had
> already worked as guest, but not as a regular user).
>
> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
> the non-signing case.  Am testing that now, but getting SMB3.11
> signing working is a big step and important for security.
>
> --
> Thanks,
>
> Steve

Comments

ronnie sahlberg March 5, 2018, 6:08 a.m. UTC | #1
Two nits

1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1
(since it is unlikely this requirement will revert back once we have
later versions of smb3.)

2, 3.2.4.1.1 says signing must be used IF the session has
EncryptData==False, but that is not what the code checks for.
It checks for is !guest user. Is that the right check?
(guest can never have encrypted sessions  but !guest can have
not-encrypted sessions.)






On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com> wrote:
> Proposed fix for the SMB3.11 (non-mandatory signing) case.
>
> See MS-SMB2 3.2.4.1.1
>
> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com> wrote:
>> SMB3.11 signing now works, thanks to Aurelien's patches (it had
>> already worked as guest, but not as a regular user).
>>
>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
>> the non-signing case.  Am testing that now, but getting SMB3.11
>> signing working is a big step and important for security.
>>
>> --
>> Thanks,
>>
>> Steve
>
>
>
> --
> Thanks,
>
> Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French March 5, 2018, 6:16 a.m. UTC | #2
Those are good points - but may be tricky to test the latter.

On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> Two nits
>
> 1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1
> (since it is unlikely this requirement will revert back once we have
> later versions of smb3.)
>
> 2, 3.2.4.1.1 says signing must be used IF the session has
> EncryptData==False, but that is not what the code checks for.
> It checks for is !guest user. Is that the right check?
> (guest can never have encrypted sessions  but !guest can have
> not-encrypted sessions.)
>
>
>
>
>
>
> On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com> wrote:
>> Proposed fix for the SMB3.11 (non-mandatory signing) case.
>>
>> See MS-SMB2 3.2.4.1.1
>>
>> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com> wrote:
>>> SMB3.11 signing now works, thanks to Aurelien's patches (it had
>>> already worked as guest, but not as a regular user).
>>>
>>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
>>> the non-signing case.  Am testing that now, but getting SMB3.11
>>> signing working is a big step and important for security.
>>>
>>> --
>>> Thanks,
>>>
>>> Steve
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
Ronnie Sahlberg March 5, 2018, 6:40 a.m. UTC | #3
It is not entirely clear in the spec,  but I think you might want to check
session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.

I will try to test this tomorrow.  If not, and your tests work for all your tests
then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>



----- Original Message -----
From: "Steve French" <smfrench@gmail.com>
To: "ronnie sahlberg" <ronniesahlberg@gmail.com>
Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel" <aaptel@suse.com>
Sent: Monday, 5 March, 2018 5:16:45 PM
Subject: Re: SMB3.11 security fixes

Those are good points - but may be tricky to test the latter.

On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> Two nits
>
> 1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1
> (since it is unlikely this requirement will revert back once we have
> later versions of smb3.)
>
> 2, 3.2.4.1.1 says signing must be used IF the session has
> EncryptData==False, but that is not what the code checks for.
> It checks for is !guest user. Is that the right check?
> (guest can never have encrypted sessions  but !guest can have
> not-encrypted sessions.)
>
>
>
>
>
>
> On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com> wrote:
>> Proposed fix for the SMB3.11 (non-mandatory signing) case.
>>
>> See MS-SMB2 3.2.4.1.1
>>
>> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com> wrote:
>>> SMB3.11 signing now works, thanks to Aurelien's patches (it had
>>> already worked as guest, but not as a regular user).
>>>
>>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
>>> the non-signing case.  Am testing that now, but getting SMB3.11
>>> signing working is a big step and important for security.
>>>
>>> --
>>> Thanks,
>>>
>>> Steve
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
Tom Talpey March 5, 2018, 4:08 p.m. UTC | #4
> -----Original Message-----

> From: linux-cifs-owner@vger.kernel.org <linux-cifs-owner@vger.kernel.org> On

> Behalf Of Ronnie Sahlberg

> Sent: Sunday, March 4, 2018 10:41 PM

> To: Steve French <smfrench@gmail.com>

> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; CIFS <linux-

> cifs@vger.kernel.org>; Aurélien Aptel <aaptel@suse.com>

> Subject: Re: SMB3.11 security fixes

> 

> It is not entirely clear in the spec,  but I think you might want to check

> session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.


The reason for the "don't sign if encrypted" rule is because the SMB3 encryption
algorithms provide integrity protection. And yes, guest accounts provide no
secrets, therefore no keys so encryption is not possible. So these are two very
different cases.

If something's not clear in the document, definitely let us know and
we'll consider clarifying.

Tom.

> 

> I will try to test this tomorrow.  If not, and your tests work for all your tests

> then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>

> 

> 

> 

> ----- Original Message -----

> From: "Steve French" <smfrench@gmail.com>

> To: "ronnie sahlberg" <ronniesahlberg@gmail.com>

> Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel" <aaptel@suse.com>

> Sent: Monday, 5 March, 2018 5:16:45 PM

> Subject: Re: SMB3.11 security fixes

> 

> Those are good points - but may be tricky to test the latter.

> 

> On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg

> <ronniesahlberg@gmail.com> wrote:

> > Two nits

> >

> > 1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1

> > (since it is unlikely this requirement will revert back once we have

> > later versions of smb3.)

> >

> > 2, 3.2.4.1.1 says signing must be used IF the session has

> > EncryptData==False, but that is not what the code checks for.

> > It checks for is !guest user. Is that the right check?

> > (guest can never have encrypted sessions  but !guest can have

> > not-encrypted sessions.)

> >

> >

> >

> >

> >

> >

> > On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com>

> wrote:

> >> Proposed fix for the SMB3.11 (non-mandatory signing) case.

> >>

> >> See MS-SMB2 3.2.4.1.1

> >>

> >> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com>

> wrote:

> >>> SMB3.11 signing now works, thanks to Aurelien's patches (it had

> >>> already worked as guest, but not as a regular user).

> >>>

> >>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix

> >>> the non-signing case.  Am testing that now, but getting SMB3.11

> >>> signing working is a big step and important for security.

> >>>

> >>> --

> >>> Thanks,

> >>>

> >>> Steve

> >>

> >>

> >>

> >> --

> >> Thanks,

> >>

> >> Steve

> 

> 

> 

> --

> Thanks,

> 

> Steve

> --

> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at

> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne

> l.org%2Fmajordomo-

> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403

> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C

> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM

> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-

> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser

> ved=0

> --

> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at

> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne

> l.org%2Fmajordomo-

> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403

> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C

> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM

> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-

> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser

> ved=0
ronnie sahlberg March 8, 2018, 3:21 a.m. UTC | #5
On Tue, Mar 6, 2018 at 2:08 AM, Tom Talpey <ttalpey@microsoft.com> wrote:
>> -----Original Message-----
>> From: linux-cifs-owner@vger.kernel.org <linux-cifs-owner@vger.kernel.org> On
>> Behalf Of Ronnie Sahlberg
>> Sent: Sunday, March 4, 2018 10:41 PM
>> To: Steve French <smfrench@gmail.com>
>> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; CIFS <linux-
>> cifs@vger.kernel.org>; Aurélien Aptel <aaptel@suse.com>
>> Subject: Re: SMB3.11 security fixes
>>
>> It is not entirely clear in the spec,  but I think you might want to check
>> session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.
>
> The reason for the "don't sign if encrypted" rule is because the SMB3 encryption
> algorithms provide integrity protection. And yes, guest accounts provide no
> secrets, therefore no keys so encryption is not possible. So these are two very
> different cases.

Ok, so then the check should look something like this ?

+ /* 3.11 tcon req must be signed when encryption is not used for all
authenticated users. See MS-SMB2 3.2.4.1.1 */
+ if ((ses->server->dialect >= SMB311_PROT_ID) &&
+    !(ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)
+    !(ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)
+    !(ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
+ req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
+

>
> If something's not clear in the document, definitely let us know and
> we'll consider clarifying.
>
> Tom.
>
>>
>> I will try to test this tomorrow.  If not, and your tests work for all your tests
>> then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>
>>
>>
>>
>> ----- Original Message -----
>> From: "Steve French" <smfrench@gmail.com>
>> To: "ronnie sahlberg" <ronniesahlberg@gmail.com>
>> Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel" <aaptel@suse.com>
>> Sent: Monday, 5 March, 2018 5:16:45 PM
>> Subject: Re: SMB3.11 security fixes
>>
>> Those are good points - but may be tricky to test the latter.
>>
>> On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg
>> <ronniesahlberg@gmail.com> wrote:
>> > Two nits
>> >
>> > 1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1
>> > (since it is unlikely this requirement will revert back once we have
>> > later versions of smb3.)
>> >
>> > 2, 3.2.4.1.1 says signing must be used IF the session has
>> > EncryptData==False, but that is not what the code checks for.
>> > It checks for is !guest user. Is that the right check?
>> > (guest can never have encrypted sessions  but !guest can have
>> > not-encrypted sessions.)
>> >
>> >
>> >
>> >
>> >
>> >
>> > On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com>
>> wrote:
>> >> Proposed fix for the SMB3.11 (non-mandatory signing) case.
>> >>
>> >> See MS-SMB2 3.2.4.1.1
>> >>
>> >> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com>
>> wrote:
>> >>> SMB3.11 signing now works, thanks to Aurelien's patches (it had
>> >>> already worked as guest, but not as a regular user).
>> >>>
>> >>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
>> >>> the non-signing case.  Am testing that now, but getting SMB3.11
>> >>> signing working is a big step and important for security.
>> >>>
>> >>> --
>> >>> Thanks,
>> >>>
>> >>> Steve
>> >>
>> >>
>> >>
>> >> --
>> >> Thanks,
>> >>
>> >> Steve
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne
>> l.org%2Fmajordomo-
>> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403
>> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
>> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
>> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser
>> ved=0
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne
>> l.org%2Fmajordomo-
>> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403
>> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
>> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
>> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser
>> ved=0
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tom Talpey March 8, 2018, 3:45 p.m. UTC | #6
This will work, but the SESSION_IS_NULL and SESSION_IS_GUEST tests are not
really needed. These two flags in the negotiate response  are somewhat advisory
from the server, and may or may not reflect the client's view. Also, it's strongly
recommended that servers not allow anonymous or guest auth, so wiring these
tests into the client implementation seems unwise.

For your purpose, it seems that simply testing dialect and FLAG_ENCRYPT_DATA
would do the trick here. If the flag is set, NULL or guest auth cannot possibly be
in force.

Tom.

> -----Original Message-----

> From: ronnie sahlberg <ronniesahlberg@gmail.com>

> Sent: Wednesday, March 7, 2018 7:21 PM

> To: Tom Talpey <ttalpey@microsoft.com>

> Cc: Ronnie Sahlberg <lsahlber@redhat.com>; Steve French

> <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>; Aurélien Aptel

> <aaptel@suse.com>

> Subject: Re: SMB3.11 security fixes

> 

> On Tue, Mar 6, 2018 at 2:08 AM, Tom Talpey <ttalpey@microsoft.com> wrote:

> >> -----Original Message-----

> >> From: linux-cifs-owner@vger.kernel.org <linux-cifs-owner@vger.kernel.org>

> On

> >> Behalf Of Ronnie Sahlberg

> >> Sent: Sunday, March 4, 2018 10:41 PM

> >> To: Steve French <smfrench@gmail.com>

> >> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; CIFS <linux-

> >> cifs@vger.kernel.org>; Aurélien Aptel <aaptel@suse.com>

> >> Subject: Re: SMB3.11 security fixes

> >>

> >> It is not entirely clear in the spec,  but I think you might want to check

> >> session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.

> >

> > The reason for the "don't sign if encrypted" rule is because the SMB3

> encryption

> > algorithms provide integrity protection. And yes, guest accounts provide no

> > secrets, therefore no keys so encryption is not possible. So these are two very

> > different cases.

> 

> Ok, so then the check should look something like this ?

> 

> + /* 3.11 tcon req must be signed when encryption is not used for all

> authenticated users. See MS-SMB2 3.2.4.1.1 */

> + if ((ses->server->dialect >= SMB311_PROT_ID) &&

> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)

> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)

> +    !(ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))

> + req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;

> +

> 

> >

> > If something's not clear in the document, definitely let us know and

> > we'll consider clarifying.

> >

> > Tom.

> >

> >>

> >> I will try to test this tomorrow.  If not, and your tests work for all your tests

> >> then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>

> >>

> >>

> >>

> >> ----- Original Message -----

> >> From: "Steve French" <smfrench@gmail.com>

> >> To: "ronnie sahlberg" <ronniesahlberg@gmail.com>

> >> Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel"

> <aaptel@suse.com>

> >> Sent: Monday, 5 March, 2018 5:16:45 PM

> >> Subject: Re: SMB3.11 security fixes

> >>

> >> Those are good points - but may be tricky to test the latter.

> >>

> >> On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg

> >> <ronniesahlberg@gmail.com> wrote:

> >> > Two nits

> >> >

> >> > 1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1

> >> > (since it is unlikely this requirement will revert back once we have

> >> > later versions of smb3.)

> >> >

> >> > 2, 3.2.4.1.1 says signing must be used IF the session has

> >> > EncryptData==False, but that is not what the code checks for.

> >> > It checks for is !guest user. Is that the right check?

> >> > (guest can never have encrypted sessions  but !guest can have

> >> > not-encrypted sessions.)

> >> >

> >> >

> >> >

> >> >

> >> >

> >> >

> >> > On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com>

> >> wrote:

> >> >> Proposed fix for the SMB3.11 (non-mandatory signing) case.

> >> >>

> >> >> See MS-SMB2 3.2.4.1.1

> >> >>

> >> >> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com>

> >> wrote:

> >> >>> SMB3.11 signing now works, thanks to Aurelien's patches (it had

> >> >>> already worked as guest, but not as a regular user).

> >> >>>

> >> >>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix

> >> >>> the non-signing case.  Am testing that now, but getting SMB3.11

> >> >>> signing working is a big step and important for security.

> >> >>>

> >> >>> --

> >> >>> Thanks,

> >> >>>

> >> >>> Steve

> >> >>

> >> >>

> >> >>

> >> >> --

> >> >> Thanks,

> >> >>

> >> >> Steve

> >>

> >>

> >>

> >> --

> >> Thanks,

> >>

> >> Steve

> >> --

> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in

> >> the body of a message to majordomo@vger.kernel.org

> >> More majordomo info at

> >>

> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne

> >> l.org%2Fmajordomo-

> >>

> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403

> >>

> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C

> >>

> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM

> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-

> >>

> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser

> >> ved=0

> >> --

> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in

> >> the body of a message to majordomo@vger.kernel.org

> >> More majordomo info at

> >>

> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne

> >> l.org%2Fmajordomo-

> >>

> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403

> >>

> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C

> >>

> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM

> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-

> >>

> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser

> >> ved=0
Steve French March 8, 2018, 3:51 p.m. UTC | #7
Presumably we also need to test if the client thinks it is unsignable
(session is guest) rather than if the server reports it authenticated
as guest?

On Thu, Mar 8, 2018 at 7:45 AM, Tom Talpey <ttalpey@microsoft.com> wrote:
> This will work, but the SESSION_IS_NULL and SESSION_IS_GUEST tests are not
> really needed. These two flags in the negotiate response  are somewhat advisory
> from the server, and may or may not reflect the client's view. Also, it's strongly
> recommended that servers not allow anonymous or guest auth, so wiring these
> tests into the client implementation seems unwise.
>
> For your purpose, it seems that simply testing dialect and FLAG_ENCRYPT_DATA
> would do the trick here. If the flag is set, NULL or guest auth cannot possibly be
> in force.
>
> Tom.
>
>> -----Original Message-----
>> From: ronnie sahlberg <ronniesahlberg@gmail.com>
>> Sent: Wednesday, March 7, 2018 7:21 PM
>> To: Tom Talpey <ttalpey@microsoft.com>
>> Cc: Ronnie Sahlberg <lsahlber@redhat.com>; Steve French
>> <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>; Aurélien Aptel
>> <aaptel@suse.com>
>> Subject: Re: SMB3.11 security fixes
>>
>> On Tue, Mar 6, 2018 at 2:08 AM, Tom Talpey <ttalpey@microsoft.com> wrote:
>> >> -----Original Message-----
>> >> From: linux-cifs-owner@vger.kernel.org <linux-cifs-owner@vger.kernel.org>
>> On
>> >> Behalf Of Ronnie Sahlberg
>> >> Sent: Sunday, March 4, 2018 10:41 PM
>> >> To: Steve French <smfrench@gmail.com>
>> >> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; CIFS <linux-
>> >> cifs@vger.kernel.org>; Aurélien Aptel <aaptel@suse.com>
>> >> Subject: Re: SMB3.11 security fixes
>> >>
>> >> It is not entirely clear in the spec,  but I think you might want to check
>> >> session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.
>> >
>> > The reason for the "don't sign if encrypted" rule is because the SMB3
>> encryption
>> > algorithms provide integrity protection. And yes, guest accounts provide no
>> > secrets, therefore no keys so encryption is not possible. So these are two very
>> > different cases.
>>
>> Ok, so then the check should look something like this ?
>>
>> + /* 3.11 tcon req must be signed when encryption is not used for all
>> authenticated users. See MS-SMB2 3.2.4.1.1 */
>> + if ((ses->server->dialect >= SMB311_PROT_ID) &&
>> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)
>> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)
>> +    !(ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
>> + req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>> +
>>
>> >
>> > If something's not clear in the document, definitely let us know and
>> > we'll consider clarifying.
>> >
>> > Tom.
>> >
>> >>
>> >> I will try to test this tomorrow.  If not, and your tests work for all your tests
>> >> then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>
>> >>
>> >>
>> >>
>> >> ----- Original Message -----
>> >> From: "Steve French" <smfrench@gmail.com>
>> >> To: "ronnie sahlberg" <ronniesahlberg@gmail.com>
>> >> Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel"
>> <aaptel@suse.com>
>> >> Sent: Monday, 5 March, 2018 5:16:45 PM
>> >> Subject: Re: SMB3.11 security fixes
>> >>
>> >> Those are good points - but may be tricky to test the latter.
>> >>
>> >> On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg
>> >> <ronniesahlberg@gmail.com> wrote:
>> >> > Two nits
>> >> >
>> >> > 1, maybe change the conditional to check for >= 3.1.1 instead of == 3.1.1
>> >> > (since it is unlikely this requirement will revert back once we have
>> >> > later versions of smb3.)
>> >> >
>> >> > 2, 3.2.4.1.1 says signing must be used IF the session has
>> >> > EncryptData==False, but that is not what the code checks for.
>> >> > It checks for is !guest user. Is that the right check?
>> >> > (guest can never have encrypted sessions  but !guest can have
>> >> > not-encrypted sessions.)
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com>
>> >> wrote:
>> >> >> Proposed fix for the SMB3.11 (non-mandatory signing) case.
>> >> >>
>> >> >> See MS-SMB2 3.2.4.1.1
>> >> >>
>> >> >> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com>
>> >> wrote:
>> >> >>> SMB3.11 signing now works, thanks to Aurelien's patches (it had
>> >> >>> already worked as guest, but not as a regular user).
>> >> >>>
>> >> >>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
>> >> >>> the non-signing case.  Am testing that now, but getting SMB3.11
>> >> >>> signing working is a big step and important for security.
>> >> >>>
>> >> >>> --
>> >> >>> Thanks,
>> >> >>>
>> >> >>> Steve
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Thanks,
>> >> >>
>> >> >> Steve
>> >>
>> >>
>> >>
>> >> --
>> >> Thanks,
>> >>
>> >> Steve
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at
>> >>
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne
>> >> l.org%2Fmajordomo-
>> >>
>> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403
>> >>
>> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
>> >>
>> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
>> >>
>> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser
>> >> ved=0
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at
>> >>
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne
>> >> l.org%2Fmajordomo-
>> >>
>> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403
>> >>
>> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
>> >>
>> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
>> >>
>> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser
>> >> ved=0
Tom Talpey March 8, 2018, 4:12 p.m. UTC | #8
I guess, but in that case I'd still suggest to look at whether the client *can* sign,
rather than test these bits. IOW if there's a signing key, use it, otherwise fail locally
and not even attempt the tcon. It would violate the protocol otherwise.

Tom.

> -----Original Message-----

> From: Steve French <smfrench@gmail.com>

> Sent: Thursday, March 8, 2018 7:51 AM

> To: Tom Talpey <ttalpey@microsoft.com>

> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; Ronnie Sahlberg

> <lsahlber@redhat.com>; CIFS <linux-cifs@vger.kernel.org>; Aurélien Aptel

> <aaptel@suse.com>

> Subject: Re: SMB3.11 security fixes

> 

> Presumably we also need to test if the client thinks it is unsignable

> (session is guest) rather than if the server reports it authenticated

> as guest?

> 

> On Thu, Mar 8, 2018 at 7:45 AM, Tom Talpey <ttalpey@microsoft.com> wrote:

> > This will work, but the SESSION_IS_NULL and SESSION_IS_GUEST tests are not

> > really needed. These two flags in the negotiate response  are somewhat

> advisory

> > from the server, and may or may not reflect the client's view. Also, it's

> strongly

> > recommended that servers not allow anonymous or guest auth, so wiring

> these

> > tests into the client implementation seems unwise.

> >

> > For your purpose, it seems that simply testing dialect and

> FLAG_ENCRYPT_DATA

> > would do the trick here. If the flag is set, NULL or guest auth cannot possibly

> be

> > in force.

> >

> > Tom.

> >

> >> -----Original Message-----

> >> From: ronnie sahlberg <ronniesahlberg@gmail.com>

> >> Sent: Wednesday, March 7, 2018 7:21 PM

> >> To: Tom Talpey <ttalpey@microsoft.com>

> >> Cc: Ronnie Sahlberg <lsahlber@redhat.com>; Steve French

> >> <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>; Aurélien Aptel

> >> <aaptel@suse.com>

> >> Subject: Re: SMB3.11 security fixes

> >>

> >> On Tue, Mar 6, 2018 at 2:08 AM, Tom Talpey <ttalpey@microsoft.com>

> wrote:

> >> >> -----Original Message-----

> >> >> From: linux-cifs-owner@vger.kernel.org <linux-cifs-

> owner@vger.kernel.org>

> >> On

> >> >> Behalf Of Ronnie Sahlberg

> >> >> Sent: Sunday, March 4, 2018 10:41 PM

> >> >> To: Steve French <smfrench@gmail.com>

> >> >> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; CIFS <linux-

> >> >> cifs@vger.kernel.org>; Aurélien Aptel <aaptel@suse.com>

> >> >> Subject: Re: SMB3.11 security fixes

> >> >>

> >> >> It is not entirely clear in the spec,  but I think you might want to check

> >> >> session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.

> >> >

> >> > The reason for the "don't sign if encrypted" rule is because the SMB3

> >> encryption

> >> > algorithms provide integrity protection. And yes, guest accounts provide

> no

> >> > secrets, therefore no keys so encryption is not possible. So these are two

> very

> >> > different cases.

> >>

> >> Ok, so then the check should look something like this ?

> >>

> >> + /* 3.11 tcon req must be signed when encryption is not used for all

> >> authenticated users. See MS-SMB2 3.2.4.1.1 */

> >> + if ((ses->server->dialect >= SMB311_PROT_ID) &&

> >> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)

> >> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)

> >> +    !(ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))

> >> + req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;

> >> +

> >>

> >> >

> >> > If something's not clear in the document, definitely let us know and

> >> > we'll consider clarifying.

> >> >

> >> > Tom.

> >> >

> >> >>

> >> >> I will try to test this tomorrow.  If not, and your tests work for all your

> tests

> >> >> then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>

> >> >>

> >> >>

> >> >>

> >> >> ----- Original Message -----

> >> >> From: "Steve French" <smfrench@gmail.com>

> >> >> To: "ronnie sahlberg" <ronniesahlberg@gmail.com>

> >> >> Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel"

> >> <aaptel@suse.com>

> >> >> Sent: Monday, 5 March, 2018 5:16:45 PM

> >> >> Subject: Re: SMB3.11 security fixes

> >> >>

> >> >> Those are good points - but may be tricky to test the latter.

> >> >>

> >> >> On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg

> >> >> <ronniesahlberg@gmail.com> wrote:

> >> >> > Two nits

> >> >> >

> >> >> > 1, maybe change the conditional to check for >= 3.1.1 instead of ==

> 3.1.1

> >> >> > (since it is unlikely this requirement will revert back once we have

> >> >> > later versions of smb3.)

> >> >> >

> >> >> > 2, 3.2.4.1.1 says signing must be used IF the session has

> >> >> > EncryptData==False, but that is not what the code checks for.

> >> >> > It checks for is !guest user. Is that the right check?

> >> >> > (guest can never have encrypted sessions  but !guest can have

> >> >> > not-encrypted sessions.)

> >> >> >

> >> >> >

> >> >> >

> >> >> >

> >> >> >

> >> >> >

> >> >> > On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com>

> >> >> wrote:

> >> >> >> Proposed fix for the SMB3.11 (non-mandatory signing) case.

> >> >> >>

> >> >> >> See MS-SMB2 3.2.4.1.1

> >> >> >>

> >> >> >> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com>

> >> >> wrote:

> >> >> >>> SMB3.11 signing now works, thanks to Aurelien's patches (it had

> >> >> >>> already worked as guest, but not as a regular user).

> >> >> >>>

> >> >> >>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix

> >> >> >>> the non-signing case.  Am testing that now, but getting SMB3.11

> >> >> >>> signing working is a big step and important for security.

> >> >> >>>

> >> >> >>> --

> >> >> >>> Thanks,

> >> >> >>>

> >> >> >>> Steve

> >> >> >>

> >> >> >>

> >> >> >>

> >> >> >> --

> >> >> >> Thanks,

> >> >> >>

> >> >> >> Steve

> >> >>

> >> >>

> >> >>

> >> >> --

> >> >> Thanks,

> >> >>

> >> >> Steve

> >> >> --

> >> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in

> >> >> the body of a message to majordomo@vger.kernel.org

> >> >> More majordomo info at

> >> >>

> >>

> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne

> >> >> l.org%2Fmajordomo-

> >> >>

> >>

> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403

> >> >>

> >>

> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C

> >> >>

> >>

> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM

> >> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-

> >> >>

> >>

> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser

> >> >> ved=0

> >> >> --

> >> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in

> >> >> the body of a message to majordomo@vger.kernel.org

> >> >> More majordomo info at

> >> >>

> >>

> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne

> >> >> l.org%2Fmajordomo-

> >> >>

> >>

> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403

> >> >>

> >>

> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C

> >> >>

> >>

> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM

> >> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-

> >> >>

> >>

> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser

> >> >> ved=0

> 

> 

> 

> --

> Thanks,

> 

> Steve
Steve French March 13, 2018, 7:16 a.m. UTC | #9
It looks like we are always setting the signing key, although I have
to test it a little more.   So based on your suggestion, it sounds
like even if the server says it is guest we should try to sign it if
it is possible (which it looks like it should be).

On Thu, Mar 8, 2018 at 10:12 AM, Tom Talpey <ttalpey@microsoft.com> wrote:
> I guess, but in that case I'd still suggest to look at whether the client *can* sign,
> rather than test these bits. IOW if there's a signing key, use it, otherwise fail locally
> and not even attempt the tcon. It would violate the protocol otherwise.
>
> Tom.
>
>> -----Original Message-----
>> From: Steve French <smfrench@gmail.com>
>> Sent: Thursday, March 8, 2018 7:51 AM
>> To: Tom Talpey <ttalpey@microsoft.com>
>> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; Ronnie Sahlberg
>> <lsahlber@redhat.com>; CIFS <linux-cifs@vger.kernel.org>; Aurélien Aptel
>> <aaptel@suse.com>
>> Subject: Re: SMB3.11 security fixes
>>
>> Presumably we also need to test if the client thinks it is unsignable
>> (session is guest) rather than if the server reports it authenticated
>> as guest?
>>
>> On Thu, Mar 8, 2018 at 7:45 AM, Tom Talpey <ttalpey@microsoft.com> wrote:
>> > This will work, but the SESSION_IS_NULL and SESSION_IS_GUEST tests are not
>> > really needed. These two flags in the negotiate response  are somewhat
>> advisory
>> > from the server, and may or may not reflect the client's view. Also, it's
>> strongly
>> > recommended that servers not allow anonymous or guest auth, so wiring
>> these
>> > tests into the client implementation seems unwise.
>> >
>> > For your purpose, it seems that simply testing dialect and
>> FLAG_ENCRYPT_DATA
>> > would do the trick here. If the flag is set, NULL or guest auth cannot possibly
>> be
>> > in force.
>> >
>> > Tom.
>> >
>> >> -----Original Message-----
>> >> From: ronnie sahlberg <ronniesahlberg@gmail.com>
>> >> Sent: Wednesday, March 7, 2018 7:21 PM
>> >> To: Tom Talpey <ttalpey@microsoft.com>
>> >> Cc: Ronnie Sahlberg <lsahlber@redhat.com>; Steve French
>> >> <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>; Aurélien Aptel
>> >> <aaptel@suse.com>
>> >> Subject: Re: SMB3.11 security fixes
>> >>
>> >> On Tue, Mar 6, 2018 at 2:08 AM, Tom Talpey <ttalpey@microsoft.com>
>> wrote:
>> >> >> -----Original Message-----
>> >> >> From: linux-cifs-owner@vger.kernel.org <linux-cifs-
>> owner@vger.kernel.org>
>> >> On
>> >> >> Behalf Of Ronnie Sahlberg
>> >> >> Sent: Sunday, March 4, 2018 10:41 PM
>> >> >> To: Steve French <smfrench@gmail.com>
>> >> >> Cc: ronnie sahlberg <ronniesahlberg@gmail.com>; CIFS <linux-
>> >> >> cifs@vger.kernel.org>; Aurélien Aptel <aaptel@suse.com>
>> >> >> Subject: Re: SMB3.11 security fixes
>> >> >>
>> >> >> It is not entirely clear in the spec,  but I think you might want to check
>> >> >> session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA instead.
>> >> >
>> >> > The reason for the "don't sign if encrypted" rule is because the SMB3
>> >> encryption
>> >> > algorithms provide integrity protection. And yes, guest accounts provide
>> no
>> >> > secrets, therefore no keys so encryption is not possible. So these are two
>> very
>> >> > different cases.
>> >>
>> >> Ok, so then the check should look something like this ?
>> >>
>> >> + /* 3.11 tcon req must be signed when encryption is not used for all
>> >> authenticated users. See MS-SMB2 3.2.4.1.1 */
>> >> + if ((ses->server->dialect >= SMB311_PROT_ID) &&
>> >> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)
>> >> +    !(ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)
>> >> +    !(ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
>> >> + req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>> >> +
>> >>
>> >> >
>> >> > If something's not clear in the document, definitely let us know and
>> >> > we'll consider clarifying.
>> >> >
>> >> > Tom.
>> >> >
>> >> >>
>> >> >> I will try to test this tomorrow.  If not, and your tests work for all your
>> tests
>> >> >> then a reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>
>> >> >>
>> >> >>
>> >> >>
>> >> >> ----- Original Message -----
>> >> >> From: "Steve French" <smfrench@gmail.com>
>> >> >> To: "ronnie sahlberg" <ronniesahlberg@gmail.com>
>> >> >> Cc: "CIFS" <linux-cifs@vger.kernel.org>, "Aurélien Aptel"
>> >> <aaptel@suse.com>
>> >> >> Sent: Monday, 5 March, 2018 5:16:45 PM
>> >> >> Subject: Re: SMB3.11 security fixes
>> >> >>
>> >> >> Those are good points - but may be tricky to test the latter.
>> >> >>
>> >> >> On Mon, Mar 5, 2018 at 12:08 AM, ronnie sahlberg
>> >> >> <ronniesahlberg@gmail.com> wrote:
>> >> >> > Two nits
>> >> >> >
>> >> >> > 1, maybe change the conditional to check for >= 3.1.1 instead of ==
>> 3.1.1
>> >> >> > (since it is unlikely this requirement will revert back once we have
>> >> >> > later versions of smb3.)
>> >> >> >
>> >> >> > 2, 3.2.4.1.1 says signing must be used IF the session has
>> >> >> > EncryptData==False, but that is not what the code checks for.
>> >> >> > It checks for is !guest user. Is that the right check?
>> >> >> > (guest can never have encrypted sessions  but !guest can have
>> >> >> > not-encrypted sessions.)
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> > On Sun, Mar 4, 2018 at 8:12 AM, Steve French <smfrench@gmail.com>
>> >> >> wrote:
>> >> >> >> Proposed fix for the SMB3.11 (non-mandatory signing) case.
>> >> >> >>
>> >> >> >> See MS-SMB2 3.2.4.1.1
>> >> >> >>
>> >> >> >> On Sat, Mar 3, 2018 at 3:08 PM, Steve French <smfrench@gmail.com>
>> >> >> wrote:
>> >> >> >>> SMB3.11 signing now works, thanks to Aurelien's patches (it had
>> >> >> >>> already worked as guest, but not as a regular user).
>> >> >> >>>
>> >> >> >>> It needs one minor fix (to send the signature on SMB3.11 tcon) to fix
>> >> >> >>> the non-signing case.  Am testing that now, but getting SMB3.11
>> >> >> >>> signing working is a big step and important for security.
>> >> >> >>>
>> >> >> >>> --
>> >> >> >>> Thanks,
>> >> >> >>>
>> >> >> >>> Steve
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >> --
>> >> >> >> Thanks,
>> >> >> >>
>> >> >> >> Steve
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Thanks,
>> >> >>
>> >> >> Steve
>> >> >> --
>> >> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> >> >> the body of a message to majordomo@vger.kernel.org
>> >> >> More majordomo info at
>> >> >>
>> >>
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne
>> >> >> l.org%2Fmajordomo-
>> >> >>
>> >>
>> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403
>> >> >>
>> >>
>> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
>> >> >>
>> >>
>> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> >> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
>> >> >>
>> >>
>> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser
>> >> >> ved=0
>> >> >> --
>> >> >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> >> >> the body of a message to majordomo@vger.kernel.org
>> >> >> More majordomo info at
>> >> >>
>> >>
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kerne
>> >> >> l.org%2Fmajordomo-
>> >> >>
>> >>
>> info.html&data=04%7C01%7Cttalpey%40microsoft.com%7Ce7d08b64d72b403
>> >> >>
>> >>
>> 179bd08d582640e7f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
>> >> >>
>> >>
>> 636558288607416690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
>> >> >> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-
>> >> >>
>> >>
>> 2&sdata=3ffZ1CqcsH97D3tfeU7Qca9%2B4GBqCSbR7jj5nef%2FQDY%3D&reser
>> >> >> ved=0
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
diff mbox

Patch

From 350477ecdbe087b790b86527db39b5e80a08c20e Mon Sep 17 00:00:00 2001
From: Steve French <sfrench@localhost.localdomain>
Date: Sat, 3 Mar 2018 16:08:45 -0600
Subject: [PATCH] [SMB3.11] Tree connect for SMB3.11 must be signed for
 non-guest users

SMB3.11 tree connect was only being signed when signing was mandatory
but needs to always be signed (for non-guest users).

See MS-SMB2 section 3.2.4.1.1

Signed-off-by: Steve French <smfrench@gmail.com>
CC: Stable <stable@vger.kernel.org>
---
 fs/cifs/smb2pdu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 4b6920de2541..fe1d52c235ee 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1305,6 +1305,11 @@  SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	iov[1].iov_base = unc_path;
 	iov[1].iov_len = unc_path_len;
 
+	/* 3.11 tcon req must be signed if not guest. See MS-SMB2 3.2.4.1.1 */
+	if ((ses->server->dialect == SMB311_PROT_ID) &&
+	    !(ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST))
+		req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
+
 	rc = smb2_send_recv(xid, ses, iov, 2, &resp_buftype, flags, &rsp_iov);
 	cifs_small_buf_release(req);
 	rsp = (struct smb2_tree_connect_rsp *)rsp_iov.iov_base;
-- 
2.14.3