Browse Source

Merge remote-tracking branch 'public/aes256'

Nick Mathewson 7 years ago
parent
commit
af01e8211e
6 changed files with 137 additions and 85 deletions
  1. 18 10
      src/common/aes.c
  2. 2 1
      src/common/aes.h
  3. 48 50
      src/common/crypto.c
  4. 5 1
      src/common/crypto.h
  5. 11 5
      src/test/bench.c
  6. 53 18
      src/test/test_crypto.c

+ 18 - 10
src/common/aes.c

@@ -48,7 +48,7 @@ ENABLE_GCC_WARNING(redundant-decls)
 
 /* We have five strategies for implementing AES counter mode.
  *
- * Best with x86 and x86_64: Use EVP_aes_ctr128() and EVP_EncryptUpdate().
+ * Best with x86 and x86_64: Use EVP_aes_*_ctr() and EVP_EncryptUpdate().
  * This is possible with OpenSSL 1.0.1, where the counter-mode implementation
  * can use bit-sliced or vectorized AES or AESNI as appropriate.
  *
@@ -89,11 +89,17 @@ ENABLE_GCC_WARNING(redundant-decls)
 /* We don't actually define the struct here. */
 
 aes_cnt_cipher_t *
-aes_new_cipher(const char *key, const char *iv)
+aes_new_cipher(const uint8_t *key, const uint8_t *iv, int key_bits)
 {
   EVP_CIPHER_CTX *cipher = EVP_CIPHER_CTX_new();
-  EVP_EncryptInit(cipher, EVP_aes_128_ctr(),
-                  (const unsigned char*)key, (const unsigned char *)iv);
+  const EVP_CIPHER *c;
+  switch (key_bits) {
+    case 128: c = EVP_aes_128_ctr(); break;
+    case 192: c = EVP_aes_192_ctr(); break;
+    case 256: c = EVP_aes_256_ctr(); break;
+    default: tor_assert(0); // LCOV_EXCL_LINE
+  }
+  EVP_EncryptInit(cipher, c, key, iv);
   return (aes_cnt_cipher_t *) cipher;
 }
 void
@@ -262,11 +268,11 @@ static void aes_set_iv(aes_cnt_cipher_t *cipher, const char *iv);
  * using the 128-bit key <b>key</b> and the 128-bit IV <b>iv</b>.
  */
 aes_cnt_cipher_t*
-aes_new_cipher(const char *key, const char *iv)
+aes_new_cipher(const uint8_t *key, const uint8_t *iv, int bits)
 {
   aes_cnt_cipher_t* result = tor_malloc_zero(sizeof(aes_cnt_cipher_t));
 
-  aes_set_key(result, key, 128);
+  aes_set_key(result, key, bits);
   aes_set_iv(result, iv);
 
   return result;
@@ -277,7 +283,7 @@ aes_new_cipher(const char *key, const char *iv)
  * the counter to 0.
  */
 static void
-aes_set_key(aes_cnt_cipher_t *cipher, const char *key, int key_bits)
+aes_set_key(aes_cnt_cipher_t *cipher, const uint8_t *key, int key_bits)
 {
   if (should_use_EVP) {
     const EVP_CIPHER *c = 0;
@@ -287,10 +293,10 @@ aes_set_key(aes_cnt_cipher_t *cipher, const char *key, int key_bits)
       case 256: c = EVP_aes_256_ecb(); break;
       default: tor_assert(0); // LCOV_EXCL_LINE
     }
-    EVP_EncryptInit(&cipher->key.evp, c, (const unsigned char*)key, NULL);
+    EVP_EncryptInit(&cipher->key.evp, c, key, NULL);
     cipher->using_evp = 1;
   } else {
-    AES_set_encrypt_key((const unsigned char *)key, key_bits,&cipher->key.aes);
+    AES_set_encrypt_key(key, key_bits,&cipher->key.aes);
     cipher->using_evp = 0;
   }
 
@@ -348,6 +354,8 @@ evp_block128_fn(const uint8_t in[16],
 void
 aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len)
 {
+  /* Note that the "128" below refers to the length of the counter,
+   * not the length of the AES key. */
   if (cipher->using_evp) {
     /* In openssl 1.0.0, there's an if'd out EVP_aes_128_ctr in evp.h.  If
      * it weren't disabled, it might be better just to use that.
@@ -374,7 +382,7 @@ aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len)
 /** Reset the 128-bit counter of <b>cipher</b> to the 16-bit big-endian value
  * in <b>iv</b>. */
 static void
-aes_set_iv(aes_cnt_cipher_t *cipher, const char *iv)
+aes_set_iv(aes_cnt_cipher_t *cipher, const uint8_t *iv)
 {
 #ifdef USING_COUNTER_VARS
   cipher->counter3 = ntohl(get_uint32(iv));

+ 2 - 1
src/common/aes.h

@@ -15,7 +15,8 @@
 
 typedef struct aes_cnt_cipher aes_cnt_cipher_t;
 
-aes_cnt_cipher_t* aes_new_cipher(const char *key, const char *iv);
+aes_cnt_cipher_t* aes_new_cipher(const uint8_t *key, const uint8_t *iv,
+                                 int key_bits);
 void aes_cipher_free(aes_cnt_cipher_t *cipher);
 void aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len);
 

+ 48 - 50
src/common/crypto.c

@@ -69,6 +69,7 @@ ENABLE_GCC_WARNING(redundant-decls)
 #endif
 
 #include "torlog.h"
+#include "torint.h"
 #include "aes.h"
 #include "util.h"
 #include "container.h"
@@ -119,15 +120,6 @@ struct crypto_pk_t
   RSA *key; /**< The key itself */
 };
 
-/** Key and stream information for a stream cipher. */
-struct crypto_cipher_t
-{
-  char key[CIPHER_KEY_LEN]; /**< The raw key. */
-  char iv[CIPHER_IV_LEN]; /**< The initial IV. */
-  aes_cnt_cipher_t *cipher; /**< The key in format usable for counter-mode AES
-                             * encryption */
-};
-
 /** A structure to hold the first half (x, g^x) of a Diffie-Hellman handshake
  * while we're waiting for the second.*/
 struct crypto_dh_t {
@@ -550,38 +542,48 @@ 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 <b>bits</b> bits long; the IV is CIPHER_IV_LEN bytes.  Both
+ * must be provided. Key length must be 128, 192, or 256 */
 crypto_cipher_t *
-crypto_cipher_new_with_iv(const char *key, const char *iv)
+crypto_cipher_new_with_iv_and_bits(const uint8_t *key,
+                                   const uint8_t *iv,
+                                   int bits)
 {
-  crypto_cipher_t *env;
-
-  env = tor_malloc_zero(sizeof(crypto_cipher_t));
-
-  if (key == NULL)
-    crypto_rand(env->key, CIPHER_KEY_LEN);
-  else
-    memcpy(env->key, key, CIPHER_KEY_LEN);
-  if (iv == NULL)
-    crypto_rand(env->iv, CIPHER_IV_LEN);
-  else
-    memcpy(env->iv, iv, CIPHER_IV_LEN);
+  tor_assert(key);
+  tor_assert(iv);
 
-  env->cipher = aes_new_cipher(env->key, env->iv);
+  return aes_new_cipher((const uint8_t*)key, (const uint8_t*)iv, bits);
+}
 
-  return 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.  Both
+ * must be provided.
+ */
+crypto_cipher_t *
+crypto_cipher_new_with_iv(const char *key, const char *iv)
+{
+  return crypto_cipher_new_with_iv_and_bits((uint8_t*)key, (uint8_t*)iv,
+                                            128);
 }
 
 /** Return a new crypto_cipher_t with the provided <b>key</b> and an IV of all
- * zero bytes.  */
+ * zero bytes and key length <b>bits</b>.  Key length must be 128, 192, or
+ * 256. */
 crypto_cipher_t *
-crypto_cipher_new(const char *key)
+crypto_cipher_new_with_bits(const char *key, int bits)
 {
   char zeroiv[CIPHER_IV_LEN];
   memset(zeroiv, 0, sizeof(zeroiv));
-  return crypto_cipher_new_with_iv(key, zeroiv);
+  return crypto_cipher_new_with_iv_and_bits((uint8_t*)key, (uint8_t*)zeroiv,
+                                            bits);
+}
+
+/** Return a new crypto_cipher_t with the provided <b>key</b> (of
+ * CIPHER_KEY_LEN bytes) and an IV of all zero bytes.  */
+crypto_cipher_t *
+crypto_cipher_new(const char *key)
+{
+  return crypto_cipher_new_with_bits(key, 128);
 }
 
 /** Free a symmetric cipher.
@@ -592,10 +594,7 @@ crypto_cipher_free(crypto_cipher_t *env)
   if (!env)
     return;
 
-  tor_assert(env->cipher);
-  aes_cipher_free(env->cipher);
-  memwipe(env, 0, sizeof(crypto_cipher_t));
-  tor_free(env);
+  aes_cipher_free(env);
 }
 
 /* public key crypto */
@@ -1266,10 +1265,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. */
@@ -1284,6 +1285,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);
@@ -1291,6 +1293,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;
@@ -1582,14 +1585,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 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.
@@ -1599,14 +1594,14 @@ crypto_cipher_encrypt(crypto_cipher_t *env, char *to,
                       const char *from, size_t fromlen)
 {
   tor_assert(env);
-  tor_assert(env->cipher);
+  tor_assert(env);
   tor_assert(from);
   tor_assert(fromlen);
   tor_assert(to);
   tor_assert(fromlen < SIZE_T_CEILING);
 
   memcpy(to, from, fromlen);
-  aes_crypt_inplace(env->cipher, to, fromlen);
+  aes_crypt_inplace(env, to, fromlen);
   return 0;
 }
 
@@ -1624,7 +1619,7 @@ crypto_cipher_decrypt(crypto_cipher_t *env, char *to,
   tor_assert(fromlen < SIZE_T_CEILING);
 
   memcpy(to, from, fromlen);
-  aes_crypt_inplace(env->cipher, to, fromlen);
+  aes_crypt_inplace(env, to, fromlen);
   return 0;
 }
 
@@ -1635,7 +1630,7 @@ void
 crypto_cipher_crypt_inplace(crypto_cipher_t *env, char *buf, size_t len)
 {
   tor_assert(len < SIZE_T_CEILING);
-  aes_crypt_inplace(env->cipher, buf, len);
+  aes_crypt_inplace(env, buf, len);
 }
 
 /** Encrypt <b>fromlen</b> bytes (at least 1) from <b>from</b> with the key in
@@ -1659,11 +1654,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);
 }
 

+ 5 - 1
src/common/crypto.h

@@ -117,7 +117,7 @@ typedef struct {
 } common_digests_t;
 
 typedef struct crypto_pk_t crypto_pk_t;
-typedef struct crypto_cipher_t crypto_cipher_t;
+typedef struct aes_cnt_cipher crypto_cipher_t;
 typedef struct crypto_digest_t crypto_digest_t;
 typedef struct crypto_xof_t crypto_xof_t;
 typedef struct crypto_dh_t crypto_dh_t;
@@ -138,7 +138,11 @@ void crypto_pk_free(crypto_pk_t *env);
 
 void crypto_set_tls_dh_prime(void);
 crypto_cipher_t *crypto_cipher_new(const char *key);
+crypto_cipher_t *crypto_cipher_new_with_bits(const char *key, int bits);
 crypto_cipher_t *crypto_cipher_new_with_iv(const char *key, const char *iv);
+crypto_cipher_t *crypto_cipher_new_with_iv_and_bits(const uint8_t *key,
+                                                    const uint8_t *iv,
+                                                    int bits);
 void crypto_cipher_free(crypto_cipher_t *env);
 
 /* public key crypto */

+ 11 - 5
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();
 

+ 53 - 18
src/test/test_crypto.c

@@ -363,15 +363,15 @@ test_crypto_rng_engine(void *arg)
   ;
 }
 
-/** Run unit tests for our AES functionality */
+/** Run unit tests for our AES128 functionality */
 static void
-test_crypto_aes(void *arg)
+test_crypto_aes128(void *arg)
 {
   char *data1 = NULL, *data2 = NULL, *data3 = NULL;
   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);
@@ -513,32 +514,61 @@ test_crypto_aes(void *arg)
 static void
 test_crypto_aes_ctr_testvec(void *arg)
 {
-  (void)arg;
+  const char *bitstr = arg;
   char *mem_op_hex_tmp=NULL;
+  crypto_cipher_t *c=NULL;
 
   /* from NIST SP800-38a, section F.5 */
-  const char key16[] = "2b7e151628aed2a6abf7158809cf4f3c";
   const char ctr16[] = "f0f1f2f3f4f5f6f7f8f9fafbfcfdfeff";
   const char plaintext16[] =
     "6bc1bee22e409f96e93d7e117393172a"
     "ae2d8a571e03ac9c9eb76fac45af8e51"
     "30c81c46a35ce411e5fbc1191a0a52ef"
     "f69f2445df4f9b17ad2b417be66c3710";
-  const char ciphertext16[] =
-    "874d6191b620e3261bef6864990db6ce"
-    "9806f66b7970fdff8617187bb9fffdff"
-    "5ae4df3edbd5d35e5b4f09020db03eab"
-    "1e031dda2fbe03d1792170a0f3009cee";
+  const char *ciphertext16;
+  const char *key16;
+  int bits;
+
+  if (!strcmp(bitstr, "128")) {
+    ciphertext16 = /* section F.5.1 */
+      "874d6191b620e3261bef6864990db6ce"
+      "9806f66b7970fdff8617187bb9fffdff"
+      "5ae4df3edbd5d35e5b4f09020db03eab"
+      "1e031dda2fbe03d1792170a0f3009cee";
+    key16 = "2b7e151628aed2a6abf7158809cf4f3c";
+    bits = 128;
+  } else if (!strcmp(bitstr, "192")) {
+    ciphertext16 = /* section F.5.3 */
+      "1abc932417521ca24f2b0459fe7e6e0b"
+      "090339ec0aa6faefd5ccc2c6f4ce8e94"
+      "1e36b26bd1ebc670d1bd1d665620abf7"
+      "4f78a7f6d29809585a97daec58c6b050";
+    key16 = "8e73b0f7da0e6452c810f32b809079e562f8ead2522c6b7b";
+    bits = 192;
+  } else if (!strcmp(bitstr, "256")) {
+    ciphertext16 = /* section F.5.5 */
+      "601ec313775789a5b7a7f504bbf3d228"
+      "f443e3ca4d62b59aca84e990cacaf5c5"
+      "2b0930daa23de94ce87017ba2d84988d"
+      "dfc9c58db67aada613c2dd08457941a6";
+    key16 =
+      "603deb1015ca71be2b73aef0857d7781"
+      "1f352c073b6108d72d9810a30914dff4";
+    bits = 256;
+  } else {
+    tt_abort_msg("AES doesn't support this number of bits.");
+  }
 
-  char key[16];
+  char key[32];
   char iv[16];
   char plaintext[16*4];
+  memset(key, 0xf9, sizeof(key)); /* poison extra bytes */
   base16_decode(key, sizeof(key), key16, strlen(key16));
   base16_decode(iv, sizeof(iv), ctr16, strlen(ctr16));
   base16_decode(plaintext, sizeof(plaintext),
                 plaintext16, strlen(plaintext16));
 
-  crypto_cipher_t *c = crypto_cipher_new_with_iv(key, iv);
+  c = crypto_cipher_new_with_iv_and_bits((uint8_t*)key, (uint8_t*)iv, bits);
   crypto_cipher_crypt_inplace(c, plaintext, sizeof(plaintext));
   test_memeq_hex(plaintext, ciphertext16);
 
@@ -2872,9 +2902,14 @@ struct testcase_t crypto_tests[] = {
   { "rng_strongest_broken", test_crypto_rng_strongest, TT_FORK,
     &passthrough_setup, (void*)"broken" },
   { "openssl_version", test_crypto_openssl_version, TT_FORK, NULL, NULL },
-  { "aes_AES", test_crypto_aes, TT_FORK, &passthrough_setup, (void*)"aes" },
-  { "aes_EVP", test_crypto_aes, TT_FORK, &passthrough_setup, (void*)"evp" },
-  { "aes_ctr_testvec", test_crypto_aes_ctr_testvec, 0, NULL, NULL },
+  { "aes_AES", test_crypto_aes128, TT_FORK, &passthrough_setup, (void*)"aes" },
+  { "aes_EVP", test_crypto_aes128, TT_FORK, &passthrough_setup, (void*)"evp" },
+  { "aes128_ctr_testvec", test_crypto_aes_ctr_testvec, 0,
+    &passthrough_setup, (void*)"128" },
+  { "aes192_ctr_testvec", test_crypto_aes_ctr_testvec, 0,
+    &passthrough_setup, (void*)"192" },
+  { "aes256_ctr_testvec", test_crypto_aes_ctr_testvec, 0,
+    &passthrough_setup, (void*)"256" },
   CRYPTO_LEGACY(sha),
   CRYPTO_LEGACY(pk),
   { "pk_fingerprints", test_crypto_pk_fingerprints, TT_FORK, NULL, NULL },