* [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support @ 2021-09-27 12:47 Namjae Jeon 2021-09-27 12:47 ` [PATCH 2/2] ksmbd: remove NTLMv1 authentication Namjae Jeon 2021-09-28 14:46 ` [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Tom Talpey 0 siblings, 2 replies; 8+ messages in thread From: Namjae Jeon @ 2021-09-27 12:47 UTC (permalink / raw) To: linux-cifs Cc: Namjae Jeon, Tom Talpey, Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee Although ksmbd doesn't send SMB2.0 support in supported dialect list of smb negotiate response, There is the leftover of smb2.0 dialect. This patch remove it not to support SMB2.0 in ksmbd. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/smb2ops.c | 5 ----- fs/ksmbd/smb2pdu.c | 15 ++++----------- fs/ksmbd/smb2pdu.h | 1 - fs/ksmbd/smb_common.c | 4 ++-- 4 files changed, 6 insertions(+), 19 deletions(-) diff --git a/fs/ksmbd/smb2ops.c b/fs/ksmbd/smb2ops.c index 197473871aa4..b06456eb587b 100644 --- a/fs/ksmbd/smb2ops.c +++ b/fs/ksmbd/smb2ops.c @@ -187,11 +187,6 @@ static struct smb_version_cmds smb2_0_server_cmds[NUMBER_OF_SMB2_COMMANDS] = { [SMB2_CHANGE_NOTIFY_HE] = { .proc = smb2_notify}, }; -int init_smb2_0_server(struct ksmbd_conn *conn) -{ - return -EOPNOTSUPP; -} - /** * init_smb2_1_server() - initialize a smb server connection with smb2.1 * command dispatcher diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 88e94a8e4a15..b7d0406d1a14 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -236,7 +236,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work) if (conn->need_neg == false) return -EINVAL; - if (!(conn->dialect >= SMB20_PROT_ID && + if (!(conn->dialect >= SMB21_PROT_ID && conn->dialect <= SMB311_PROT_ID)) return -EINVAL; @@ -1166,13 +1166,6 @@ int smb2_handle_negotiate(struct ksmbd_work *work) case SMB21_PROT_ID: init_smb2_1_server(conn); break; - case SMB20_PROT_ID: - rc = init_smb2_0_server(conn); - if (rc) { - rsp->hdr.Status = STATUS_NOT_SUPPORTED; - goto err_out; - } - break; case SMB2X_PROT_ID: case BAD_PROT_ID: default: @@ -1191,7 +1184,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work) rsp->MaxReadSize = cpu_to_le32(conn->vals->max_read_size); rsp->MaxWriteSize = cpu_to_le32(conn->vals->max_write_size); - if (conn->dialect > SMB20_PROT_ID) { + if (conn->dialect >= SMB21_PROT_ID) { memcpy(conn->ClientGUID, req->ClientGUID, SMB2_CLIENT_GUID_SIZE); conn->cli_sec_mode = le16_to_cpu(req->SecurityMode); @@ -1537,7 +1530,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) } } - if (conn->dialect > SMB20_PROT_ID) { + if (conn->dialect >= SMB21_PROT_ID) { if (!ksmbd_conn_lookup_dialect(conn)) { pr_err("fail to verify the dialect\n"); return -ENOENT; @@ -1623,7 +1616,7 @@ static int krb5_authenticate(struct ksmbd_work *work) } } - if (conn->dialect > SMB20_PROT_ID) { + if (conn->dialect >= SMB21_PROT_ID) { if (!ksmbd_conn_lookup_dialect(conn)) { pr_err("fail to verify the dialect\n"); return -ENOENT; diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h index 261825d06391..a6dec5ec6a54 100644 --- a/fs/ksmbd/smb2pdu.h +++ b/fs/ksmbd/smb2pdu.h @@ -1637,7 +1637,6 @@ struct smb2_posix_info { } __packed; /* functions */ -int init_smb2_0_server(struct ksmbd_conn *conn); void init_smb2_1_server(struct ksmbd_conn *conn); void init_smb3_0_server(struct ksmbd_conn *conn); void init_smb3_02_server(struct ksmbd_conn *conn); diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c index b6c4c7e960fa..435ca8df590b 100644 --- a/fs/ksmbd/smb_common.c +++ b/fs/ksmbd/smb_common.c @@ -88,7 +88,7 @@ unsigned int ksmbd_server_side_copy_max_total_size(void) inline int ksmbd_min_protocol(void) { - return SMB2_PROT; + return SMB21_PROT; } inline int ksmbd_max_protocol(void) @@ -427,7 +427,7 @@ int ksmbd_extract_shortname(struct ksmbd_conn *conn, const char *longname, static int __smb2_negotiate(struct ksmbd_conn *conn) { - return (conn->dialect >= SMB20_PROT_ID && + return (conn->dialect >= SMB21_PROT_ID && conn->dialect <= SMB311_PROT_ID); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ksmbd: remove NTLMv1 authentication 2021-09-27 12:47 [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Namjae Jeon @ 2021-09-27 12:47 ` Namjae Jeon 2021-09-28 14:32 ` Tom Talpey 2021-09-28 14:46 ` [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Tom Talpey 1 sibling, 1 reply; 8+ messages in thread From: Namjae Jeon @ 2021-09-27 12:47 UTC (permalink / raw) To: linux-cifs Cc: Namjae Jeon, Tom Talpey, Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee Remove insecure NTLMv1 authentication. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/auth.c | 205 ------------------------------------------ fs/ksmbd/crypto_ctx.c | 16 ---- fs/ksmbd/crypto_ctx.h | 8 -- 3 files changed, 229 deletions(-) diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c index de36f12070bf..71c989f1568d 100644 --- a/fs/ksmbd/auth.c +++ b/fs/ksmbd/auth.c @@ -68,125 +68,6 @@ void ksmbd_copy_gss_neg_header(void *buf) memcpy(buf, NEGOTIATE_GSS_HEADER, AUTH_GSS_LENGTH); } -static void -str_to_key(unsigned char *str, unsigned char *key) -{ - int i; - - key[0] = str[0] >> 1; - key[1] = ((str[0] & 0x01) << 6) | (str[1] >> 2); - key[2] = ((str[1] & 0x03) << 5) | (str[2] >> 3); - key[3] = ((str[2] & 0x07) << 4) | (str[3] >> 4); - key[4] = ((str[3] & 0x0F) << 3) | (str[4] >> 5); - key[5] = ((str[4] & 0x1F) << 2) | (str[5] >> 6); - key[6] = ((str[5] & 0x3F) << 1) | (str[6] >> 7); - key[7] = str[6] & 0x7F; - for (i = 0; i < 8; i++) - key[i] = (key[i] << 1); -} - -static int -smbhash(unsigned char *out, const unsigned char *in, unsigned char *key) -{ - unsigned char key2[8]; - struct des_ctx ctx; - - if (fips_enabled) { - ksmbd_debug(AUTH, "FIPS compliance enabled: DES not permitted\n"); - return -ENOENT; - } - - str_to_key(key, key2); - des_expand_key(&ctx, key2, DES_KEY_SIZE); - des_encrypt(&ctx, out, in); - memzero_explicit(&ctx, sizeof(ctx)); - return 0; -} - -static int ksmbd_enc_p24(unsigned char *p21, const unsigned char *c8, unsigned char *p24) -{ - int rc; - - rc = smbhash(p24, c8, p21); - if (rc) - return rc; - rc = smbhash(p24 + 8, c8, p21 + 7); - if (rc) - return rc; - return smbhash(p24 + 16, c8, p21 + 14); -} - -/* produce a md4 message digest from data of length n bytes */ -static int ksmbd_enc_md4(unsigned char *md4_hash, unsigned char *link_str, - int link_len) -{ - int rc; - struct ksmbd_crypto_ctx *ctx; - - ctx = ksmbd_crypto_ctx_find_md4(); - if (!ctx) { - ksmbd_debug(AUTH, "Crypto md4 allocation error\n"); - return -ENOMEM; - } - - rc = crypto_shash_init(CRYPTO_MD4(ctx)); - if (rc) { - ksmbd_debug(AUTH, "Could not init md4 shash\n"); - goto out; - } - - rc = crypto_shash_update(CRYPTO_MD4(ctx), link_str, link_len); - if (rc) { - ksmbd_debug(AUTH, "Could not update with link_str\n"); - goto out; - } - - rc = crypto_shash_final(CRYPTO_MD4(ctx), md4_hash); - if (rc) - ksmbd_debug(AUTH, "Could not generate md4 hash\n"); -out: - ksmbd_release_crypto_ctx(ctx); - return rc; -} - -static int ksmbd_enc_update_sess_key(unsigned char *md5_hash, char *nonce, - char *server_challenge, int len) -{ - int rc; - struct ksmbd_crypto_ctx *ctx; - - ctx = ksmbd_crypto_ctx_find_md5(); - if (!ctx) { - ksmbd_debug(AUTH, "Crypto md5 allocation error\n"); - return -ENOMEM; - } - - rc = crypto_shash_init(CRYPTO_MD5(ctx)); - if (rc) { - ksmbd_debug(AUTH, "Could not init md5 shash\n"); - goto out; - } - - rc = crypto_shash_update(CRYPTO_MD5(ctx), server_challenge, len); - if (rc) { - ksmbd_debug(AUTH, "Could not update with challenge\n"); - goto out; - } - - rc = crypto_shash_update(CRYPTO_MD5(ctx), nonce, len); - if (rc) { - ksmbd_debug(AUTH, "Could not update with nonce\n"); - goto out; - } - - rc = crypto_shash_final(CRYPTO_MD5(ctx), md5_hash); - if (rc) - ksmbd_debug(AUTH, "Could not generate md5 hash\n"); -out: - ksmbd_release_crypto_ctx(ctx); - return rc; -} - /** * ksmbd_gen_sess_key() - function to generate session key * @sess: session of connection @@ -324,43 +205,6 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash, return ret; } -/** - * ksmbd_auth_ntlm() - NTLM authentication handler - * @sess: session of connection - * @pw_buf: NTLM challenge response - * @passkey: user password - * - * Return: 0 on success, error number on error - */ -int ksmbd_auth_ntlm(struct ksmbd_session *sess, char *pw_buf) -{ - int rc; - unsigned char p21[21]; - char key[CIFS_AUTH_RESP_SIZE]; - - memset(p21, '\0', 21); - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); - rc = ksmbd_enc_p24(p21, sess->ntlmssp.cryptkey, key); - if (rc) { - pr_err("password processing failed\n"); - return rc; - } - - ksmbd_enc_md4(sess->sess_key, user_passkey(sess->user), - CIFS_SMB1_SESSKEY_SIZE); - memcpy(sess->sess_key + CIFS_SMB1_SESSKEY_SIZE, key, - CIFS_AUTH_RESP_SIZE); - sess->sequence_number = 1; - - if (strncmp(pw_buf, key, CIFS_AUTH_RESP_SIZE) != 0) { - ksmbd_debug(AUTH, "ntlmv1 authentication failed\n"); - return -EINVAL; - } - - ksmbd_debug(AUTH, "ntlmv1 authentication pass\n"); - return 0; -} - /** * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler * @sess: session of connection @@ -441,44 +285,6 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2, return rc; } -/** - * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication handler - * @sess: session of connection - * @client_nonce: client nonce from LM response. - * @ntlm_resp: ntlm response data from client. - * - * Return: 0 on success, error number on error - */ -static int __ksmbd_auth_ntlmv2(struct ksmbd_session *sess, char *client_nonce, - char *ntlm_resp) -{ - char sess_key[CIFS_SMB1_SESSKEY_SIZE] = {0}; - int rc; - unsigned char p21[21]; - char key[CIFS_AUTH_RESP_SIZE]; - - rc = ksmbd_enc_update_sess_key(sess_key, - client_nonce, - (char *)sess->ntlmssp.cryptkey, 8); - if (rc) { - pr_err("password processing failed\n"); - goto out; - } - - memset(p21, '\0', 21); - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); - rc = ksmbd_enc_p24(p21, sess_key, key); - if (rc) { - pr_err("password processing failed\n"); - goto out; - } - - if (memcmp(ntlm_resp, key, CIFS_AUTH_RESP_SIZE) != 0) - rc = -EINVAL; -out: - return rc; -} - /** * ksmbd_decode_ntlmssp_auth_blob() - helper function to construct * authenticate blob @@ -512,17 +318,6 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob, nt_off = le32_to_cpu(authblob->NtChallengeResponse.BufferOffset); nt_len = le16_to_cpu(authblob->NtChallengeResponse.Length); - /* process NTLM authentication */ - if (nt_len == CIFS_AUTH_RESP_SIZE) { - if (le32_to_cpu(authblob->NegotiateFlags) & - NTLMSSP_NEGOTIATE_EXTENDED_SEC) - return __ksmbd_auth_ntlmv2(sess, (char *)authblob + - lm_off, (char *)authblob + nt_off); - else - return ksmbd_auth_ntlm(sess, (char *)authblob + - nt_off); - } - /* TODO : use domain name that imported from configuration file */ domain_name = smb_strndup_from_utf16((const char *)authblob + le32_to_cpu(authblob->DomainName.BufferOffset), diff --git a/fs/ksmbd/crypto_ctx.c b/fs/ksmbd/crypto_ctx.c index 5f4b1008d17e..81488d04199d 100644 --- a/fs/ksmbd/crypto_ctx.c +++ b/fs/ksmbd/crypto_ctx.c @@ -81,12 +81,6 @@ static struct shash_desc *alloc_shash_desc(int id) case CRYPTO_SHASH_SHA512: tfm = crypto_alloc_shash("sha512", 0, 0); break; - case CRYPTO_SHASH_MD4: - tfm = crypto_alloc_shash("md4", 0, 0); - break; - case CRYPTO_SHASH_MD5: - tfm = crypto_alloc_shash("md5", 0, 0); - break; default: return NULL; } @@ -214,16 +208,6 @@ struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void) return ____crypto_shash_ctx_find(CRYPTO_SHASH_SHA512); } -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void) -{ - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD4); -} - -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void) -{ - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD5); -} - static struct ksmbd_crypto_ctx *____crypto_aead_ctx_find(int id) { struct ksmbd_crypto_ctx *ctx; diff --git a/fs/ksmbd/crypto_ctx.h b/fs/ksmbd/crypto_ctx.h index ef11154b43df..4a367c62f653 100644 --- a/fs/ksmbd/crypto_ctx.h +++ b/fs/ksmbd/crypto_ctx.h @@ -15,8 +15,6 @@ enum { CRYPTO_SHASH_CMACAES, CRYPTO_SHASH_SHA256, CRYPTO_SHASH_SHA512, - CRYPTO_SHASH_MD4, - CRYPTO_SHASH_MD5, CRYPTO_SHASH_MAX, }; @@ -43,8 +41,6 @@ struct ksmbd_crypto_ctx { #define CRYPTO_CMACAES(c) ((c)->desc[CRYPTO_SHASH_CMACAES]) #define CRYPTO_SHA256(c) ((c)->desc[CRYPTO_SHASH_SHA256]) #define CRYPTO_SHA512(c) ((c)->desc[CRYPTO_SHASH_SHA512]) -#define CRYPTO_MD4(c) ((c)->desc[CRYPTO_SHASH_MD4]) -#define CRYPTO_MD5(c) ((c)->desc[CRYPTO_SHASH_MD5]) #define CRYPTO_HMACMD5_TFM(c) ((c)->desc[CRYPTO_SHASH_HMACMD5]->tfm) #define CRYPTO_HMACSHA256_TFM(c)\ @@ -52,8 +48,6 @@ struct ksmbd_crypto_ctx { #define CRYPTO_CMACAES_TFM(c) ((c)->desc[CRYPTO_SHASH_CMACAES]->tfm) #define CRYPTO_SHA256_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA256]->tfm) #define CRYPTO_SHA512_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA512]->tfm) -#define CRYPTO_MD4_TFM(c) ((c)->desc[CRYPTO_SHASH_MD4]->tfm) -#define CRYPTO_MD5_TFM(c) ((c)->desc[CRYPTO_SHASH_MD5]->tfm) #define CRYPTO_GCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_GCM]) #define CRYPTO_CCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_CCM]) @@ -64,8 +58,6 @@ struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_hmacsha256(void); struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_cmacaes(void); struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void); struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha256(void); -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void); -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void); struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_gcm(void); struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_ccm(void); void ksmbd_crypto_destroy(void); -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ksmbd: remove NTLMv1 authentication 2021-09-27 12:47 ` [PATCH 2/2] ksmbd: remove NTLMv1 authentication Namjae Jeon @ 2021-09-28 14:32 ` Tom Talpey 2021-09-29 0:34 ` Namjae Jeon 0 siblings, 1 reply; 8+ messages in thread From: Tom Talpey @ 2021-09-28 14:32 UTC (permalink / raw) To: Namjae Jeon, linux-cifs Cc: Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee On 9/27/2021 8:47 AM, Namjae Jeon wrote: > Remove insecure NTLMv1 authentication. There are some extremely confusing name overloads in this file. Apparently "ksmbd_auth_ntlmv2()" and "__ksmb2_auth_ntlmv2()" are entirely different things! Yes, this patch removes one, but it's not easy to review. > /** > * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler > * @sess: session of connection > * @ntlmv2: NTLMv2 challenge response > * @blen: NTLMv2 blob length > * @domain_name: domain name > * > * Return: 0 on success, error number on error > */ > /** > * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication handler > * @sess: session of connection > * @client_nonce: client nonce from LM response. > * @ntlm_resp: ntlm response data from client. > * > * Return: 0 on success, error number on error > */ Two questions: 1) Have you tested this does not remove existing NTLMv2 support? 2) Does this fully clean up the rather insane function naming? Tom. > Cc: Tom Talpey <tom@talpey.com> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Ralph Böhme <slow@samba.org> > Cc: Steve French <smfrench@gmail.com> > Cc: Sergey Senozhatsky <senozhatsky@chromium.org> > Cc: Hyunchul Lee <hyc.lee@gmail.com> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/auth.c | 205 ------------------------------------------ > fs/ksmbd/crypto_ctx.c | 16 ---- > fs/ksmbd/crypto_ctx.h | 8 -- > 3 files changed, 229 deletions(-) > > diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c > index de36f12070bf..71c989f1568d 100644 > --- a/fs/ksmbd/auth.c > +++ b/fs/ksmbd/auth.c > @@ -68,125 +68,6 @@ void ksmbd_copy_gss_neg_header(void *buf) > memcpy(buf, NEGOTIATE_GSS_HEADER, AUTH_GSS_LENGTH); > } > > -static void > -str_to_key(unsigned char *str, unsigned char *key) > -{ > - int i; > - > - key[0] = str[0] >> 1; > - key[1] = ((str[0] & 0x01) << 6) | (str[1] >> 2); > - key[2] = ((str[1] & 0x03) << 5) | (str[2] >> 3); > - key[3] = ((str[2] & 0x07) << 4) | (str[3] >> 4); > - key[4] = ((str[3] & 0x0F) << 3) | (str[4] >> 5); > - key[5] = ((str[4] & 0x1F) << 2) | (str[5] >> 6); > - key[6] = ((str[5] & 0x3F) << 1) | (str[6] >> 7); > - key[7] = str[6] & 0x7F; > - for (i = 0; i < 8; i++) > - key[i] = (key[i] << 1); > -} > - > -static int > -smbhash(unsigned char *out, const unsigned char *in, unsigned char *key) > -{ > - unsigned char key2[8]; > - struct des_ctx ctx; > - > - if (fips_enabled) { > - ksmbd_debug(AUTH, "FIPS compliance enabled: DES not permitted\n"); > - return -ENOENT; > - } > - > - str_to_key(key, key2); > - des_expand_key(&ctx, key2, DES_KEY_SIZE); > - des_encrypt(&ctx, out, in); > - memzero_explicit(&ctx, sizeof(ctx)); > - return 0; > -} > - > -static int ksmbd_enc_p24(unsigned char *p21, const unsigned char *c8, unsigned char *p24) > -{ > - int rc; > - > - rc = smbhash(p24, c8, p21); > - if (rc) > - return rc; > - rc = smbhash(p24 + 8, c8, p21 + 7); > - if (rc) > - return rc; > - return smbhash(p24 + 16, c8, p21 + 14); > -} > - > -/* produce a md4 message digest from data of length n bytes */ > -static int ksmbd_enc_md4(unsigned char *md4_hash, unsigned char *link_str, > - int link_len) > -{ > - int rc; > - struct ksmbd_crypto_ctx *ctx; > - > - ctx = ksmbd_crypto_ctx_find_md4(); > - if (!ctx) { > - ksmbd_debug(AUTH, "Crypto md4 allocation error\n"); > - return -ENOMEM; > - } > - > - rc = crypto_shash_init(CRYPTO_MD4(ctx)); > - if (rc) { > - ksmbd_debug(AUTH, "Could not init md4 shash\n"); > - goto out; > - } > - > - rc = crypto_shash_update(CRYPTO_MD4(ctx), link_str, link_len); > - if (rc) { > - ksmbd_debug(AUTH, "Could not update with link_str\n"); > - goto out; > - } > - > - rc = crypto_shash_final(CRYPTO_MD4(ctx), md4_hash); > - if (rc) > - ksmbd_debug(AUTH, "Could not generate md4 hash\n"); > -out: > - ksmbd_release_crypto_ctx(ctx); > - return rc; > -} > - > -static int ksmbd_enc_update_sess_key(unsigned char *md5_hash, char *nonce, > - char *server_challenge, int len) > -{ > - int rc; > - struct ksmbd_crypto_ctx *ctx; > - > - ctx = ksmbd_crypto_ctx_find_md5(); > - if (!ctx) { > - ksmbd_debug(AUTH, "Crypto md5 allocation error\n"); > - return -ENOMEM; > - } > - > - rc = crypto_shash_init(CRYPTO_MD5(ctx)); > - if (rc) { > - ksmbd_debug(AUTH, "Could not init md5 shash\n"); > - goto out; > - } > - > - rc = crypto_shash_update(CRYPTO_MD5(ctx), server_challenge, len); > - if (rc) { > - ksmbd_debug(AUTH, "Could not update with challenge\n"); > - goto out; > - } > - > - rc = crypto_shash_update(CRYPTO_MD5(ctx), nonce, len); > - if (rc) { > - ksmbd_debug(AUTH, "Could not update with nonce\n"); > - goto out; > - } > - > - rc = crypto_shash_final(CRYPTO_MD5(ctx), md5_hash); > - if (rc) > - ksmbd_debug(AUTH, "Could not generate md5 hash\n"); > -out: > - ksmbd_release_crypto_ctx(ctx); > - return rc; > -} > - > /** > * ksmbd_gen_sess_key() - function to generate session key > * @sess: session of connection > @@ -324,43 +205,6 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash, > return ret; > } > > -/** > - * ksmbd_auth_ntlm() - NTLM authentication handler > - * @sess: session of connection > - * @pw_buf: NTLM challenge response > - * @passkey: user password > - * > - * Return: 0 on success, error number on error > - */ > -int ksmbd_auth_ntlm(struct ksmbd_session *sess, char *pw_buf) > -{ > - int rc; > - unsigned char p21[21]; > - char key[CIFS_AUTH_RESP_SIZE]; > - > - memset(p21, '\0', 21); > - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); > - rc = ksmbd_enc_p24(p21, sess->ntlmssp.cryptkey, key); > - if (rc) { > - pr_err("password processing failed\n"); > - return rc; > - } > - > - ksmbd_enc_md4(sess->sess_key, user_passkey(sess->user), > - CIFS_SMB1_SESSKEY_SIZE); > - memcpy(sess->sess_key + CIFS_SMB1_SESSKEY_SIZE, key, > - CIFS_AUTH_RESP_SIZE); > - sess->sequence_number = 1; > - > - if (strncmp(pw_buf, key, CIFS_AUTH_RESP_SIZE) != 0) { > - ksmbd_debug(AUTH, "ntlmv1 authentication failed\n"); > - return -EINVAL; > - } > - > - ksmbd_debug(AUTH, "ntlmv1 authentication pass\n"); > - return 0; > -} > - > /** > * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler > * @sess: session of connection > @@ -441,44 +285,6 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2, > return rc; > } > > -/** > - * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication handler > - * @sess: session of connection > - * @client_nonce: client nonce from LM response. > - * @ntlm_resp: ntlm response data from client. > - * > - * Return: 0 on success, error number on error > - */ > -static int __ksmbd_auth_ntlmv2(struct ksmbd_session *sess, char *client_nonce, > - char *ntlm_resp) > -{ > - char sess_key[CIFS_SMB1_SESSKEY_SIZE] = {0}; > - int rc; > - unsigned char p21[21]; > - char key[CIFS_AUTH_RESP_SIZE]; > - > - rc = ksmbd_enc_update_sess_key(sess_key, > - client_nonce, > - (char *)sess->ntlmssp.cryptkey, 8); > - if (rc) { > - pr_err("password processing failed\n"); > - goto out; > - } > - > - memset(p21, '\0', 21); > - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); > - rc = ksmbd_enc_p24(p21, sess_key, key); > - if (rc) { > - pr_err("password processing failed\n"); > - goto out; > - } > - > - if (memcmp(ntlm_resp, key, CIFS_AUTH_RESP_SIZE) != 0) > - rc = -EINVAL; > -out: > - return rc; > -} > - > /** > * ksmbd_decode_ntlmssp_auth_blob() - helper function to construct > * authenticate blob > @@ -512,17 +318,6 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob, > nt_off = le32_to_cpu(authblob->NtChallengeResponse.BufferOffset); > nt_len = le16_to_cpu(authblob->NtChallengeResponse.Length); > > - /* process NTLM authentication */ > - if (nt_len == CIFS_AUTH_RESP_SIZE) { > - if (le32_to_cpu(authblob->NegotiateFlags) & > - NTLMSSP_NEGOTIATE_EXTENDED_SEC) > - return __ksmbd_auth_ntlmv2(sess, (char *)authblob + > - lm_off, (char *)authblob + nt_off); > - else > - return ksmbd_auth_ntlm(sess, (char *)authblob + > - nt_off); > - } > - > /* TODO : use domain name that imported from configuration file */ > domain_name = smb_strndup_from_utf16((const char *)authblob + > le32_to_cpu(authblob->DomainName.BufferOffset), > diff --git a/fs/ksmbd/crypto_ctx.c b/fs/ksmbd/crypto_ctx.c > index 5f4b1008d17e..81488d04199d 100644 > --- a/fs/ksmbd/crypto_ctx.c > +++ b/fs/ksmbd/crypto_ctx.c > @@ -81,12 +81,6 @@ static struct shash_desc *alloc_shash_desc(int id) > case CRYPTO_SHASH_SHA512: > tfm = crypto_alloc_shash("sha512", 0, 0); > break; > - case CRYPTO_SHASH_MD4: > - tfm = crypto_alloc_shash("md4", 0, 0); > - break; > - case CRYPTO_SHASH_MD5: > - tfm = crypto_alloc_shash("md5", 0, 0); > - break; > default: > return NULL; > } > @@ -214,16 +208,6 @@ struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void) > return ____crypto_shash_ctx_find(CRYPTO_SHASH_SHA512); > } > > -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void) > -{ > - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD4); > -} > - > -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void) > -{ > - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD5); > -} > - > static struct ksmbd_crypto_ctx *____crypto_aead_ctx_find(int id) > { > struct ksmbd_crypto_ctx *ctx; > diff --git a/fs/ksmbd/crypto_ctx.h b/fs/ksmbd/crypto_ctx.h > index ef11154b43df..4a367c62f653 100644 > --- a/fs/ksmbd/crypto_ctx.h > +++ b/fs/ksmbd/crypto_ctx.h > @@ -15,8 +15,6 @@ enum { > CRYPTO_SHASH_CMACAES, > CRYPTO_SHASH_SHA256, > CRYPTO_SHASH_SHA512, > - CRYPTO_SHASH_MD4, > - CRYPTO_SHASH_MD5, > CRYPTO_SHASH_MAX, > }; > > @@ -43,8 +41,6 @@ struct ksmbd_crypto_ctx { > #define CRYPTO_CMACAES(c) ((c)->desc[CRYPTO_SHASH_CMACAES]) > #define CRYPTO_SHA256(c) ((c)->desc[CRYPTO_SHASH_SHA256]) > #define CRYPTO_SHA512(c) ((c)->desc[CRYPTO_SHASH_SHA512]) > -#define CRYPTO_MD4(c) ((c)->desc[CRYPTO_SHASH_MD4]) > -#define CRYPTO_MD5(c) ((c)->desc[CRYPTO_SHASH_MD5]) > > #define CRYPTO_HMACMD5_TFM(c) ((c)->desc[CRYPTO_SHASH_HMACMD5]->tfm) > #define CRYPTO_HMACSHA256_TFM(c)\ > @@ -52,8 +48,6 @@ struct ksmbd_crypto_ctx { > #define CRYPTO_CMACAES_TFM(c) ((c)->desc[CRYPTO_SHASH_CMACAES]->tfm) > #define CRYPTO_SHA256_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA256]->tfm) > #define CRYPTO_SHA512_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA512]->tfm) > -#define CRYPTO_MD4_TFM(c) ((c)->desc[CRYPTO_SHASH_MD4]->tfm) > -#define CRYPTO_MD5_TFM(c) ((c)->desc[CRYPTO_SHASH_MD5]->tfm) > > #define CRYPTO_GCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_GCM]) > #define CRYPTO_CCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_CCM]) > @@ -64,8 +58,6 @@ struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_hmacsha256(void); > struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_cmacaes(void); > struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void); > struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha256(void); > -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void); > -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void); > struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_gcm(void); > struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_ccm(void); > void ksmbd_crypto_destroy(void); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ksmbd: remove NTLMv1 authentication 2021-09-28 14:32 ` Tom Talpey @ 2021-09-29 0:34 ` Namjae Jeon 2021-09-29 15:02 ` Tom Talpey 0 siblings, 1 reply; 8+ messages in thread From: Namjae Jeon @ 2021-09-29 0:34 UTC (permalink / raw) To: Tom Talpey Cc: linux-cifs, Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee 2021-09-28 23:32 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 9/27/2021 8:47 AM, Namjae Jeon wrote: >> Remove insecure NTLMv1 authentication. > > There are some extremely confusing name overloads in this file. > Apparently "ksmbd_auth_ntlmv2()" and "__ksmb2_auth_ntlmv2()" > are entirely different things! Yes, this patch removes one, > but it's not easy to review. A long time ago, There was a mistake to rename this function to current name during clean-up. > >> /** >> * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler >> * @sess: session of connection >> * @ntlmv2: NTLMv2 challenge response >> * @blen: NTLMv2 blob length >> * @domain_name: domain name >> * >> * Return: 0 on success, error number on error >> */ > >> /** >> * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication >> handler >> * @sess: session of connection >> * @client_nonce: client nonce from LM response. >> * @ntlm_resp: ntlm response data from client. >> * >> * Return: 0 on success, error number on error >> */ > > Two questions: > 1) Have you tested this does not remove existing NTLMv2 support? Yes, tested. This is NTLM2 not NTLMv2. > 2) Does this fully clean up the rather insane function naming? Yes, This patch will do all(remove NTLM and insane fucntion name) :) > > Tom. Thank you for your review! > >> Cc: Tom Talpey <tom@talpey.com> >> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> Cc: Ralph Böhme <slow@samba.org> >> Cc: Steve French <smfrench@gmail.com> >> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> >> Cc: Hyunchul Lee <hyc.lee@gmail.com> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/auth.c | 205 ------------------------------------------ >> fs/ksmbd/crypto_ctx.c | 16 ---- >> fs/ksmbd/crypto_ctx.h | 8 -- >> 3 files changed, 229 deletions(-) >> >> diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c >> index de36f12070bf..71c989f1568d 100644 >> --- a/fs/ksmbd/auth.c >> +++ b/fs/ksmbd/auth.c >> @@ -68,125 +68,6 @@ void ksmbd_copy_gss_neg_header(void *buf) >> memcpy(buf, NEGOTIATE_GSS_HEADER, AUTH_GSS_LENGTH); >> } >> >> -static void >> -str_to_key(unsigned char *str, unsigned char *key) >> -{ >> - int i; >> - >> - key[0] = str[0] >> 1; >> - key[1] = ((str[0] & 0x01) << 6) | (str[1] >> 2); >> - key[2] = ((str[1] & 0x03) << 5) | (str[2] >> 3); >> - key[3] = ((str[2] & 0x07) << 4) | (str[3] >> 4); >> - key[4] = ((str[3] & 0x0F) << 3) | (str[4] >> 5); >> - key[5] = ((str[4] & 0x1F) << 2) | (str[5] >> 6); >> - key[6] = ((str[5] & 0x3F) << 1) | (str[6] >> 7); >> - key[7] = str[6] & 0x7F; >> - for (i = 0; i < 8; i++) >> - key[i] = (key[i] << 1); >> -} >> - >> -static int >> -smbhash(unsigned char *out, const unsigned char *in, unsigned char *key) >> -{ >> - unsigned char key2[8]; >> - struct des_ctx ctx; >> - >> - if (fips_enabled) { >> - ksmbd_debug(AUTH, "FIPS compliance enabled: DES not permitted\n"); >> - return -ENOENT; >> - } >> - >> - str_to_key(key, key2); >> - des_expand_key(&ctx, key2, DES_KEY_SIZE); >> - des_encrypt(&ctx, out, in); >> - memzero_explicit(&ctx, sizeof(ctx)); >> - return 0; >> -} >> - >> -static int ksmbd_enc_p24(unsigned char *p21, const unsigned char *c8, >> unsigned char *p24) >> -{ >> - int rc; >> - >> - rc = smbhash(p24, c8, p21); >> - if (rc) >> - return rc; >> - rc = smbhash(p24 + 8, c8, p21 + 7); >> - if (rc) >> - return rc; >> - return smbhash(p24 + 16, c8, p21 + 14); >> -} >> - >> -/* produce a md4 message digest from data of length n bytes */ >> -static int ksmbd_enc_md4(unsigned char *md4_hash, unsigned char >> *link_str, >> - int link_len) >> -{ >> - int rc; >> - struct ksmbd_crypto_ctx *ctx; >> - >> - ctx = ksmbd_crypto_ctx_find_md4(); >> - if (!ctx) { >> - ksmbd_debug(AUTH, "Crypto md4 allocation error\n"); >> - return -ENOMEM; >> - } >> - >> - rc = crypto_shash_init(CRYPTO_MD4(ctx)); >> - if (rc) { >> - ksmbd_debug(AUTH, "Could not init md4 shash\n"); >> - goto out; >> - } >> - >> - rc = crypto_shash_update(CRYPTO_MD4(ctx), link_str, link_len); >> - if (rc) { >> - ksmbd_debug(AUTH, "Could not update with link_str\n"); >> - goto out; >> - } >> - >> - rc = crypto_shash_final(CRYPTO_MD4(ctx), md4_hash); >> - if (rc) >> - ksmbd_debug(AUTH, "Could not generate md4 hash\n"); >> -out: >> - ksmbd_release_crypto_ctx(ctx); >> - return rc; >> -} >> - >> -static int ksmbd_enc_update_sess_key(unsigned char *md5_hash, char >> *nonce, >> - char *server_challenge, int len) >> -{ >> - int rc; >> - struct ksmbd_crypto_ctx *ctx; >> - >> - ctx = ksmbd_crypto_ctx_find_md5(); >> - if (!ctx) { >> - ksmbd_debug(AUTH, "Crypto md5 allocation error\n"); >> - return -ENOMEM; >> - } >> - >> - rc = crypto_shash_init(CRYPTO_MD5(ctx)); >> - if (rc) { >> - ksmbd_debug(AUTH, "Could not init md5 shash\n"); >> - goto out; >> - } >> - >> - rc = crypto_shash_update(CRYPTO_MD5(ctx), server_challenge, len); >> - if (rc) { >> - ksmbd_debug(AUTH, "Could not update with challenge\n"); >> - goto out; >> - } >> - >> - rc = crypto_shash_update(CRYPTO_MD5(ctx), nonce, len); >> - if (rc) { >> - ksmbd_debug(AUTH, "Could not update with nonce\n"); >> - goto out; >> - } >> - >> - rc = crypto_shash_final(CRYPTO_MD5(ctx), md5_hash); >> - if (rc) >> - ksmbd_debug(AUTH, "Could not generate md5 hash\n"); >> -out: >> - ksmbd_release_crypto_ctx(ctx); >> - return rc; >> -} >> - >> /** >> * ksmbd_gen_sess_key() - function to generate session key >> * @sess: session of connection >> @@ -324,43 +205,6 @@ static int calc_ntlmv2_hash(struct ksmbd_session >> *sess, char *ntlmv2_hash, >> return ret; >> } >> >> -/** >> - * ksmbd_auth_ntlm() - NTLM authentication handler >> - * @sess: session of connection >> - * @pw_buf: NTLM challenge response >> - * @passkey: user password >> - * >> - * Return: 0 on success, error number on error >> - */ >> -int ksmbd_auth_ntlm(struct ksmbd_session *sess, char *pw_buf) >> -{ >> - int rc; >> - unsigned char p21[21]; >> - char key[CIFS_AUTH_RESP_SIZE]; >> - >> - memset(p21, '\0', 21); >> - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); >> - rc = ksmbd_enc_p24(p21, sess->ntlmssp.cryptkey, key); >> - if (rc) { >> - pr_err("password processing failed\n"); >> - return rc; >> - } >> - >> - ksmbd_enc_md4(sess->sess_key, user_passkey(sess->user), >> - CIFS_SMB1_SESSKEY_SIZE); >> - memcpy(sess->sess_key + CIFS_SMB1_SESSKEY_SIZE, key, >> - CIFS_AUTH_RESP_SIZE); >> - sess->sequence_number = 1; >> - >> - if (strncmp(pw_buf, key, CIFS_AUTH_RESP_SIZE) != 0) { >> - ksmbd_debug(AUTH, "ntlmv1 authentication failed\n"); >> - return -EINVAL; >> - } >> - >> - ksmbd_debug(AUTH, "ntlmv1 authentication pass\n"); >> - return 0; >> -} >> - >> /** >> * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler >> * @sess: session of connection >> @@ -441,44 +285,6 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, >> struct ntlmv2_resp *ntlmv2, >> return rc; >> } >> >> -/** >> - * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication >> handler >> - * @sess: session of connection >> - * @client_nonce: client nonce from LM response. >> - * @ntlm_resp: ntlm response data from client. >> - * >> - * Return: 0 on success, error number on error >> - */ >> -static int __ksmbd_auth_ntlmv2(struct ksmbd_session *sess, char >> *client_nonce, >> - char *ntlm_resp) >> -{ >> - char sess_key[CIFS_SMB1_SESSKEY_SIZE] = {0}; >> - int rc; >> - unsigned char p21[21]; >> - char key[CIFS_AUTH_RESP_SIZE]; >> - >> - rc = ksmbd_enc_update_sess_key(sess_key, >> - client_nonce, >> - (char *)sess->ntlmssp.cryptkey, 8); >> - if (rc) { >> - pr_err("password processing failed\n"); >> - goto out; >> - } >> - >> - memset(p21, '\0', 21); >> - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); >> - rc = ksmbd_enc_p24(p21, sess_key, key); >> - if (rc) { >> - pr_err("password processing failed\n"); >> - goto out; >> - } >> - >> - if (memcmp(ntlm_resp, key, CIFS_AUTH_RESP_SIZE) != 0) >> - rc = -EINVAL; >> -out: >> - return rc; >> -} >> - >> /** >> * ksmbd_decode_ntlmssp_auth_blob() - helper function to construct >> * authenticate blob >> @@ -512,17 +318,6 @@ int ksmbd_decode_ntlmssp_auth_blob(struct >> authenticate_message *authblob, >> nt_off = le32_to_cpu(authblob->NtChallengeResponse.BufferOffset); >> nt_len = le16_to_cpu(authblob->NtChallengeResponse.Length); >> >> - /* process NTLM authentication */ >> - if (nt_len == CIFS_AUTH_RESP_SIZE) { >> - if (le32_to_cpu(authblob->NegotiateFlags) & >> - NTLMSSP_NEGOTIATE_EXTENDED_SEC) >> - return __ksmbd_auth_ntlmv2(sess, (char *)authblob + >> - lm_off, (char *)authblob + nt_off); >> - else >> - return ksmbd_auth_ntlm(sess, (char *)authblob + >> - nt_off); >> - } >> - >> /* TODO : use domain name that imported from configuration file */ >> domain_name = smb_strndup_from_utf16((const char *)authblob + >> le32_to_cpu(authblob->DomainName.BufferOffset), >> diff --git a/fs/ksmbd/crypto_ctx.c b/fs/ksmbd/crypto_ctx.c >> index 5f4b1008d17e..81488d04199d 100644 >> --- a/fs/ksmbd/crypto_ctx.c >> +++ b/fs/ksmbd/crypto_ctx.c >> @@ -81,12 +81,6 @@ static struct shash_desc *alloc_shash_desc(int id) >> case CRYPTO_SHASH_SHA512: >> tfm = crypto_alloc_shash("sha512", 0, 0); >> break; >> - case CRYPTO_SHASH_MD4: >> - tfm = crypto_alloc_shash("md4", 0, 0); >> - break; >> - case CRYPTO_SHASH_MD5: >> - tfm = crypto_alloc_shash("md5", 0, 0); >> - break; >> default: >> return NULL; >> } >> @@ -214,16 +208,6 @@ struct ksmbd_crypto_ctx >> *ksmbd_crypto_ctx_find_sha512(void) >> return ____crypto_shash_ctx_find(CRYPTO_SHASH_SHA512); >> } >> >> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void) >> -{ >> - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD4); >> -} >> - >> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void) >> -{ >> - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD5); >> -} >> - >> static struct ksmbd_crypto_ctx *____crypto_aead_ctx_find(int id) >> { >> struct ksmbd_crypto_ctx *ctx; >> diff --git a/fs/ksmbd/crypto_ctx.h b/fs/ksmbd/crypto_ctx.h >> index ef11154b43df..4a367c62f653 100644 >> --- a/fs/ksmbd/crypto_ctx.h >> +++ b/fs/ksmbd/crypto_ctx.h >> @@ -15,8 +15,6 @@ enum { >> CRYPTO_SHASH_CMACAES, >> CRYPTO_SHASH_SHA256, >> CRYPTO_SHASH_SHA512, >> - CRYPTO_SHASH_MD4, >> - CRYPTO_SHASH_MD5, >> CRYPTO_SHASH_MAX, >> }; >> >> @@ -43,8 +41,6 @@ struct ksmbd_crypto_ctx { >> #define CRYPTO_CMACAES(c) ((c)->desc[CRYPTO_SHASH_CMACAES]) >> #define CRYPTO_SHA256(c) ((c)->desc[CRYPTO_SHASH_SHA256]) >> #define CRYPTO_SHA512(c) ((c)->desc[CRYPTO_SHASH_SHA512]) >> -#define CRYPTO_MD4(c) ((c)->desc[CRYPTO_SHASH_MD4]) >> -#define CRYPTO_MD5(c) ((c)->desc[CRYPTO_SHASH_MD5]) >> >> #define CRYPTO_HMACMD5_TFM(c) ((c)->desc[CRYPTO_SHASH_HMACMD5]->tfm) >> #define CRYPTO_HMACSHA256_TFM(c)\ >> @@ -52,8 +48,6 @@ struct ksmbd_crypto_ctx { >> #define CRYPTO_CMACAES_TFM(c) ((c)->desc[CRYPTO_SHASH_CMACAES]->tfm) >> #define CRYPTO_SHA256_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA256]->tfm) >> #define CRYPTO_SHA512_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA512]->tfm) >> -#define CRYPTO_MD4_TFM(c) ((c)->desc[CRYPTO_SHASH_MD4]->tfm) >> -#define CRYPTO_MD5_TFM(c) ((c)->desc[CRYPTO_SHASH_MD5]->tfm) >> >> #define CRYPTO_GCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_GCM]) >> #define CRYPTO_CCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_CCM]) >> @@ -64,8 +58,6 @@ struct ksmbd_crypto_ctx >> *ksmbd_crypto_ctx_find_hmacsha256(void); >> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_cmacaes(void); >> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void); >> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha256(void); >> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void); >> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void); >> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_gcm(void); >> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_ccm(void); >> void ksmbd_crypto_destroy(void); >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ksmbd: remove NTLMv1 authentication 2021-09-29 0:34 ` Namjae Jeon @ 2021-09-29 15:02 ` Tom Talpey 2021-09-29 21:19 ` Steve French 0 siblings, 1 reply; 8+ messages in thread From: Tom Talpey @ 2021-09-29 15:02 UTC (permalink / raw) To: Namjae Jeon Cc: linux-cifs, Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee Ok, feel to free to add my Reviewed-By: Tom Talpey <tom@talpey.com> On 9/28/2021 8:34 PM, Namjae Jeon wrote: > 2021-09-28 23:32 GMT+09:00, Tom Talpey <tom@talpey.com>: >> On 9/27/2021 8:47 AM, Namjae Jeon wrote: >>> Remove insecure NTLMv1 authentication. >> >> There are some extremely confusing name overloads in this file. >> Apparently "ksmbd_auth_ntlmv2()" and "__ksmb2_auth_ntlmv2()" >> are entirely different things! Yes, this patch removes one, >> but it's not easy to review. > A long time ago, There was a mistake to rename this function to > current name during clean-up. >> >>> /** >>> * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler >>> * @sess: session of connection >>> * @ntlmv2: NTLMv2 challenge response >>> * @blen: NTLMv2 blob length >>> * @domain_name: domain name >>> * >>> * Return: 0 on success, error number on error >>> */ >> >>> /** >>> * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication >>> handler >>> * @sess: session of connection >>> * @client_nonce: client nonce from LM response. >>> * @ntlm_resp: ntlm response data from client. >>> * >>> * Return: 0 on success, error number on error >>> */ >> >> Two questions: >> 1) Have you tested this does not remove existing NTLMv2 support? > Yes, tested. This is NTLM2 not NTLMv2. >> 2) Does this fully clean up the rather insane function naming? > Yes, This patch will do all(remove NTLM and insane fucntion name) :) >> >> Tom. > Thank you for your review! >> >>> Cc: Tom Talpey <tom@talpey.com> >>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >>> Cc: Ralph Böhme <slow@samba.org> >>> Cc: Steve French <smfrench@gmail.com> >>> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> >>> Cc: Hyunchul Lee <hyc.lee@gmail.com> >>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >>> --- >>> fs/ksmbd/auth.c | 205 ------------------------------------------ >>> fs/ksmbd/crypto_ctx.c | 16 ---- >>> fs/ksmbd/crypto_ctx.h | 8 -- >>> 3 files changed, 229 deletions(-) >>> >>> diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c >>> index de36f12070bf..71c989f1568d 100644 >>> --- a/fs/ksmbd/auth.c >>> +++ b/fs/ksmbd/auth.c >>> @@ -68,125 +68,6 @@ void ksmbd_copy_gss_neg_header(void *buf) >>> memcpy(buf, NEGOTIATE_GSS_HEADER, AUTH_GSS_LENGTH); >>> } >>> >>> -static void >>> -str_to_key(unsigned char *str, unsigned char *key) >>> -{ >>> - int i; >>> - >>> - key[0] = str[0] >> 1; >>> - key[1] = ((str[0] & 0x01) << 6) | (str[1] >> 2); >>> - key[2] = ((str[1] & 0x03) << 5) | (str[2] >> 3); >>> - key[3] = ((str[2] & 0x07) << 4) | (str[3] >> 4); >>> - key[4] = ((str[3] & 0x0F) << 3) | (str[4] >> 5); >>> - key[5] = ((str[4] & 0x1F) << 2) | (str[5] >> 6); >>> - key[6] = ((str[5] & 0x3F) << 1) | (str[6] >> 7); >>> - key[7] = str[6] & 0x7F; >>> - for (i = 0; i < 8; i++) >>> - key[i] = (key[i] << 1); >>> -} >>> - >>> -static int >>> -smbhash(unsigned char *out, const unsigned char *in, unsigned char *key) >>> -{ >>> - unsigned char key2[8]; >>> - struct des_ctx ctx; >>> - >>> - if (fips_enabled) { >>> - ksmbd_debug(AUTH, "FIPS compliance enabled: DES not permitted\n"); >>> - return -ENOENT; >>> - } >>> - >>> - str_to_key(key, key2); >>> - des_expand_key(&ctx, key2, DES_KEY_SIZE); >>> - des_encrypt(&ctx, out, in); >>> - memzero_explicit(&ctx, sizeof(ctx)); >>> - return 0; >>> -} >>> - >>> -static int ksmbd_enc_p24(unsigned char *p21, const unsigned char *c8, >>> unsigned char *p24) >>> -{ >>> - int rc; >>> - >>> - rc = smbhash(p24, c8, p21); >>> - if (rc) >>> - return rc; >>> - rc = smbhash(p24 + 8, c8, p21 + 7); >>> - if (rc) >>> - return rc; >>> - return smbhash(p24 + 16, c8, p21 + 14); >>> -} >>> - >>> -/* produce a md4 message digest from data of length n bytes */ >>> -static int ksmbd_enc_md4(unsigned char *md4_hash, unsigned char >>> *link_str, >>> - int link_len) >>> -{ >>> - int rc; >>> - struct ksmbd_crypto_ctx *ctx; >>> - >>> - ctx = ksmbd_crypto_ctx_find_md4(); >>> - if (!ctx) { >>> - ksmbd_debug(AUTH, "Crypto md4 allocation error\n"); >>> - return -ENOMEM; >>> - } >>> - >>> - rc = crypto_shash_init(CRYPTO_MD4(ctx)); >>> - if (rc) { >>> - ksmbd_debug(AUTH, "Could not init md4 shash\n"); >>> - goto out; >>> - } >>> - >>> - rc = crypto_shash_update(CRYPTO_MD4(ctx), link_str, link_len); >>> - if (rc) { >>> - ksmbd_debug(AUTH, "Could not update with link_str\n"); >>> - goto out; >>> - } >>> - >>> - rc = crypto_shash_final(CRYPTO_MD4(ctx), md4_hash); >>> - if (rc) >>> - ksmbd_debug(AUTH, "Could not generate md4 hash\n"); >>> -out: >>> - ksmbd_release_crypto_ctx(ctx); >>> - return rc; >>> -} >>> - >>> -static int ksmbd_enc_update_sess_key(unsigned char *md5_hash, char >>> *nonce, >>> - char *server_challenge, int len) >>> -{ >>> - int rc; >>> - struct ksmbd_crypto_ctx *ctx; >>> - >>> - ctx = ksmbd_crypto_ctx_find_md5(); >>> - if (!ctx) { >>> - ksmbd_debug(AUTH, "Crypto md5 allocation error\n"); >>> - return -ENOMEM; >>> - } >>> - >>> - rc = crypto_shash_init(CRYPTO_MD5(ctx)); >>> - if (rc) { >>> - ksmbd_debug(AUTH, "Could not init md5 shash\n"); >>> - goto out; >>> - } >>> - >>> - rc = crypto_shash_update(CRYPTO_MD5(ctx), server_challenge, len); >>> - if (rc) { >>> - ksmbd_debug(AUTH, "Could not update with challenge\n"); >>> - goto out; >>> - } >>> - >>> - rc = crypto_shash_update(CRYPTO_MD5(ctx), nonce, len); >>> - if (rc) { >>> - ksmbd_debug(AUTH, "Could not update with nonce\n"); >>> - goto out; >>> - } >>> - >>> - rc = crypto_shash_final(CRYPTO_MD5(ctx), md5_hash); >>> - if (rc) >>> - ksmbd_debug(AUTH, "Could not generate md5 hash\n"); >>> -out: >>> - ksmbd_release_crypto_ctx(ctx); >>> - return rc; >>> -} >>> - >>> /** >>> * ksmbd_gen_sess_key() - function to generate session key >>> * @sess: session of connection >>> @@ -324,43 +205,6 @@ static int calc_ntlmv2_hash(struct ksmbd_session >>> *sess, char *ntlmv2_hash, >>> return ret; >>> } >>> >>> -/** >>> - * ksmbd_auth_ntlm() - NTLM authentication handler >>> - * @sess: session of connection >>> - * @pw_buf: NTLM challenge response >>> - * @passkey: user password >>> - * >>> - * Return: 0 on success, error number on error >>> - */ >>> -int ksmbd_auth_ntlm(struct ksmbd_session *sess, char *pw_buf) >>> -{ >>> - int rc; >>> - unsigned char p21[21]; >>> - char key[CIFS_AUTH_RESP_SIZE]; >>> - >>> - memset(p21, '\0', 21); >>> - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); >>> - rc = ksmbd_enc_p24(p21, sess->ntlmssp.cryptkey, key); >>> - if (rc) { >>> - pr_err("password processing failed\n"); >>> - return rc; >>> - } >>> - >>> - ksmbd_enc_md4(sess->sess_key, user_passkey(sess->user), >>> - CIFS_SMB1_SESSKEY_SIZE); >>> - memcpy(sess->sess_key + CIFS_SMB1_SESSKEY_SIZE, key, >>> - CIFS_AUTH_RESP_SIZE); >>> - sess->sequence_number = 1; >>> - >>> - if (strncmp(pw_buf, key, CIFS_AUTH_RESP_SIZE) != 0) { >>> - ksmbd_debug(AUTH, "ntlmv1 authentication failed\n"); >>> - return -EINVAL; >>> - } >>> - >>> - ksmbd_debug(AUTH, "ntlmv1 authentication pass\n"); >>> - return 0; >>> -} >>> - >>> /** >>> * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler >>> * @sess: session of connection >>> @@ -441,44 +285,6 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, >>> struct ntlmv2_resp *ntlmv2, >>> return rc; >>> } >>> >>> -/** >>> - * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication >>> handler >>> - * @sess: session of connection >>> - * @client_nonce: client nonce from LM response. >>> - * @ntlm_resp: ntlm response data from client. >>> - * >>> - * Return: 0 on success, error number on error >>> - */ >>> -static int __ksmbd_auth_ntlmv2(struct ksmbd_session *sess, char >>> *client_nonce, >>> - char *ntlm_resp) >>> -{ >>> - char sess_key[CIFS_SMB1_SESSKEY_SIZE] = {0}; >>> - int rc; >>> - unsigned char p21[21]; >>> - char key[CIFS_AUTH_RESP_SIZE]; >>> - >>> - rc = ksmbd_enc_update_sess_key(sess_key, >>> - client_nonce, >>> - (char *)sess->ntlmssp.cryptkey, 8); >>> - if (rc) { >>> - pr_err("password processing failed\n"); >>> - goto out; >>> - } >>> - >>> - memset(p21, '\0', 21); >>> - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); >>> - rc = ksmbd_enc_p24(p21, sess_key, key); >>> - if (rc) { >>> - pr_err("password processing failed\n"); >>> - goto out; >>> - } >>> - >>> - if (memcmp(ntlm_resp, key, CIFS_AUTH_RESP_SIZE) != 0) >>> - rc = -EINVAL; >>> -out: >>> - return rc; >>> -} >>> - >>> /** >>> * ksmbd_decode_ntlmssp_auth_blob() - helper function to construct >>> * authenticate blob >>> @@ -512,17 +318,6 @@ int ksmbd_decode_ntlmssp_auth_blob(struct >>> authenticate_message *authblob, >>> nt_off = le32_to_cpu(authblob->NtChallengeResponse.BufferOffset); >>> nt_len = le16_to_cpu(authblob->NtChallengeResponse.Length); >>> >>> - /* process NTLM authentication */ >>> - if (nt_len == CIFS_AUTH_RESP_SIZE) { >>> - if (le32_to_cpu(authblob->NegotiateFlags) & >>> - NTLMSSP_NEGOTIATE_EXTENDED_SEC) >>> - return __ksmbd_auth_ntlmv2(sess, (char *)authblob + >>> - lm_off, (char *)authblob + nt_off); >>> - else >>> - return ksmbd_auth_ntlm(sess, (char *)authblob + >>> - nt_off); >>> - } >>> - >>> /* TODO : use domain name that imported from configuration file */ >>> domain_name = smb_strndup_from_utf16((const char *)authblob + >>> le32_to_cpu(authblob->DomainName.BufferOffset), >>> diff --git a/fs/ksmbd/crypto_ctx.c b/fs/ksmbd/crypto_ctx.c >>> index 5f4b1008d17e..81488d04199d 100644 >>> --- a/fs/ksmbd/crypto_ctx.c >>> +++ b/fs/ksmbd/crypto_ctx.c >>> @@ -81,12 +81,6 @@ static struct shash_desc *alloc_shash_desc(int id) >>> case CRYPTO_SHASH_SHA512: >>> tfm = crypto_alloc_shash("sha512", 0, 0); >>> break; >>> - case CRYPTO_SHASH_MD4: >>> - tfm = crypto_alloc_shash("md4", 0, 0); >>> - break; >>> - case CRYPTO_SHASH_MD5: >>> - tfm = crypto_alloc_shash("md5", 0, 0); >>> - break; >>> default: >>> return NULL; >>> } >>> @@ -214,16 +208,6 @@ struct ksmbd_crypto_ctx >>> *ksmbd_crypto_ctx_find_sha512(void) >>> return ____crypto_shash_ctx_find(CRYPTO_SHASH_SHA512); >>> } >>> >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void) >>> -{ >>> - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD4); >>> -} >>> - >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void) >>> -{ >>> - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD5); >>> -} >>> - >>> static struct ksmbd_crypto_ctx *____crypto_aead_ctx_find(int id) >>> { >>> struct ksmbd_crypto_ctx *ctx; >>> diff --git a/fs/ksmbd/crypto_ctx.h b/fs/ksmbd/crypto_ctx.h >>> index ef11154b43df..4a367c62f653 100644 >>> --- a/fs/ksmbd/crypto_ctx.h >>> +++ b/fs/ksmbd/crypto_ctx.h >>> @@ -15,8 +15,6 @@ enum { >>> CRYPTO_SHASH_CMACAES, >>> CRYPTO_SHASH_SHA256, >>> CRYPTO_SHASH_SHA512, >>> - CRYPTO_SHASH_MD4, >>> - CRYPTO_SHASH_MD5, >>> CRYPTO_SHASH_MAX, >>> }; >>> >>> @@ -43,8 +41,6 @@ struct ksmbd_crypto_ctx { >>> #define CRYPTO_CMACAES(c) ((c)->desc[CRYPTO_SHASH_CMACAES]) >>> #define CRYPTO_SHA256(c) ((c)->desc[CRYPTO_SHASH_SHA256]) >>> #define CRYPTO_SHA512(c) ((c)->desc[CRYPTO_SHASH_SHA512]) >>> -#define CRYPTO_MD4(c) ((c)->desc[CRYPTO_SHASH_MD4]) >>> -#define CRYPTO_MD5(c) ((c)->desc[CRYPTO_SHASH_MD5]) >>> >>> #define CRYPTO_HMACMD5_TFM(c) ((c)->desc[CRYPTO_SHASH_HMACMD5]->tfm) >>> #define CRYPTO_HMACSHA256_TFM(c)\ >>> @@ -52,8 +48,6 @@ struct ksmbd_crypto_ctx { >>> #define CRYPTO_CMACAES_TFM(c) ((c)->desc[CRYPTO_SHASH_CMACAES]->tfm) >>> #define CRYPTO_SHA256_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA256]->tfm) >>> #define CRYPTO_SHA512_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA512]->tfm) >>> -#define CRYPTO_MD4_TFM(c) ((c)->desc[CRYPTO_SHASH_MD4]->tfm) >>> -#define CRYPTO_MD5_TFM(c) ((c)->desc[CRYPTO_SHASH_MD5]->tfm) >>> >>> #define CRYPTO_GCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_GCM]) >>> #define CRYPTO_CCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_CCM]) >>> @@ -64,8 +58,6 @@ struct ksmbd_crypto_ctx >>> *ksmbd_crypto_ctx_find_hmacsha256(void); >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_cmacaes(void); >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void); >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha256(void); >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void); >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void); >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_gcm(void); >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_ccm(void); >>> void ksmbd_crypto_destroy(void); >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ksmbd: remove NTLMv1 authentication 2021-09-29 15:02 ` Tom Talpey @ 2021-09-29 21:19 ` Steve French 0 siblings, 0 replies; 8+ messages in thread From: Steve French @ 2021-09-29 21:19 UTC (permalink / raw) To: Tom Talpey Cc: Namjae Jeon, CIFS, Ronnie Sahlberg, Ralph Böhme, Sergey Senozhatsky, Hyunchul Lee merged into cifsd-for-next (and ksmbd-for-next) On Wed, Sep 29, 2021 at 10:02 AM Tom Talpey <tom@talpey.com> wrote: > > Ok, feel to free to add my > Reviewed-By: Tom Talpey <tom@talpey.com> > > On 9/28/2021 8:34 PM, Namjae Jeon wrote: > > 2021-09-28 23:32 GMT+09:00, Tom Talpey <tom@talpey.com>: > >> On 9/27/2021 8:47 AM, Namjae Jeon wrote: > >>> Remove insecure NTLMv1 authentication. > >> > >> There are some extremely confusing name overloads in this file. > >> Apparently "ksmbd_auth_ntlmv2()" and "__ksmb2_auth_ntlmv2()" > >> are entirely different things! Yes, this patch removes one, > >> but it's not easy to review. > > A long time ago, There was a mistake to rename this function to > > current name during clean-up. > >> > >>> /** > >>> * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler > >>> * @sess: session of connection > >>> * @ntlmv2: NTLMv2 challenge response > >>> * @blen: NTLMv2 blob length > >>> * @domain_name: domain name > >>> * > >>> * Return: 0 on success, error number on error > >>> */ > >> > >>> /** > >>> * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication > >>> handler > >>> * @sess: session of connection > >>> * @client_nonce: client nonce from LM response. > >>> * @ntlm_resp: ntlm response data from client. > >>> * > >>> * Return: 0 on success, error number on error > >>> */ > >> > >> Two questions: > >> 1) Have you tested this does not remove existing NTLMv2 support? > > Yes, tested. This is NTLM2 not NTLMv2. > >> 2) Does this fully clean up the rather insane function naming? > > Yes, This patch will do all(remove NTLM and insane fucntion name) :) > >> > >> Tom. > > Thank you for your review! > >> > >>> Cc: Tom Talpey <tom@talpey.com> > >>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > >>> Cc: Ralph Böhme <slow@samba.org> > >>> Cc: Steve French <smfrench@gmail.com> > >>> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> > >>> Cc: Hyunchul Lee <hyc.lee@gmail.com> > >>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > >>> --- > >>> fs/ksmbd/auth.c | 205 ------------------------------------------ > >>> fs/ksmbd/crypto_ctx.c | 16 ---- > >>> fs/ksmbd/crypto_ctx.h | 8 -- > >>> 3 files changed, 229 deletions(-) > >>> > >>> diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c > >>> index de36f12070bf..71c989f1568d 100644 > >>> --- a/fs/ksmbd/auth.c > >>> +++ b/fs/ksmbd/auth.c > >>> @@ -68,125 +68,6 @@ void ksmbd_copy_gss_neg_header(void *buf) > >>> memcpy(buf, NEGOTIATE_GSS_HEADER, AUTH_GSS_LENGTH); > >>> } > >>> > >>> -static void > >>> -str_to_key(unsigned char *str, unsigned char *key) > >>> -{ > >>> - int i; > >>> - > >>> - key[0] = str[0] >> 1; > >>> - key[1] = ((str[0] & 0x01) << 6) | (str[1] >> 2); > >>> - key[2] = ((str[1] & 0x03) << 5) | (str[2] >> 3); > >>> - key[3] = ((str[2] & 0x07) << 4) | (str[3] >> 4); > >>> - key[4] = ((str[3] & 0x0F) << 3) | (str[4] >> 5); > >>> - key[5] = ((str[4] & 0x1F) << 2) | (str[5] >> 6); > >>> - key[6] = ((str[5] & 0x3F) << 1) | (str[6] >> 7); > >>> - key[7] = str[6] & 0x7F; > >>> - for (i = 0; i < 8; i++) > >>> - key[i] = (key[i] << 1); > >>> -} > >>> - > >>> -static int > >>> -smbhash(unsigned char *out, const unsigned char *in, unsigned char *key) > >>> -{ > >>> - unsigned char key2[8]; > >>> - struct des_ctx ctx; > >>> - > >>> - if (fips_enabled) { > >>> - ksmbd_debug(AUTH, "FIPS compliance enabled: DES not permitted\n"); > >>> - return -ENOENT; > >>> - } > >>> - > >>> - str_to_key(key, key2); > >>> - des_expand_key(&ctx, key2, DES_KEY_SIZE); > >>> - des_encrypt(&ctx, out, in); > >>> - memzero_explicit(&ctx, sizeof(ctx)); > >>> - return 0; > >>> -} > >>> - > >>> -static int ksmbd_enc_p24(unsigned char *p21, const unsigned char *c8, > >>> unsigned char *p24) > >>> -{ > >>> - int rc; > >>> - > >>> - rc = smbhash(p24, c8, p21); > >>> - if (rc) > >>> - return rc; > >>> - rc = smbhash(p24 + 8, c8, p21 + 7); > >>> - if (rc) > >>> - return rc; > >>> - return smbhash(p24 + 16, c8, p21 + 14); > >>> -} > >>> - > >>> -/* produce a md4 message digest from data of length n bytes */ > >>> -static int ksmbd_enc_md4(unsigned char *md4_hash, unsigned char > >>> *link_str, > >>> - int link_len) > >>> -{ > >>> - int rc; > >>> - struct ksmbd_crypto_ctx *ctx; > >>> - > >>> - ctx = ksmbd_crypto_ctx_find_md4(); > >>> - if (!ctx) { > >>> - ksmbd_debug(AUTH, "Crypto md4 allocation error\n"); > >>> - return -ENOMEM; > >>> - } > >>> - > >>> - rc = crypto_shash_init(CRYPTO_MD4(ctx)); > >>> - if (rc) { > >>> - ksmbd_debug(AUTH, "Could not init md4 shash\n"); > >>> - goto out; > >>> - } > >>> - > >>> - rc = crypto_shash_update(CRYPTO_MD4(ctx), link_str, link_len); > >>> - if (rc) { > >>> - ksmbd_debug(AUTH, "Could not update with link_str\n"); > >>> - goto out; > >>> - } > >>> - > >>> - rc = crypto_shash_final(CRYPTO_MD4(ctx), md4_hash); > >>> - if (rc) > >>> - ksmbd_debug(AUTH, "Could not generate md4 hash\n"); > >>> -out: > >>> - ksmbd_release_crypto_ctx(ctx); > >>> - return rc; > >>> -} > >>> - > >>> -static int ksmbd_enc_update_sess_key(unsigned char *md5_hash, char > >>> *nonce, > >>> - char *server_challenge, int len) > >>> -{ > >>> - int rc; > >>> - struct ksmbd_crypto_ctx *ctx; > >>> - > >>> - ctx = ksmbd_crypto_ctx_find_md5(); > >>> - if (!ctx) { > >>> - ksmbd_debug(AUTH, "Crypto md5 allocation error\n"); > >>> - return -ENOMEM; > >>> - } > >>> - > >>> - rc = crypto_shash_init(CRYPTO_MD5(ctx)); > >>> - if (rc) { > >>> - ksmbd_debug(AUTH, "Could not init md5 shash\n"); > >>> - goto out; > >>> - } > >>> - > >>> - rc = crypto_shash_update(CRYPTO_MD5(ctx), server_challenge, len); > >>> - if (rc) { > >>> - ksmbd_debug(AUTH, "Could not update with challenge\n"); > >>> - goto out; > >>> - } > >>> - > >>> - rc = crypto_shash_update(CRYPTO_MD5(ctx), nonce, len); > >>> - if (rc) { > >>> - ksmbd_debug(AUTH, "Could not update with nonce\n"); > >>> - goto out; > >>> - } > >>> - > >>> - rc = crypto_shash_final(CRYPTO_MD5(ctx), md5_hash); > >>> - if (rc) > >>> - ksmbd_debug(AUTH, "Could not generate md5 hash\n"); > >>> -out: > >>> - ksmbd_release_crypto_ctx(ctx); > >>> - return rc; > >>> -} > >>> - > >>> /** > >>> * ksmbd_gen_sess_key() - function to generate session key > >>> * @sess: session of connection > >>> @@ -324,43 +205,6 @@ static int calc_ntlmv2_hash(struct ksmbd_session > >>> *sess, char *ntlmv2_hash, > >>> return ret; > >>> } > >>> > >>> -/** > >>> - * ksmbd_auth_ntlm() - NTLM authentication handler > >>> - * @sess: session of connection > >>> - * @pw_buf: NTLM challenge response > >>> - * @passkey: user password > >>> - * > >>> - * Return: 0 on success, error number on error > >>> - */ > >>> -int ksmbd_auth_ntlm(struct ksmbd_session *sess, char *pw_buf) > >>> -{ > >>> - int rc; > >>> - unsigned char p21[21]; > >>> - char key[CIFS_AUTH_RESP_SIZE]; > >>> - > >>> - memset(p21, '\0', 21); > >>> - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); > >>> - rc = ksmbd_enc_p24(p21, sess->ntlmssp.cryptkey, key); > >>> - if (rc) { > >>> - pr_err("password processing failed\n"); > >>> - return rc; > >>> - } > >>> - > >>> - ksmbd_enc_md4(sess->sess_key, user_passkey(sess->user), > >>> - CIFS_SMB1_SESSKEY_SIZE); > >>> - memcpy(sess->sess_key + CIFS_SMB1_SESSKEY_SIZE, key, > >>> - CIFS_AUTH_RESP_SIZE); > >>> - sess->sequence_number = 1; > >>> - > >>> - if (strncmp(pw_buf, key, CIFS_AUTH_RESP_SIZE) != 0) { > >>> - ksmbd_debug(AUTH, "ntlmv1 authentication failed\n"); > >>> - return -EINVAL; > >>> - } > >>> - > >>> - ksmbd_debug(AUTH, "ntlmv1 authentication pass\n"); > >>> - return 0; > >>> -} > >>> - > >>> /** > >>> * ksmbd_auth_ntlmv2() - NTLMv2 authentication handler > >>> * @sess: session of connection > >>> @@ -441,44 +285,6 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, > >>> struct ntlmv2_resp *ntlmv2, > >>> return rc; > >>> } > >>> > >>> -/** > >>> - * __ksmbd_auth_ntlmv2() - NTLM2(extended security) authentication > >>> handler > >>> - * @sess: session of connection > >>> - * @client_nonce: client nonce from LM response. > >>> - * @ntlm_resp: ntlm response data from client. > >>> - * > >>> - * Return: 0 on success, error number on error > >>> - */ > >>> -static int __ksmbd_auth_ntlmv2(struct ksmbd_session *sess, char > >>> *client_nonce, > >>> - char *ntlm_resp) > >>> -{ > >>> - char sess_key[CIFS_SMB1_SESSKEY_SIZE] = {0}; > >>> - int rc; > >>> - unsigned char p21[21]; > >>> - char key[CIFS_AUTH_RESP_SIZE]; > >>> - > >>> - rc = ksmbd_enc_update_sess_key(sess_key, > >>> - client_nonce, > >>> - (char *)sess->ntlmssp.cryptkey, 8); > >>> - if (rc) { > >>> - pr_err("password processing failed\n"); > >>> - goto out; > >>> - } > >>> - > >>> - memset(p21, '\0', 21); > >>> - memcpy(p21, user_passkey(sess->user), CIFS_NTHASH_SIZE); > >>> - rc = ksmbd_enc_p24(p21, sess_key, key); > >>> - if (rc) { > >>> - pr_err("password processing failed\n"); > >>> - goto out; > >>> - } > >>> - > >>> - if (memcmp(ntlm_resp, key, CIFS_AUTH_RESP_SIZE) != 0) > >>> - rc = -EINVAL; > >>> -out: > >>> - return rc; > >>> -} > >>> - > >>> /** > >>> * ksmbd_decode_ntlmssp_auth_blob() - helper function to construct > >>> * authenticate blob > >>> @@ -512,17 +318,6 @@ int ksmbd_decode_ntlmssp_auth_blob(struct > >>> authenticate_message *authblob, > >>> nt_off = le32_to_cpu(authblob->NtChallengeResponse.BufferOffset); > >>> nt_len = le16_to_cpu(authblob->NtChallengeResponse.Length); > >>> > >>> - /* process NTLM authentication */ > >>> - if (nt_len == CIFS_AUTH_RESP_SIZE) { > >>> - if (le32_to_cpu(authblob->NegotiateFlags) & > >>> - NTLMSSP_NEGOTIATE_EXTENDED_SEC) > >>> - return __ksmbd_auth_ntlmv2(sess, (char *)authblob + > >>> - lm_off, (char *)authblob + nt_off); > >>> - else > >>> - return ksmbd_auth_ntlm(sess, (char *)authblob + > >>> - nt_off); > >>> - } > >>> - > >>> /* TODO : use domain name that imported from configuration file */ > >>> domain_name = smb_strndup_from_utf16((const char *)authblob + > >>> le32_to_cpu(authblob->DomainName.BufferOffset), > >>> diff --git a/fs/ksmbd/crypto_ctx.c b/fs/ksmbd/crypto_ctx.c > >>> index 5f4b1008d17e..81488d04199d 100644 > >>> --- a/fs/ksmbd/crypto_ctx.c > >>> +++ b/fs/ksmbd/crypto_ctx.c > >>> @@ -81,12 +81,6 @@ static struct shash_desc *alloc_shash_desc(int id) > >>> case CRYPTO_SHASH_SHA512: > >>> tfm = crypto_alloc_shash("sha512", 0, 0); > >>> break; > >>> - case CRYPTO_SHASH_MD4: > >>> - tfm = crypto_alloc_shash("md4", 0, 0); > >>> - break; > >>> - case CRYPTO_SHASH_MD5: > >>> - tfm = crypto_alloc_shash("md5", 0, 0); > >>> - break; > >>> default: > >>> return NULL; > >>> } > >>> @@ -214,16 +208,6 @@ struct ksmbd_crypto_ctx > >>> *ksmbd_crypto_ctx_find_sha512(void) > >>> return ____crypto_shash_ctx_find(CRYPTO_SHASH_SHA512); > >>> } > >>> > >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void) > >>> -{ > >>> - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD4); > >>> -} > >>> - > >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void) > >>> -{ > >>> - return ____crypto_shash_ctx_find(CRYPTO_SHASH_MD5); > >>> -} > >>> - > >>> static struct ksmbd_crypto_ctx *____crypto_aead_ctx_find(int id) > >>> { > >>> struct ksmbd_crypto_ctx *ctx; > >>> diff --git a/fs/ksmbd/crypto_ctx.h b/fs/ksmbd/crypto_ctx.h > >>> index ef11154b43df..4a367c62f653 100644 > >>> --- a/fs/ksmbd/crypto_ctx.h > >>> +++ b/fs/ksmbd/crypto_ctx.h > >>> @@ -15,8 +15,6 @@ enum { > >>> CRYPTO_SHASH_CMACAES, > >>> CRYPTO_SHASH_SHA256, > >>> CRYPTO_SHASH_SHA512, > >>> - CRYPTO_SHASH_MD4, > >>> - CRYPTO_SHASH_MD5, > >>> CRYPTO_SHASH_MAX, > >>> }; > >>> > >>> @@ -43,8 +41,6 @@ struct ksmbd_crypto_ctx { > >>> #define CRYPTO_CMACAES(c) ((c)->desc[CRYPTO_SHASH_CMACAES]) > >>> #define CRYPTO_SHA256(c) ((c)->desc[CRYPTO_SHASH_SHA256]) > >>> #define CRYPTO_SHA512(c) ((c)->desc[CRYPTO_SHASH_SHA512]) > >>> -#define CRYPTO_MD4(c) ((c)->desc[CRYPTO_SHASH_MD4]) > >>> -#define CRYPTO_MD5(c) ((c)->desc[CRYPTO_SHASH_MD5]) > >>> > >>> #define CRYPTO_HMACMD5_TFM(c) ((c)->desc[CRYPTO_SHASH_HMACMD5]->tfm) > >>> #define CRYPTO_HMACSHA256_TFM(c)\ > >>> @@ -52,8 +48,6 @@ struct ksmbd_crypto_ctx { > >>> #define CRYPTO_CMACAES_TFM(c) ((c)->desc[CRYPTO_SHASH_CMACAES]->tfm) > >>> #define CRYPTO_SHA256_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA256]->tfm) > >>> #define CRYPTO_SHA512_TFM(c) ((c)->desc[CRYPTO_SHASH_SHA512]->tfm) > >>> -#define CRYPTO_MD4_TFM(c) ((c)->desc[CRYPTO_SHASH_MD4]->tfm) > >>> -#define CRYPTO_MD5_TFM(c) ((c)->desc[CRYPTO_SHASH_MD5]->tfm) > >>> > >>> #define CRYPTO_GCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_GCM]) > >>> #define CRYPTO_CCM(c) ((c)->ccmaes[CRYPTO_AEAD_AES_CCM]) > >>> @@ -64,8 +58,6 @@ struct ksmbd_crypto_ctx > >>> *ksmbd_crypto_ctx_find_hmacsha256(void); > >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_cmacaes(void); > >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha512(void); > >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_sha256(void); > >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md4(void); > >>> -struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_md5(void); > >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_gcm(void); > >>> struct ksmbd_crypto_ctx *ksmbd_crypto_ctx_find_ccm(void); > >>> void ksmbd_crypto_destroy(void); > >>> > >> > > -- Thanks, Steve ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support 2021-09-27 12:47 [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Namjae Jeon 2021-09-27 12:47 ` [PATCH 2/2] ksmbd: remove NTLMv1 authentication Namjae Jeon @ 2021-09-28 14:46 ` Tom Talpey 2021-09-29 1:40 ` Namjae Jeon 1 sibling, 1 reply; 8+ messages in thread From: Tom Talpey @ 2021-09-28 14:46 UTC (permalink / raw) To: Namjae Jeon, linux-cifs Cc: Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee On 9/27/2021 8:47 AM, Namjae Jeon wrote: > Although ksmbd doesn't send SMB2.0 support in supported dialect list of smb > negotiate response, There is the leftover of smb2.0 dialect. > This patch remove it not to support SMB2.0 in ksmbd. > > Cc: Tom Talpey <tom@talpey.com> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Ralph Böhme <slow@samba.org> > Cc: Steve French <smfrench@gmail.com> > Cc: Sergey Senozhatsky <senozhatsky@chromium.org> > Cc: Hyunchul Lee <hyc.lee@gmail.com> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/smb2ops.c | 5 ----- > fs/ksmbd/smb2pdu.c | 15 ++++----------- > fs/ksmbd/smb2pdu.h | 1 - > fs/ksmbd/smb_common.c | 4 ++-- > 4 files changed, 6 insertions(+), 19 deletions(-) > > diff --git a/fs/ksmbd/smb2ops.c b/fs/ksmbd/smb2ops.c > index 197473871aa4..b06456eb587b 100644 > --- a/fs/ksmbd/smb2ops.c > +++ b/fs/ksmbd/smb2ops.c > @@ -187,11 +187,6 @@ static struct smb_version_cmds smb2_0_server_cmds[NUMBER_OF_SMB2_COMMANDS] = { > [SMB2_CHANGE_NOTIFY_HE] = { .proc = smb2_notify}, > }; > > -int init_smb2_0_server(struct ksmbd_conn *conn) > -{ > - return -EOPNOTSUPP; > -} > - > /** > * init_smb2_1_server() - initialize a smb server connection with smb2.1 > * command dispatcher > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index 88e94a8e4a15..b7d0406d1a14 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -236,7 +236,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work) > > if (conn->need_neg == false) > return -EINVAL; > - if (!(conn->dialect >= SMB20_PROT_ID && > + if (!(conn->dialect >= SMB21_PROT_ID && > conn->dialect <= SMB311_PROT_ID)) > return -EINVAL; This would accept any in-between value! That, um, would be bad. This needs to be a much stronger check, especially since significant state is being built in the lines that follow this segment. > @@ -1166,13 +1166,6 @@ int smb2_handle_negotiate(struct ksmbd_work *work) > case SMB21_PROT_ID: > init_smb2_1_server(conn); > break; > - case SMB20_PROT_ID: > - rc = init_smb2_0_server(conn); > - if (rc) { > - rsp->hdr.Status = STATUS_NOT_SUPPORTED; > - goto err_out; > - } > - break; > case SMB2X_PROT_ID: > case BAD_PROT_ID: > default: > @@ -1191,7 +1184,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work) > rsp->MaxReadSize = cpu_to_le32(conn->vals->max_read_size); > rsp->MaxWriteSize = cpu_to_le32(conn->vals->max_write_size); > > - if (conn->dialect > SMB20_PROT_ID) { > + if (conn->dialect >= SMB21_PROT_ID) { > memcpy(conn->ClientGUID, req->ClientGUID, > SMB2_CLIENT_GUID_SIZE); If SMB2.1 is now the minimum supported dialect, why is this GUID insertion made conditional? > conn->cli_sec_mode = le16_to_cpu(req->SecurityMode); > @@ -1537,7 +1530,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) > } > } > > - if (conn->dialect > SMB20_PROT_ID) { > + if (conn->dialect >= SMB21_PROT_ID) { > if (!ksmbd_conn_lookup_dialect(conn)) { > pr_err("fail to verify the dialect\n"); > return -ENOENT; Why is verifying the dialect *ever* conditional on the dialect value??? > @@ -1623,7 +1616,7 @@ static int krb5_authenticate(struct ksmbd_work *work) > } > } > > - if (conn->dialect > SMB20_PROT_ID) { > + if (conn->dialect >= SMB21_PROT_ID) { > if (!ksmbd_conn_lookup_dialect(conn)) { > pr_err("fail to verify the dialect\n"); > return -ENOENT; Ditto previous comment. > diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h > index 261825d06391..a6dec5ec6a54 100644 > --- a/fs/ksmbd/smb2pdu.h > +++ b/fs/ksmbd/smb2pdu.h > @@ -1637,7 +1637,6 @@ struct smb2_posix_info { > } __packed; > > /* functions */ > -int init_smb2_0_server(struct ksmbd_conn *conn); > void init_smb2_1_server(struct ksmbd_conn *conn); > void init_smb3_0_server(struct ksmbd_conn *conn); > void init_smb3_02_server(struct ksmbd_conn *conn); > diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c > index b6c4c7e960fa..435ca8df590b 100644 > --- a/fs/ksmbd/smb_common.c > +++ b/fs/ksmbd/smb_common.c > @@ -88,7 +88,7 @@ unsigned int ksmbd_server_side_copy_max_total_size(void) > > inline int ksmbd_min_protocol(void) > { > - return SMB2_PROT; > + return SMB21_PROT; > } > > inline int ksmbd_max_protocol(void) > @@ -427,7 +427,7 @@ int ksmbd_extract_shortname(struct ksmbd_conn *conn, const char *longname, > > static int __smb2_negotiate(struct ksmbd_conn *conn) > { > - return (conn->dialect >= SMB20_PROT_ID && > + return (conn->dialect >= SMB21_PROT_ID && > conn->dialect <= SMB311_PROT_ID); > } Ditto previous comment. And after fixing it, why is this helper not used everywhere? Tom. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support 2021-09-28 14:46 ` [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Tom Talpey @ 2021-09-29 1:40 ` Namjae Jeon 0 siblings, 0 replies; 8+ messages in thread From: Namjae Jeon @ 2021-09-29 1:40 UTC (permalink / raw) To: Tom Talpey Cc: linux-cifs, Ronnie Sahlberg, Ralph Böhme, Steve French, Sergey Senozhatsky, Hyunchul Lee 2021-09-28 23:46 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 9/27/2021 8:47 AM, Namjae Jeon wrote: >> Although ksmbd doesn't send SMB2.0 support in supported dialect list of >> smb >> negotiate response, There is the leftover of smb2.0 dialect. >> This patch remove it not to support SMB2.0 in ksmbd. >> >> Cc: Tom Talpey <tom@talpey.com> >> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> Cc: Ralph Böhme <slow@samba.org> >> Cc: Steve French <smfrench@gmail.com> >> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> >> Cc: Hyunchul Lee <hyc.lee@gmail.com> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/smb2ops.c | 5 ----- >> fs/ksmbd/smb2pdu.c | 15 ++++----------- >> fs/ksmbd/smb2pdu.h | 1 - >> fs/ksmbd/smb_common.c | 4 ++-- >> 4 files changed, 6 insertions(+), 19 deletions(-) >> >> diff --git a/fs/ksmbd/smb2ops.c b/fs/ksmbd/smb2ops.c >> index 197473871aa4..b06456eb587b 100644 >> --- a/fs/ksmbd/smb2ops.c >> +++ b/fs/ksmbd/smb2ops.c >> @@ -187,11 +187,6 @@ static struct smb_version_cmds >> smb2_0_server_cmds[NUMBER_OF_SMB2_COMMANDS] = { >> [SMB2_CHANGE_NOTIFY_HE] = { .proc = smb2_notify}, >> }; >> >> -int init_smb2_0_server(struct ksmbd_conn *conn) >> -{ >> - return -EOPNOTSUPP; >> -} >> - >> /** >> * init_smb2_1_server() - initialize a smb server connection with >> smb2.1 >> * command dispatcher >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >> index 88e94a8e4a15..b7d0406d1a14 100644 >> --- a/fs/ksmbd/smb2pdu.c >> +++ b/fs/ksmbd/smb2pdu.c >> @@ -236,7 +236,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work) >> >> if (conn->need_neg == false) >> return -EINVAL; >> - if (!(conn->dialect >= SMB20_PROT_ID && >> + if (!(conn->dialect >= SMB21_PROT_ID && >> conn->dialect <= SMB311_PROT_ID)) >> return -EINVAL; > Hi Tom, > This would accept any in-between value! That, um, would be bad. > > This needs to be a much stronger check, especially since significant > state is being built in the lines that follow this segment. Will fix. > > >> @@ -1166,13 +1166,6 @@ int smb2_handle_negotiate(struct ksmbd_work *work) >> case SMB21_PROT_ID: >> init_smb2_1_server(conn); >> break; >> - case SMB20_PROT_ID: >> - rc = init_smb2_0_server(conn); >> - if (rc) { >> - rsp->hdr.Status = STATUS_NOT_SUPPORTED; >> - goto err_out; >> - } >> - break; >> case SMB2X_PROT_ID: >> case BAD_PROT_ID: >> default: >> @@ -1191,7 +1184,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work) >> rsp->MaxReadSize = cpu_to_le32(conn->vals->max_read_size); >> rsp->MaxWriteSize = cpu_to_le32(conn->vals->max_write_size); >> >> - if (conn->dialect > SMB20_PROT_ID) { >> + if (conn->dialect >= SMB21_PROT_ID) { >> memcpy(conn->ClientGUID, req->ClientGUID, >> SMB2_CLIENT_GUID_SIZE); > > If SMB2.1 is now the minimum supported dialect, why is this GUID > insertion made conditional? Yep, Will remove this condition. > >> conn->cli_sec_mode = le16_to_cpu(req->SecurityMode); >> @@ -1537,7 +1530,7 @@ static int ntlm_authenticate(struct ksmbd_work >> *work) >> } >> } >> >> - if (conn->dialect > SMB20_PROT_ID) { >> + if (conn->dialect >= SMB21_PROT_ID) { >> if (!ksmbd_conn_lookup_dialect(conn)) { >> pr_err("fail to verify the dialect\n"); >> return -ENOENT; > > Why is verifying the dialect *ever* conditional on the dialect value??? Will remove it. > >> @@ -1623,7 +1616,7 @@ static int krb5_authenticate(struct ksmbd_work >> *work) >> } >> } >> >> - if (conn->dialect > SMB20_PROT_ID) { >> + if (conn->dialect >= SMB21_PROT_ID) { >> if (!ksmbd_conn_lookup_dialect(conn)) { >> pr_err("fail to verify the dialect\n"); >> return -ENOENT; > > Ditto previous comment. Will remove it. > >> diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h >> index 261825d06391..a6dec5ec6a54 100644 >> --- a/fs/ksmbd/smb2pdu.h >> +++ b/fs/ksmbd/smb2pdu.h >> @@ -1637,7 +1637,6 @@ struct smb2_posix_info { >> } __packed; >> >> /* functions */ >> -int init_smb2_0_server(struct ksmbd_conn *conn); >> void init_smb2_1_server(struct ksmbd_conn *conn); >> void init_smb3_0_server(struct ksmbd_conn *conn); >> void init_smb3_02_server(struct ksmbd_conn *conn); >> diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c >> index b6c4c7e960fa..435ca8df590b 100644 >> --- a/fs/ksmbd/smb_common.c >> +++ b/fs/ksmbd/smb_common.c >> @@ -88,7 +88,7 @@ unsigned int >> ksmbd_server_side_copy_max_total_size(void) >> >> inline int ksmbd_min_protocol(void) >> { >> - return SMB2_PROT; >> + return SMB21_PROT; >> } >> >> inline int ksmbd_max_protocol(void) >> @@ -427,7 +427,7 @@ int ksmbd_extract_shortname(struct ksmbd_conn *conn, >> const char *longname, >> >> static int __smb2_negotiate(struct ksmbd_conn *conn) >> { >> - return (conn->dialect >= SMB20_PROT_ID && >> + return (conn->dialect >= SMB21_PROT_ID && >> conn->dialect <= SMB311_PROT_ID); >> } > > Ditto previous comment. And after fixing it, why is this helper not used > everywhere? Will use this helper. Thanks for your review! > > Tom. > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-29 21:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-27 12:47 [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Namjae Jeon 2021-09-27 12:47 ` [PATCH 2/2] ksmbd: remove NTLMv1 authentication Namjae Jeon 2021-09-28 14:32 ` Tom Talpey 2021-09-29 0:34 ` Namjae Jeon 2021-09-29 15:02 ` Tom Talpey 2021-09-29 21:19 ` Steve French 2021-09-28 14:46 ` [PATCH 1/2] ksmbd: remove the leftover of smb2.0 dialect support Tom Talpey 2021-09-29 1:40 ` Namjae Jeon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).