[Dovecot] [PATCH] Generalize HMAC implementation

[Florian Zeitz]

Am 11.09.2012 18:02, schrieb Timo Sirainen:
> On 4.9.2012, at 21.25, Florian Zeitz wrote:
> 
>> Hello everyone and Timo in particular,
>>
>> about a year ago I implemented a SHA-1 variant of the HMAC(-MD5) present
>> in Dovecot.
>> I had always disliked this a bit, because it replicates a lot of code.
>> This patch generalizes the HMAC function to take a hash_method struct as
>> parameter, and changes existing code which uses the "old" HMAC function
>> to use this new one.
>>
>> I'm not really sure this is actually a good idea, but I still felt I
>> should provide the code in case you would want to merge it upstream.
> 
> It's otherwise good, but this isn't safe:
> 
> +	ctx->ctx = t_malloc(meth->context_size);
> +	ctx->ctxo = t_malloc(meth->context_size);
> 
> It assumes that the hmac_init() + hmac_final() is called close to each others.
I had in fact noticed that. The assumption is currently true for all
occurrences, and probably will remain such, but I agree it's better to
be safe then sorry.

> I think we could simply #define the largest allowed context_size, use it for these buffers' sizes and then add i_assert(meth->context_size <= HMAC_MAX_CONTEXT_SIZE)
> 
Well, either that, or we could use a union of all known context structs
there. Possibly plus an
i_assert(meth->context_size <= sizeof(union hmac_ctxts)).
Or we could use i_malloc() and i_free() under the assumption hmac_init()
+ hmac_final() calls are always matched.

I've a certain preference for the union variant, but it's your call.

Regards,
Florian