[openssl-dev] sizeof (HMAC_CTX) changes with update, breaks binary compatibility

[Matt Caswell]

On 12/06/15 11:16, Timo Teras wrote:
>>>> Why is separate key_init needid? Could we not use md!=NULL or
>>>> key_length!=0 checks to see if the context is initialized?
>>>
>>> Seems it'd be along with the line to just use key_length instead.
>>>
>>> Something along the lines of:
>>
>> Your patch does introduce a change in behaviour if key != NULL but len
>> == 0. Previously this would set ctx->key to all 0s, and key_init to 1,
>> and would then continue to use that all zero key. A subsequent
>> invocation of HMAC_Init_ex with key == NULL would reuse that all zero
>> key. Your patch would allow the first invocation, but error out on the
>> second.
>>
>> Should it be a valid use case to allow an all zero key in this way?
> 
> This raises another concern. If md is changed, but key is not, things
> go wrong anyway.

Hmmm...yes, this is a problem.

> I think we should just disallow chaning md without
> key.
> 
> The problem is that if md is changed, we need to rehash using the new
> md (in case they key >= HMAC_MAX_MD_CBLOCK). This was not allowed
> earlier. So let's just require specifying key if md changes.
> 
> We can in fact remove using key_length altogether then. I think
> key_length should be assigned to EVP_MD_block_size(md) always. Because
> they key is technically zero-padded anyway to this length. 
> 

Previously, it would work to do this:

HMAC_Init_ex(ctx, NULL, 0, md, NULL);
HMAC_Init_ex(ctx, key, len, NULL, NULL);

The first call above would actually read uninitialised ctx->key
data...but then throw away any results in the second call.

I'm not sure we could get rid of key_length altogether and deal with the
above?

Matt