[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