[tor-commits] [tor/master] crypto config: do not change the user's value of HardwareAccel.

[nickm at torproject.org]

commit 0d8504e70b0c6ff92f38fc7f4b4c38037d121e69
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Nov 5 10:04:39 2019 -0500

    crypto config: do not change the user's value of HardwareAccel.
    
    We still interpret "AccelName" as turning on the "HardwareAccel"
    feature, but we no longer modify the user's options here.
    
    Fixes bug 32382; bugfix on 0.2.2.1-alpha when we added openssl
    engine support.
---
 changes/ticket32382             |  4 ++++
 src/lib/crypt_ops/crypto_init.c | 23 +++--------------------
 src/test/test_options.c         |  2 +-
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/changes/ticket32382 b/changes/ticket32382
new file mode 100644
index 000000000..85cf07b2e
--- /dev/null
+++ b/changes/ticket32382
@@ -0,0 +1,4 @@
+  o Minor bugfixes (configuration):
+    - Avoid changing the user's value of HardwareAccel as stored by SAVECONF,
+      when AccelName is set but HardwareAccel is not.
+      Fixes bug 32382; bugfix on 0.2.2.1-alpha.
diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c
index 999eac75f..4b0845619 100644
--- a/src/lib/crypt_ops/crypto_init.c
+++ b/src/lib/crypt_ops/crypto_init.c
@@ -258,24 +258,6 @@ subsys_crypto_thread_cleanup(void)
 /** Magic number for crypto_options_t. */
 #define CRYPTO_OPTIONS_MAGIC 0x68757368
 
-/** Invoked before validating crypto options: makes sure that if
- * AccelName is set, HardwareAccel is turned on.
- **/
-static int
-crypto_options_prenormalize(void *arg, char **msg_out)
-{
-  crypto_options_t *opt = arg;
-  tor_assert(opt->magic == CRYPTO_OPTIONS_MAGIC);
-  (void)msg_out;
-
-  // TODO: It would be cleaner to remove this code, but right now the
-  // tests depend on it.
-  if (opt->AccelName && !opt->HardwareAccel)
-    opt->HardwareAccel = 1;
-
-  return 0;
-}
-
 /**
  * Return 0 if <b>arg</b> is a valid crypto_options_t.  Otherwise return -1
  * and set *<b>msg_out</b> to a freshly allocated error string.
@@ -310,7 +292,6 @@ static const config_format_t crypto_options_fmt = {
              CRYPTO_OPTIONS_MAGIC,
              offsetof(crypto_options_t, magic) },
   .vars = crypto_options_t_vars,
-  .pre_normalize_fn = crypto_options_prenormalize,
   .validate_fn = crypto_options_validate,
   .config_suite_offset = -1,
 };
@@ -322,9 +303,11 @@ static int
 crypto_set_options(void *arg)
 {
   const crypto_options_t *options = arg;
+  const bool hardware_accel = options->HardwareAccel || options->AccelName;
+
   // This call already checks for crypto_global_initialized_, so it
   // will only initialize the subsystem the first time it's called.
-  if (crypto_global_init(options->HardwareAccel,
+  if (crypto_global_init(hardware_accel,
                          options->AccelName,
                          options->AccelDir)) {
     log_err(LD_BUG, "Unable to initialize the crypto subsystem. Exiting.");
diff --git a/src/test/test_options.c b/src/test/test_options.c
index 6a933bec4..8aa4bf090 100644
--- a/src/test/test_options.c
+++ b/src/test/test_options.c
@@ -4008,7 +4008,7 @@ test_options_validate__accel(void *ignored)
   tdata = get_options_test_data("AccelName foo\n");
   ret = options_validate(NULL, tdata->opt, &msg);
   tt_int_op(ret, OP_EQ, 0);
-  tt_int_op(get_crypto_options(tdata->opt)->HardwareAccel, OP_EQ, 1);
+  tt_int_op(get_crypto_options(tdata->opt)->HardwareAccel, OP_EQ, 0);
   tor_free(msg);
 
   free_options_test_data(tdata);