commit ff116b780896bb735f887a669d87694bb6d8a964 Author: Nick Mathewson nickm@torproject.org Date: Fri Sep 16 10:12:30 2016 -0400

Simplify the crypto_cipher_t interface and structure

Previously, the IV and key were stored in the structure, even though they mostly weren't needed. The only purpose they had was to support a seldom-used API where you could pass NULL when creating a cipher in order to get a random key/IV, and then pull that key/IV back out.

This saves 32 bytes per AES instance, and makes it easier to support different key lengths. --- src/common/crypto.c | 45 +++++++++++++++++---------------------------- src/test/bench.c | 16 +++++++++++----- src/test/test_crypto.c | 9 +++++---- 3 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/src/common/crypto.c b/src/common/crypto.c index 7be43d7..f6ee6b0 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -123,8 +123,6 @@ struct crypto_pk_t /** Key and stream information for a stream cipher. */ struct crypto_cipher_t { - uint8_t key[CIPHER_KEY_LEN]; /**< The raw key. */ - uint8_t iv[CIPHER_IV_LEN]; /**< The initial IV. */ aes_cnt_cipher_t *cipher; /**< The key in format usable for counter-mode AES * encryption */ }; @@ -551,26 +549,18 @@ crypto_pk_free(crypto_pk_t *env) }

/** Allocate and return a new symmetric cipher using the provided key and iv. - * The key is CIPHER_KEY_LEN bytes; the IV is CIPHER_IV_LEN bytes. If you - * provide NULL in place of either one, it is generated at random. + * The key is CIPHER_KEY_LEN bytes; the IV is CIPHER_IV_LEN bytes. Both + * must be provided. */ crypto_cipher_t * crypto_cipher_new_with_iv(const char *key, const char *iv) { crypto_cipher_t *env; + tor_assert(key); + tor_assert(iv);

- env = tor_malloc_zero(sizeof(crypto_cipher_t)); - - if (key == NULL) - crypto_rand((char*)env->key, CIPHER_KEY_LEN); - else - memcpy(env->key, key, CIPHER_KEY_LEN); - if (iv == NULL) - crypto_rand((char*)env->iv, CIPHER_IV_LEN); - else - memcpy(env->iv, iv, CIPHER_IV_LEN); - - env->cipher = aes_new_cipher(env->key, env->iv, 128); + env = tor_malloc(sizeof(crypto_cipher_t)); + env->cipher = aes_new_cipher((const uint8_t*)key, (const uint8_t*)iv, 128);

return env; } @@ -1267,10 +1257,12 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, tor_assert(tolen >= fromlen + overhead + CIPHER_KEY_LEN); tor_assert(tolen >= pkeylen);

- cipher = crypto_cipher_new(NULL); /* generate a new key. */ + char key[CIPHER_KEY_LEN]; + crypto_rand(key, sizeof(key)); /* generate a new key. */ + cipher = crypto_cipher_new(key);

buf = tor_malloc(pkeylen+1); - memcpy(buf, cipher->key, CIPHER_KEY_LEN); + memcpy(buf, key, CIPHER_KEY_LEN); memcpy(buf+CIPHER_KEY_LEN, from, pkeylen-overhead-CIPHER_KEY_LEN);

/* Length of symmetrically encrypted data. */ @@ -1285,6 +1277,7 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env,

if (r<0) goto err; memwipe(buf, 0, pkeylen); + memwipe(key, 0, sizeof(key)); tor_free(buf); crypto_cipher_free(cipher); tor_assert(outlen+symlen < INT_MAX); @@ -1292,6 +1285,7 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, err:

memwipe(buf, 0, pkeylen); + memwipe(key, 0, sizeof(key)); tor_free(buf); crypto_cipher_free(cipher); return -1; @@ -1583,14 +1577,6 @@ crypto_pk_base64_decode(const char *str, size_t len)

/* symmetric crypto */

-/** Return a pointer to the key set for the cipher in <b>env</b>. - */ -const char * -crypto_cipher_get_key(crypto_cipher_t *env) -{ - return (const char *)env->key; -} - /** Encrypt <b>fromlen</b> bytes from <b>from</b> using the cipher * <b>env</b>; on success, store the result to <b>to</b> and return 0. * Does not check for failure. @@ -1660,11 +1646,14 @@ crypto_cipher_encrypt_with_iv(const char *key, if (tolen < fromlen + CIPHER_IV_LEN) return -1;

- cipher = crypto_cipher_new_with_iv(key, NULL); + char iv[CIPHER_IV_LEN]; + crypto_rand(iv, sizeof(iv)); + cipher = crypto_cipher_new_with_iv(key, iv);

- memcpy(to, cipher->iv, CIPHER_IV_LEN); + memcpy(to, iv, CIPHER_IV_LEN); crypto_cipher_encrypt(cipher, to+CIPHER_IV_LEN, from, fromlen); crypto_cipher_free(cipher); + memwipe(iv, 0, sizeof(iv)); return (int)(fromlen + CIPHER_IV_LEN); }

diff --git a/src/test/bench.c b/src/test/bench.c index f373019..30984fd 100644 --- a/src/test/bench.c +++ b/src/test/bench.c @@ -90,7 +90,9 @@ bench_aes(void) uint64_t start, end; const int bytes_per_iter = (1<<24); reset_perftime(); - c = crypto_cipher_new(NULL); + char key[CIPHER_KEY_LEN]; + crypto_rand(key, sizeof(key)); + c = crypto_cipher_new(key);

for (len = 1; len <= 8192; len *= 2) { int iters = bytes_per_iter / len; @@ -328,8 +330,9 @@ bench_cell_aes(void) char *b = tor_malloc(len+max_misalign); crypto_cipher_t *c; int i, misalign; - - c = crypto_cipher_new(NULL); + char key[CIPHER_KEY_LEN]; + crypto_rand(key, sizeof(key)); + c = crypto_cipher_new(key);

reset_perftime(); for (misalign = 0; misalign <= max_misalign; ++misalign) { @@ -501,8 +504,11 @@ bench_cell_ops(void) or_circ->base_.purpose = CIRCUIT_PURPOSE_OR;

/* Initialize crypto */ - or_circ->p_crypto = crypto_cipher_new(NULL); - or_circ->n_crypto = crypto_cipher_new(NULL); + char key1[CIPHER_KEY_LEN], key2[CIPHER_KEY_LEN]; + crypto_rand(key1, sizeof(key1)); + crypto_rand(key2, sizeof(key2)); + or_circ->p_crypto = crypto_cipher_new(key1); + or_circ->n_crypto = crypto_cipher_new(key2); or_circ->p_digest = crypto_digest_new(); or_circ->n_digest = crypto_digest_new();

diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index d83b93b..6586c06 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -371,7 +371,7 @@ test_crypto_aes(void *arg) crypto_cipher_t *env1 = NULL, *env2 = NULL; int i, j; char *mem_op_hex_tmp=NULL; - + char key[CIPHER_KEY_LEN]; int use_evp = !strcmp(arg,"evp"); evaluate_evp_for_aes(use_evp); evaluate_ctr_for_aes(); @@ -387,9 +387,10 @@ test_crypto_aes(void *arg)

memset(data2, 0, 1024); memset(data3, 0, 1024); - env1 = crypto_cipher_new(NULL); + crypto_rand(key, sizeof(key)); + env1 = crypto_cipher_new(key); tt_ptr_op(env1, OP_NE, NULL); - env2 = crypto_cipher_new(crypto_cipher_get_key(env1)); + env2 = crypto_cipher_new(key); tt_ptr_op(env2, OP_NE, NULL);

/* Try encrypting 512 chars. */ @@ -420,7 +421,7 @@ test_crypto_aes(void *arg) env2 = NULL;

memset(data3, 0, 1024); - env2 = crypto_cipher_new(crypto_cipher_get_key(env1)); + env2 = crypto_cipher_new(key); tt_ptr_op(env2, OP_NE, NULL); for (j = 0; j < 1024-16; j += 17) { crypto_cipher_encrypt(env2, data3+j, data1+j, 17);