From 00b4784575c88d5de15886b440096c1e2b9fb080 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 8 Mar 2012 14:51:57 -0500 Subject: [PATCH 1/4] Remove support for PK_NO_PADDING in crypto_pk_public_hybrid_encrypt We never use it, and it would be a stupid thing if we started using it. --- src/common/crypto.c | 15 ++------------- src/test/test_crypto.c | 7 ++----- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index 41f81ebd77..02f3d2fbba 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1001,8 +1001,7 @@ crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen, * bytes of data from from, with padding type 'padding', * storing the results on to. * - * If no padding is used, the public key must be at least as large as - * from. + * (Padding is required; the PK_NO_PADDING value is not supported.) * * Returns the number of bytes written on success, -1 on failure. * @@ -1030,13 +1029,11 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, tor_assert(from); tor_assert(to); tor_assert(fromlen < SIZE_T_CEILING); + tor_assert(padding != PK_NO_PADDING); overhead = crypto_get_rsa_padding_overhead(crypto_get_rsa_padding(padding)); pkeylen = crypto_pk_keysize(env); - if (padding == PK_NO_PADDING && fromlen < pkeylen) - return -1; - if (!force && fromlen+overhead <= pkeylen) { /* It all fits in a single encrypt. */ return crypto_pk_public_encrypt(env,to, @@ -1050,14 +1047,6 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, if (!cipher) return -1; if (crypto_cipher_generate_key(cipher)<0) goto err; - /* You can't just run around RSA-encrypting any bitstream: if it's - * greater than the RSA key, then OpenSSL will happily encrypt, and - * later decrypt to the wrong value. So we set the first bit of - * 'cipher->key' to 0 if we aren't padding. This means that our - * symmetric key is really only 127 bits. - */ - if (padding == PK_NO_PADDING) - cipher->key[0] &= 0x7f; if (crypto_cipher_encrypt_init_cipher(cipher)<0) goto err; buf = tor_malloc(pkeylen+1); diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index bf0962e98b..3f9029a8f8 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -485,14 +485,11 @@ test_crypto_pk(void) /* Try with hybrid encryption wrappers. */ crypto_rand(data1, 1024); - for (i = 0; i < 3; ++i) { + for (i = 0; i < 2; ++i) { for (j = 85; j < 140; ++j) { memset(data2,0,1024); memset(data3,0,1024); - if (i == 0 && j < 129) - continue; - p = (i==0)?PK_NO_PADDING: - (i==1)?PK_PKCS1_PADDING:PK_PKCS1_OAEP_PADDING; + p = (i==0)?PK_PKCS1_PADDING:PK_PKCS1_OAEP_PADDING; len = crypto_pk_public_hybrid_encrypt(pk1,data2,sizeof(data2), data1,j,p,0); test_assert(len>=0); From de0dca0de76d9d50aeb5955fe3f435c6c190f8d7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 20 Mar 2012 15:35:43 -0400 Subject: [PATCH 2/4] Refactor the API for setting up a block cipher. It allows us more flexibility on the backend if the user needs to specify the key and IV at setup time. --- changes/crypto_api | 3 + src/common/aes.c | 16 +++- src/common/aes.h | 4 +- src/common/crypto.c | 168 +++++++++++------------------------------ src/common/crypto.h | 17 +---- src/or/circuitbuild.c | 4 +- src/or/rendcommon.c | 13 ++-- src/or/routerparse.c | 15 ++-- src/test/bench.c | 17 ++--- src/test/test_crypto.c | 135 +++++++++++++-------------------- 10 files changed, 134 insertions(+), 258 deletions(-) create mode 100644 changes/crypto_api diff --git a/changes/crypto_api b/changes/crypto_api new file mode 100644 index 0000000000..608999f47d --- /dev/null +++ b/changes/crypto_api @@ -0,0 +1,3 @@ + o Code refactoring: + - Change the symmetric cipher interface so that creating and + initializing a stream cipher are no longer separate functions. diff --git a/src/common/aes.c b/src/common/aes.c index cfd931fab1..3121891068 100644 --- a/src/common/aes.c +++ b/src/common/aes.c @@ -209,14 +209,22 @@ _aes_fill_buf(aes_cnt_cipher_t *cipher) } } +static void aes_set_key(aes_cnt_cipher_t *cipher, const char *key, + int key_bits); +static void aes_set_iv(aes_cnt_cipher_t *cipher, const char *iv); + /** - * Return a newly allocated counter-mode AES128 cipher implementation. + * Return a newly allocated counter-mode AES128 cipher implementation, + * using the 128-bit key key and the 128-bit IV iv. */ aes_cnt_cipher_t* -aes_new_cipher(void) +aes_new_cipher(const char *key, const char *iv) { aes_cnt_cipher_t* result = tor_malloc_zero(sizeof(aes_cnt_cipher_t)); + aes_set_key(result, key, 128); + aes_set_iv(result, iv); + return result; } @@ -224,7 +232,7 @@ aes_new_cipher(void) * key_bits bits long (must be 128, 192, or 256). Also resets * the counter to 0. */ -void +static void aes_set_key(aes_cnt_cipher_t *cipher, const char *key, int key_bits) { if (should_use_EVP) { @@ -398,7 +406,7 @@ aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len) /** Reset the 128-bit counter of cipher to the 16-bit big-endian value * in iv. */ -void +static void aes_set_iv(aes_cnt_cipher_t *cipher, const char *iv) { #ifdef USING_COUNTER_VARS diff --git a/src/common/aes.h b/src/common/aes.h index f9de68a1bf..04b424ec77 100644 --- a/src/common/aes.h +++ b/src/common/aes.h @@ -16,13 +16,11 @@ struct aes_cnt_cipher; typedef struct aes_cnt_cipher aes_cnt_cipher_t; -aes_cnt_cipher_t* aes_new_cipher(void); +aes_cnt_cipher_t* aes_new_cipher(const char *key, const char *iv); void aes_cipher_free(aes_cnt_cipher_t *cipher); -void aes_set_key(aes_cnt_cipher_t *cipher, const char *key, int key_bits); void aes_crypt(aes_cnt_cipher_t *cipher, const char *input, size_t len, char *output); void aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len); -void aes_set_iv(aes_cnt_cipher_t *cipher, const char *iv); int evaluate_evp_for_aes(int force_value); int evaluate_ctr_for_aes(void); diff --git a/src/common/crypto.c b/src/common/crypto.c index 02f3d2fbba..2bd2e1e4d2 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -119,6 +119,7 @@ struct crypto_pk_t 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 */ }; @@ -383,50 +384,39 @@ crypto_pk_free(crypto_pk_t *env) tor_free(env); } -/** Create a new symmetric cipher for a given key and encryption flag - * (1=encrypt, 0=decrypt). Return the crypto object on success; NULL - * on failure. +/** 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. */ crypto_cipher_t * -crypto_create_init_cipher(const char *key, int encrypt_mode) -{ - int r; - crypto_cipher_t *crypto = NULL; - - if (! (crypto = crypto_cipher_new())) { - log_warn(LD_CRYPTO, "Unable to allocate crypto object"); - return NULL; - } - - crypto_cipher_set_key(crypto, key); - - if (encrypt_mode) - r = crypto_cipher_encrypt_init_cipher(crypto); - else - r = crypto_cipher_decrypt_init_cipher(crypto); - - if (r) - goto error; - return crypto; - - error: - if (crypto) - crypto_cipher_free(crypto); - return NULL; -} - -/** Allocate and return a new symmetric cipher. - */ -crypto_cipher_t * -crypto_cipher_new(void) +crypto_cipher_new_with_iv(const char *key, const char *iv) { crypto_cipher_t *env; env = tor_malloc_zero(sizeof(crypto_cipher_t)); - env->cipher = aes_new_cipher(); + + 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); + + env->cipher = aes_new_cipher(env->key, env->iv); + return env; } +crypto_cipher_t * +crypto_cipher_new(const char *key) +{ + char zeroiv[CIPHER_IV_LEN]; + memset(zeroiv, 0, sizeof(zeroiv)); + return crypto_cipher_new_with_iv(key, zeroiv); +} + /** Free a symmetric cipher. */ void @@ -1043,12 +1033,8 @@ 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(); - if (!cipher) return -1; - if (crypto_cipher_generate_key(cipher)<0) - goto err; - if (crypto_cipher_encrypt_init_cipher(cipher)<0) - goto err; + cipher = crypto_cipher_new(NULL); /* generate a new key. */ + buf = tor_malloc(pkeylen+1); memcpy(buf, cipher->key, CIPHER_KEY_LEN); memcpy(buf+CIPHER_KEY_LEN, from, pkeylen-overhead-CIPHER_KEY_LEN); @@ -1113,7 +1099,7 @@ crypto_pk_private_hybrid_decrypt(crypto_pk_t *env, "No room for a symmetric key"); goto err; } - cipher = crypto_create_init_cipher(buf, 0); + cipher = crypto_cipher_new(buf); if (!cipher) { goto err; } @@ -1301,49 +1287,6 @@ crypto_pk_check_fingerprint_syntax(const char *s) /* symmetric crypto */ -/** Generate a new random key for the symmetric cipher in env. - * Return 0 on success, -1 on failure. Does not initialize the cipher. - */ -int -crypto_cipher_generate_key(crypto_cipher_t *env) -{ - tor_assert(env); - - return crypto_rand(env->key, CIPHER_KEY_LEN); -} - -/** Set the symmetric key for the cipher in env to the first - * CIPHER_KEY_LEN bytes of key. Does not initialize the cipher. - */ -void -crypto_cipher_set_key(crypto_cipher_t *env, const char *key) -{ - tor_assert(env); - tor_assert(key); - - memcpy(env->key, key, CIPHER_KEY_LEN); -} - -/** Generate an initialization vector for our AES-CTR cipher; store it - * in the first CIPHER_IV_LEN bytes of iv_out. */ -void -crypto_cipher_generate_iv(char *iv_out) -{ - crypto_rand(iv_out, CIPHER_IV_LEN); -} - -/** Adjust the counter of env to point to the first byte of the block - * corresponding to the encryption of the CIPHER_IV_LEN bytes at - * iv. */ -int -crypto_cipher_set_iv(crypto_cipher_t *env, const char *iv) -{ - tor_assert(env); - tor_assert(iv); - aes_set_iv(env->cipher, iv); - return 0; -} - /** Return a pointer to the key set for the cipher in env. */ const char * @@ -1352,30 +1295,6 @@ crypto_cipher_get_key(crypto_cipher_t *env) return env->key; } -/** Initialize the cipher in env for encryption. Return 0 on - * success, -1 on failure. - */ -int -crypto_cipher_encrypt_init_cipher(crypto_cipher_t *env) -{ - tor_assert(env); - - aes_set_key(env->cipher, env->key, CIPHER_KEY_LEN*8); - return 0; -} - -/** Initialize the cipher in env for decryption. Return 0 on - * success, -1 on failure. - */ -int -crypto_cipher_decrypt_init_cipher(crypto_cipher_t *env) -{ - tor_assert(env); - - aes_set_key(env->cipher, env->key, CIPHER_KEY_LEN*8); - return 0; -} - /** Encrypt fromlen bytes from from using the cipher * env; on success, store the result to to and return 0. * On failure, return -1. @@ -1424,20 +1343,17 @@ crypto_cipher_crypt_inplace(crypto_cipher_t *env, char *buf, size_t len) } /** Encrypt fromlen bytes (at least 1) from from with the key in - * cipher to the buffer in to of length + * key to the buffer in to of length * tolen. tolen must be at least fromlen plus * CIPHER_IV_LEN bytes for the initialization vector. On success, return the * number of bytes written, on failure, return -1. - * - * This function adjusts the current position of the counter in cipher - * to immediately after the encrypted data. */ int -crypto_cipher_encrypt_with_iv(crypto_cipher_t *cipher, +crypto_cipher_encrypt_with_iv(const char *key, char *to, size_t tolen, const char *from, size_t fromlen) { - tor_assert(cipher); + crypto_cipher_t *cipher; tor_assert(from); tor_assert(to); tor_assert(fromlen < INT_MAX); @@ -1447,28 +1363,27 @@ crypto_cipher_encrypt_with_iv(crypto_cipher_t *cipher, if (tolen < fromlen + CIPHER_IV_LEN) return -1; - crypto_cipher_generate_iv(to); - if (crypto_cipher_set_iv(cipher, to)<0) - return -1; + cipher = crypto_cipher_new_with_iv(key, NULL); + + memcpy(to, cipher->iv, CIPHER_IV_LEN); crypto_cipher_encrypt(cipher, to+CIPHER_IV_LEN, from, fromlen); + crypto_cipher_free(cipher); return (int)(fromlen + CIPHER_IV_LEN); } /** Decrypt fromlen bytes (at least 1+CIPHER_IV_LEN) from from - * with the key in cipher to the buffer in to of length + * with the key in key to the buffer in to of length * tolen. tolen must be at least fromlen minus * CIPHER_IV_LEN bytes for the initialization vector. On success, return the * number of bytes written, on failure, return -1. - * - * This function adjusts the current position of the counter in cipher - * to immediately after the decrypted data. */ int -crypto_cipher_decrypt_with_iv(crypto_cipher_t *cipher, +crypto_cipher_decrypt_with_iv(const char *key, char *to, size_t tolen, const char *from, size_t fromlen) { - tor_assert(cipher); + crypto_cipher_t *cipher; + tor_assert(key); tor_assert(from); tor_assert(to); tor_assert(fromlen < INT_MAX); @@ -1478,9 +1393,10 @@ crypto_cipher_decrypt_with_iv(crypto_cipher_t *cipher, if (tolen < fromlen - CIPHER_IV_LEN) return -1; - if (crypto_cipher_set_iv(cipher, from)<0) - return -1; + cipher = crypto_cipher_new_with_iv(key, from); + crypto_cipher_encrypt(cipher, to, from+CIPHER_IV_LEN, fromlen-CIPHER_IV_LEN); + crypto_cipher_free(cipher); return (int)(fromlen - CIPHER_IV_LEN); } diff --git a/src/common/crypto.h b/src/common/crypto.h index 1c5ee0d23e..00ac26ba26 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -125,11 +125,8 @@ void crypto_pk_free(crypto_pk_t *env); void crypto_set_tls_dh_prime(const char *dynamic_dh_modulus_fname); -/* convenience function: wraps crypto_cipher_new, set_key, and init. */ -crypto_cipher_t *crypto_create_init_cipher(const char *key, - int encrypt_mode); - -crypto_cipher_t *crypto_cipher_new(void); +crypto_cipher_t *crypto_cipher_new(const char *key); +crypto_cipher_t *crypto_cipher_new_with_iv(const char *key, const char *iv); void crypto_cipher_free(crypto_cipher_t *env); /* public key crypto */ @@ -189,13 +186,7 @@ int crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out,int add_space); int crypto_pk_check_fingerprint_syntax(const char *s); /* symmetric crypto */ -int crypto_cipher_generate_key(crypto_cipher_t *env); -void crypto_cipher_set_key(crypto_cipher_t *env, const char *key); -void crypto_cipher_generate_iv(char *iv_out); -int crypto_cipher_set_iv(crypto_cipher_t *env, const char *iv); const char *crypto_cipher_get_key(crypto_cipher_t *env); -int crypto_cipher_encrypt_init_cipher(crypto_cipher_t *env); -int crypto_cipher_decrypt_init_cipher(crypto_cipher_t *env); int crypto_cipher_encrypt(crypto_cipher_t *env, char *to, const char *from, size_t fromlen); @@ -203,10 +194,10 @@ int crypto_cipher_decrypt(crypto_cipher_t *env, char *to, const char *from, size_t fromlen); int crypto_cipher_crypt_inplace(crypto_cipher_t *env, char *d, size_t len); -int crypto_cipher_encrypt_with_iv(crypto_cipher_t *env, +int crypto_cipher_encrypt_with_iv(const char *key, char *to, size_t tolen, const char *from, size_t fromlen); -int crypto_cipher_decrypt_with_iv(crypto_cipher_t *env, +int crypto_cipher_decrypt_with_iv(const char *key, char *to, size_t tolen, const char *from, size_t fromlen); diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 3948008775..1c7367a3fc 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2334,12 +2334,12 @@ circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, crypto_digest_add_bytes(cpath->b_digest, key_data+DIGEST_LEN, DIGEST_LEN); if (!(cpath->f_crypto = - crypto_create_init_cipher(key_data+(2*DIGEST_LEN),1))) { + crypto_cipher_new(key_data+(2*DIGEST_LEN)))) { log_warn(LD_BUG,"Forward cipher initialization failed."); return -1; } if (!(cpath->b_crypto = - crypto_create_init_cipher(key_data+(2*DIGEST_LEN)+CIPHER_KEY_LEN,0))) { + crypto_cipher_new(key_data+(2*DIGEST_LEN)+CIPHER_KEY_LEN))) { log_warn(LD_BUG,"Backward cipher initialization failed."); return -1; } diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 9c7bf518d1..20bbdafec9 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -290,11 +290,10 @@ rend_encrypt_v2_intro_points_basic(char **encrypted_out, enc[1] = (uint8_t)client_blocks; /* Encrypt with random session key. */ - cipher = crypto_create_init_cipher(session_key, 1); - enclen = crypto_cipher_encrypt_with_iv(cipher, + enclen = crypto_cipher_encrypt_with_iv(session_key, enc + 2 + client_entries_len, CIPHER_IV_LEN + strlen(encoded), encoded, strlen(encoded)); - crypto_cipher_free(cipher); + if (enclen < 0) { log_warn(LD_REND, "Could not encrypt introduction point string."); goto done; @@ -307,7 +306,7 @@ rend_encrypt_v2_intro_points_basic(char **encrypted_out, SMARTLIST_FOREACH_BEGIN(client_cookies, const char *, cookie) { client_part = tor_malloc_zero(REND_BASIC_AUTH_CLIENT_ENTRY_LEN); /* Encrypt session key. */ - cipher = crypto_create_init_cipher(cookie, 1); + cipher = crypto_cipher_new(cookie); if (crypto_cipher_encrypt(cipher, client_part + REND_BASIC_AUTH_CLIENT_ID_LEN, session_key, CIPHER_KEY_LEN) < 0) { @@ -374,18 +373,16 @@ rend_encrypt_v2_intro_points_stealth(char **encrypted_out, const char *descriptor_cookie) { int r = -1, enclen; - crypto_cipher_t *cipher; char *enc; tor_assert(encoded); tor_assert(descriptor_cookie); enc = tor_malloc_zero(1 + CIPHER_IV_LEN + strlen(encoded)); enc[0] = 0x02; /* Auth type */ - cipher = crypto_create_init_cipher(descriptor_cookie, 1); - enclen = crypto_cipher_encrypt_with_iv(cipher, enc + 1, + enclen = crypto_cipher_encrypt_with_iv(descriptor_cookie, + enc + 1, CIPHER_IV_LEN+strlen(encoded), encoded, strlen(encoded)); - crypto_cipher_free(cipher); if (enclen < 0) { log_warn(LD_REND, "Could not encrypt introduction point string."); goto done; diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 95cef93521..0aae0aa949 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4887,7 +4887,7 @@ rend_decrypt_introduction_points(char **ipos_decrypted, if (tor_memeq(ipos_encrypted + pos, client_id, REND_BASIC_AUTH_CLIENT_ID_LEN)) { /* Attempt to decrypt introduction points. */ - cipher = crypto_create_init_cipher(descriptor_cookie, 0); + cipher = crypto_cipher_new(descriptor_cookie); if (crypto_cipher_decrypt(cipher, session_key, ipos_encrypted + pos + REND_BASIC_AUTH_CLIENT_ID_LEN, CIPHER_KEY_LEN) < 0) { @@ -4896,13 +4896,13 @@ rend_decrypt_introduction_points(char **ipos_decrypted, return -1; } crypto_cipher_free(cipher); - cipher = crypto_create_init_cipher(session_key, 0); + len = ipos_encrypted_size - 2 - client_entries_len - CIPHER_IV_LEN; dec = tor_malloc(len); - declen = crypto_cipher_decrypt_with_iv(cipher, dec, len, + declen = crypto_cipher_decrypt_with_iv(session_key, dec, len, ipos_encrypted + 2 + client_entries_len, ipos_encrypted_size - 2 - client_entries_len); - crypto_cipher_free(cipher); + if (declen < 0) { log_warn(LD_REND, "Could not decrypt introduction point string."); tor_free(dec); @@ -4923,7 +4923,6 @@ rend_decrypt_introduction_points(char **ipos_decrypted, "check your authorization for this service!"); return -1; } else if (ipos_encrypted[0] == (int)REND_STEALTH_AUTH) { - crypto_cipher_t *cipher; char *dec; int declen; if (ipos_encrypted_size < CIPHER_IV_LEN + 2) { @@ -4932,13 +4931,13 @@ rend_decrypt_introduction_points(char **ipos_decrypted, return -1; } dec = tor_malloc_zero(ipos_encrypted_size - CIPHER_IV_LEN - 1); - cipher = crypto_create_init_cipher(descriptor_cookie, 0); - declen = crypto_cipher_decrypt_with_iv(cipher, dec, + + declen = crypto_cipher_decrypt_with_iv(descriptor_cookie, dec, ipos_encrypted_size - CIPHER_IV_LEN - 1, ipos_encrypted + 1, ipos_encrypted_size - 1); - crypto_cipher_free(cipher); + if (declen < 0) { log_warn(LD_REND, "Decrypting introduction points failed!"); tor_free(dec); diff --git a/src/test/bench.c b/src/test/bench.c index a662bd23e8..3081814802 100644 --- a/src/test/bench.c +++ b/src/test/bench.c @@ -77,9 +77,8 @@ bench_aes(void) uint64_t start, end; const int bytes_per_iter = (1<<24); reset_perftime(); - c = crypto_cipher_new(); - crypto_cipher_generate_key(c); - crypto_cipher_encrypt_init_cipher(c); + c = crypto_cipher_new(NULL); + for (len = 1; len <= 8192; len *= 2) { int iters = bytes_per_iter / len; b1 = tor_malloc_zero(len); @@ -108,9 +107,7 @@ bench_cell_aes(void) crypto_cipher_t *c; int i, misalign; - c = crypto_cipher_new(); - crypto_cipher_generate_key(c); - crypto_cipher_encrypt_init_cipher(c); + c = crypto_cipher_new(NULL); reset_perftime(); for (misalign = 0; misalign <= max_misalign; ++misalign) { @@ -221,12 +218,8 @@ bench_cell_ops(void) or_circ->_base.purpose = CIRCUIT_PURPOSE_OR; /* Initialize crypto */ - or_circ->p_crypto = crypto_cipher_new(); - crypto_cipher_generate_key(or_circ->p_crypto); - crypto_cipher_encrypt_init_cipher(or_circ->p_crypto); - or_circ->n_crypto = crypto_cipher_new(); - crypto_cipher_generate_key(or_circ->n_crypto); - crypto_cipher_encrypt_init_cipher(or_circ->n_crypto); + or_circ->p_crypto = crypto_cipher_new(NULL); + or_circ->n_crypto = crypto_cipher_new(NULL); 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 3f9029a8f8..95a33613af 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -118,14 +118,10 @@ test_crypto_aes(void *arg) memset(data2, 0, 1024); memset(data3, 0, 1024); - env1 = crypto_cipher_new(); + env1 = crypto_cipher_new(NULL); test_neq(env1, 0); - env2 = crypto_cipher_new(); + env2 = crypto_cipher_new(crypto_cipher_get_key(env1)); test_neq(env2, 0); - j = crypto_cipher_generate_key(env1); - crypto_cipher_set_key(env2, crypto_cipher_get_key(env1)); - crypto_cipher_encrypt_init_cipher(env1); - crypto_cipher_decrypt_init_cipher(env2); /* Try encrypting 512 chars. */ crypto_cipher_encrypt(env1, data2, data1, 512); @@ -155,10 +151,8 @@ test_crypto_aes(void *arg) env2 = NULL; memset(data3, 0, 1024); - env2 = crypto_cipher_new(); + env2 = crypto_cipher_new(crypto_cipher_get_key(env1)); test_neq(env2, 0); - crypto_cipher_set_key(env2, crypto_cipher_get_key(env1)); - crypto_cipher_encrypt_init_cipher(env2); for (j = 0; j < 1024-16; j += 17) { crypto_cipher_encrypt(env2, data3+j, data1+j, 17); } @@ -174,10 +168,9 @@ test_crypto_aes(void *arg) env2 = NULL; /* NIST test vector for aes. */ - env1 = crypto_cipher_new(); /* IV starts at 0 */ - crypto_cipher_set_key(env1, "\x80\x00\x00\x00\x00\x00\x00\x00" - "\x00\x00\x00\x00\x00\x00\x00\x00"); - crypto_cipher_encrypt_init_cipher(env1); + /* IV starts at 0 */ + env1 = crypto_cipher_new("\x80\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00"); crypto_cipher_encrypt(env1, data1, "\x00\x00\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x00\x00\x00", 16); @@ -185,37 +178,55 @@ test_crypto_aes(void *arg) /* Now test rollover. All these values are originally from a python * script. */ - crypto_cipher_set_iv(env1, "\x00\x00\x00\x00\x00\x00\x00\x00" - "\xff\xff\xff\xff\xff\xff\xff\xff"); + crypto_cipher_free(env1); + env1 = crypto_cipher_new_with_iv( + "\x80\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00", + "\x00\x00\x00\x00\x00\x00\x00\x00" + "\xff\xff\xff\xff\xff\xff\xff\xff"); memset(data2, 0, 1024); crypto_cipher_encrypt(env1, data1, data2, 32); test_memeq_hex(data1, "335fe6da56f843199066c14a00a40231" "cdd0b917dbc7186908a6bfb5ffd574d3"); - - crypto_cipher_set_iv(env1, "\x00\x00\x00\x00\xff\xff\xff\xff" - "\xff\xff\xff\xff\xff\xff\xff\xff"); + crypto_cipher_free(env1); + env1 = crypto_cipher_new_with_iv( + "\x80\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00", + "\x00\x00\x00\x00\xff\xff\xff\xff" + "\xff\xff\xff\xff\xff\xff\xff\xff"); memset(data2, 0, 1024); crypto_cipher_encrypt(env1, data1, data2, 32); test_memeq_hex(data1, "e627c6423fa2d77832a02b2794094b73" "3e63c721df790d2c6469cc1953a3ffac"); - - crypto_cipher_set_iv(env1, "\xff\xff\xff\xff\xff\xff\xff\xff" - "\xff\xff\xff\xff\xff\xff\xff\xff"); + crypto_cipher_free(env1); + env1 = crypto_cipher_new_with_iv( + "\x80\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00", + "\xff\xff\xff\xff\xff\xff\xff\xff" + "\xff\xff\xff\xff\xff\xff\xff\xff"); memset(data2, 0, 1024); crypto_cipher_encrypt(env1, data1, data2, 32); test_memeq_hex(data1, "2aed2bff0de54f9328efd070bf48f70a" "0EDD33D3C621E546455BD8BA1418BEC8"); /* Now check rollover on inplace cipher. */ - crypto_cipher_set_iv(env1, "\xff\xff\xff\xff\xff\xff\xff\xff" - "\xff\xff\xff\xff\xff\xff\xff\xff"); + crypto_cipher_free(env1); + env1 = crypto_cipher_new_with_iv( + "\x80\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00", + "\xff\xff\xff\xff\xff\xff\xff\xff" + "\xff\xff\xff\xff\xff\xff\xff\xff"); crypto_cipher_crypt_inplace(env1, data2, 64); test_memeq_hex(data2, "2aed2bff0de54f9328efd070bf48f70a" "0EDD33D3C621E546455BD8BA1418BEC8" "93e2c5243d6839eac58503919192f7ae" "1908e67cafa08d508816659c2e693191"); - crypto_cipher_set_iv(env1, "\xff\xff\xff\xff\xff\xff\xff\xff" - "\xff\xff\xff\xff\xff\xff\xff\xff"); + crypto_cipher_free(env1); + env1 = crypto_cipher_new_with_iv( + "\x80\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00", + "\xff\xff\xff\xff\xff\xff\xff\xff" + "\xff\xff\xff\xff\xff\xff\xff\xff"); crypto_cipher_crypt_inplace(env1, data2, 64); test_assert(tor_mem_is_zero(data2, 64)); @@ -674,7 +685,6 @@ test_crypto_s2k(void) static void test_crypto_aes_iv(void *arg) { - crypto_cipher_t *cipher; char *plain, *encrypted1, *encrypted2, *decrypted1, *decrypted2; char plain_1[1], plain_15[15], plain_16[16], plain_17[17]; char key1[16], key2[16]; @@ -698,113 +708,76 @@ test_crypto_aes_iv(void *arg) crypto_rand(plain_17, 17); key1[0] = key2[0] + 128; /* Make sure that contents are different. */ /* Encrypt and decrypt with the same key. */ - cipher = crypto_create_init_cipher(key1, 1); - encrypted_size = crypto_cipher_encrypt_with_iv(cipher, encrypted1, 16 + 4095, + encrypted_size = crypto_cipher_encrypt_with_iv(key1, encrypted1, 16 + 4095, plain, 4095); - crypto_cipher_free(cipher); - cipher = NULL; + test_eq(encrypted_size, 16 + 4095); tt_assert(encrypted_size > 0); /* This is obviously true, since 4111 is * greater than 0, but its truth is not * obvious to all analysis tools. */ - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted1, 4095, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted1, 4095, encrypted1, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; + test_eq(decrypted_size, 4095); tt_assert(decrypted_size > 0); test_memeq(plain, decrypted1, 4095); /* Encrypt a second time (with a new random initialization vector). */ - cipher = crypto_create_init_cipher(key1, 1); - encrypted_size = crypto_cipher_encrypt_with_iv(cipher, encrypted2, 16 + 4095, + encrypted_size = crypto_cipher_encrypt_with_iv(key1, encrypted2, 16 + 4095, plain, 4095); - crypto_cipher_free(cipher); - cipher = NULL; + test_eq(encrypted_size, 16 + 4095); tt_assert(encrypted_size > 0); - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted2, 4095, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted2, 4095, encrypted2, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(decrypted_size, 4095); tt_assert(decrypted_size > 0); test_memeq(plain, decrypted2, 4095); test_memneq(encrypted1, encrypted2, encrypted_size); /* Decrypt with the wrong key. */ - cipher = crypto_create_init_cipher(key2, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted2, 4095, + decrypted_size = crypto_cipher_decrypt_with_iv(key2, decrypted2, 4095, encrypted1, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; test_memneq(plain, decrypted2, encrypted_size); /* Alter the initialization vector. */ encrypted1[0] += 42; - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted1, 4095, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted1, 4095, encrypted1, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; test_memneq(plain, decrypted2, 4095); /* Special length case: 1. */ - cipher = crypto_create_init_cipher(key1, 1); - encrypted_size = crypto_cipher_encrypt_with_iv(cipher, encrypted1, 16 + 1, + encrypted_size = crypto_cipher_encrypt_with_iv(key1, encrypted1, 16 + 1, plain_1, 1); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(encrypted_size, 16 + 1); tt_assert(encrypted_size > 0); - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted1, 1, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted1, 1, encrypted1, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(decrypted_size, 1); tt_assert(decrypted_size > 0); test_memeq(plain_1, decrypted1, 1); /* Special length case: 15. */ - cipher = crypto_create_init_cipher(key1, 1); - encrypted_size = crypto_cipher_encrypt_with_iv(cipher, encrypted1, 16 + 15, + encrypted_size = crypto_cipher_encrypt_with_iv(key1, encrypted1, 16 + 15, plain_15, 15); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(encrypted_size, 16 + 15); tt_assert(encrypted_size > 0); - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted1, 15, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted1, 15, encrypted1, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(decrypted_size, 15); tt_assert(decrypted_size > 0); test_memeq(plain_15, decrypted1, 15); /* Special length case: 16. */ - cipher = crypto_create_init_cipher(key1, 1); - encrypted_size = crypto_cipher_encrypt_with_iv(cipher, encrypted1, 16 + 16, + encrypted_size = crypto_cipher_encrypt_with_iv(key1, encrypted1, 16 + 16, plain_16, 16); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(encrypted_size, 16 + 16); tt_assert(encrypted_size > 0); - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted1, 16, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted1, 16, encrypted1, encrypted_size); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(decrypted_size, 16); tt_assert(decrypted_size > 0); test_memeq(plain_16, decrypted1, 16); /* Special length case: 17. */ - cipher = crypto_create_init_cipher(key1, 1); - encrypted_size = crypto_cipher_encrypt_with_iv(cipher, encrypted1, 16 + 17, + encrypted_size = crypto_cipher_encrypt_with_iv(key1, encrypted1, 16 + 17, plain_17, 17); - crypto_cipher_free(cipher); - cipher = NULL; test_eq(encrypted_size, 16 + 17); tt_assert(encrypted_size > 0); - cipher = crypto_create_init_cipher(key1, 0); - decrypted_size = crypto_cipher_decrypt_with_iv(cipher, decrypted1, 17, + decrypted_size = crypto_cipher_decrypt_with_iv(key1, decrypted1, 17, encrypted1, encrypted_size); test_eq(decrypted_size, 17); tt_assert(decrypted_size > 0); @@ -817,8 +790,6 @@ test_crypto_aes_iv(void *arg) tor_free(encrypted2); tor_free(decrypted1); tor_free(decrypted2); - if (cipher) - crypto_cipher_free(cipher); } /** Test base32 decoding. */ From 55c3e29669214e7a51b16c7273caf30b29079157 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 20 Mar 2012 13:51:11 -0400 Subject: [PATCH 3/4] Use OpenSSL 1.0.1's EVP aes_ctr implementation when available This should be really fast on Intel chips. --- changes/openssl101_aes | 6 +++ src/common/aes.c | 87 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 changes/openssl101_aes diff --git a/changes/openssl101_aes b/changes/openssl101_aes new file mode 100644 index 0000000000..565a48d415 --- /dev/null +++ b/changes/openssl101_aes @@ -0,0 +1,6 @@ + o Major features (performance): + - When built to use the newly OpenSSL 1.0.1, and built for an x86 or + x86_64 instruction set, take advantage of OpenSSL's AESNI, bitsliced, + or vectorized AES implementations as appropriate. These can be + much, much faster than other AES implementations. + diff --git a/src/common/aes.c b/src/common/aes.c index 3121891068..c6bc2a821e 100644 --- a/src/common/aes.c +++ b/src/common/aes.c @@ -33,15 +33,35 @@ #define DISABLE_ENGINES #endif -/* We have 2 strategies for getting AES: Via OpenSSL's AES_encrypt function, - * via OpenSSL's EVP_EncryptUpdate function. +/* We have five strategies for implementing AES counter mode. + * + * Best with x86 and x86_64: Use EVP_aes_ctr128() 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. + * + * Otherwise: Pick the best possible AES block implementation that OpenSSL + * gives us, and the best possible counter-mode implementation, and combine + * them. + */ +#if OPENSSL_VERSION_NUMBER >= OPENSSL_V_NOPATCH(1,0,1) && \ + (defined(__i386) || defined(__i386__) || defined(_M_IX86) || \ + defined(__x86_64) || defined(__x86_64__) || \ + defined(_M_AMD64) || defined(_M_X64) || defined(__INTEL__)) \ + +#define USE_EVP_AES_CTR + +#endif + +/* We have 2 strategies for getting the AES block cipher: Via OpenSSL's + * AES_encrypt function, or via OpenSSL's EVP_EncryptUpdate function. * * If there's any hardware acceleration in play, we want to be using EVP_* so * we can get it. Otherwise, we'll want AES_*, which seems to be about 5% * faster than indirecting through the EVP layer. */ -/* We have 2 strategies for counter mode: use our own, or use OpenSSL's. +/* We have 2 strategies for getting a plug-in counter mode: use our own, or + * use OpenSSL's. * * Here we have a counter mode that's faster than the one shipping with * OpenSSL pre-1.0 (by about 10%!). But OpenSSL 1.0.0 added a counter mode @@ -51,6 +71,66 @@ * make sure that we have a fixed version.) */ +#ifdef USE_EVP_AES_CTR + +struct aes_cnt_cipher { + EVP_CIPHER_CTX evp; +}; + +aes_cnt_cipher_t * +aes_new_cipher(const char *key, const char *iv) +{ + aes_cnt_cipher_t *cipher; + cipher = tor_malloc_zero(sizeof(aes_cnt_cipher_t)); + EVP_EncryptInit(&cipher->evp, EVP_aes_128_ctr(), + (const unsigned char*)key, (const unsigned char *)iv); + return cipher; +} +void +aes_cipher_free(aes_cnt_cipher_t *cipher) +{ + if (!cipher) + return; + EVP_CIPHER_CTX_cleanup(&cipher->evp); + memset(cipher, 0, sizeof(aes_cnt_cipher_t)); + tor_free(cipher); +} +void +aes_crypt(aes_cnt_cipher_t *cipher, const char *input, size_t len, + char *output) +{ + int outl; + + tor_assert(len < INT_MAX); + + EVP_EncryptUpdate(&cipher->evp, (unsigned char*)output, + &outl, (const unsigned char *)input, (int)len); +} +void +aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len) +{ + int outl; + + tor_assert(len < INT_MAX); + + EVP_EncryptUpdate(&cipher->evp, (unsigned char*)data, + &outl, (unsigned char*)data, (int)len); +} +int +evaluate_evp_for_aes(int force_val) +{ + (void) force_val; + log_notice(LD_CRYPTO, "This version of OpenSSL has a known-good EVP " + "counter-mode implementation. Using it."); + return 0; +} +int +evaluate_ctr_for_aes(void) +{ + return 0; +} +#else + /*======================================================================*/ /* Interface to AES code, and counter implementation */ @@ -424,3 +504,4 @@ aes_set_iv(aes_cnt_cipher_t *cipher, const char *iv) _aes_fill_buf(cipher); } +#endif From 01905a6ef9f00705e9db95cf134f1a08787ab92f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 27 Mar 2012 17:57:18 -0400 Subject: [PATCH 4/4] Excise PK_NO_PADDING entirely: Unpadded RSA is silly. We never use it, so having it around is pointless. Suggested by Sebastian --- changes/pad_rsa_always | 3 +++ src/common/crypto.c | 5 ----- src/common/crypto.h | 6 ++---- 3 files changed, 5 insertions(+), 9 deletions(-) create mode 100644 changes/pad_rsa_always diff --git a/changes/pad_rsa_always b/changes/pad_rsa_always new file mode 100644 index 0000000000..a049d61c4f --- /dev/null +++ b/changes/pad_rsa_always @@ -0,0 +1,3 @@ + o Code removal: + - Remove all internal support for unpadded RSA. We never used it, and + it would be a bad idea to start. diff --git a/src/common/crypto.c b/src/common/crypto.c index 2bd2e1e4d2..e79666f955 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -140,7 +140,6 @@ crypto_get_rsa_padding_overhead(int padding) { switch (padding) { - case RSA_NO_PADDING: return 0; case RSA_PKCS1_OAEP_PADDING: return 42; case RSA_PKCS1_PADDING: return 11; default: tor_assert(0); return -1; @@ -154,7 +153,6 @@ crypto_get_rsa_padding(int padding) { switch (padding) { - case PK_NO_PADDING: return RSA_NO_PADDING; case PK_PKCS1_PADDING: return RSA_PKCS1_PADDING; case PK_PKCS1_OAEP_PADDING: return RSA_PKCS1_OAEP_PADDING; default: tor_assert(0); return -1; @@ -991,8 +989,6 @@ crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen, * bytes of data from from, with padding type 'padding', * storing the results on to. * - * (Padding is required; the PK_NO_PADDING value is not supported.) - * * Returns the number of bytes written on success, -1 on failure. * * The encrypted data consists of: @@ -1019,7 +1015,6 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, tor_assert(from); tor_assert(to); tor_assert(fromlen < SIZE_T_CEILING); - tor_assert(padding != PK_NO_PADDING); overhead = crypto_get_rsa_padding_overhead(crypto_get_rsa_padding(padding)); pkeylen = crypto_pk_keysize(env); diff --git a/src/common/crypto.h b/src/common/crypto.h index 00ac26ba26..0482e1af65 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -69,11 +69,9 @@ * signs removed. */ #define BASE64_DIGEST256_LEN 43 -/** Constants used to indicate no padding for public-key encryption */ -#define PK_NO_PADDING 60000 -/** Constants used to indicate PKCS1 padding for public-key encryption */ +/** Constant used to indicate PKCS1 padding for public-key encryption */ #define PK_PKCS1_PADDING 60001 -/** Constants used to indicate OAEP padding for public-key encryption */ +/** Constant used to indicate OAEP padding for public-key encryption */ #define PK_PKCS1_OAEP_PADDING 60002 /** Number of bytes added for PKCS1 padding. */