[tor-commits] [tor/master] Use new configuration architecture for crypto options
nickm at torproject.org
nickm at torproject.org
Thu Nov 7 13:59:49 UTC 2019
commit 8cd3e66d93a7b3f61afc3bc0c8868fb50c85af22
Author: Nick Mathewson <nickm at torproject.org>
Date: Sat Oct 26 18:05:58 2019 -0400
Use new configuration architecture for crypto options
This is a comparatively simple change.
---
src/app/config/config.c | 8 ----
src/app/config/or_options_st.h | 4 --
src/app/main/main.c | 10 -----
src/feature/relay/router.c | 9 ----
src/lib/crypt_ops/.may_include | 3 ++
src/lib/crypt_ops/crypto_init.c | 84 +++++++++++++++++++++++++++++++++++
src/lib/crypt_ops/crypto_options.inc | 19 ++++++++
src/lib/crypt_ops/crypto_options_st.h | 23 ++++++++++
src/lib/crypt_ops/include.am | 2 +
src/test/test_options.c | 17 +++++--
10 files changed, 145 insertions(+), 34 deletions(-)
diff --git a/src/app/config/config.c b/src/app/config/config.c
index 892934970..480d225e9 100644
--- a/src/app/config/config.c
+++ b/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;
}
diff --git a/src/app/config/or_options_st.h b/src/app/config/or_options_st.h
index 5511763da..a3d63d920 100644
--- a/src/app/config/or_options_st.h
+++ b/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? */
diff --git a/src/app/main/main.c b/src/app/main/main.c
index fad2e0b62..6029ed3d2 100644
--- a/src/app/main/main.c
+++ b/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();
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 7f80b288d..410ed8c2f 100644
--- a/src/feature/relay/router.c
+++ b/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;
}
diff --git a/src/lib/crypt_ops/.may_include b/src/lib/crypt_ops/.may_include
index 073969968..810e77727 100644
--- a/src/lib/crypt_ops/.may_include
+++ b/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
diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c
index a16bf4e11..999eac75f 100644
--- a/src/lib/crypt_ops/crypto_init.c
+++ b/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,
};
diff --git a/src/lib/crypt_ops/crypto_options.inc b/src/lib/crypt_ops/crypto_options.inc
new file mode 100644
index 000000000..5bee0daac
--- /dev/null
+++ b/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)
diff --git a/src/lib/crypt_ops/crypto_options_st.h b/src/lib/crypt_ops/crypto_options_st.h
new file mode 100644
index 000000000..8127b41ee
--- /dev/null
+++ b/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) */
diff --git a/src/lib/crypt_ops/include.am b/src/lib/crypt_ops/include.am
index 1f58a33d3..7644cab41 100644
--- a/src/lib/crypt_ops/include.am
+++ b/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 \
diff --git a/src/test/test_options.c b/src/test/test_options.c
index c06fb998f..6a933bec4 100644
--- a/src/test/test_options.c
+++ b/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);
More information about the tor-commits
mailing list