Minor change to not log error message http://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=ff1c038addc4f205d5f1ede449426c7d316c0eed On Tue, Nov 19, 2013 at 11:50 PM, Steve French <smfrench@xxxxxxxxx> wrote: > Updated patch with the minor change you suggested is now in > cifs-2.6.git for-next > > http://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=6f3219c1bc3b0f33a513ba63cea8278f1cd89ab3 > > On Tue, Nov 19, 2013 at 11:34 PM, Steve French <smfrench@xxxxxxxxx> wrote: >> On Tue, Nov 19, 2013 at 11:33 PM, Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote: >>> 2013/11/19 Steve French <smfrench@xxxxxxxxx>: >>>> From e4608aa57f9948a5e160ecb8590c885f76796e34 Mon Sep 17 00:00:00 2001 >>>> From: Steve French <smfrench@xxxxxxxxx> >>>> Date: Tue, 19 Nov 2013 12:58:08 -0600 >>>> Subject: [PATCH] [CIFS] Check SMB3 dialects against downgrade attacks >>>> >>>> When we are running SMB3 or SMB3.02 connections which are signed >>>> we need to validate the protocol negotiation information, >>>> to ensure that thenegotiate protocol response was not tampered with. >>>> >>>> Add the missing FSCTL which is sent at mount time (immediately after >>>> the SMB3 Tree Connect) to validate that the capabilities match >>>> what we think the server sent. >>>> >>>> "Secure dialect negotiation is introduced in SMB3 to protect against >>>> man-in-the-middle attempt to downgrade dialect negotiation. >>>> The idea is to prevent an eavesdropper from downgrading the initially >>>> negotiated dialect and capabilities between the client and the server." >>>> >>>> For more explanation see 2.2.31.4 of MS-SMB2 or >>>> http://blogs.msdn.com/b/openspecification/archive/2012/06/28/smb3-secure-dialect-negotiation.aspx >>>> >>>> Signed-off-by: Steve French <smfrench@xxxxxxxxx> >>>> --- >>>> fs/cifs/cifsglob.h | 1 + >>>> fs/cifs/smb2ops.c | 1 + >>>> fs/cifs/smb2pdu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> fs/cifs/smb2pdu.h | 12 ++++++--- >>>> fs/cifs/smb2proto.h | 1 + >>>> fs/cifs/smbfsctl.h | 2 +- >>>> 6 files changed, 89 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h >>>> index d9ea7ad..f918a99 100644 >>>> --- a/fs/cifs/cifsglob.h >>>> +++ b/fs/cifs/cifsglob.h >>>> @@ -384,6 +384,7 @@ struct smb_version_operations { >>>> int (*clone_range)(const unsigned int, struct cifsFileInfo *src_file, >>>> struct cifsFileInfo *target_file, u64 src_off, u64 len, >>>> u64 dest_off); >>>> + int (*validate_negotiate)(const unsigned int, struct cifs_tcon *); >>>> }; >>>> >>>> struct smb_version_values { >>>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c >>>> index a3968ee..757da3e 100644 >>>> --- a/fs/cifs/smb2ops.c >>>> +++ b/fs/cifs/smb2ops.c >>>> @@ -1319,6 +1319,7 @@ struct smb_version_operations smb30_operations = { >>>> .create_lease_buf = smb3_create_lease_buf, >>>> .parse_lease_buf = smb3_parse_lease_buf, >>>> .clone_range = smb2_clone_range, >>>> + .validate_negotiate = smb3_validate_negotiate, >>>> }; >>>> >>>> struct smb_version_values smb20_values = { >>>> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c >>>> index 1e136ee..7756f9b 100644 >>>> --- a/fs/cifs/smb2pdu.c >>>> +++ b/fs/cifs/smb2pdu.c >>>> @@ -454,6 +454,80 @@ neg_exit: >>>> return rc; >>>> } >>>> >>>> +int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) >>>> +{ >>>> + int rc = 0; >>>> + struct validate_negotiate_info_req vneg_inbuf; >>>> + struct validate_negotiate_info_rsp *pneg_rsp; >>>> + u32 rsplen; >>>> + >>>> + cifs_dbg(VFS, "validate negotiate "); >>>> + vneg_inbuf.Capabilities = >>>> + cpu_to_le32(tcon->ses->server->vals->req_capabilities); >>>> + memcpy(vneg_inbuf.Guid, cifs_client_guid, SMB2_CLIENT_GUID_SIZE); >>>> + >>>> + /* >>>> + * validation ioctl must be signed, so no point sending this if we >>>> + * can not sign it. We could eventually change this to selectively >>>> + * sign just this, the first and only signed request on a connection. >>>> + * This is good enough for now since a user who wants better security >>>> + * would also enable signing on the mount. Having validation of >>>> + * negotiate info for signed connections helps reduce attack vectors >>>> + */ >>>> + if (tcon->ses->server->sign == false) >>>> + return 0; /* validation requires signing */ >>> >>> Shouldn't we move the above check and comments before we set >>> Capabilities and Guid of vneg_inbuf? >> >> Yes - will do >> >> >> -- >> 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 http://vger.kernel.org/majordomo-info.html
