Owen Kirby <[email protected]> writes:
> I've incorporated a few of your suggestions and updated my patch for the CCM
> cipher
> modes. This improves the API coverage in the CCM test suite, adds the
> all-at-once
> API for message processing, and fixes the copyright of the CCM mode source
> code.
Thanks!
> --- /dev/null
> +++ b/ccm.c
> +/* Pad an unaligned CBC-MAC digest with zero, or initialize B0 if no adata.
> */
> +static void
> +ccm_pad(struct ccm_ctx *ctx, void *cipher, nettle_crypt_func *f)
> +{
> + if (ctx->blen) f(cipher, CCM_BLOCK_SIZE, ctx->tag.b, ctx->tag.b);
> + ctx->blen = 0;
> +}
Can you explain what this is intended for? It's called unconditionally
by ccm_encrypt and ccm_decrypt, and I imagine it's a nop for all calls
but the first (otherwise, ccm_encrypt (..., 16, msg...); ccm_encrypt
(..., 16, msg+16...); would seem to give different output than
ccm_encrypt(...32, msg))?
It is intended that _update and encrypt_/_decrypt can be called multiple
times, as long as the total length equals the corresponding value passed
to _set_nonce, right?
> +void
> +ccm_update(struct ccm_ctx *ctx, void *cipher, nettle_crypt_func *f,
> + size_t length, const uint8_t *data)
> +{
> + const uint8_t *end = data + length;
> +
> + /* nop */
> + if (!length) return;
> +
> + /* On the first call, encrypt B0 and encode L(a) */
> + if (ctx->blen < 0) {
> + if (!ctx->alen) ctx->alen = length; /* If alen unknown, set it now. */
> + ctx->tag.b[CCM_OFFSET_FLAGS] |= CCM_FLAG_ADATA;
Why the special handling of zero ctx->alen here? As far as I can see,
this can happen only if ccm_set_nonce is called with alength = 0, and
then nevertheless calls ccm_update with length > 0, which seems like an
invalid use of the api.
> + f(cipher, CCM_BLOCK_SIZE, ctx->tag.b, ctx->tag.b);
> + ctx->blen = 0;
> + if (ctx->alen >= (0x01ULL << 32)) {
> + /* Encode L(a) as 0xff || 0xff || <64-bit integer> */
> + ctx->tag.b[ctx->blen++] ^= 0xff;
> + ctx->tag.b[ctx->blen++] ^= 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 56) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 48) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 40) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 32) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 24) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 16) & 0xff;
> + }
> + else if (ctx->alen >= ((0x1ULL << 16) - (0x1ULL << 8))) {
> + /* Encode L(a) as 0xff || 0xfe || <32-bit integer> */
> + ctx->tag.b[ctx->blen++] ^= 0xff;
> + ctx->tag.b[ctx->blen++] ^= 0xfe;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 24) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 16) & 0xff;
> + }
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 8) & 0xff;
> + ctx->tag.b[ctx->blen++] ^= (ctx->alen >> 0) & 0xff;
> + }
Is it possible to move this initial processing to ccm_set_nonce? The
alength *is* known there, and that would let you eliminate the ctx->blen
< 0 cases, and maybe you could eliminate the alen state variable too (or
keep it for sanity checking only).
> +void
> +ccm_digest(struct ccm_ctx *ctx, void *cipher, nettle_crypt_func *f,
> + size_t length, uint8_t *digest)
> +{
> + int i = CCM_BLOCK_SIZE - CCM_FLAG_GET_L(ctx->ctr.b[CCM_OFFSET_FLAGS]);
Maybe it would be nicer to stick to a one-way encoding of the ctr block
(done by ccm_build_iv, if I understand correctly), and never decode that
format? One would need to store the needed information in some simpler
"uncoded" way in the ctx. That's a judgment call, adding redundant info
to the context is also a bit ugly.
> +void
> +ccm_encrypt_message(void *cipher, nettle_crypt_func *f,
> + size_t nlength, const uint8_t *nonce,
> + size_t alength, const uint8_t *adata,
> + size_t tlength, uint8_t *tag,
> + size_t mlength, uint8_t *dst, const uint8_t *src)
> +{
> + struct ccm_ctx ctx;
> + ccm_set_nonce(&ctx, nlength, nonce, alength, mlength, tlength);
> + ccm_update(&ctx, cipher, f, alength, adata);
> + ccm_encrypt(&ctx, cipher, f, mlength, dst, src);
> + ccm_digest(&ctx, cipher, f, tlength, tag);
> +}
I think this function should append the tag to the ciphertext (assuming
ccm is specified as an aead with a single output string?).
> +void
> +ccm_decrypt_message(void *cipher, nettle_crypt_func *f,
> + size_t nlength, const uint8_t *nonce,
> + size_t alength, const uint8_t *adata,
> + size_t tlength, uint8_t *tag,
> + size_t mlength, uint8_t *dst, const uint8_t *src)
> +{
> + struct ccm_ctx ctx;
> + ccm_set_nonce(&ctx, nlength, nonce, alength, mlength, tlength);
> + ccm_update(&ctx, cipher, f, alength, adata);
> + ccm_decrypt(&ctx, cipher, f, mlength, dst, src);
> + ccm_digest(&ctx, cipher, f, tlength, tag);
> +}
I think this function should have a tag *input* (possibly reading it at
te end of the cryptotext), compare it to the tag it computes, and return
1 on success, 0 for failure.
If the tag is appended to the cryptotext, I'm not sure if the mlength
should be the message length with or without the tag (i.e., the length
of the clear text message, or of the encrypted and authenticated
message).
Maybe it's best to let it be the length of the clear text message, same
as passed to ccm_encrypt_message. In any case, the caller must be aware
of both lengths.
One *could* pass both message length, and derive the tag length as the
difference. But I'm afraid that style won't generalize nicely to other
aead algorithms.
> diff --git a/ccm.h b/ccm.h
> new file mode 100644
> index 0000000..76b4cc4
> +/* Per-message state */
> +struct ccm_ctx {
> + union nettle_block16 ctr; /* Counter for CTR encryption. */
> + union nettle_block16 tag; /* CBC-MAC message tag. */
> + uint64_t alen; /* Length of adata set during the nonce generation. */
> + int blen; /* <0 on init or # of bytes since the last aligned block */
> +};
In nettle, length is usually not abbreviated to "len". If negative blen
values can be eliminated, it should be changed to unsigned.
> --- /dev/null
> +++ b/testsuite/ccm-test.c
> + /* Ensure we get the same answers using the all-in-one API. */
> + if (repeat <= 1) {
> + memset(en_data, 0, len); memset(de_data, 0, len);
> + memset(en_digest, 0, tlen); memset(de_digest, 0, tlen);
> +
> + ccm_encrypt_message(ctx, cipher->encrypt, nonce->length, nonce->data,
> + authdata->length, authdata->data, tlen, en_digest, len, en_data,
> cleartext->data);
> +
> + ccm_decrypt_message(ctx, cipher->encrypt, nonce->length, nonce->data,
> + authdata->length, authdata->data, tlen, de_digest, len, de_data,
> ciphertext->data);
If the all-in-one interface is changed as suggested above, adding a
return value for ccm_decrypt_message, one should also check that
ccm_decrypt_message returns 1 for the correct data, and 0 if any of
message, adata or or tag is corrupted.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
