Dmitry Eremin-Solenikov <[email protected]> writes:
> --- a/md5.c
> +++ b/md5.c
> @@ -56,19 +56,19 @@ md5_init(struct md5_ctx *ctx)
> 0x98badcfe,
> 0x10325476,
> };
> - memcpy(ctx->state, iv, sizeof(ctx->state));
> - ctx->count = 0;
> + memcpy(ctx->state.state, iv, sizeof(ctx->state.state));
> + ctx->state.count = 0;
> ctx->index = 0;
> }
Could have the same memcpy handle all of state.
> --- a/md5.h
> +++ b/md5.h
> @@ -53,10 +53,15 @@ extern "C" {
> /* Digest is kept internally as 4 32-bit words. */
> #define _MD5_DIGEST_LENGTH 4
>
> -struct md5_ctx
> +struct md5_state
> {
> uint32_t state[_MD5_DIGEST_LENGTH];
> uint64_t count; /* Block count */
> +};
> +
> +struct md5_ctx
> +{
> + struct md5_state state;
> uint8_t block[MD5_BLOCK_SIZE]; /* Block buffer */
> unsigned index; /* Into buffer */
> };
When reworking these structs, I think we should generally have index
before block, since index has a larger required alignment. Even if it
doesn't matter in this particular case, with MD5_BLOCK_SIZE == 64.
I wonder if we can find some better naming. "state.state" isn't so
pretty, and I think the conventional terminology would be to refer to
the 4 32-bit words as the md5 "state", not including the block count.
What we're trying to capture actually is somewhat hmac-specific.
It's the part that needs to be copied to the start at an md5_ctx to
reset it to some block boundary, and the reason we need a named type
(rather than offsetof(struct md5_ctx, block), is that code needs to
allocate it. Perhaps
struct md5_init_state
{
uint32_t state[_MD5_DIGEST_LENGTH];
uint64_t count; /* Block count */
unsigned index;
};
or struct md5_internal_ctx_no_buffer or so.
Then both plain md5 reset and md5-hmac could use a single memcpy. And we
could use some internal variant of md5_digest to avoid the redundant
memcpy done by md5_digest (in your patch set, the hmac code depends on
md5_digest resetting the index field, while it overwrites the rest of
the state).
I'm starting to think that it probably was a mistake to advertise and
document the internal hmac_set_key, hmac_update and hmac_digest methods.
Maybe we can deprecate them (without immediately breaking them); I find
no usage on codesearch.debian.net. We'de get more flexibility if we
could implement hmac_md5_* without going via struct nettle_hash
nettle_md5.
I think it would make sense to start with reordering fields in the
current context structs.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
