Headline
CVE-2021-45100: Do not set SMB2_GLOBAL_CAP_ENCRYPTION for SMB 3.1.1. Fixes #550 by socram8888 · Pull Request #551 · cifsd-team/ksmbd
The ksmbd server through 3.4.2, as used in the Linux kernel through 5.15.8, sometimes communicates in cleartext even though encryption has been enabled. This occurs because it sets the SMB2_GLOBAL_CAP_ENCRYPTION flag when using the SMB 3.1.1 protocol, which is a violation of the SMB protocol specification. When Windows 10 detects this protocol violation, it disables encryption.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
According to the official Microsoft MS-SMB2 document section 3.3.5.4, this
flag should be used only for 3.0 and 3.0.2 dialects. Setting it for 3.1.1 is
a violation of the specification.
This caused Windows 10 to detect a mistake in the protocol, and disable
encryption despite being enabled in the settings.
Copy link
Member
@@ -323,9 +323,6 @@ int init_smb3_11_server(struct ksmbd_conn *conn)
if (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB2_LEASES)
conn->vals->capabilities |= SMB2_GLOBAL_CAP_LEASING;
if (conn->cipher_type)
conn->vals->capabilities |= SMB2_GLOBAL_CAP_ENCRYPTION;
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the root cause of your encryption problem?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, Looks great!
* SMB 3.1.1 uses the cipher_type field.
*/
encrequested = conn->cipher_type ||
(conn->vals->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION);
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don’t need to use encrequested variable.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created the variable to make clearer the conditions on which the encryption applied, since there were already a sizeable amount of operands in the if statement.
Mixing complex and, ors and bit-twiddling in the same if would be a big no for me since it requires thinking about C’s operand priorities, but if you’d prefer I can change it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve moved the logic into a function, which makes it IMHO clearer and reduces duplicate code.
I’ll send it now to the LKML for further review.
According to the official Microsoft MS-SMB2 document section 3.3.5.4, this flag should be used only for 3.0 and 3.0.2 dialects. Setting it for 3.1.1 is a violation of the specification.
*
* Return: true if should be encrypted, else false
*/
static bool should_encrypt(struct ksmbd_conn *conn)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t like this naming sense. how about smb3_encryption_negotiated()
I’ve just sent the patch to the LKML, so I am gonna close this PR :)