Nicolas Mora nicolas@babelouest.org writes:
I've added a merge request to implement AES key wrap and unwrap in Nettle [1].
Thanks. Can you give a bit more details on the usecase? I've understood that it's part of web-related specs, but do you know any examples of protocols or applicatinos using it, and how?
I've only had a quick look at the spec, and it looks like it works a bit like an AEAD algorithm (authenticated encryption with associated data), but with no associated data, no nonce (that's often an advantage, I guess), and message length contrained to be a multiple of 8 bytes. So not obvious to me when it's useful to use this key wrap method over any general AEAD construction.
Should it really be AES only? It could be generalized to arbitrary block ciphers (as long as block size is an even number of bytes), or more easily to any 16-byte block cipher. I think the spec https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-38F.pdf mentioned a variant for "TDEA" (I guess that's triple-DES?), which doesn't seem that relevant.
Some comments on the code:
* Nettle usually doesn't return any status codes based on validation of inputs. The key wrap function should not have a return value. It can assume passed in pointers are valid, and use assert to check that lengths are valid. The unwrap function should obviously have a return value, but it doesn't need to check conditions that are trivial to check for the caller (not an obvious call if invalid ciphertext length for unwrap should trigger an assert or give a zero return value; whatever choice is made it needs to be clearly documented).
* Try to avoid conditions on algorithm type. The common style in nettle is to have a general function taking an opaque context and needed function pointers as input, and then small convenience wrappers for algorithms of interest. So for the key wrap function, the cipher could be represented as a const void * for a cipher context already initialized with the "kek" key, and a nettle_cipher_func for invoking the cipher. If we want to support arbitrary block size (which is a bit questionable), it would also need a block size parametert.
* If we restrict the implementation to 16-byte block size, it would be good to use wider types than uint8_t for the internal data shuffling. I think most or all internal state could be represented as union nettle_block16 or uint64_t. Movement (and xor) is independent of endianness. Easiest way to test for big-endian is likely to target mips, using cross compiler and qemu (like the mips builds in the gitlab ci). The
I[7] ^= (n*j)+(i+1);
line would differ depending on endianness, if we want to do on a 64-bit state variable. (Staying with bytes and memcpy may be fine if you think it makes the code simpler and there's no significant performance difference).
* For the final memcmp in the unwrap function, you could use memeql_sec, to make the comparison side-channel silent (i.e., not leak information about the first differing byte).
* Since relation bewteen input and output size is very simple, we don't need both arguments. Just document the relation and provide appropriate #defined constants. The convention for similar functions is that the single length argument specifies the size of the *output* buffer.
So to be concrete on the interface comments, I'd suggest something like
void nist_keywrap16(const void *ctx, nettle_cipher_func *encrypt, const uint8_t *iv, size_t ciphertext_length, uint8_t *ciphertext, cosnt uint8_t *cleartext);
int nist_keyunwrap16(const void *ctx, nettle_cipher_func *decrypt, const uint8_t *iv, size_t cleartext_length, uint8_t *cleartext, const uint8_t *ciphertext);
Not clear if default iv should be handled at this level. What are the usecases for specifying a different iv?
I can add tests based on the tests vectors in the RFC [2], but I'm not sure how the test suites are build, should I need to create test_wrap functions like in aes-test.c or something else?
I think it would make sense to put tests in a new file. test_main should contain one fucntion call per test case, calling whatever helper functions are needed. It's probably sufficient to test algorithm-specific convenience functions.
Regards, /Niels