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
