Use new configuration architecture for crypto options

This is a comparatively simple change.

src/app/config/config.c

@@ -498,11 +498,8 @@ static const config_var_t option_vars_[] = {
 #endif /* defined(_WIN32) */
   OBSOLETE("Group"),
   V(GuardLifetime,               INTERVAL, "0 minutes"),
-  V_IMMUTABLE(HardwareAccel,     BOOL,     "0"),
   V(HeartbeatPeriod,             INTERVAL, "6 hours"),
   V(MainloopStats,               BOOL,     "0"),
-  V_IMMUTABLE(AccelName,         STRING,   NULL),
-  V_IMMUTABLE(AccelDir,          FILENAME, NULL),
   V(HashedControlPassword,       LINELIST, NULL),
   OBSOLETE("HidServDirectoryV2"),
   VAR("HiddenServiceDir",    LINELIST_S, RendConfigLines,    NULL),
@@ -3938,11 +3935,6 @@ options_validate_cb(const void *old_options_, void *options_, char **msg)
                         "testing Tor network!");
   }
 
-  if (options->AccelName && !options->HardwareAccel)
-    options->HardwareAccel = 1;
-  if (options->AccelDir && !options->AccelName)
-    REJECT("Can't use hardware crypto accelerator dir without engine name.");
-
   if (options_validate_scheduler(options, msg) < 0) {
     return -1;
   }

src/app/config/or_options_st.h

@@ -536,12 +536,8 @@ struct or_options_t {
                          * protocol, is it a warn or an info in our logs? */
   int TestSocks; /**< Boolean: when we get a socks connection, do we loudly
                   * log whether it was DNS-leaking or not? */
-  int HardwareAccel; /**< Boolean: Should we enable OpenSSL hardware
-                      * acceleration where available? */
   /** Token Bucket Refill resolution in milliseconds. */
   int TokenBucketRefillInterval;
-  char *AccelName; /**< Optional hardware acceleration engine name. */
-  char *AccelDir; /**< Optional hardware acceleration engine search dir. */
 
   /** Boolean: Do we try to enter from a smallish number
    * of fixed nodes? */

src/app/main/main.c

@@ -592,9 +592,6 @@ tor_init(int argc, char *argv[])
     return 1;
   }
 
-  /* The options are now initialised */
-  const or_options_t *options = get_options();
-
   /* Initialize channelpadding and circpad parameters to defaults
    * until we get a consensus */
   channelpadding_new_consensus_params(NULL);
@@ -616,13 +613,6 @@ tor_init(int argc, char *argv[])
              "and you probably shouldn't.");
 #endif
 
-  if (crypto_global_init(options->HardwareAccel,
-                         options->AccelName,
-                         options->AccelDir)) {
-    log_err(LD_BUG, "Unable to initialize OpenSSL. Exiting.");
-    return -1;
-  }
-
   /* Scan/clean unparseable descriptors; after reading config */
   routerparse_init();
 

src/feature/relay/router.c

@@ -887,15 +887,6 @@ init_keys_common(void)
   if (!key_lock)
     key_lock = tor_mutex_new();
 
-  /* There are a couple of paths that put us here before we've asked
-   * openssl to initialize itself. */
-  if (crypto_global_init(get_options()->HardwareAccel,
-                         get_options()->AccelName,
-                         get_options()->AccelDir)) {
-    log_err(LD_BUG, "Unable to initialize OpenSSL. Exiting.");
-    return -1;
-  }
-
   return 0;
 }
 

src/lib/crypt_ops/.may_include

@@ -1,6 +1,7 @@
 orconfig.h
 lib/arch/*.h
 lib/cc/*.h
+lib/conf/*.h
 lib/container/*.h
 lib/crypt_ops/*.h
 lib/ctime/*.h
@@ -17,6 +18,8 @@ lib/testsupport/*.h
 lib/thread/*.h
 lib/log/*.h
 
+lib/crypt_ops/*.inc
+
 trunnel/pwbox.h
 
 keccak-tiny/*.h

src/lib/crypt_ops/crypto_init.c

@@ -23,6 +23,9 @@
 #include "lib/crypt_ops/crypto_nss_mgt.h"
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_sys.h"
+#include "lib/crypt_ops/crypto_options_st.h"
+#include "lib/conf/conftypes.h"
+#include "lib/log/util_bug.h"
 
 #include "lib/subsys/subsys.h"
 
@@ -252,6 +255,84 @@ subsys_crypto_thread_cleanup(void)
   crypto_thread_cleanup();
 }
 
+/** 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.
+ **/
+static int
+crypto_options_validate(const void *arg, char **msg_out)
+{
+  const crypto_options_t *opt = arg;
+  tor_assert(opt->magic == CRYPTO_OPTIONS_MAGIC);
+  tor_assert(msg_out);
+
+  if (opt->AccelDir && !opt->AccelName) {
+    *msg_out = tor_strdup("Can't use hardware crypto accelerator dir "
+                          "without engine name.");
+    return -1;
+  }
+
+  return 0;
+}
+
+/* Declare the options field table for crypto_options */
+#define CONF_CONTEXT LL_TABLE
+#include "lib/crypt_ops/crypto_options.inc"
+#undef CONF_CONTEXT
+
+/**
+ * Declares the configuration options for this module.
+ **/
+static const config_format_t crypto_options_fmt = {
+  .size = sizeof(crypto_options_t),
+  .magic = { "crypto_options_t",
+             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,
+};
+
+/**
+ * Invoked from subsysmgr.c when a new set of options arrives.
+ **/
+static int
+crypto_set_options(void *arg)
+{
+  const crypto_options_t *options = arg;
+  // 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,
+                         options->AccelName,
+                         options->AccelDir)) {
+    log_err(LD_BUG, "Unable to initialize the crypto subsystem. Exiting.");
+    return -1;
+  }
+  return 0;
+}
+
 const struct subsys_fns_t sys_crypto = {
   .name = "crypto",
   .supported = true,
@@ -261,4 +342,7 @@ const struct subsys_fns_t sys_crypto = {
   .prefork = subsys_crypto_prefork,
   .postfork = subsys_crypto_postfork,
   .thread_cleanup = subsys_crypto_thread_cleanup,
+
+  .options_format = &crypto_options_fmt,
+  .set_options = crypto_set_options,
 };

src/lib/crypt_ops/crypto_options.inc

@@ -0,0 +1,19 @@
+
+/**
+ * @file crypto_options.inc
+ * @brief Declare configuration options for the crypto_ops module.
+ **/
+
+/** Holds configuration about our cryptography options. */
+BEGIN_CONF_STRUCT(crypto_options_t)
+
+/** Should we enable extra OpenSSL hardware acceleration (where available)?  */
+CONF_VAR(HardwareAccel, BOOL, CFLG_IMMUTABLE, "0")
+
+/** Optional OpenSSL hardware-acceleration engine name */
+CONF_VAR(AccelName, STRING, CFLG_IMMUTABLE, NULL)
+
+/** Optional OpenSSL hardware-acceleration engine search directory. */
+CONF_VAR(AccelDir, FILENAME, CFLG_IMMUTABLE, NULL)
+
+END_CONF_STRUCT(crypto_options_t)

src/lib/crypt_ops/crypto_options_st.h

@@ -0,0 +1,23 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file crypto_options_st.h
+ * @brief Header for lib/crypt_ops/crypto_options_st.c
+ **/
+
+#ifndef TOR_LIB_CRYPT_OPS_CRYPTO_OPTIONS_ST_H
+#define TOR_LIB_CRYPT_OPS_CRYPTO_OPTIONS_ST_H
+
+#include "lib/conf/confdecl.h"
+
+#define CONF_CONTEXT STRUCT
+#include "lib/crypt_ops/crypto_options.inc"
+#undef CONF_CONTEXT
+
+typedef struct crypto_options_t crypto_options_t;
+
+#endif /* !defined(TOR_LIB_CRYPT_OPS_CRYPTO_OPTIONS_ST_H) */

src/lib/crypt_ops/include.am

@@ -68,6 +68,8 @@ noinst_HEADERS +=					\
 	src/lib/crypt_ops/crypto_nss_mgt.h		\
 	src/lib/crypt_ops/crypto_openssl_mgt.h		\
 	src/lib/crypt_ops/crypto_ope.h          	\
+	src/lib/crypt_ops/crypto_options.inc		\
+	src/lib/crypt_ops/crypto_options_st.h		\
 	src/lib/crypt_ops/crypto_pwbox.h		\
 	src/lib/crypt_ops/crypto_rand.h			\
 	src/lib/crypt_ops/crypto_rsa.h			\

src/test/test_options.c

@@ -17,8 +17,11 @@
 #define ROUTERSET_PRIVATE
 #include "feature/nodelist/routerset.h"
 #include "core/mainloop/mainloop.h"
+#include "app/main/subsysmgr.h"
 #include "test/log_test_helpers.h"
 #include "test/resolve_test_helpers.h"
+#include "lib/crypt_ops/crypto_options_st.h"
+#include "lib/crypt_ops/crypto_sys.h"
 
 #include "lib/sandbox/sandbox.h"
 #include "lib/memarea/memarea.h"
@@ -3985,6 +3988,14 @@ test_options_validate__testing_options(void *ignored)
   tor_free(msg);
 }
 
+static crypto_options_t *
+get_crypto_options(or_options_t *opt)
+{
+  int idx = subsystems_get_options_idx(&sys_crypto);
+  tor_assert(idx >= 0);
+  return config_mgr_get_obj_mutable(get_options_mgr(), opt, idx);
+}
+
 static void
 test_options_validate__accel(void *ignored)
 {
@@ -3997,15 +4008,15 @@ 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(tdata->opt->HardwareAccel, OP_EQ, 1);
+  tt_int_op(get_crypto_options(tdata->opt)->HardwareAccel, OP_EQ, 1);
   tor_free(msg);
 
   free_options_test_data(tdata);
   tdata = get_options_test_data("AccelName foo\n");
-  tdata->opt->HardwareAccel = 2;
+  get_crypto_options(tdata->opt)->HardwareAccel = 2;
   ret = options_validate(NULL, tdata->opt, &msg);
   tt_int_op(ret, OP_EQ, 0);
-  tt_int_op(tdata->opt->HardwareAccel, OP_EQ, 2);
+  tt_int_op(get_crypto_options(tdata->opt)->HardwareAccel, OP_EQ, 2);
   tor_free(msg);
 
   free_options_test_data(tdata);