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 -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
