On Thu,  7 Nov 2013 16:40:57 -0700
Tim Gardner <tim.gard...@canonical.com> wrote:

> A bit of cleanup plus some gratuitous variable renaming. I think using
> structures instead of numeric offsets makes this code much more
> understandable.
> 
> Also added a comment about current time range expected by
> the server.
> 
> Cc: Jeff Layton <jlay...@redhat.com>
> Cc: Steve French <sfre...@samba.org>
> Signed-off-by: Tim Gardner <tim.gard...@canonical.com>
> ---
> 
> The comment about time of day needing to be within 5 minutes is important (to 
> me
> at least). I spent the best part of a week thinking I had endian issues on 
> powerpc
> when in truth I was just too stupid to notice that the clock
> was not updated. Danged embedded platforms...
> 
> checkpatch has some problems with this patch regarding attribute packed, but 
> I chose
> to remain consistent with existing code.
> 
> WARNING: __packed is preferred over __attribute__((packed))
> #141: FILE: fs/cifs/cifspdu.h:705:
> +         } __attribute__((packed)) challenge;
> 
> WARNING: __packed is preferred over __attribute__((packed))
> #142: FILE: fs/cifs/cifspdu.h:706:
> +     } __attribute__((packed));
> 
> total: 0 errors, 2 warnings, 99 lines checked
> 
> Tested on cifs-2.6 for-linus (c481e9feee78c6ce1ba0a1c8c892049f6514f6cf) by 
> mounting
> to iOS 10.8 and Win 8.0 Pro.
> 
> rtg
> 
>  fs/cifs/cifsencrypt.c |   40 ++++++++++++++++++++++++----------------
>  fs/cifs/cifspdu.h     |    8 +++++++-
>  2 files changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index fc6f4f3..4934347 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -548,7 +548,13 @@ static int
>  CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
>  {
>       int rc;
> -     unsigned int offset = CIFS_SESS_KEY_SIZE + 8;
> +     struct ntlmv2_resp *ntlmv2 = (struct ntlmv2_resp *)
> +         (ses->auth_key.response + CIFS_SESS_KEY_SIZE);
> +     unsigned int hash_len;
> +
> +     /* The MD5 hash starts at challenge_key.key */
> +     hash_len = ses->auth_key.len - (CIFS_SESS_KEY_SIZE +
> +             offsetof(struct ntlmv2_resp, challenge.key[0]));
>  
>       if (!ses->server->secmech.sdeschmacmd5) {
>               cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
> @@ -556,7 +562,7 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char 
> *ntlmv2_hash)
>       }
>  
>       rc = crypto_shash_setkey(ses->server->secmech.hmacmd5,
> -                             ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
> +                              ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
>       if (rc) {
>               cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
>                        __func__);
> @@ -570,20 +576,21 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char 
> *ntlmv2_hash)
>       }
>  
>       if (ses->server->negflavor == CIFS_NEGFLAVOR_EXTENDED)
> -             memcpy(ses->auth_key.response + offset,
> -                     ses->ntlmssp->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
> +             memcpy(ntlmv2->challenge.key,
> +                    ses->ntlmssp->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
>       else
> -             memcpy(ses->auth_key.response + offset,
> -                     ses->server->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
> +             memcpy(ntlmv2->challenge.key,
> +                    ses->server->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
>       rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
> -             ses->auth_key.response + offset, ses->auth_key.len - offset);
> +                              ntlmv2->challenge.key, hash_len);
>       if (rc) {
>               cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
>               return rc;
>       }
>  
> +     /* Note that the MD5 digest over writes anon.challenge_key.key */
>       rc = crypto_shash_final(&ses->server->secmech.sdeschmacmd5->shash,
> -             ses->auth_key.response + CIFS_SESS_KEY_SIZE);
> +                             ntlmv2->ntlmv2_hash);
>       if (rc)
>               cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
>  
> @@ -627,7 +634,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct 
> nls_table *nls_cp)
>       int rc;
>       int baselen;
>       unsigned int tilen;
> -     struct ntlmv2_resp *buf;
> +     struct ntlmv2_resp *ntlmv2;
>       char ntlmv2_hash[16];
>       unsigned char *tiblob = NULL; /* target info blob */
>  
> @@ -660,13 +667,14 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct 
> nls_table *nls_cp)
>       }
>       ses->auth_key.len += baselen;
>  
> -     buf = (struct ntlmv2_resp *)
> +     ntlmv2 = (struct ntlmv2_resp *)
>                       (ses->auth_key.response + CIFS_SESS_KEY_SIZE);
> -     buf->blob_signature = cpu_to_le32(0x00000101);
> -     buf->reserved = 0;
> -     buf->time = cpu_to_le64(cifs_UnixTimeToNT(CURRENT_TIME));
> -     get_random_bytes(&buf->client_chal, sizeof(buf->client_chal));
> -     buf->reserved2 = 0;
> +     ntlmv2->blob_signature = cpu_to_le32(0x00000101);
> +     ntlmv2->reserved = 0;
> +     /* Must be within 5 minutes of the server */
> +     ntlmv2->time = cpu_to_le64(cifs_UnixTimeToNT(CURRENT_TIME));
> +     get_random_bytes(&ntlmv2->client_chal, sizeof(ntlmv2->client_chal));
> +     ntlmv2->reserved2 = 0;
>  
>       memcpy(ses->auth_key.response + baselen, tiblob, tilen);
>  
> @@ -706,7 +714,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct 
> nls_table *nls_cp)
>       }
>  
>       rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
> -             ses->auth_key.response + CIFS_SESS_KEY_SIZE,
> +             ntlmv2->ntlmv2_hash,
>               CIFS_HMAC_MD5_HASH_SIZE);
>       if (rc) {
>               cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
> diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
> index 9e5ee34..33df36e 100644
> --- a/fs/cifs/cifspdu.h
> +++ b/fs/cifs/cifspdu.h
> @@ -697,7 +697,13 @@ struct ntlmssp2_name {
>  } __attribute__((packed));
>  
>  struct ntlmv2_resp {
> -     char ntlmv2_hash[CIFS_ENCPWD_SIZE];
> +     union {
> +         char ntlmv2_hash[CIFS_ENCPWD_SIZE];
> +         struct {
> +             __u8 reserved[8];
> +             __u8 key[CIFS_SERVER_CHALLENGE_SIZE];
> +         } __attribute__((packed)) challenge;
> +     } __attribute__((packed));
>       __le32 blob_signature;
>       __u32  reserved;
>       __le64  time;

(cc'ing Shirish since he wrote most of this code...)

Nice cleanup, I like it.

Acked-by: Jeff Layton <jlay...@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to