Dmitry Eremin-Solenikov dbaryshkov@gmail.com writes:

I'm still playing around HMAC/digest refactoring, but I'm not happy with the way code looks in my tree.

Below patch is a faitly simple way to do it. It adds internal functions with a state_size argument, uses that when saving and restoring state, and switches hmac_md5 to a new smaller context struct.

Key change is defining

struct hmac_md5_ctx { /* Same as struct md5_ctx without the buffer. */ struct { uint32_t state[_MD5_DIGEST_LENGTH]; uint64_t count; /* Block count */ unsigned index; /* Into buffer */ } outer, inner; struct md5_ctx state; };

It would be a bit cleaner to not include count and index in this struct. hmac_md5_digest can know that when resetting the state, we always have index == 0 and count == 1. But the general _hmac_digest doesn't know how to write those fields.

One could consider rewriting hmac_md5_digest without using _hmac_digest. Or rethink what helpers really are needed to implement hmac_foo_set_key and hmac_foo_digest without too much duplication.

If one looks closer, the hmac_set_key has code for the specal case to do an initial hashing the key if key_size > block_size, but except for that, it's really wants to invoke the compression function.

I imagine what you're working on is quite similar?

Regards, /Niels

diff --git a/hmac-internal.h b/hmac-internal.h new file mode 100644 index 00000000..ee486dab --- /dev/null +++ b/hmac-internal.h @@ -0,0 +1,51 @@ +/* hmac-internal.h + + HMAC message authentication code (RFC-2104). + + Copyright (C) 2001, 2002, 2019 Niels Möller + + This file is part of GNU Nettle. + + GNU Nettle is free software: you can redistribute it and/or + modify it under the terms of either: + + * the GNU Lesser General Public License as published by the Free + Software Foundation; either version 3 of the License, or (at your + option) any later version. + + or + + * the GNU General Public License as published by the Free + Software Foundation; either version 2 of the License, or (at your + option) any later version. + + or both in parallel, as here. + + GNU Nettle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received copies of the GNU General Public License and + the GNU Lesser General Public License along with this program. If + not, see http://www.gnu.org/licenses/. +*/ + +#ifndef NETTLE_HMAC_INTERNAL_H_INCLUDED +#define NETTLE_HMAC_INTERNAL_H_INCLUDED + +/* Namespace mangling */ +#define _hmac_set_key _nettle_hmac_set_key +#define _hmac_digest _nettle_hmac_digest + +void +_hmac_set_key(void *outer, void *inner, size_t state_size, + void *ctx, const struct nettle_hash *hash, + size_t length, const uint8_t *key); + +void +_hmac_digest(const void *outer, const void *inner, size_t state_size, + void *ctx, const struct nettle_hash *hash, + size_t length, uint8_t *digest); + +#endif /* NETTLE_HMAC_H_INCLUDED */ diff --git a/hmac-md5.c b/hmac-md5.c index a27e64f6..e0da397c 100644 --- a/hmac-md5.c +++ b/hmac-md5.c @@ -35,13 +35,24 @@ # include "config.h" #endif

+#include <stddef.h> + #include "hmac.h" +#include "hmac-internal.h" + +#define MIN(a, b) ((a) < (b) ? (a) : (b)) + +/* Padding may be different in struct md5_ctx and struct hmac_md5_ctx, + so use the smallest one when saving and restoring state. */ +#define MD5_STATE_SIZE MIN(sizeof(((struct hmac_md5_ctx *)NULL)->outer), \ + offsetof(struct md5_ctx, block))

void hmac_md5_set_key(struct hmac_md5_ctx *ctx, size_t key_length, const uint8_t *key) { - HMAC_SET_KEY(ctx, &nettle_md5, key_length, key); + _hmac_set_key(&ctx->outer, &ctx->inner, MD5_STATE_SIZE, + &ctx->state, &nettle_md5, key_length, key); }

void @@ -55,5 +66,7 @@ void hmac_md5_digest(struct hmac_md5_ctx *ctx, size_t length, uint8_t *digest) { - HMAC_DIGEST(ctx, &nettle_md5, length, digest); + _hmac_digest(&ctx->outer, &ctx->inner, MD5_STATE_SIZE, + &ctx->state, &nettle_md5, + length, digest); } diff --git a/hmac.c b/hmac.c index 6ac5e11a..318de8a2 100644 --- a/hmac.c +++ b/hmac.c @@ -2,7 +2,7 @@

HMAC message authentication code (RFC-2104).

- Copyright (C) 2001 Niels Möller + Copyright (C) 2001, 2019 Niels Möller

This file is part of GNU Nettle.

@@ -42,6 +42,7 @@

#include "hmac.h"

+#include "hmac-internal.h" #include "memxor.h" #include "nettle-internal.h"

@@ -49,15 +50,15 @@ #define OPAD 0x5c

void -hmac_set_key(void *outer, void *inner, void *state, - const struct nettle_hash *hash, - size_t key_length, const uint8_t *key) +_hmac_set_key(void *outer, void *inner, size_t state_size, + void *ctx, + const struct nettle_hash *hash, + size_t key_length, const uint8_t *key) { TMP_DECL(pad, uint8_t, NETTLE_MAX_HASH_BLOCK_SIZE); TMP_ALLOC(pad, hash->block_size);

- hash->init(outer); - hash->init(inner); + hash->init(ctx);

if (key_length > hash->block_size) { @@ -67,9 +68,8 @@ hmac_set_key(void *outer, void *inner, void *state, TMP_DECL(digest, uint8_t, NETTLE_MAX_HASH_DIGEST_SIZE); TMP_ALLOC(digest, hash->digest_size);

- hash->init(state); - hash->update(state, key_length, key); - hash->digest(state, hash->digest_size, digest); + hash->update(ctx, key_length, key); + hash->digest(ctx, hash->digest_size, digest);

key = digest; key_length = hash->digest_size; @@ -80,14 +80,24 @@ hmac_set_key(void *outer, void *inner, void *state, memset(pad, OPAD, hash->block_size); memxor(pad, key, key_length);

- hash->update(outer, hash->block_size, pad); + hash->update(ctx, hash->block_size, pad); + memcpy(outer, ctx, state_size);

memset(pad, IPAD, hash->block_size); memxor(pad, key, key_length); + hash->init(ctx); + hash->update(ctx, hash->block_size, pad);

- hash->update(inner, hash->block_size, pad); + memcpy(inner, ctx, state_size); +}

- memcpy(state, inner, hash->context_size); +void +hmac_set_key(void *outer, void *inner, void *state, + const struct nettle_hash *hash, + size_t key_length, const uint8_t *key) +{ + _hmac_set_key(outer, inner, hash->context_size, + state, hash, key_length, key); }

void @@ -99,19 +109,28 @@ hmac_update(void *state, }

void -hmac_digest(const void *outer, const void *inner, void *state, - const struct nettle_hash *hash, - size_t length, uint8_t *dst) +_hmac_digest(const void *outer, const void *inner, size_t state_size, + void *ctx, + const struct nettle_hash *hash, + size_t length, uint8_t *dst) { TMP_DECL(digest, uint8_t, NETTLE_MAX_HASH_DIGEST_SIZE); TMP_ALLOC(digest, hash->digest_size);

- hash->digest(state, hash->digest_size, digest); + hash->digest(ctx, hash->digest_size, digest);

- memcpy(state, outer, hash->context_size); + memcpy(ctx, outer, state_size);

- hash->update(state, hash->digest_size, digest); - hash->digest(state, length, dst); + hash->update(ctx, hash->digest_size, digest); + hash->digest(ctx, length, dst);

- memcpy(state, inner, hash->context_size); + memcpy(ctx, inner, state_size); +} + +void +hmac_digest(const void *outer, const void *inner, void *state, + const struct nettle_hash *hash, + size_t length, uint8_t *dst) +{ + _hmac_digest(outer, inner, hash->context_size, state, hash, length, dst); } diff --git a/hmac.h b/hmac.h index 40a8e77a..b574ab2f 100644 --- a/hmac.h +++ b/hmac.h @@ -101,7 +101,16 @@ hmac_digest(const void *outer, const void *inner, void *state, /* HMAC using specific hash functions */

/* hmac-md5 */ -struct hmac_md5_ctx HMAC_CTX(struct md5_ctx); +struct hmac_md5_ctx +{ + /* Same as struct md5_ctx without the buffer. */ + struct { + uint32_t state[_MD5_DIGEST_LENGTH]; + uint64_t count; /* Block count */ + unsigned index; /* Into buffer */ + } outer, inner; + struct md5_ctx state; +};

void hmac_md5_set_key(struct hmac_md5_ctx *ctx,