Tim Kosse tim.kosse@filezilla-project.org writes:
Thanks. It seems naming is one of the more difficult parts here. Some comments below.
--- a/nettle.texinfo +++ b/nettle.texinfo +The standalone Poly1305 algorithm is also available.
I wonder if we should either discourage direct use, or make some recommendations on how to use it without shooting oneself in the foot? E.g., say that if one can arrange to have a unique but predictable nonce for each message, like a sequence number, one can generate at least S, and preferably both R and S, by encrypting that nonce using a cipher with a secret key? (This could go somewhere else, e.g., in the intro paragraphs, I'm not saying it must be at this place).
+@defvr Constant POLY1305_KEYART_R_SIZE +@defvrx Constant POLY1305_KEYPART_S_SIZE +The two parts @code{r} and @code{s} of the key, each 16 octets. +@end defvr
+@defvr Constant POLY1305_DIGEST_SIZE +Size of the digest or ``authenticator'', 16 octets. +@end defvr
+@deftp {Context struct} {struct poly1305_mac_ctx} +The poly1305 context struct. +@end deftp
+@deftypefun void poly1305_set_key (struct poly1305_mac_ctx *@var{ctx}, const uint8_t *@var{r}) +Initialize the context struct. +@end deftypefun
+@deftypefun void poly1305_update (struct poly1305_mac_ctx *@var{ctx}, size_t @var{length}, const uint8_t *@var{data}) +Process more data. +@end deftypefun
+@deftypefun void poly1305_digest (struct poly1305_mac_ctx *@var{ctx}, const uint8_t *s, uint8_t *@var{digest}) +Extracts the digest, writing @code{POLY1305_DIGEST_SIZE} octets to +@var{digest}. Also prepares the context for processing a new message.
...new message with the same R (and to process several messages in this way, each call to _digest *must* use a new secret S).
--- /dev/null +++ b/poly1305.c +void +poly1305_digest(struct poly1305_mac_ctx* ctx, const uint8_t *s, uint8_t* digest) +{
- union nettle_block16 buffer;
- if (ctx->index > 0) {
- ctx->block[ctx->index] = 1;
- memset(ctx->block + ctx->index + 1, 0, POLY1305_BLOCK_SIZE - 1 - ctx->index);
- _nettle_poly1305_block (&ctx->pctx, ctx->block, 0);
- }
- memcpy(&buffer.b, s, POLY1305_KEYPART_S_SIZE);
- _nettle_poly1305_digest(&ctx->pctx, &buffer);
- memcpy(digest, buffer.b, POLY1305_DIGEST_SIZE);
I think this should reset ctx->index = 0 at the end?
--- a/poly1305.h +++ b/poly1305.h @@ -42,6 +42,9 @@ extern "C" { #endif
/* Name mangling */ +#define poly1305_set_key nettle_poly1305_set_key +#define poly1305_update nettle_poly1305_update +#define poly1305_digest nettle_poly1305_digest #define poly1305_aes_set_key nettle_poly1305_aes_set_key #define poly1305_aes_set_nonce nettle_poly1305_aes_set_nonce #define poly1305_aes_update nettle_poly1305_aes_update @@ -70,6 +73,27 @@ struct poly1305_ctx { } h; };
+struct poly1305_mac_ctx {
- struct poly1305_ctx pctx;
- uint8_t block[POLY1305_BLOCK_SIZE];
- unsigned index;
+};
If we use "poly1305_mac" for the context, I think it would make sense to use the same prefix for constants and functions.
+#define POLY1305_DIGEST_SIZE 16
So this would be POLY1305_MAC_DIGEST_SIZE. (Alternatively, stick to POLY1305_DIGEST_SIZE, and think of POLY1305_AES_DIGEST_SIZE as a redundant alias, kept only for backwards caompatibility.
+#define POLY1305_KEYPART_S_SIZE 16 +#define POLY1305_KEYPART_R_SIZE 16
Maybe strike KEYPART, and name as POLY1305_MAC_R_SIZE and POLY1305_MAC_S_SIZE ?
+/* Also sets index to 0 */ +void +poly1305_set_key(struct poly1305_mac_ctx* ctx, const uint8_t *r);
The comment looks a bit out of place. I think _mac_ should be in the name. Could also consider if _set_r is clearer (since it doesn't set all of the key).
+void +poly1305_update(struct poly1305_mac_ctx* ctx, size_t length, const uint8_t *data);
poly1305_mac_update (and then there's no need to rename the static function in chacha-poly1305.c).
+void +poly1305_digest(struct poly1305_mac_ctx* ctx, const uint8_t *s, uint8_t* digest);
poly1305_mac_digest, or possibly something also referencing the role of s (but I have no great idea. Maybe poly1305_mac_s_digest?).
To follow nettle conventions for _digest functions, it would mak sense if it is possible to start processing a new message after _digest. Which would then reuse the same r, and it would be essential to have a fresh secret s for the next _digest call. Is that how you intend it to work? A test case for this usage would also be nice.
--- a/testsuite/poly1305-test.c +++ b/testsuite/poly1305-test.c @@ -218,6 +218,35 @@ test_random(void) } }
+static void +test_poly1305_mac(const struct tstring *r,
const struct tstring *s,const struct tstring *msg,const struct tstring *ref)+{
- struct poly1305_mac_ctx ctx;
- uint8_t tag[POLY1305_DIGEST_SIZE];
- ASSERT (r->length == POLY1305_KEYPART_R_SIZE);
- ASSERT (s->length == POLY1305_KEYPART_S_SIZE);
- ASSERT (ref->length == POLY1305_DIGEST_SIZE);
- poly1305_set_key(&ctx, r->data);
- poly1305_update(&ctx, msg->length, msg->data);
- poly1305_digest(&ctx, s->data, tag);
- if (!MEMEQ(ref->length, ref->data, tag))
- {
- printf ("poly1305 failed\n");
- printf ("msg: "); print_hex (msg->length, msg->data);
- printf ("length: %u\n", (unsigned) msg->length);
- printf ("tag: "); print_hex (ref->length, tag);
- printf ("ref: "); print_hex (ref->length, ref->data);
- abort ();
- }
+}
For testing, you could think about defining a larger test context that includes the s value, and write the glue to test it using test_mac (defined in testutils.c). That way we get somewhat better test coverage for providing the message incrementally to the _update function. Alternatively, add some variants of with two calls to _update for different splitting of the message; that may be simpler.
Regards, /Niels