Skip to content

Commit 0603c96

Browse files
committed
SMB: Validate negotiate (to protect against downgrade) even if signing off
As long as signing is supported (ie not a guest user connection) and connection is SMB3 or SMB3.02, then validate negotiate (protect against man in the middle downgrade attacks). We had been doing this only when signing was required, not when signing was just enabled, but this more closely matches recommended SMB3 behavior and is better security. Suggested by Metze. Signed-off-by: Steve French <[email protected]> Reviewed-by: Jeremy Allison <[email protected]> Acked-by: Stefan Metzmacher <[email protected]> Reviewed-by: Ronnie Sahlberg <[email protected]> CC: Stable <[email protected]>
1 parent f5c4ba8 commit 0603c96

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

fs/cifs/smb2pdu.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -656,15 +656,22 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
656656

657657
/*
658658
* validation ioctl must be signed, so no point sending this if we
659-
* can not sign it. We could eventually change this to selectively
659+
* can not sign it (ie are not known user). Even if signing is not
660+
* required (enabled but not negotiated), in those cases we selectively
660661
* sign just this, the first and only signed request on a connection.
661-
* This is good enough for now since a user who wants better security
662-
* would also enable signing on the mount. Having validation of
663-
* negotiate info for signed connections helps reduce attack vectors
662+
* Having validation of negotiate info helps reduce attack vectors.
664663
*/
665-
if (tcon->ses->server->sign == false)
664+
if (tcon->ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)
666665
return 0; /* validation requires signing */
667666

667+
if (tcon->ses->user_name == NULL) {
668+
cifs_dbg(FYI, "Can't validate negotiate: null user mount\n");
669+
return 0; /* validation requires signing */
670+
}
671+
672+
if (tcon->ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)
673+
cifs_dbg(VFS, "Unexpected null user (anonymous) auth flag sent by server\n");
674+
668675
vneg_inbuf.Capabilities =
669676
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
670677
memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,

0 commit comments

Comments
 (0)