Owen Kirby osk@exegin.com 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