On Fri, Oct 15, 2010 at 9:02 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> On Tue, 12 Oct 2010 14:45:19 -0500
> shirishpargaonkar@xxxxxxxxx wrote:
>
>> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>
>>
>> Have to move av_pair/ti blob to session key as part of ntlmv2 response.
>> This entire blob is part of the response and is use to calculate
>> signature for cases like ntlmv2 (without extended security) authentication.
>> The size of the blob is hard coded to 1024 byte which should be
>> sufficient to accomodate all 11 types of av pair types and their
>> lenghts/values.
>>
>>
>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>
> [...]
>
>> hmac_md5_final(v2_session_response, &context);
>> /* cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 7d834b1..5dd5659 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -105,6 +105,7 @@ struct session_key {
>> struct {
>> char key[16];
>> struct ntlmv2_resp resp;
>> + unsigned char tiblob[CIFS_MAX_NTLMV2_AVPAIR_SIZE];
>
> ^^^^^
> Is this really the best way to do this? We're increasing the size of
> this struct substantially, thereby increasing the size of every
> TCP_Server_Info and cifsSesInfo struct by 1k.
>
> Would it be better to keep the tiblob dynamically allocated, and hand
> off the pointer to it? If it really is necessary to add this field to
> the session_key struct in this way, would it be better to change how
> session keys are managed? i.e. not embed them inside of TCP_Server_Info
> and cifsSesInfo?
Jeff, we need to have a contiguous block of ntlmv2 client response i.e.
First part of client response (CR1), the 16 bytes and the variable second part
of client response which consists of struct ntlmv2_resp and tiblob.
Without contiguous blob, i.e. if tiblob is a pointer pointing to a dynamically
allocated memory, when it comes to calculating signature, this code,
crypto_shash_update(&server->secmech.sdescmd5->shash,
(char *)&server->session_key.data, len);
we end up copying incorrect contents.
I agree 1K is a lot but I can reduce it. av pair/target info blob can vary
in size, we can only guestimate how large it could.
Perhaps I can keep the way it is, and do two updates, one update with fixed
part, key + ntlmv2_resp structure and another with tiblob.
Let me try that, I think that will work and if it works, we do not
need this patch,
I can make that change as part of current 4/4 patch.
>
>> } ntlmv2;
>> } data;
>> };
>> @@ -226,7 +227,6 @@ struct cifsSesInfo {
>> struct session_key auth_key;
>> char ntlmv2_hash[16];
>> unsigned int tilen; /* length of the target info blob */
>> - unsigned char *tiblob; /* target info blob in challenge response */
>> bool need_reconnect:1; /* connection reset, uid now invalid */
>> };
>> /* no more than one of the following three session flags may be set */
>> diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
>> index b0f4b56..e778165 100644
>> --- a/fs/cifs/cifspdu.h
>> +++ b/fs/cifs/cifspdu.h
>> @@ -136,6 +136,11 @@
>> #define CIFS_SESS_KEY_SIZE (24)
>>
>> /*
>> + * Max expected size of an av pair blob in ntlmv2 auth exchange
>> + */
>> +#define CIFS_MAX_NTLMV2_AVPAIR_SIZE (1024)
>> +
>> +/*
>
> --
> Jeff Layton <jlayton@xxxxxxxxx>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
