tor-commits October 2017

tor-commits@lists.torproject.org

17 participants
2004 discussions

[tor/master] Return from instead of exit()ing when ed25519 key check fails.
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit 853e73e815c1be9c9e533160f803de56e7d21147 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 20 11:03:53 2017 -0400 Return from instead of exit()ing when ed25519 key check fails. --- src/or/main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 8bfea7895..d47c8f4dd 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1609,8 +1609,7 @@ check_ed_keys_callback(time_t now, const or_options_t *options) if (new_signing_key < 0 || generate_ed_link_cert(options, now, new_signing_key > 0)) { log_err(LD_OR, "Unable to update Ed25519 keys! Exiting."); - tor_cleanup(); - exit(1); // XXXX bad exit + tor_shutdown_event_loop_and_exit(1); } } return 30;

1 0

[tor/master] Merge remote-tracking branch 'public/exit_carefully'
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit 30a681553f2ecc7747236085dd3331e9875651d5 Merge: fa78546db a62f59f00 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 27 11:13:05 2017 -0400 Merge remote-tracking branch 'public/exit_carefully' changes/bug23848 | 8 +++ src/common/compat_libevent.h | 1 + src/or/config.c | 67 +++++++++++++++++------- src/or/connection_edge.c | 2 +- src/or/control.c | 3 +- src/or/hibernate.c | 7 ++- src/or/main.c | 119 ++++++++++++++++++++++++++++++++++++------- src/or/main.h | 4 +- src/or/ntmain.c | 2 +- src/test/test_keygen.sh | 109 +++++++++++++++++++++++++++++++++++++++ 10 files changed, 278 insertions(+), 44 deletions(-) diff --cc src/or/config.c index 63a1cac99,64e42c93c..82c35e952 --- a/src/or/config.c +++ b/src/or/config.c @@@ -1684,13 -1667,11 +1687,16 @@@ options_act(const or_options_t *old_opt return -1; } + if (options->ProtocolWarnings) + protocol_warning_severity_level = LOG_WARN; + else + protocol_warning_severity_level = LOG_INFO; + - if (consider_adding_dir_servers(options, old_options) < 0) + if (consider_adding_dir_servers(options, old_options) < 0) { + // XXXX This should get validated earlier, and committed here, to + // XXXX lower opportunities for reaching an error case. return -1; + } if (rend_non_anonymous_mode_enabled(options)) { log_warn(LD_GENERAL, "This copy of Tor was compiled or configured to run "

1 0

[tor/master] Add a comment about removing the failsafe event loop exit code
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit 79e7c0af0e4bd9a813ddd9226b43cc49112edd19 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 27 11:15:43 2017 -0400 Add a comment about removing the failsafe event loop exit code --- src/or/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/or/main.c b/src/or/main.c index a615a6be0..bb5935690 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -691,6 +691,7 @@ tor_shutdown_event_loop_and_exit(int exitcode) /* Die with an assertion failure in ten seconds, if for some reason we don't * exit normally. */ + /* XXXX We should consider this code if it's never used. */ struct timeval ten_seconds = { 10, 0 }; event_base_once(tor_libevent_get_base(), -1, EV_TIMEOUT, shutdown_did_not_work_callback, NULL,

1 0

[tor/master] Add unit tests for cases of starting with bogus keygen arguments
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit c4a07b261b552c39b64c8d47f1ba75f4bb4f37d6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 20 19:10:05 2017 -0400 Add unit tests for cases of starting with bogus keygen arguments In particular, this tests that we give an appropriate warning when we are told to use some keygen argument, but --keygen is not specified. --- src/test/test_keygen.sh | 109 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/src/test/test_keygen.sh b/src/test/test_keygen.sh index 87012cd28..b3d4d8e39 100755 --- a/src/test/test_keygen.sh +++ b/src/test/test_keygen.sh @@ -40,6 +40,12 @@ fi CASE8=$dflt CASE9=$dflt CASE10=$dflt + CASE11A=$dflt + CASE11B=$dflt + CASE11C=$dflt + CASE11D=$dflt + CASE11E=$dflt + CASE11F=$dflt if [ $# -ge 1 ]; then eval "CASE${1}"=1 @@ -363,6 +369,109 @@ echo "==== Case 10 ok" fi +# Case 11a: -passphrase-fd without --keygen + +if [ "$CASE11A" = 1 ]; then + +ME="${DATA_DIR}/case11a" + +mkdir -p "${ME}/keys" + +${TOR} --DataDirectory "${ME}" --passphrase-fd 1 > "${ME}/stdout" && die "Successfully started with passphrase-fd but no keygen?" || true + +grep "passphrase-fd specified without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments." + +echo "==== Case 11A ok" + +fi + +# Case 11b: --no-passphrase without --keygen + +if [ "$CASE11B" = 1 ]; then + +ME="${DATA_DIR}/case11b" + +mkdir -p "${ME}/keys" + +${TOR} --DataDirectory "${ME}" --no-passphrase > "${ME}/stdout" && die "Successfully started with no-passphrase but no keygen?" || true + +grep "no-passphrase specified without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments." + +echo "==== Case 11B ok" + +fi + +# Case 11c: --newpass without --keygen + +if [ "$CASE11C" = 1 ]; then + +ME="${DATA_DIR}/case11C" + +mkdir -p "${ME}/keys" + +${TOR} --DataDirectory "${ME}" --newpass > "${ME}/stdout" && die "Successfully started with newpass but no keygen?" || true + +grep "newpass specified without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments." + +echo "==== Case 11C ok" + +fi + +######## --master-key does not work yet, but this will test the error case +######## when it does. +# +# Case 11d: --master-key without --keygen +# +if [ "$CASE11D" = 1 ]; then +# +# ME="${DATA_DIR}/case11d" +# +# mkdir -p "${ME}/keys" +# +# ${TOR} --DataDirectory "${ME}" --master-key "${ME}/foobar" > "${ME}/stdout" && die "Successfully started with master-key but no keygen?" || true +# +# cat "${ME}/stdout" +# +# grep "master-key without --keygen" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments." + + echo "==== Case 11D skipped" + +fi + + +# Case 11E: Silly passphrase-fd + +if [ "$CASE11E" = 1 ]; then + +ME="${DATA_DIR}/case11E" + +mkdir -p "${ME}/keys" + +${TOR} --DataDirectory "${ME}" --keygen --passphrase-fd ewigeblumenkraft > "${ME}/stdout" && die "Successfully started with bogus passphrase-fd?" || true + +grep "Invalid --passphrase-fd value" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments." + +echo "==== Case 11E ok" + +fi + + +# Case 11F: --no-passphrase with --passphrase-fd + +if [ "$CASE11F" = 1 ]; then + +ME="${DATA_DIR}/case11F" + +mkdir -p "${ME}/keys" + +${TOR} --DataDirectory "${ME}" --keygen --passphrase-fd 1 --no-passphrase > "${ME}/stdout" && die "Successfully started with bogus passphrase-fd combination?" || true + +grep "no-passphrase specified with --passphrase-fd" "${ME}/stdout" >/dev/null || die "Tor didn't declare that there was a problem with the arguments." + +echo "==== Case 11F ok" + +fi + # Check cert-only.

1 0

[tor/master] Mark "previously validated foo could not be set" blocks as unreachable.
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit a62f59f000f05ac5f5a7a22387007a1b8c2ee2a4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 20 19:28:12 2017 -0400 Mark "previously validated foo could not be set" blocks as unreachable. --- src/or/config.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index 6f7bf23df..64e42c93c 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1717,9 +1717,11 @@ options_act(const or_options_t *old_options) for (cl = options->Bridges; cl; cl = cl->next) { bridge_line_t *bridge_line = parse_bridge_line(cl->value); if (!bridge_line) { + // LCOV_EXCL_START log_warn(LD_BUG, "Previously validated Bridge line could not be added!"); return -1; + // LCOV_EXCL_STOP } bridge_add_from_config(bridge_line); } @@ -1727,15 +1729,19 @@ options_act(const or_options_t *old_options) } if (running_tor && hs_config_service_all(options, 0)<0) { + // LCOV_EXCL_START log_warn(LD_BUG, "Previously validated hidden services line could not be added!"); return -1; + // LCOV_EXCL_STOP } if (running_tor && rend_parse_service_authorization(options, 0) < 0) { + // LCOV_EXCL_START log_warn(LD_BUG, "Previously validated client authorization for " "hidden services could not be added!"); return -1; + // LCOV_EXCL_STOP } /* Load state */ @@ -1758,10 +1764,12 @@ options_act(const or_options_t *old_options) if (options->ClientTransportPlugin) { for (cl = options->ClientTransportPlugin; cl; cl = cl->next) { if (parse_transport_line(options, cl->value, 0, 0) < 0) { + // LCOV_EXCL_START log_warn(LD_BUG, "Previously validated ClientTransportPlugin line " "could not be added!"); return -1; + // LCOV_EXCL_STOP } } } @@ -1769,10 +1777,12 @@ options_act(const or_options_t *old_options) if (options->ServerTransportPlugin && server_mode(options)) { for (cl = options->ServerTransportPlugin; cl; cl = cl->next) { if (parse_transport_line(options, cl->value, 0, 1) < 0) { + // LCOV_EXCL_START log_warn(LD_BUG, "Previously validated ServerTransportPlugin line " "could not be added!"); return -1; + // LCOV_EXCL_STOP } } } @@ -1858,8 +1868,10 @@ options_act(const or_options_t *old_options) /* Set up accounting */ if (accounting_parse_options(options, 0)<0) { + // LCOV_EXCL_START log_warn(LD_BUG,"Error in previously validated accounting options"); return -1; + // LCOV_EXCL_STOP } if (accounting_is_enabled(options)) configure_accounting(time(NULL)); @@ -1895,10 +1907,12 @@ options_act(const or_options_t *old_options) } if (parse_outbound_addresses(options, 0, &msg) < 0) { + // LCOV_EXCL_START log_warn(LD_BUG, "Failed parsing previously validated outbound " "bind addresses: %s", msg); tor_free(msg); return -1; + // LCOV_EXCL_STOP } config_maybe_load_geoip_files_(options, old_options);

1 0

[tor/master] Return instead of exiting in options_init_from_torrc()
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit 1df43aff41eb8198712af0aa14ec2d574b26682c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 20 11:15:32 2017 -0400 Return instead of exiting in options_init_from_torrc() --- src/or/config.c | 31 +++++++++++++++++++------------ src/or/control.c | 3 +-- src/or/main.c | 29 ++++++++++++++++++++++++----- src/or/ntmain.c | 2 +- 4 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 55ab76c81..3795aeaa3 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -4981,7 +4981,8 @@ load_torrc_from_disk(config_line_t *cmd_arg, int defaults_file) /** Read a configuration file into <b>options</b>, finding the configuration * file location based on the command line. After loading the file * call options_init_from_string() to load the config. - * Return 0 if success, -1 if failure. */ + * Return 0 if success, -1 if failure, and 1 if we succeeded but should exit + * anyway. */ int options_init_from_torrc(int argc, char **argv) { @@ -5008,22 +5009,22 @@ options_init_from_torrc(int argc, char **argv) if (config_line_find(cmdline_only_options, "-h") || config_line_find(cmdline_only_options, "--help")) { print_usage(); - exit(0); // XXXX bad exit, though probably harmless + return 1; } if (config_line_find(cmdline_only_options, "--list-torrc-options")) { /* For validating whether we've documented everything. */ list_torrc_options(); - exit(0); // XXXX bad exit, though probably harmless + return 1; } if (config_line_find(cmdline_only_options, "--list-deprecated-options")) { /* For validating whether what we have deprecated really exists. */ list_deprecated_options(); - exit(0); // XXXX bad exit, though probably harmless + return 1; } if (config_line_find(cmdline_only_options, "--version")) { printf("Tor version %s.\n",get_version()); - exit(0); // XXXX bad exit, though probably harmless + return 1; } if (config_line_find(cmdline_only_options, "--library-versions")) { @@ -5051,7 +5052,7 @@ options_init_from_torrc(int argc, char **argv) tor_compress_header_version_str(ZSTD_METHOD)); } //TODO: Hex versions? - exit(0); // XXXX bad exit, though probably harmless + return 1; } command = CMD_RUN_TOR; @@ -5112,7 +5113,8 @@ options_init_from_torrc(int argc, char **argv) get_options_mutable()->keygen_force_passphrase = FORCE_PASSPHRASE_OFF; } else { log_err(LD_CONFIG, "--no-passphrase specified without --keygen!"); - exit(1); // XXXX bad exit + retval = -1; + goto err; } } @@ -5121,7 +5123,8 @@ options_init_from_torrc(int argc, char **argv) get_options_mutable()->change_key_passphrase = 1; } else { log_err(LD_CONFIG, "--newpass specified without --keygen!"); - exit(1); // XXXX bad exit + retval = -1; + goto err; } } @@ -5131,17 +5134,20 @@ options_init_from_torrc(int argc, char **argv) if (fd_line) { if (get_options()->keygen_force_passphrase == FORCE_PASSPHRASE_OFF) { log_err(LD_CONFIG, "--no-passphrase specified with --passphrase-fd!"); - exit(1); // XXXX bad exit + retval = -1; + goto err; } else if (command != CMD_KEYGEN) { log_err(LD_CONFIG, "--passphrase-fd specified without --keygen!"); - exit(1); // XXXX bad exit + retval = -1; + goto err; } else { const char *v = fd_line->value; int ok = 1; long fd = tor_parse_long(v, 10, 0, INT_MAX, &ok, NULL); if (fd < 0 || ok == 0) { log_err(LD_CONFIG, "Invalid --passphrase-fd value %s", escaped(v)); - exit(1); // XXXX bad exit + retval = -1; + goto err; } get_options_mutable()->keygen_passphrase_fd = (int)fd; get_options_mutable()->use_keygen_passphrase_fd = 1; @@ -5156,7 +5162,8 @@ options_init_from_torrc(int argc, char **argv) if (key_line) { if (command != CMD_KEYGEN) { log_err(LD_CONFIG, "--master-key without --keygen!"); - exit(1); // XXXX bad exit + retval = -1; + goto err; } else { get_options_mutable()->master_key_fname = tor_strdup(key_line->value); } diff --git a/src/or/control.c b/src/or/control.c index ab164700e..7b89a1370 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -6571,8 +6571,7 @@ monitor_owning_controller_process(const char *process_spec) "owning controller: %s. Exiting.", msg); owning_controller_process_spec = NULL; - tor_cleanup(); - exit(1); // XXXX bad exit: or questionable, at least. + tor_shutdown_event_loop_and_exit(1); } } diff --git a/src/or/main.c b/src/or/main.c index d47c8f4dd..9121e5216 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2420,10 +2420,18 @@ do_hup(void) /* first, reload config variables, in case they've changed */ if (options->ReloadTorrcOnSIGHUP) { /* no need to provide argc/v, they've been cached in init_from_config */ - if (options_init_from_torrc(0, NULL) < 0) { + int init_rv = options_init_from_torrc(0, NULL); + if (init_rv < 0) { log_err(LD_CONFIG,"Reading config failed--see warnings above. " "For usage, try -h."); return -1; + } else if (BUG(init_rv > 0)) { + // LCOV_EXCL_START + /* This should be impossible: the only "return 1" cases in + * options_init_from_torrc are ones caused by command-line arguments; + * but they can't change while Tor is running. */ + return -1; + // LCOV_EXCL_STOP } options = get_options(); /* they have changed now */ /* Logs are only truncated the first time they are opened, but were @@ -3085,7 +3093,8 @@ activate_signal(int signal_num) } } -/** Main entry point for the Tor command-line client. +/** Main entry point for the Tor command-line client. Return 0 on "success", + * negative on "failure", and positive on "success and exit". */ int tor_init(int argc, char *argv[]) @@ -3192,9 +3201,14 @@ tor_init(int argc, char *argv[]) } atexit(exit_function); - if (options_init_from_torrc(argc,argv) < 0) { + int init_rv = options_init_from_torrc(argc,argv); + if (init_rv < 0) { log_err(LD_CONFIG,"Reading config failed--see warnings above."); return -1; + } else if (init_rv > 0) { + // We succeeded, and should exit anyway -- probably the user just said + // "--version" or something like that. + return 1; } /* The options are now initialised */ @@ -3820,8 +3834,13 @@ tor_main(int argc, char *argv[]) if (done) return result; } #endif /* defined(NT_SERVICE) */ - if (tor_init(argc, argv)<0) - return -1; + { + int init_rv = tor_init(argc, argv); + if (init_rv < 0) + return -1; + else if (init_rv > 0) + return 0; + } if (get_options()->Sandbox && get_options()->command == CMD_RUN_TOR) { sandbox_cfg_t* cfg = sandbox_init_filter(); diff --git a/src/or/ntmain.c b/src/or/ntmain.c index 9ce03e1f5..ebbe0018b 100644 --- a/src/or/ntmain.c +++ b/src/or/ntmain.c @@ -318,7 +318,7 @@ nt_service_main(void) printf("Service error %d : %s\n", (int) result, errmsg); tor_free(errmsg); if (result == ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) { - if (tor_init(backup_argc, backup_argv) < 0) + if (tor_init(backup_argc, backup_argv)) return; switch (get_options()->command) { case CMD_RUN_TOR:

1 0

[tor/master] Clarify doc on tell_event_loop_to_run_external_code()
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit 0a10335d2e23e43de401cf955c8f188ce19106a6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 27 11:14:37 2017 -0400 Clarify doc on tell_event_loop_to_run_external_code() --- src/or/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/or/main.c b/src/or/main.c index 5510df253..a615a6be0 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -648,7 +648,8 @@ connection_should_read_from_linked_conn(connection_t *conn) /** If we called event_base_loop() and told it to never stop until it * runs out of events, now we've changed our mind: tell it we want it to - * finish. */ + * exit once the current round of callbacks is done, so that we can + * run external code, and then return to the main loop. */ void tell_event_loop_to_run_external_code(void) {

1 0

[tor/master] Make sure all C files have copyright/license notices
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit fa78546dbc0e62fc1b24daef881c460412f04e7e Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 27 10:59:36 2017 -0400 Make sure all C files have copyright/license notices --- src/common/crypto_pwbox.h | 3 +++ src/test/test-memwipe.c | 3 +++ src/test/test_accounting.c | 3 +++ src/test/test_channelpadding.c | 3 +++ src/test/test_dns.c | 3 +++ src/test/test_routerset.c | 3 +++ src/test/test_shared_random.c | 3 +++ src/test/test_status.c | 3 +++ 8 files changed, 24 insertions(+) diff --git a/src/common/crypto_pwbox.h b/src/common/crypto_pwbox.h index cee865358..a26b6d2c1 100644 --- a/src/common/crypto_pwbox.h +++ b/src/common/crypto_pwbox.h @@ -1,3 +1,6 @@ +/* Copyright (c) 2014-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #ifndef CRYPTO_PWBOX_H_INCLUDED_ #define CRYPTO_PWBOX_H_INCLUDED_ diff --git a/src/test/test-memwipe.c b/src/test/test-memwipe.c index 484f13dd0..89d946d50 100644 --- a/src/test/test-memwipe.c +++ b/src/test/test-memwipe.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2015-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #include "orconfig.h" #include <string.h> #include <stdio.h> diff --git a/src/test/test_accounting.c b/src/test/test_accounting.c index 7edba988a..b0d37b298 100644 --- a/src/test/test_accounting.c +++ b/src/test/test_accounting.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2014-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #include "or.h" #include "test.h" #define HIBERNATE_PRIVATE diff --git a/src/test/test_channelpadding.c b/src/test/test_channelpadding.c index d54c9cc52..fe21057c8 100644 --- a/src/test/test_channelpadding.c +++ b/src/test/test_channelpadding.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2016-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #define TOR_CHANNEL_INTERNAL_ #define MAIN_PRIVATE #define NETWORKSTATUS_PRIVATE diff --git a/src/test/test_dns.c b/src/test/test_dns.c index 19dcb0293..66b231b6d 100644 --- a/src/test/test_dns.c +++ b/src/test/test_dns.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2015-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #include "or.h" #include "test.h" diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c index c9c69911d..6da2275c3 100644 --- a/src/test/test_routerset.c +++ b/src/test/test_routerset.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2014-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #define ROUTERSET_PRIVATE #include "or.h" diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index a9d58e6b8..80494f64c 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2016-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #define SHARED_RANDOM_PRIVATE #define SHARED_RANDOM_STATE_PRIVATE #define CONFIG_PRIVATE diff --git a/src/test/test_status.c b/src/test/test_status.c index f86f8e3b9..50ea203e4 100644 --- a/src/test/test_status.c +++ b/src/test/test_status.c @@ -1,3 +1,6 @@ +/* Copyright (c) 2014-2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + #define STATUS_PRIVATE #define HIBERNATE_PRIVATE #define LOG_PRIVATE

1 0

[tor/master] remove unneeded dependencies, remove types where unnecessary
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit d14a83f74fcbe5fae97d944ac8f7f8645e160d3a Author: Chelsea Holland Komlo <me(a)chelseakomlo.com> Date: Sat Oct 14 22:27:36 2017 -0400 remove unneeded dependencies, remove types where unnecessary --- src/rust/protover/protover.rs | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index f85d16b73..37589a83f 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -1,10 +1,8 @@ use external::c_tor_version_as_new_as; use std::str::FromStr; -use std::str::SplitN; use std::fmt; -use std::collections::HashMap; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::string::String; /// The first version of Tor that included "proto" entries in its descriptors. @@ -191,14 +189,14 @@ fn get_versions(version_string: &str) -> Result<HashSet<u32>, &'static str> { fn get_proto_and_vers<'a>( protocol_entry: &'a str, ) -> Result<(Proto, HashSet<u32>), &'static str> { - let mut parts: SplitN<'a, &str> = protocol_entry.splitn(2, "="); + let mut parts = protocol_entry.splitn(2, "="); - let proto: &str = match parts.next() { + let proto = match parts.next() { Some(n) => n, None => return Err("invalid protover entry"), }; - let vers: &str = match parts.next() { + let vers = match parts.next() { Some(n) => n, None => return Err("invalid protover entry"), }; @@ -272,7 +270,7 @@ fn contains_only_supported_protocols(proto_entry: &str) -> bool { /// assert_eq!("Link=5-6", unsupported); /// pub fn all_supported(protocols: &str) -> (bool, String) { - let unsupported: Vec<&str> = protocols + let unsupported = protocols .split_whitespace() .filter(|v| !contains_only_supported_protocols(v)) .collect::<Vec<&str>>(); @@ -348,19 +346,19 @@ fn expand_version_range(range: &str) -> Result<Vec<u32>, &'static str> { let mut parts = range.split("-"); - let lower_string: &str = parts.next().ok_or( + let lower_string = parts.next().ok_or( "cannot parse protocol range lower bound", )?; - let lower: u32 = u32::from_str_radix(lower_string, 10).or(Err( + let lower = u32::from_str_radix(lower_string, 10).or(Err( "cannot parse protocol range lower bound", ))?; - let higher_string: &str = parts.next().ok_or( + let higher_string = parts.next().ok_or( "cannot parse protocol range upper bound", )?; - let higher: u32 = u32::from_str_radix(higher_string, 10).or(Err( + let higher = u32::from_str_radix(higher_string, 10).or(Err( "cannot parse protocol range upper bound", ))?; @@ -391,7 +389,7 @@ fn find_range(list: &Vec<u32>) -> (bool, u32) { } let mut iterable = list.iter().peekable(); - let mut range_end: u32 = match iterable.next() { + let mut range_end = match iterable.next() { Some(n) => *n, None => return (false, 0), }; @@ -478,20 +476,20 @@ fn contract_protocol_list<'a>(supported_set: &'a HashSet<u32>) -> String { fn parse_protocols_from_string_with_no_validation<'a>( protocol_string: &'a str, ) -> Result<HashMap<String, HashSet<u32>>, &'static str> { - let protocols: &[&'a str] = - &protocol_string.split(" ").collect::<Vec<&'a str>>()[..]; + let protocols = &protocol_string.split(" ") + .collect::<Vec<&'a str>>()[..]; let mut parsed: HashMap<String, HashSet<u32>> = HashMap::new(); for subproto in protocols { - let mut parts: SplitN<'a, &str> = subproto.splitn(2, "="); + let mut parts = subproto.splitn(2, "="); - let name: &str = match parts.next() { + let name = match parts.next() { Some(n) => n, None => return Err("invalid protover entry"), }; - let vers: &str = match parts.next() { + let vers = match parts.next() { Some(n) => n, None => return Err("invalid protover entry"), };

1 0

[tor/master] add tor allocator for rust
by nickm＠torproject.org 27 Oct '17

27 Oct '17

commit 76bbdfbfa9eca46b53d3ec5a44deafce51d2875a Author: Chelsea Holland Komlo <me(a)chelseakomlo.com> Date: Sat Oct 14 23:05:31 2017 -0400 add tor allocator for rust --- src/rust/protover/ffi.rs | 20 ++++---- src/rust/tor_allocate/Cargo.toml | 13 +++++ src/rust/tor_allocate/include.am | 13 +++++ src/rust/tor_allocate/lib.rs | 12 +++++ src/rust/tor_allocate/tor_allocate.rs | 90 +++++++++++++++++++++++++++++++++++ 5 files changed, 138 insertions(+), 10 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 539b6c0ba..23a289bd5 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -8,7 +8,7 @@ use std::ffi::CString; use protover::*; use smartlist::*; -use tor_allocate::allocate_string; +use tor_allocate::allocate_and_copy_string; /// Translate C enums to Rust Proto enums, using the integer value of the C /// enum to map to its associated Rust enum @@ -124,17 +124,17 @@ pub extern "C" fn protover_compute_vote( ) -> *mut c_char { if list.is_null() { - let mut empty = String::new(); - return allocate_string(&mut empty); + let empty = String::new(); + return allocate_and_copy_string(&empty); } // Dereference of raw pointer requires an unsafe block. The pointer is // checked above to ensure it is not null. let data: Vec<String> = unsafe { (*list).get_list() }; - let mut vote = compute_vote(data, threshold); + let vote = compute_vote(data, threshold); - allocate_string(&mut vote) + allocate_and_copy_string(&vote) } /// Provide an interface for C to translate arguments and return types for @@ -162,10 +162,10 @@ pub extern "C" fn protover_compute_for_old_tor( ) -> *mut c_char { // Not handling errors when unwrapping as the content is controlled // and is an empty string - let mut empty = String::new(); + let empty = String::new(); if version.is_null() { - return allocate_string(&mut empty); + return allocate_and_copy_string(&empty); } // Require an unsafe block to read the version from a C string. The pointer @@ -174,10 +174,10 @@ pub extern "C" fn protover_compute_for_old_tor( let version = match c_str.to_str() { Ok(n) => n, - Err(_) => return allocate_string(&mut empty), + Err(_) => return allocate_and_copy_string(&empty), }; - let mut supported = compute_for_old_tor(&version); + let supported = compute_for_old_tor(&version); - allocate_string(&mut supported) + allocate_and_copy_string(&supported) } diff --git a/src/rust/tor_allocate/Cargo.toml b/src/rust/tor_allocate/Cargo.toml new file mode 100644 index 000000000..ceb08b78a --- /dev/null +++ b/src/rust/tor_allocate/Cargo.toml @@ -0,0 +1,13 @@ +[package] +authors = ["The Tor Project"] +version = "0.0.1" +name = "tor_allocate" + +[dependencies] +libc = "0.2.22" + +[lib] +name = "tor_allocate" +path = "lib.rs" +crate_type = ["rlib", "staticlib"] + diff --git a/src/rust/tor_allocate/include.am b/src/rust/tor_allocate/include.am new file mode 100644 index 000000000..9e770dbc0 --- /dev/null +++ b/src/rust/tor_allocate/include.am @@ -0,0 +1,13 @@ +EXTRA_DIST +=\ + src/rust/tor_allocate/Cargo.toml \ + src/rust/tor_allocate/lib.rs \ + src/rust/tor_allocate/tor_allocate.rs + +src/rust/target/release/@TOR_RUST_C_STRING_STATIC_NAME@: FORCE + ( cd "$(abs_top_srcdir)/src/rust/tor_allocate" ; \ + CARGO_TARGET_DIR="$(abs_top_builddir)/src/rust/target" \ + CARGO_HOME="$(abs_top_builddir)/src/rust" \ + $(CARGO) build --release --quiet $(CARGO_ONLINE) ) + +FORCE: + diff --git a/src/rust/tor_allocate/lib.rs b/src/rust/tor_allocate/lib.rs new file mode 100644 index 000000000..81afd095f --- /dev/null +++ b/src/rust/tor_allocate/lib.rs @@ -0,0 +1,12 @@ +//! Allocation helper functions that allow data to be allocated in Rust +//! using tor's specified allocator. In doing so, this can be later freed +//! from C. +//! +//! This is currently a temporary solution, we will later use tor's allocator +//! by default for any allocation that occurs in Rust. However, as this will +//! stabalize in 2018, we can use this as a temporary measure. + +extern crate libc; + +mod tor_allocate; +pub use tor_allocate::*; diff --git a/src/rust/tor_allocate/tor_allocate.rs b/src/rust/tor_allocate/tor_allocate.rs new file mode 100644 index 000000000..de1d13942 --- /dev/null +++ b/src/rust/tor_allocate/tor_allocate.rs @@ -0,0 +1,90 @@ +use libc::{c_char, c_void}; +use std::{ptr, slice}; + +#[cfg(not(test))] +extern "C" { + fn tor_malloc_ ( size: usize) -> *mut c_void; +} + +// Defined only for tests, used for testing purposes, so that we don't need +// to link to tor C files. Uses the system allocator +#[cfg(test)] +extern "C" fn tor_malloc_ ( size: usize) -> *mut c_void { + use libc::malloc; + unsafe { malloc(size) } +} + +/// Allocate memory using tor_malloc_ and copy an existing string into the +/// allocated buffer, returning a pointer that can later be called in C. +/// +/// # Inputs +/// +/// * `src`, a reference to a String that will be copied. +/// +/// # Returns +/// +/// A `String` that should be freed by tor_free in C +/// +pub fn allocate_and_copy_string(src: &String) -> *mut c_char { + let bytes = s.as_bytes(); + + let size = s.len(); + let size_with_null_byte = size + 1; + + let dest = unsafe { tor_malloc_(size_with_null_byte) as *mut u8 }; + + if dest.is_null() { + return dest as *mut c_char; + } + + unsafe { ptr::copy_nonoverlapping(bytes.as_ptr(), dest, size) }; + + // set the last byte as null, using the ability to index into a slice + // rather than doing pointer arithmatic + let slice = unsafe { slice::from_raw_parts_mut(dest, size_with_null_byte) }; + slice[size] = 0; // add a null terminator + + dest as *mut c_char +} + +#[cfg(test)] +mod test { + + #[test] + fn test_allocate_and_copy_string_with_empty() { + use std::ffi::CStr; + use libc::{free, c_void}; + + use tor_allocate::allocate_and_copy_string; + + let empty = String::new(); + let allocated_empty = allocate_and_copy_string(&empty); + + let allocated_empty_rust = unsafe { + CStr::from_ptr(allocated_empty).to_str().unwrap() + }; + + assert_eq!("", allocated_empty_rust); + + unsafe { free(allocated_empty as *mut c_void) }; + } + + #[test] + fn test_allocate_and_copy_string_with_not_empty_string() { + use std::ffi::CStr; + use libc::{free, c_void}; + + use tor_allocate::allocate_and_copy_string; + + let empty = String::from("foo bar biz"); + let allocated_empty = allocate_and_copy_string(&empty); + + let allocated_empty_rust = unsafe { + CStr::from_ptr(allocated_empty).to_str().unwrap() + }; + + assert_eq!("foo bar biz", allocated_empty_rust); + + unsafe { free(allocated_empty as *mut c_void) }; + } +}

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits October 2017