Tarsnap / libcperciva

BSD-licensed C99/POSIX library code shared between tarsnap, scrypt, kivaloo, spiped, and bsdiff.
Other
112 stars 11 forks source link

crypto_aes{,ctr}: use ${key_unexpanded} for unexpanded key #483

Closed gperciva closed 1 year ago

gperciva commented 1 year ago

Previously, we used ${key} to refer both to the unexpanded and expanded AES keys. For example,

/**
 * crypto_aes_key_expand(key, len):
 * Expand the ${len}-byte AES key ${key} into a structure which can be passed
 * to crypto_aes_encrypt_block().  The length must be 16 or 32.
 */
struct crypto_aes_key * crypto_aes_key_expand(const uint8_t *, size_t);

/**
 * crypto_aes_encrypt_block(in, out, key):
 * Using the expanded AES key ${key}, encrypt the block ${in} and write the
 * resulting ciphertext to ${out}.  ${in} and ${out} can overlap.
 */
void crypto_aes_encrypt_block(const uint8_t[16], uint8_t[16],
    const struct crypto_aes_key *);

This seemed sub-optimal.

(That said, if there's a strong tradition of using the same variable name in the cryptographic community, of course we shouldn't rename these. It's true that although we've reused the name, the type of variable differs -- uint8_t * vs. struct crypto_aes_key.)

I have a separate branch which renamed the relevant ${key} to ${key_expanded} instead, but that was a bigger patch.

cperciva commented 1 year ago

LGTM aside from one missing change.

gperciva commented 1 year ago

Fixed, plus a few other inconsistencies: sometimes I added "unexpanded" in the API comment text; other places, I only changed the ${key_unexpanded} variable.

I went with adding it to the API comment text, so there's 3 additions of _unexpanded in each. For example,

 * crypto_aes_key_expand_arm(key_unexpanded, len):
 * Expand the ${len}-byte unexpanded AES key ${key_unexpanded} into a
cperciva commented 1 year ago

LGTM, please rebase.

gperciva commented 1 year ago

Rebased, ready for merge.