On Sat, Mar 15, 2014 at 12:07 AM, Niels Möller <ni...@lysator.liu.se> wrote:
>
> > --- /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?
The Input to the CBC-MAC used in CCM mode takes the form:
L(adata) | adata | padding | plaintext | padding
Where L(adata) is a variable length transformation that encodes the length
of adata, and padding is the minimum number of 0-bits necessary to align
the input to a multiple of the block size (note that input to the CBC-MAC is
just an XOR with the previous block, so padding becomes a NOP)
Because ccm_update() might be called multiple times, with data that doesn't
necessarily leave the input aligned to the block size, blen is used to count
the number of bytes that were already added to block, but haven't been
processed by another iteration of the CBC-MAC. If the last call to left the
input data aligned to the block size, then blen will be zero. In other words,
blen is the length of data input to the CBC-MAC modulus the block size.
Once we find the end of the adata (this is currently done on the first call to
ccm_encrypt() or ccm_decrypt()), we need to insert the padding before the
plaintext, which is what ccm_pad() does. Then, because the CTR functions
must always in a multiple of the block size, blen should always be zero
except for the last block of the message.
>
> > +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.
Since the IV only contains a single bit that indicates whether adata exists
at all, in theory the alength parameter could be omitted from set_nonce()
entirely and then handled in ccm_update(), which is what this check is doing.
This is more or less an artifact of an attempt to support the AEAD API. Note
that if you call ccm_set_nonce() with alength==0, and then pass the entire
adata to ccm_update(), you would still get the correct authentication tag and
ciphertext because of this special check on alen.
>
> > + 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).
Yes, that could be done, but this operation requires the use of the cipher
encrypt function, which would need to be added to the set_nonce() API.
>
> > +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.
Yes, this was a bit of a judgement call, whether it was uglier to store the
nonce length in the ctx, or to parse it out of the flags byte of the counter.
There are other possible ways to do this, but they all require extra storage
in the ctx, which I was hoping to avoid.
>
> > +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?).
CCM is indeed specified as an AEAD function with a single output string,
for example, RFC 5166 specifies that the ciphertext is always exactly 16
bytes longer than the plaintext, so this might be a good way to simplify
the function call API.
>
> > +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).
In the language of the CCM publications, mlen is always the length of
the plaintext message. clen would be the length of the ciphertext, and
can always be determined by: clen = mlen + tlen
>
> 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.
Generally, I would prefer that the length provided should be the length of
data that actually gets written (ie: ciphertext in this case). If a naive
programmer using this API just passes buffer size as a sizeof() they
might miss that the encrypt function would write passed the end of the
buffer and cause an overflow.
>
> 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.
I didn't see any similar functions for the EAX or CGM modes, so I'm not sure
that we really need to be concerned with breaking a precedent here.
>
> > 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.
I'll change the names accordingly with the next patch.
The negative value of blen is currently only used as a special value to
flag the first time that ccm_update() gets called so that we can
encrypt the IV to the CBC-MAC (B0) and generate the L(a). These
operations could be moved into ccm_set_nonce(), provided that
ccm_set_nonce() is also provided with the cipher encrypt function.
>
> > --- /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.
Of course, I'll make sure that any changes to the API will be tested in the
ccm-test program.
Cheers,
Owen
_______________________________________________
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
