On Sun, 2019-04-14 at 09:33 +0200, Niels Möller wrote:
- assert(nc->context_size <= NETTLE_MAX_CIPHER16_CONTEXT_SIZE);
- /* ensure we have enough size of context plus any padding size
*/
- CMAC128_SET_KEY(&ctx, nc->set_encrypt_key, nc->encrypt, s2vk);
- if (nlength == 0 && alength == 0) {
- CMAC128_UPDATE(&ctx, nc->encrypt, 16, const_one);
- CMAC128_DIGEST(&ctx, nc->encrypt, 16, v);
- return;
- }
Shouldn't the plaintext, plength, pdata, still be processed in this case?
Right, there should be an and plength == 0 as well. I've added an two additional test cases to check these cases, and the case where everything is zero, doesn't seem to interoperate with two libs I tried.
Hopefully it is issue of this code.
https://github.com/miscreant/miscreant/issues/194 https://github.com/dfoxfranke/libaes_siv/issues/14
In this function, you treat empty associated data or nonce as those elements missing in the input vector to S2V. E.g., if both adata and nonce are empty, the input vector is { plaintext }, one single element. But it could also be { "", "", plaintext }, with three elements, the first two being empty strings.
While the low level function could handle it, it is not exposed to be called directly (mainly intentionally as this cipher introduces a very new paradigm which I do not quite see much of practical uses).
This patch only adds the higher level AEAD API only, so this case cannot happen as we don't have the notion of empty string in nettle. We can introduce it of course, though we may be opening a can of worms as not only empty strings are undefined in terms of AEAD API [0], but what would these mean in the other implementations?
[0]. https://tools.ietf.org/html/rfc5116#section-2
To me, this sounds like a likely source of interop problems. Since RFC 5297 is general and allows the application to decide on the number of elements and meaning of the input vector, it doesn't give much guidance on this, as far as I see. The crucial case is when an application specifies that SIV is used with associated data and/or a nonce, but allows an empty string for either of those.
I agree on that. That's one of the reasons I stuck on the higher level AEAD API (expressed by the message APIs in nettle). I added two sentences in the documentation about it.
I think this function should do the underlying key setup also for
the
cipher instance used for s2v, not just store the key for later. So then the function would be
void siv_cmac_set_key(void *cmac_cipher, void *ctr_cipher,
The idea of the set_key function is to do all preparations that don't depend on the actual message, so they don't have to be repeated. And I think it's a bit odd to handle the keying of the two involved cipher contexts so differently.
Done. It needed some reorganization, and cmac128_syn is still needed in an ugly simulation of the CMAC structure setup to use the macros. I have kept the union
The attached version should address the comments so far and also changes cmac128_set_key to use nettle_block16 as well.
regards, Nikos