2021-09-28 23:32 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
> 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@xxxxxxxxxx>
>> Cc: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> Cc: Ralph Böhme <slow@xxxxxxxxx>
>> Cc: Steve French <smfrench@xxxxxxxxx>
>> Cc: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
>> Cc: Hyunchul Lee <hyc.lee@xxxxxxxxx>
>> Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
>> ---
>> 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);
>>
>
