[tor-commits] [stegotorus/master] Prepare for having more than one file that touches OpenSSL.

zwol at torproject.org zwol at torproject.org
Fri Jul 20 23:17:08 UTC 2012


commit 8a0c099d09284c6717b599c6d70c4cc1913ecc55
Author: Zack Weinberg <zackw at cmu.edu>
Date:   Tue Jun 5 13:49:59 2012 -0700

    Prepare for having more than one file that touches OpenSSL.
    
    * Expose init_crypto, log_crypto_abort, and log_crypto_warn.
    * Use an explicit teardown function for OpenSSL state instead of
      relying on atexit().
    
    Portability/reliability fixes en passant.
    
    * Clarify situation with malloc/realloc in configure.ac.
    * Actually check whether we need -lm for floor().
    * Use CRYPTO_set_mem_functions to send OpenSSL-internal
      memory allocations through xmalloc/xrealloc.
---
 configure.ac         |   11 +++++---
 src/audit-globals.sh |    4 +-
 src/crypt.cc         |   72 +++++++++++++++++++++++++++++--------------------
 src/crypt.h          |   27 ++++++++++++++++++
 src/main.cc          |    3 ++
 src/test/unittest.cc |    4 +++
 6 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/configure.ac b/configure.ac
index ab56311..889cfae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,14 +16,12 @@ dnl with a rationale for each:
 dnl
 dnl Defined by C89, therefore unnecessary to check for nowadays:
 dnl
-dnl   AC_CHECK_FUNCS([atexit floor memset strcasecmp strchr strerror
-dnl                   strrchr strstr strtoul])
+dnl   AC_CHECK_FUNCS([memset strcasecmp strchr strerror strrchr strstr strtoul])
 dnl   AC_CHECK_HEADERS([limits.h stddef.h stdlib.h string.h])
 dnl   AC_CHECK_TYPES([ptrdiff_t])
 dnl   AC_TYPE_SIZE_T
 dnl
-dnl We don't make use of the unspecified-behavior corner cases that
-dnl these pin down:
+dnl Dealt with by our 'xmalloc' and 'xrealloc' wrappers:
 dnl
 dnl   AC_FUNC_MALLOC
 dnl   AC_FUNC_REALLOC
@@ -136,6 +134,11 @@ AC_SUBST(lib_CPPFLAGS)
 AX_LIB_WINSOCK2
 LIBS="$LIBS $ws32_LIBS"
 
+# We might need to explicitly link -lm for floor().
+AC_SEARCH_LIBS([floor], [m], [], [
+  AC_MSG_ERROR([unable to find 'floor'])
+])
+
 ### System features ###
 
 AC_CHECK_HEADERS([execinfo.h paths.h],,,[/**/])
diff --git a/src/audit-globals.sh b/src/audit-globals.sh
index 13565aa..025549f 100644
--- a/src/audit-globals.sh
+++ b/src/audit-globals.sh
@@ -35,8 +35,8 @@ sed '
   /^connections last_ckt_serial$/d
   /^connections last_conn_serial$/d
   /^connections shutting_down$/d
-  /^crypt init_crypto()::initialized$/d
-  /^crypt log_crypto()::initialized$/d
+  /^crypt crypto_initialized$/d
+  /^crypt crypto_errs_initialized$/d
   /^main allow_kq$/d
   /^main handle_signal_cb(int, short, void\*)::got_sigint$/d
   /^main registration_helper$/d
diff --git a/src/crypt.cc b/src/crypt.cc
index 053f7e6..d4af5e0 100644
--- a/src/crypt.cc
+++ b/src/crypt.cc
@@ -11,30 +11,46 @@
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
 
-static void
+static bool crypto_initialized = false;
+static bool crypto_errs_initialized = false;
+
+#define REQUIRE_INIT_CRYPTO() \
+  log_assert(crypto_initialized)
+
+void
 init_crypto()
 {
-  static bool initialized = false;
-  if (!initialized) {
-    initialized = true;
-    ENGINE_load_builtin_engines();
-    ENGINE_register_all_complete();
-    atexit(ENGINE_cleanup);
-
-    // we don't need to call OpenSSL_add_all_algorithms or EVP_cleanup,
-    // since we never look up ciphers by textual name.
-  }
+  log_assert(!crypto_initialized);
+
+  crypto_initialized = true;
+  CRYPTO_set_mem_functions(xmalloc, xrealloc, free);
+  ENGINE_load_builtin_engines();
+  ENGINE_register_all_complete();
+
+  // we don't need to call OpenSSL_add_all_algorithms, since we never
+  // look up ciphers by textual name.
+}
+
+void
+free_crypto()
+{
+  // we don't need to call EVP_cleanup, since we never called
+  // OpenSSL_add_all_algorithms.
+
+  if (crypto_initialized)
+    ENGINE_cleanup();
+  if (crypto_errs_initialized)
+    ERR_free_strings();
 }
 
 static void
 log_crypto()
 {
-  static bool initialized = false;
-  if (!initialized) {
-    initialized = true;
+  if (!crypto_errs_initialized) {
+    crypto_errs_initialized = true;
     ERR_load_crypto_strings();
-    atexit(ERR_free_strings);
   }
+
   unsigned long err;
   while ((err = ERR_get_error()) != 0)
     log_warn("%s: %s: %s",
@@ -43,14 +59,14 @@ log_crypto()
              ERR_reason_error_string(err));
 }
 
-static void ATTR_NORETURN
+void ATTR_NORETURN
 log_crypto_abort(const char *msg)
 {
   log_crypto();
   log_abort("libcrypto error in %s", msg);
 }
 
-static int
+int
 log_crypto_warn(const char *msg)
 {
   log_crypto();
@@ -145,7 +161,7 @@ namespace {
 ecb_encryptor *
 ecb_encryptor::create(const uint8_t *key, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   ecb_encryptor_impl *enc = new ecb_encryptor_impl;
   if (!EVP_EncryptInit_ex(&enc->ctx, aes_ecb_by_size(keylen), 0, key, 0))
@@ -159,7 +175,7 @@ ecb_encryptor::create(const uint8_t *key, size_t keylen)
 ecb_encryptor *
 ecb_encryptor::create(key_generator *gen, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   MemBlock key(keylen);
   size_t got = gen->generate(key, keylen);
@@ -177,7 +193,7 @@ ecb_encryptor::create(key_generator *gen, size_t keylen)
 ecb_decryptor *
 ecb_decryptor::create(const uint8_t *key, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   ecb_decryptor_impl *dec = new ecb_decryptor_impl;
   if (!EVP_DecryptInit_ex(&dec->ctx, aes_ecb_by_size(keylen), 0, key, 0))
@@ -191,7 +207,7 @@ ecb_decryptor::create(const uint8_t *key, size_t keylen)
 ecb_decryptor *
 ecb_decryptor::create(key_generator *gen, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   MemBlock key(keylen);
   size_t got = gen->generate(key, keylen);
@@ -272,7 +288,7 @@ namespace {
 gcm_encryptor *
 gcm_encryptor::create(const uint8_t *key, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   gcm_encryptor_impl *enc = new gcm_encryptor_impl;
   if (!EVP_EncryptInit_ex(&enc->ctx, aes_gcm_by_size(keylen), 0, key, 0))
@@ -284,7 +300,7 @@ gcm_encryptor::create(const uint8_t *key, size_t keylen)
 gcm_encryptor *
 gcm_encryptor::create(key_generator *gen, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   MemBlock key(keylen);
   size_t got = gen->generate(key, keylen);
@@ -300,7 +316,7 @@ gcm_encryptor::create(key_generator *gen, size_t keylen)
 gcm_decryptor *
 gcm_decryptor::create(const uint8_t *key, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   gcm_decryptor_impl *dec = new gcm_decryptor_impl;
   if (!EVP_DecryptInit_ex(&dec->ctx, aes_gcm_by_size(keylen), 0, key, 0))
@@ -312,7 +328,7 @@ gcm_decryptor::create(const uint8_t *key, size_t keylen)
 gcm_decryptor *
 gcm_decryptor::create(key_generator *gen, size_t keylen)
 {
-  init_crypto();
+  REQUIRE_INIT_CRYPTO();
 
   MemBlock key(keylen);
   size_t got = gen->generate(key, keylen);
@@ -430,6 +446,7 @@ key_generator::from_random_secret(const uint8_t *key,  size_t klen,
                                   const uint8_t *ctxt, size_t clen)
 {
   log_assert(klen <= INT_MAX && slen < INT_MAX && clen < INT_MAX);
+  REQUIRE_INIT_CRYPTO();
 
   MemBlock prk(SHA256_LEN);
 
@@ -438,8 +455,6 @@ key_generator::from_random_secret(const uint8_t *key,  size_t klen,
     slen = SHA256_LEN;
   }
 
-  init_crypto();
-
   if (HMAC(EVP_sha256(), salt, slen, key, klen, prk, 0) == 0)
     log_crypto_abort("key_generator::from_random_secret");
 
@@ -460,6 +475,7 @@ key_generator::from_passphrase(const uint8_t *phra, size_t plen,
   // salt, then put the result through HKDF-Extract with the salt.
 
   log_assert(plen <= INT_MAX && slen < INT_MAX);
+  REQUIRE_INIT_CRYPTO();
 
   MemBlock prk(SHA256_LEN);
 
@@ -468,8 +484,6 @@ key_generator::from_passphrase(const uint8_t *phra, size_t plen,
     slen = SHA256_LEN;
   }
 
-  init_crypto();
-
   if (!PKCS5_PBKDF2_HMAC((const char *)phra, plen, salt, slen,
                          10000, EVP_sha256(), SHA256_LEN, prk))
     log_crypto_abort("key_generator::from_passphrase");
diff --git a/src/crypt.h b/src/crypt.h
index d7e5fbf..4da3309 100644
--- a/src/crypt.h
+++ b/src/crypt.h
@@ -9,6 +9,33 @@ const size_t AES_BLOCK_LEN = 16;
 const size_t GCM_TAG_LEN   = 16;
 const size_t SHA256_LEN    = 32;
 
+/**
+ * Initialize cryptography library.  Must be called before anything that
+ * uses any of the APIs below.
+ */
+void init_crypto();
+
+/**
+ * Tear down cryptography library.
+ */
+void free_crypto();
+
+/**
+ * Report a cryptography failure.
+ * @msg should describe the operation that failed.
+ * Always returns -1; this allows you to write
+ * if (some operation failed)
+ *   return log_crypto_warn("some operation");
+ */
+int log_crypto_warn(const char *msg);
+
+/**
+ * Report a cryptography failure which is a fatal error.
+ * @msg should describe the operation that failed.
+ * Does not return.
+ */
+void ATTR_NORETURN log_crypto_abort(const char *msg);
+
 struct key_generator;
 
 struct ecb_encryptor
diff --git a/src/main.cc b/src/main.cc
index 9cc1300..dd69e68 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -381,6 +381,8 @@ main(int, const char *const *argv)
 
   /* Configurations have been established; proceed with initialization. */
 
+  init_crypto();
+
   /* Ugly method to fix a Windows problem:
      http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */
 #ifdef _WIN32
@@ -499,6 +501,7 @@ main(int, const char *const *argv)
   free(stdin_eof);
   event_base_free(the_event_base);
   event_config_free(evcfg);
+  free_crypto();
   log_close();
 
   return 0;
diff --git a/src/test/unittest.cc b/src/test/unittest.cc
index cba4906..958c82c 100644
--- a/src/test/unittest.cc
+++ b/src/test/unittest.cc
@@ -108,6 +108,8 @@ main(int argc, const char **argv)
     log_set_method(LOG_METHOD_NULL, 0);
   }
 
+  init_crypto();
+
   /* Ugly method to fix a Windows problem:
      http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */
 #ifdef _WIN32
@@ -118,7 +120,9 @@ main(int argc, const char **argv)
 #endif
 
   rv = tinytest_main(argc, argv, unittest_groups);
+
   conn_start_shutdown(1);
+  free_crypto();
 
   return rv;
 }





More information about the tor-commits mailing list