commit 8cd3e66d93a7b3f61afc3bc0c8868fb50c85af22 Author: Nick Mathewson nickm@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);