November 2019 - tor-commits - lists.torproject.org

[tor/master] Stop using "config_suite_offset=-1" to indicate "no config suite."
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 0f0a9bdf332002bb0542dae6bb00e922af5dcf63 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 5 10:18:47 2019 -0500 Stop using "config_suite_offset=-1" to indicate "no config suite." Instead, create a separate "has_config_suite" boolean, so that only top-level formats with config_suites need to declare an offset at all. --- src/app/config/config.c | 1 + src/app/config/statefile.c | 1 + src/feature/dirauth/shared_random_state.c | 1 - src/lib/conf/conftypes.h | 10 ++++++++-- src/lib/confmgt/confmgt.c | 9 +++++++-- src/lib/crypt_ops/crypto_init.c | 1 - src/test/test_confmgr.c | 3 +-- src/test/test_confparse.c | 2 -- 8 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index 480d225e9..c121775c0 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -880,6 +880,7 @@ static const config_format_t options_format = { .legacy_validate_fn = options_validate_cb, .check_transition_fn = options_check_transition_cb, .clear_fn = options_clear_cb, + .has_config_suite = true, .config_suite_offset = offsetof(or_options_t, subconfigs_), }; diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index 5fdb15ace..3e793ff40 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -169,6 +169,7 @@ static const config_format_t state_format = { .vars = state_vars_, .legacy_validate_fn = or_state_validate_cb, .extra = &state_extra_var, + .has_config_suite = true, .config_suite_offset = offsetof(or_state_t, substates_), }; diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c index 759b3b810..bf4302f16 100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@ -92,7 +92,6 @@ static const config_format_t state_format = { }, .vars = state_vars, .extra = &state_extra_var, - .config_suite_offset = -1, }; /** Global configuration manager for the shared-random state file */ diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h index d4e2ea218..dfe51cfba 100644 --- a/src/lib/conf/conftypes.h +++ b/src/lib/conf/conftypes.h @@ -335,8 +335,14 @@ typedef struct config_format_t { /** If present, extra denotes a LINELIST variable for unrecognized * lines. Otherwise, unrecognized lines are an error. */ const struct_member_t *extra; - /** The position of a config_suite_t pointer within the toplevel object, - * or -1 if there is no such pointer. */ + /** + * If true, this format describes a top-level configuration, with + * a suite containing multiple sub-configuration objects. + */ + bool has_config_suite; + /** The position of a config_suite_t pointer within the toplevel object. + * Ignored unless have_config_suite is true. + */ ptrdiff_t config_suite_offset; } config_format_t; diff --git a/src/lib/confmgt/confmgt.c b/src/lib/confmgt/confmgt.c index 1c1a1595e..a96c7f96b 100644 --- a/src/lib/confmgt/confmgt.c +++ b/src/lib/confmgt/confmgt.c @@ -169,9 +169,14 @@ config_mgr_register_fmt(config_mgr_t *mgr, "it had been frozen."); if (object_idx != IDX_TOPLEVEL) { - tor_assertf(fmt->config_suite_offset < 0, + tor_assertf(! fmt->has_config_suite, "Tried to register a toplevel format in a non-toplevel position"); } + if (fmt->config_suite_offset) { + tor_assertf(fmt->has_config_suite, + "config_suite_offset was set, but has_config_suite was not."); + } + tor_assertf(fmt != mgr->toplevel && ! smartlist_contains(mgr->subconfigs, fmt), "Tried to register an already-registered format."); @@ -223,7 +228,7 @@ config_mgr_add_format(config_mgr_t *mgr, static inline config_suite_t ** config_mgr_get_suite_ptr(const config_mgr_t *mgr, void *toplevel) { - if (mgr->toplevel->config_suite_offset < 0) + if (! mgr->toplevel->has_config_suite) return NULL; return STRUCT_VAR_P(toplevel, mgr->toplevel->config_suite_offset); } diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c index 4b0845619..fbd4da470 100644 --- a/src/lib/crypt_ops/crypto_init.c +++ b/src/lib/crypt_ops/crypto_init.c @@ -293,7 +293,6 @@ static const config_format_t crypto_options_fmt = { offsetof(crypto_options_t, magic) }, .vars = crypto_options_t_vars, .validate_fn = crypto_options_validate, - .config_suite_offset = -1, }; /** diff --git a/src/test/test_confmgr.c b/src/test/test_confmgr.c index 375a513c0..b59bd8c6a 100644 --- a/src/test/test_confmgr.c +++ b/src/test/test_confmgr.c @@ -193,6 +193,7 @@ static const config_format_t pasture_fmt = { offsetof(pasture_cfg_t, magic) }, .vars = pasture_vars, + .has_config_suite = true, .config_suite_offset = offsetof(pasture_cfg_t, subobjs), .legacy_validate_fn = legacy_validate_pasture, }; @@ -205,7 +206,6 @@ static const config_format_t llama_fmt = { offsetof(llama_cfg_t, magic) }, .vars = llama_vars, - .config_suite_offset = -1, .deprecations = llama_deprecations, .abbrevs = llama_abbrevs, .clear_fn = clear_llama_cfg, @@ -221,7 +221,6 @@ static const config_format_t alpaca_fmt = { offsetof(alpaca_cfg_t, magic) }, .vars = alpaca_vars, - .config_suite_offset = -1, .deprecations = alpaca_deprecations, .pre_normalize_fn = pre_normalize_alpaca, .check_transition_fn = check_transition_alpaca, diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c index 39e2de866..3e122a512 100644 --- a/src/test/test_confparse.c +++ b/src/test/test_confparse.c @@ -129,7 +129,6 @@ static const config_format_t test_fmt = { .deprecations = test_deprecation_notes, .vars = test_vars, .legacy_validate_fn = test_validate_cb, - .config_suite_offset = -1, }; /* Make sure that config_init sets everything to the right defaults. */ @@ -824,7 +823,6 @@ static config_format_t etest_fmt = { .vars = test_vars, .legacy_validate_fn = test_validate_cb, .extra = &extra, - .config_suite_offset = -1, }; /* Try out the feature where we can store unrecognized lines and dump them

1 0

[tor/master] Add testing-only functions to get the subsystem config/state indices
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit b06e9d8ad58bfe11d2c1f6a921ba059658a578f2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 1 10:41:02 2019 -0400 Add testing-only functions to get the subsystem config/state indices --- src/app/main/subsysmgr.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ src/app/main/subsysmgr.h | 5 +++++ 2 files changed, 53 insertions(+) diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index bd9af1144..4189519ca 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -335,6 +335,54 @@ subsystems_register_state_formats(config_mgr_t *mgr) return 0; } +#ifdef TOR_UNIT_TESTS +/** + * Helper: look up the index for sys. Return -1 if the subsystem + * is not recognized. + **/ +static int +subsys_get_idx(const subsys_fns_t *sys) +{ + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + if (sys == tor_subsystems[i]) + return (int)i; + } + return -1; +} + +/** + * Return the current state-manager's index for any state held by the + * subsystem sys. If sys has no options, return -1. + * + * Using raw indices can be error-prone: only do this from the unit + * tests. If you need a way to access another subsystem's configuration, + * that subsystem should provide access functions. + **/ +int +subsystems_get_options_idx(const subsys_fns_t *sys) +{ + int i = subsys_get_idx(sys); + tor_assert(i >= 0); + return sys_status[i].options_idx; +} + +/** + * Return the current state-manager's index for any state held by the + * subsystem sys. If sys has no state, return -1. + * + * Using raw indices can be error-prone: only do this from the unit + * tests. If you need a way to access another subsystem's state + * that subsystem should provide access functions. + **/ +int +subsystems_get_state_idx(const subsys_fns_t *sys) +{ + int i = subsys_get_idx(sys); + tor_assert(i >= 0); + return sys_status[i].state_idx; +} +#endif + /** * Call all appropriate set_options() methods to tell the various subsystems * about a new set of torrc options. Return 0 on success, -1 on diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index 7e5fe7636..c1138e1ff 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -43,4 +43,9 @@ int subsystems_set_state(const struct config_mgr_t *mgr, int subsystems_flush_state(const struct config_mgr_t *mgr, struct or_state_t *state); +#ifdef TOR_UNIT_TESTS +int subsystems_get_options_idx(const subsys_fns_t *sys); +int subsystems_get_state_idx(const subsys_fns_t *sys); +#endif + #endif /* !defined(TOR_SUBSYSMGR_T) */

1 0

[tor/master] Give subsystems optional config formats and state formats.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 7ac4f9d5ec1b31a0d4b76ab62c1afc039c8fe627 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 30 09:28:34 2019 -0400 Give subsystems optional config formats and state formats. The formats, when provided, are now added to the global config_mgr_t objects. --- src/app/config/config.c | 2 ++ src/app/config/statefile.c | 3 ++ src/app/main/subsysmgr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++- src/app/main/subsysmgr.h | 4 +++ src/lib/subsys/subsys.h | 13 +++++++++ 5 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index 5f9a55ed1..7317a5d2f 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -918,6 +918,8 @@ get_options_mgr(void) { if (PREDICT_UNLIKELY(options_mgr == NULL)) { options_mgr = config_mgr_new(&options_format); + int rv = subsystems_register_options_formats(options_mgr); + tor_assert(rv == 0); config_mgr_freeze(options_mgr); } return options_mgr; diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index db4d780a7..f6b0915da 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -45,6 +45,7 @@ #include "feature/relay/routermode.h" #include "lib/sandbox/sandbox.h" #include "app/config/statefile.h" +#include "app/main/subsysmgr.h" #include "lib/encoding/confline.h" #include "lib/net/resolve.h" #include "lib/version/torversion.h" @@ -180,6 +181,8 @@ get_state_mgr(void) { if (PREDICT_UNLIKELY(state_mgr == NULL)) { state_mgr = config_mgr_new(&state_format); + int rv = subsystems_register_state_formats(state_mgr); + tor_assert(rv == 0); config_mgr_freeze(state_mgr); } return state_mgr; diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 5fc298dbf..e5c76b8e5 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -14,10 +14,12 @@ #include "orconfig.h" #include "app/main/subsysmgr.h" +#include "lib/confmgt/confmgt.h" #include "lib/dispatch/dispatch_naming.h" #include "lib/dispatch/msgtypes.h" #include "lib/err/torerr.h" #include "lib/log/log.h" +#include "lib/log/util_bug.h" #include "lib/malloc/malloc.h" #include "lib/pubsub/pubsub_build.h" #include "lib/pubsub/pubsub_connect.h" @@ -37,6 +39,10 @@ static bool subsystem_array_validated = false; typedef struct subsys_status_t { /** True if the given subsystem is initialized. */ bool initialized; + /** Index for this subsystem's options object, or -1 for none. */ + int options_idx; + /** Index for this subsystem's state object, or -1 for none. */ + int state_idx; } subsys_status_t; /** An overestimate of the number of subsystems. */ @@ -48,6 +54,18 @@ typedef struct subsys_status_t { **/ static subsys_status_t sys_status[N_SYS_STATUS]; +/** Set status to a default (not set-up) state. */ +static void +subsys_status_clear(subsys_status_t *status) +{ + if (!status) + return; + memset(status, 0, sizeof(*status)); + status->initialized = false; + status->state_idx = -1; + status->options_idx = -1; +} + /** * Exit with a raw assertion if the subsystems list is inconsistent; * initialize the subsystem_initialized array. @@ -77,6 +95,8 @@ check_and_setup(void) sys->name, i, sys->level, last_level); raw_assert_unreached_msg("There is a bug in subsystem_list.c"); } + subsys_status_clear(&sys_status[i]); + last_level = sys->level; } @@ -202,7 +222,7 @@ subsystems_shutdown_downto(int target_level) log_debug(LD_GENERAL, "Shutting down %s", sys->name); sys->shutdown(); } - sys_status[i].initialized = false; + subsys_status_clear(&sys_status[i]); } } @@ -268,3 +288,49 @@ subsystems_thread_cleanup(void) } } } + +/** + * Register all subsystem-declared options formats in mgr. + * + * Return 0 on success, -1 on failure. + **/ +int +subsystems_register_options_formats(config_mgr_t *mgr) +{ + tor_assert(mgr); + check_and_setup(); + + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys->options_format) { + int options_idx = config_mgr_add_format(mgr, sys->options_format); + sys_status[i].options_idx = options_idx; + log_debug(LD_CONFIG, "Added options format for %s with index %d", + sys->name, options_idx); + } + } + return 0; +} + +/** + * Register all subsystem-declared state formats in mgr. + * + * Return 0 on success, -1 on failure. + **/ +int +subsystems_register_state_formats(config_mgr_t *mgr) +{ + tor_assert(mgr); + check_and_setup(); + + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys->state_format) { + int state_idx = config_mgr_add_format(mgr, sys->state_format); + sys_status[i].state_idx = state_idx; + log_debug(LD_CONFIG, "Added state format for %s with index %d", + sys->name, state_idx); + } + } + return 0; +} diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index f8bc83e0a..54193f964 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -31,4 +31,8 @@ void subsystems_prefork(void); void subsystems_postfork(void); void subsystems_thread_cleanup(void); +struct config_mgr_t; +int subsystems_register_options_formats(struct config_mgr_t *mgr); +int subsystems_register_state_formats(struct config_mgr_t *mgr); + #endif /* !defined(TOR_SUBSYSMGR_T) */ diff --git a/src/lib/subsys/subsys.h b/src/lib/subsys/subsys.h index 91abdb7d7..1cb3fe94a 100644 --- a/src/lib/subsys/subsys.h +++ b/src/lib/subsys/subsys.h @@ -14,6 +14,7 @@ #include <stdbool.h> struct pubsub_connector_t; +struct config_format_t; /** * A subsystem is a part of Tor that is initialized, shut down, configured, @@ -88,6 +89,18 @@ typedef struct subsys_fns_t { **/ void (*shutdown)(void); + /** + * A config_format_t describing all of the torrc fields owned by this + * subsystem. + **/ + const struct config_format_t *options_format; + + /** + * A config_format_t describing all of the DataDir/state fields owned by + * this subsystem. + **/ + const struct config_format_t *state_format; + } subsys_fns_t; /**

1 0

[tor/master] subsysmgr: use IDX_NONE is an exception value, not -1.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 3afbb29bee060b191e10aaec134a819047c3cf5e Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 5 10:24:34 2019 -0500 subsysmgr: use IDX_NONE is an exception value, not -1. --- src/app/main/subsysmgr.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 4189519ca..8be4a7d75 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -33,15 +33,19 @@ **/ static bool subsystem_array_validated = false; +/** Index value indicating that a subsystem has no options/state object, and + * so that object does not have an index. */ +#define IDX_NONE (-1) + /** * Runtime status of a single subsystem. **/ typedef struct subsys_status_t { /** True if the given subsystem is initialized. */ bool initialized; - /** Index for this subsystem's options object, or -1 for none. */ + /** Index for this subsystem's options object, or IDX_NONE for none. */ int options_idx; - /** Index for this subsystem's state object, or -1 for none. */ + /** Index for this subsystem's state object, or IDX_NONE for none. */ int state_idx; } subsys_status_t; @@ -62,8 +66,8 @@ subsys_status_clear(subsys_status_t *status) return; memset(status, 0, sizeof(*status)); status->initialized = false; - status->state_idx = -1; - status->options_idx = -1; + status->state_idx = IDX_NONE; + status->options_idx = IDX_NONE; } /**

1 0

[tor/master] crypto config: do not change the user's value of HardwareAccel.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 0d8504e70b0c6ff92f38fc7f4b4c38037d121e69 Author: Nick Mathewson <nickm(a)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 arg is a valid crypto_options_t. Otherwise return -1 * and set *msg_out 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);

1 0

[tor/master] Expose get_obj() functions from confmgt.c.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 86389893080d86f6437476c3ff6f3fb07ad3bd99 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 30 10:15:22 2019 -0400 Expose get_obj() functions from confmgt.c. The subsysmgr code wants to use them. --- src/lib/confmgt/confmgt.c | 4 ++-- src/lib/confmgt/confmgt.h | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/lib/confmgt/confmgt.c b/src/lib/confmgt/confmgt.c index 937773611..1c1a1595e 100644 --- a/src/lib/confmgt/confmgt.c +++ b/src/lib/confmgt/confmgt.c @@ -237,7 +237,7 @@ config_mgr_get_suite_ptr(const config_mgr_t *mgr, void *toplevel) * to configuration objects for other modules. This function gets * the sub-object for a particular module. */ -STATIC void * +void * config_mgr_get_obj_mutable(const config_mgr_t *mgr, void *toplevel, int idx) { tor_assert(mgr); @@ -256,7 +256,7 @@ config_mgr_get_obj_mutable(const config_mgr_t *mgr, void *toplevel, int idx) } /** As config_mgr_get_obj_mutable(), but return a const pointer. */ -STATIC const void * +const void * config_mgr_get_obj(const config_mgr_t *mgr, const void *toplevel, int idx) { return config_mgr_get_obj_mutable(mgr, (void*)toplevel, idx); diff --git a/src/lib/confmgt/confmgt.h b/src/lib/confmgt/confmgt.h index f565742c5..11f0de03a 100644 --- a/src/lib/confmgt/confmgt.h +++ b/src/lib/confmgt/confmgt.h @@ -123,13 +123,14 @@ bool config_var_is_listable(const config_var_t *var); #define CFG_EQ_LINELIST(a,b,opt) config_lines_eq((a)->opt, (b)->opt) #define CFG_EQ_ROUTERSET(a,b,opt) routerset_equal((a)->opt, (b)->opt) +void *config_mgr_get_obj_mutable(const config_mgr_t *mgr, + void *toplevel, int idx); +const void *config_mgr_get_obj(const config_mgr_t *mgr, + const void *toplevel, int idx); + #ifdef CONFMGT_PRIVATE STATIC void config_reset_line(const config_mgr_t *mgr, void *options, const char *key, int use_defaults); -STATIC void *config_mgr_get_obj_mutable(const config_mgr_t *mgr, - void *toplevel, int idx); -STATIC const void *config_mgr_get_obj(const config_mgr_t *mgr, - const void *toplevel, int idx); #endif /* defined(CONFMGT_PRIVATE) */ #endif /* !defined(TOR_CONFMGT_H) */

1 0

[tor/master] Add subsys functions for receiving/flushing states and options.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 52c0ab4af3ec152c4b78669acf8877ca27d66097 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 30 10:29:43 2019 -0400 Add subsys functions for receiving/flushing states and options. These functions are in the subsystem, not in the config_format_t, since they are about how the format is _used_, not about _what it is_. --- src/app/config/config.c | 3 +- src/app/config/statefile.c | 4 +++ src/app/main/subsysmgr.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++ src/app/main/subsysmgr.h | 8 +++++ src/lib/subsys/subsys.h | 29 ++++++++++++++++++ 5 files changed, 119 insertions(+), 1 deletion(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index 7317a5d2f..892934970 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -987,7 +987,8 @@ set_options(or_options_t *new_val, char **msg) global_options = old_options; return -1; } - if (options_act(old_options) < 0) { /* acting on the options failed. die. */ + if (subsystems_set_options(get_options_mgr(), new_val) < 0 || + options_act(old_options) < 0) { /* acting on the options failed. die. */ if (! tor_event_loop_shutdown_is_pending()) { log_err(LD_BUG, "Acting on config options left us in a broken state. Dying."); diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index f6b0915da..5fdb15ace 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -316,6 +316,9 @@ or_state_set(or_state_t *new_state) tor_assert(new_state); config_free(get_state_mgr(), global_state); global_state = new_state; + if (subsystems_set_state(get_state_mgr(), global_state) < 0) { + ret = -1; + } if (entry_guards_parse_state(global_state, 1, &err)<0) { log_warn(LD_GENERAL,"%s",err); tor_free(err); @@ -519,6 +522,7 @@ or_state_save(time_t now) /* Call everything else that might dirty the state even more, in order * to avoid redundant writes. */ + (void) subsystems_flush_state(get_state_mgr(), global_state); entry_guards_update_state(global_state); rep_hist_update_state(global_state); circuit_build_times_update_state(get_circuit_build_times(), global_state); diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index e5c76b8e5..bd9af1144 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -334,3 +334,79 @@ subsystems_register_state_formats(config_mgr_t *mgr) } return 0; } + +/** + * Call all appropriate set_options() methods to tell the various subsystems + * about a new set of torrc options. Return 0 on success, -1 on + * nonrecoverable failure. + **/ +int +subsystems_set_options(const config_mgr_t *mgr, struct or_options_t *options) +{ + /* XXXX This does not yet handle reversible option assignment; I'll + * do that later in this branch. */ + + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys_status[i].options_idx >= 0 && sys->set_options) { + void *obj = config_mgr_get_obj_mutable(mgr, options, + sys_status[i].options_idx); + int rv = sys->set_options(obj); + if (rv < 0) { + log_err(LD_CONFIG, "Error when handling option for %s; " + "cannot proceed.", sys->name); + return -1; + } + } + } + return 0; +} + +/** + * Call all appropriate set_state() methods to tell the various subsystems + * about an initial DataDir/state file. Return 0 on success, -1 on + * nonrecoverable failure. + **/ +int +subsystems_set_state(const config_mgr_t *mgr, struct or_state_t *state) +{ + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys_status[i].state_idx >= 0 && sys->set_state) { + void *obj = config_mgr_get_obj_mutable(mgr, state, + sys_status[i].state_idx); + int rv = sys->set_state(obj); + if (rv < 0) { + log_err(LD_CONFIG, "Error when handling state for %s; " + "cannot proceed.", sys->name); + return -1; + } + } + } + return 0; +} + +/** + * Call all appropriate flush_state() methods to tell the various subsystems + * to update the state objects in state. Return 0 on success, + * -1 on failure. + **/ +int +subsystems_flush_state(const config_mgr_t *mgr, struct or_state_t *state) +{ + int result = 0; + for (unsigned i = 0; i < n_tor_subsystems; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + if (sys_status[i].state_idx >= 0 && sys->flush_state) { + void *obj = config_mgr_get_obj_mutable(mgr, state, + sys_status[i].state_idx); + int rv = sys->flush_state(obj); + if (rv < 0) { + log_warn(LD_CONFIG, "Error when flushing state to state object for %s", + sys->name); + result = -1; + } + } + } + return result; +} diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index 54193f964..7e5fe7636 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -34,5 +34,13 @@ void subsystems_thread_cleanup(void); struct config_mgr_t; int subsystems_register_options_formats(struct config_mgr_t *mgr); int subsystems_register_state_formats(struct config_mgr_t *mgr); +struct or_options_t; +struct or_state_t; +int subsystems_set_options(const struct config_mgr_t *mgr, + struct or_options_t *options); +int subsystems_set_state(const struct config_mgr_t *mgr, + struct or_state_t *state); +int subsystems_flush_state(const struct config_mgr_t *mgr, + struct or_state_t *state); #endif /* !defined(TOR_SUBSYSMGR_T) */ diff --git a/src/lib/subsys/subsys.h b/src/lib/subsys/subsys.h index 1cb3fe94a..29a90c049 100644 --- a/src/lib/subsys/subsys.h +++ b/src/lib/subsys/subsys.h @@ -101,6 +101,35 @@ typedef struct subsys_fns_t { **/ const struct config_format_t *state_format; + /** + * Receive an options object as defined by options_format. Return 0 + * on success, -1 on failure. + * + * It is safe to store the pointer to the object until set_options() + * is called again. */ + int (*set_options)(void *); + + /* XXXX Add an implementation for options_act_reversible() later in this + * branch. */ + + /** + * Receive a state object as defined by state_format. Return 0 on success, + * -1 on failure. + * + * It is safe to store the pointer to the object; set_state() is only + * called on startup. + **/ + int (*set_state)(void *); + + /** + * Update any information that needs to be stored in the provided state + * object (as defined by state_format). Return 0 on success, -1 on failure. + * + * The object provided here will be the same one as provided earlier to + * set_state(). This method is called when we are about to save the state + * to disk. + **/ + int (*flush_state)(void *); } subsys_fns_t; /**

1 0

[tor/master] conf_examples test for crypto_accel
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 27a4438805dcac7fa46a2c47f0b247c7f8d3668e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Nov 6 11:02:37 2019 -0500 conf_examples test for crypto_accel --- src/test/conf_examples/crypto_accel/expected | 2 ++ src/test/conf_examples/crypto_accel/torrc | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/test/conf_examples/crypto_accel/expected b/src/test/conf_examples/crypto_accel/expected new file mode 100644 index 000000000..ea80ca19d --- /dev/null +++ b/src/test/conf_examples/crypto_accel/expected @@ -0,0 +1,2 @@ +AccelName nonexistent_chartreuse_accelerator +HardwareAccel 1 diff --git a/src/test/conf_examples/crypto_accel/torrc b/src/test/conf_examples/crypto_accel/torrc new file mode 100644 index 000000000..9ca18903b --- /dev/null +++ b/src/test/conf_examples/crypto_accel/torrc @@ -0,0 +1,3 @@ + +AccelName nonexistent_chartreuse_accelerator +HardwareAccel 1

1 0

[tor/master] Make a new structure for tracking subsystem status.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit a7cfddc8d18c39be8fb212ee2a96da2d1905d9c8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 30 09:49:55 2019 -0400 Make a new structure for tracking subsystem status. We used to have only one boolean per subsystem, but we're about to have a little more information. --- src/app/main/subsysmgr.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 1f4bc840f..5fc298dbf 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -32,11 +32,21 @@ static bool subsystem_array_validated = false; /** + * Runtime status of a single subsystem. + **/ +typedef struct subsys_status_t { + /** True if the given subsystem is initialized. */ + bool initialized; +} subsys_status_t; + +/** An overestimate of the number of subsystems. */ +#define N_SYS_STATUS 128 +/** * True if a given subsystem is initialized. Expand this array if there * are more than this number of subsystems. (We'd rather not * dynamically allocate in this module.) **/ -static bool sys_initialized[128]; +static subsys_status_t sys_status[N_SYS_STATUS]; /** * Exit with a raw assertion if the subsystems list is inconsistent; @@ -48,8 +58,8 @@ check_and_setup(void) if (subsystem_array_validated) return; - raw_assert(ARRAY_LENGTH(sys_initialized) >= n_tor_subsystems); - memset(sys_initialized, 0, sizeof(sys_initialized)); + raw_assert(ARRAY_LENGTH(sys_status) >= n_tor_subsystems); + memset(sys_status, 0, sizeof(sys_status)); int last_level = MIN_SUBSYS_LEVEL; @@ -97,7 +107,7 @@ subsystems_init_upto(int target_level) continue; if (sys->level > target_level) break; - if (sys_initialized[i]) + if (sys_status[i].initialized) continue; int r = 0; if (sys->initialize) { @@ -112,7 +122,7 @@ subsystems_init_upto(int target_level) sys->name, i); raw_assert_unreached_msg("A subsystem couldn't be initialized."); } - sys_initialized[i] = true; + sys_status[i].initialized = true; } return 0; @@ -132,7 +142,7 @@ subsystems_add_pubsub_upto(pubsub_builder_t *builder, continue; if (sys->level > target_level) break; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; int r = 0; if (sys->add_pubsub) { @@ -186,13 +196,13 @@ subsystems_shutdown_downto(int target_level) continue; if (sys->level <= target_level) break; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->shutdown) { log_debug(LD_GENERAL, "Shutting down %s", sys->name); sys->shutdown(); } - sys_initialized[i] = false; + sys_status[i].initialized = false; } } @@ -208,7 +218,7 @@ subsystems_prefork(void) const subsys_fns_t *sys = tor_subsystems[i]; if (!sys->supported) continue; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->prefork) { log_debug(LD_GENERAL, "Pre-fork: %s", sys->name); @@ -229,7 +239,7 @@ subsystems_postfork(void) const subsys_fns_t *sys = tor_subsystems[i]; if (!sys->supported) continue; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->postfork) { log_debug(LD_GENERAL, "Post-fork: %s", sys->name); @@ -250,7 +260,7 @@ subsystems_thread_cleanup(void) const subsys_fns_t *sys = tor_subsystems[i]; if (!sys->supported) continue; - if (! sys_initialized[i]) + if (! sys_status[i].initialized) continue; if (sys->thread_cleanup) { log_debug(LD_GENERAL, "Thread cleanup: %s", sys->name);

1 0

[tor/master] Add the ability to fail when a hardware accelerator is missing.
by nickm＠torproject.org 07 Nov '19

07 Nov '19

commit 32a2f96f82eaadafd4faf2c6184093e441661a4e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Nov 6 11:12:09 2019 -0500 Add the ability to fail when a hardware accelerator is missing. Closes ticket 32406. --- changes/ticket32406 | 4 ++++ doc/tor.1.txt | 3 +++ src/lib/crypt_ops/crypto_openssl_mgt.c | 20 +++++++++++++++++--- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/changes/ticket32406 b/changes/ticket32406 new file mode 100644 index 000000000..c0c60207c --- /dev/null +++ b/changes/ticket32406 @@ -0,0 +1,4 @@ + o Minor features (configuration): + - If the configured hardware crypto accelerator in AccelName + is prefixed with "!", Tor now exits when it cannot be found. + Closes ticket 32406. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 5e3953e33..a3c05961e 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -843,6 +843,9 @@ GENERAL OPTIONS engine of this name. This must be used for any dynamic hardware engine. Names can be verified with the openssl engine command. Can not be changed while tor is running. + + + If the engine name is prefixed with a "!", then Tor will exit if the + engine cannot be loaded. [[AccelDir]] **AccelDir** __DIR__:: Specify this option if using dynamic hardware acceleration and the engine diff --git a/src/lib/crypt_ops/crypto_openssl_mgt.c b/src/lib/crypt_ops/crypto_openssl_mgt.c index 2fbebd87e..d7a3e0692 100644 --- a/src/lib/crypt_ops/crypto_openssl_mgt.c +++ b/src/lib/crypt_ops/crypto_openssl_mgt.c @@ -275,8 +275,14 @@ log_engine(const char *fn, ENGINE *e) } #endif /* !defined(DISABLE_ENGINES) */ -/** Initialize engines for openssl (if enabled). */ -static void +/** Initialize engines for openssl (if enabled). Load all the built-in + * engines, along with the one called accelName (which may be NULL). + * If accelName is prefixed with "!", then it is required: return -1 + * if it can't be loaded. Otherwise return 0. + * + * If accelDir is not NULL, it is the path from which the engine should + * be loaded. */ +static int crypto_openssl_init_engines(const char *accelName, const char *accelDir) { @@ -284,6 +290,7 @@ crypto_openssl_init_engines(const char *accelName, (void)accelName; (void)accelDir; log_warn(LD_CRYPTO, "No OpenSSL hardware acceleration support enabled."); + return 0; #else ENGINE *e = NULL; @@ -292,6 +299,9 @@ crypto_openssl_init_engines(const char *accelName, ENGINE_register_all_complete(); if (accelName) { + const bool required = accelName[0] == '!'; + if (required) + ++accelName; if (accelDir) { log_info(LD_CRYPTO, "Trying to load dynamic OpenSSL engine \"%s\"" " via path \"%s\".", accelName, accelDir); @@ -304,6 +314,8 @@ crypto_openssl_init_engines(const char *accelName, if (!e) { log_warn(LD_CRYPTO, "Unable to load dynamic OpenSSL engine \"%s\".", accelName); + if (required) + return -1; } else { log_info(LD_CRYPTO, "Loaded dynamic OpenSSL engine \"%s\".", accelName); @@ -340,6 +352,7 @@ crypto_openssl_init_engines(const char *accelName, #ifdef NID_aes_256_gcm log_engine("AES-256-GCM", ENGINE_get_cipher_engine(NID_aes_256_gcm)); #endif + return 0; #endif /* defined(DISABLE_ENGINES) */ } @@ -350,7 +363,8 @@ crypto_openssl_late_init(int useAccel, const char *accelName, const char *accelDir) { if (useAccel > 0) { - crypto_openssl_init_engines(accelName, accelDir); + if (crypto_openssl_init_engines(accelName, accelDir) < 0) + return -1; } else { log_info(LD_CRYPTO, "NOT using OpenSSL engine support."); }

1 0