Merge remote-tracking branch 'public/feature16794_more'

7 files changed, 108 insertions, 9 deletions

diff --git a/changes/17926 b/changes/17926
new file mode 100644
index 0000000000..09bdbbeae7
--- /dev/null
+++ b/changes/17926
@@ -0,0 +1,3 @@
+  o Removed code:
+    - Remove code for OpenSSL dynamic locks; OpenSSL doesn't use them.
+      Closes ticket 17926.
diff --git a/changes/bug16794_ed b/changes/bug16794_ed
new file mode 100644
index 0000000000..a43c401ed4
--- /dev/null
+++ b/changes/bug16794_ed
@@ -0,0 +1,3 @@
+  o Testing:
+    - Always test both ed25519 backends, so that we can be sure that
+      our batch-open replacement code works. Part of ticket 16794.
diff --git a/src/common/crypto.c b/src/common/crypto.c
index 134e69aa20..2f498ac6be 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -252,7 +252,7 @@ crypto_openssl_get_header_version_str(void)
 
 /** Make sure that openssl is using its default PRNG. Return 1 if we had to
  * adjust it; 0 otherwise. */
-static int
+STATIC int
 crypto_force_rand_ssleay(void)
 {
   RAND_METHOD *default_method;
@@ -3018,6 +3018,10 @@ openssl_locking_cb_(int mode, int n, const char *file, int line)
     tor_mutex_release(openssl_mutexes_[n]);
 }
 
+#if 0
+/* This code is disabled, because OpenSSL never actually uses these callbacks.
+ */
+
 /** OpenSSL helper type: wraps a Tor mutex so that OpenSSL can use it
  * as a lock. */
 struct CRYPTO_dynlock_value {
@@ -3062,6 +3066,7 @@ openssl_dynlock_destroy_cb_(struct CRYPTO_dynlock_value *v,
   tor_mutex_free(v->lock);
   tor_free(v);
 }
+#endif
 
 static void
 tor_set_openssl_thread_id(CRYPTO_THREADID *threadid)
@@ -3083,9 +3088,11 @@ setup_openssl_threading(void)
     openssl_mutexes_[i] = tor_mutex_new();
   CRYPTO_set_locking_callback(openssl_locking_cb_);
   CRYPTO_THREADID_set_callback(tor_set_openssl_thread_id);
+#if 0
   CRYPTO_set_dynlock_create_callback(openssl_dynlock_create_cb_);
   CRYPTO_set_dynlock_lock_callback(openssl_dynlock_lock_cb_);
   CRYPTO_set_dynlock_destroy_callback(openssl_dynlock_destroy_cb_);
+#endif
   return 0;
 }
 
diff --git a/src/common/crypto.h b/src/common/crypto.h
index d0bbf7b47a..fa2ed610c7 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -313,5 +313,9 @@ struct dh_st *crypto_dh_get_dh_(crypto_dh_t *dh);
 
 void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in);
 
+#ifdef CRYPTO_PRIVATE
+STATIC int crypto_force_rand_ssleay(void);
+#endif
+
 #endif
 
diff --git a/src/common/crypto_ed25519.c b/src/common/crypto_ed25519.c
index 41ec486f0a..9df665f66a 100644
--- a/src/common/crypto_ed25519.c
+++ b/src/common/crypto_ed25519.c
@@ -96,6 +96,28 @@ get_ed_impl(void)
   return ed25519_impl;
 }
 
+#ifdef TOR_UNIT_TESTS
+static const ed25519_impl_t *saved_ed25519_impl = NULL;
+void
+crypto_ed25519_testing_force_impl(const char *name)
+{
+  tor_assert(saved_ed25519_impl == NULL);
+  saved_ed25519_impl = ed25519_impl;
+  if (! strcmp(name, "donna")) {
+    ed25519_impl = &impl_donna;
+  } else {
+    tor_assert(!strcmp(name, "ref10"));
+    ed25519_impl = &impl_ref10;
+  }
+}
+void
+crypto_ed25519_testing_restore_impl(void)
+{
+  ed25519_impl = saved_ed25519_impl;
+  saved_ed25519_impl = NULL;
+}
+#endif
+
 /**
  * Initialize a new ed25519 secret key in <b>seckey_out</b>.  If
  * <b>extra_strong</b>, take the RNG inputs directly from the operating
diff --git a/src/common/crypto_ed25519.h b/src/common/crypto_ed25519.h
index bdac12eb27..4fa7ea11cf 100644
--- a/src/common/crypto_ed25519.h
+++ b/src/common/crypto_ed25519.h
@@ -111,5 +111,10 @@ int ed25519_pubkey_eq(const ed25519_public_key_t *key1,
 void ed25519_set_impl_params(int use_donna);
 void ed25519_init(void);
 
+#ifdef TOR_UNIT_TESTS
+void crypto_ed25519_testing_force_impl(const char *name);
+void crypto_ed25519_testing_restore_impl(void);
+#endif
+
 #endif
 
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index e343779bb8..b3e0e1b944 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -5,6 +5,7 @@
 
 #include "orconfig.h"
 #define CRYPTO_CURVE25519_PRIVATE
+#define CRYPTO_PRIVATE
 #include "or.h"
 #include "test.h"
 #include "aes.h"
@@ -15,6 +16,7 @@
 #include "ed25519_vectors.inc"
 
 #include <openssl/evp.h>
+#include <openssl/rand.h>
 
 extern const char AUTHORITY_SIGNKEY_3[];
 extern const char AUTHORITY_SIGNKEY_A_DIGEST[];
@@ -131,6 +133,32 @@ test_crypto_rng_range(void *arg)
   ;
 }
 
+/* Test for rectifying openssl RAND engine. */
+static void
+test_crypto_rng_engine(void *arg)
+{
+  (void)arg;
+  RAND_METHOD dummy_method;
+  memset(&dummy_method, 0, sizeof(dummy_method));
+
+  /* We should be a no-op if we're already on RAND_OpenSSL */
+  tt_int_op(0, ==, crypto_force_rand_ssleay());
+  tt_assert(RAND_get_rand_method() == RAND_OpenSSL());
+
+  /* We should correct the method if it's a dummy. */
+  RAND_set_rand_method(&dummy_method);
+  tt_assert(RAND_get_rand_method() == &dummy_method);
+  tt_int_op(1, ==, crypto_force_rand_ssleay());
+  tt_assert(RAND_get_rand_method() == RAND_OpenSSL());
+
+  /* Make sure we aren't calling dummy_method */
+  crypto_rand((void *) &dummy_method, sizeof(dummy_method));
+  crypto_rand((void *) &dummy_method, sizeof(dummy_method));
+
+ done:
+  ;
+}
+
 /** Run unit tests for our AES functionality */
 static void
 test_crypto_aes(void *arg)
@@ -1688,6 +1716,25 @@ test_crypto_curve25519_persist(void *arg)
   tor_free(tag);
 }
 
+static void *
+ed25519_testcase_setup(const struct testcase_t *testcase)
+{
+  crypto_ed25519_testing_force_impl(testcase->setup_data);
+  return testcase->setup_data;
+}
+static int
+ed25519_testcase_cleanup(const struct testcase_t *testcase, void *ptr)
+{
+  (void)testcase;
+  (void)ptr;
+  crypto_ed25519_testing_restore_impl();
+  return 1;
+}
+static const struct testcase_setup_t ed25519_test_setup = {
+  ed25519_testcase_setup, ed25519_testcase_cleanup
+};
+
+
 static void
 test_crypto_ed25519_simple(void *arg)
 {
@@ -2327,10 +2374,19 @@ test_crypto_failure_modes(void *arg)
 #define CRYPTO_LEGACY(name)                                            \
   { #name, test_crypto_ ## name , 0, NULL, NULL }
 
+#define ED25519_TEST_ONE(name, fl, which)                               \
+  { #name "/ed25519_" which, test_crypto_ed25519_ ## name, (fl),        \
+    &ed25519_test_setup, (void*)which }
+
+#define ED25519_TEST(name, fl)                  \
+  ED25519_TEST_ONE(name, (fl), "donna"),        \
+  ED25519_TEST_ONE(name, (fl), "ref10")
+
 struct testcase_t crypto_tests[] = {
   CRYPTO_LEGACY(formats),
   CRYPTO_LEGACY(rng),
   { "rng_range", test_crypto_rng_range, 0, NULL, NULL },
+  { "rng_engine", test_crypto_rng_engine, 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" },
   CRYPTO_LEGACY(sha),
@@ -2355,14 +2411,13 @@ struct testcase_t crypto_tests[] = {
   { "curve25519_wrappers", test_crypto_curve25519_wrappers, 0, NULL, NULL },
   { "curve25519_encode", test_crypto_curve25519_encode, 0, NULL, NULL },
   { "curve25519_persist", test_crypto_curve25519_persist, 0, NULL, NULL },
-  { "ed25519_simple", test_crypto_ed25519_simple, 0, NULL, NULL },
-  { "ed25519_test_vectors", test_crypto_ed25519_test_vectors, 0, NULL, NULL },
-  { "ed25519_encode", test_crypto_ed25519_encode, 0, NULL, NULL },
-  { "ed25519_convert", test_crypto_ed25519_convert, 0, NULL, NULL },
-  { "ed25519_blinding", test_crypto_ed25519_blinding, 0, NULL, NULL },
-  { "ed25519_testvectors", test_crypto_ed25519_testvectors, 0, NULL, NULL },
-  { "ed25519_fuzz_donna", test_crypto_ed25519_fuzz_donna, TT_FORK, NULL,
-    NULL },
+  ED25519_TEST(simple, 0),
+  ED25519_TEST(test_vectors, 0),
+  ED25519_TEST(encode, 0),
+  ED25519_TEST(convert, 0),
+  ED25519_TEST(blinding, 0),
+  ED25519_TEST(testvectors, 0),
+  ED25519_TEST(fuzz_donna, TT_FORK),
   { "siphash", test_crypto_siphash, 0, NULL, NULL },
   { "failure_modes", test_crypto_failure_modes, TT_FORK, NULL, NULL },
   END_OF_TESTCASES