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@xxxxxxxxx> > Sent: Thursday, March 8, 2018 7:51 AM > To: Tom Talpey <ttalpey@xxxxxxxxxxxxx> > Cc: ronnie sahlberg <ronniesahlberg@xxxxxxxxx>; Ronnie Sahlberg > <lsahlber@xxxxxxxxxx>; CIFS <linux-cifs@xxxxxxxxxxxxxxx>; Aurélien Aptel > <aaptel@xxxxxxxx> > 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@xxxxxxxxxxxxx> 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@xxxxxxxxx> > >> Sent: Wednesday, March 7, 2018 7:21 PM > >> To: Tom Talpey <ttalpey@xxxxxxxxxxxxx> > >> Cc: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>; Steve French > >> <smfrench@xxxxxxxxx>; CIFS <linux-cifs@xxxxxxxxxxxxxxx>; Aurélien Aptel > >> <aaptel@xxxxxxxx> > >> Subject: Re: SMB3.11 security fixes > >> > >> On Tue, Mar 6, 2018 at 2:08 AM, Tom Talpey <ttalpey@xxxxxxxxxxxxx> > wrote: > >> >> -----Original Message----- > >> >> From: linux-cifs-owner@xxxxxxxxxxxxxxx <linux-cifs- > owner@xxxxxxxxxxxxxxx> > >> On > >> >> Behalf Of Ronnie Sahlberg > >> >> Sent: Sunday, March 4, 2018 10:41 PM > >> >> To: Steve French <smfrench@xxxxxxxxx> > >> >> Cc: ronnie sahlberg <ronniesahlberg@xxxxxxxxx>; CIFS <linux- > >> >> cifs@xxxxxxxxxxxxxxx>; Aurélien Aptel <aaptel@xxxxxxxx> > >> >> 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@xxxxxxxxxx> > >> >> > >> >> > >> >> > >> >> ----- Original Message ----- > >> >> From: "Steve French" <smfrench@xxxxxxxxx> > >> >> To: "ronnie sahlberg" <ronniesahlberg@xxxxxxxxx> > >> >> Cc: "CIFS" <linux-cifs@xxxxxxxxxxxxxxx>, "Aurélien Aptel" > >> <aaptel@xxxxxxxx> > >> >> 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@xxxxxxxxx> 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@xxxxxxxxx> > >> >> 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@xxxxxxxxx> > >> >> 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@xxxxxxxxxxxxxxx > >> >> 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@xxxxxxxxxxxxxxx > >> >> 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 ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
