From dca2629480f143ed8deb71b73b15c7c5ff890571 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 6 Jan 2020 09:02:49 -0500 Subject: [PATCH 1/3] Use tor_api.h entry points in ntmain.c. When we updated main.c to use the same entry points as tor_api.h, we didn't update ntmain.c. This led to bugs as the two got out of sync. There are two calls that we drop in this patch: first, I drop the call to set_main_thread(), since that's redundant with the init function in compat_winthreads.c (but see #32884). Second, I drop the check for CMD_RUN_TOR: I'll add that back with a subsequent commit. Closes ticket 32883. --- changes/bug32883 | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/bug32883 diff --git a/changes/bug32883 b/changes/bug32883 new file mode 100644 index 0000000000..6a15e0221b --- /dev/null +++ b/changes/bug32883 @@ -0,0 +1,5 @@ + o Code simplification and refactoring (windows services): + - The windows service logic now uses the tor_api.h entry points, to + avoid needless code duplication, and to prevent bugs related to + the different entry points getting out of sync. Closes ticket + 32883. From 5c240db0bf7751d74ba438a1ca4ef0d051a53df7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 6 Jan 2020 09:14:22 -0500 Subject: [PATCH 2/3] Restore feature where nt-services detect non-"run_tor" modes. Followup for #32883. --- src/app/main/main.c | 7 ++++ src/app/main/ntmain.c | 52 ++++++++++-------------------- src/feature/api/tor_api_internal.h | 5 +++ 3 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/app/main/main.c b/src/app/main/main.c index 4275a98bd0..2a5cb4aa07 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -1279,6 +1279,13 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) #endif } + if (tor_cfg->run_tor_only && get_options()->command != CMD_RUN_TOR) { + log_err(LD_CONFIG, "Unsupported command when running as an NT service."); + result = -1; + tor_cleanup(); + goto done; + } + switch (get_options()->command) { case CMD_RUN_TOR: #ifdef NT_SERVICE diff --git a/src/app/main/ntmain.c b/src/app/main/ntmain.c index a49dfdbbc9..3d923edf57 100644 --- a/src/app/main/ntmain.c +++ b/src/app/main/ntmain.c @@ -29,6 +29,8 @@ #include "lib/evloop/compat_libevent.h" #include "lib/fs/winlib.h" #include "lib/log/win32err.h" +#include "feature/api/tor_api.h" +#include "feature/api/tor_api_internal.h" #include #define GENSRV_SERVICENAME "tor" @@ -263,7 +265,6 @@ nt_service_control(DWORD request) static void nt_service_body(int argc, char **argv) { - int r; (void) argc; /* unused */ (void) argv; /* unused */ nt_service_loadlibrary(); @@ -283,24 +284,20 @@ nt_service_body(int argc, char **argv) return; } - r = tor_init(backup_argc, backup_argv); - if (r) { - /* Failed to start the Tor service */ - r = NT_SERVICE_ERROR_TORINIT_FAILED; - service_status.dwCurrentState = SERVICE_STOPPED; - service_status.dwWin32ExitCode = r; - service_status.dwServiceSpecificExitCode = r; - service_fns.SetServiceStatus_fn(hStatus, &service_status); + tor_main_configuration_t *cfg = tor_main_configuration_new(); + cfg->run_tor_only = 1; + if (tor_main_configuration_set_command_line(cfg, backup_argc, + backup_argv) < 0) return; - } /* Set the service's status to SERVICE_RUNNING and start the main * event loop */ service_status.dwCurrentState = SERVICE_RUNNING; service_fns.SetServiceStatus_fn(hStatus, &service_status); - set_main_thread(); - run_tor_main_loop(); - tor_cleanup(); + + tor_run_main(cfg); + + tor_main_configuration_free(cfg); } /** Main service entry point. Starts the service control dispatcher and waits @@ -323,29 +320,14 @@ 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)) + tor_main_configuration_t *cfg = tor_main_configuration_new(); + cfg->run_tor_only = 1; + if (tor_main_configuration_set_command_line(cfg, backup_argc, + backup_argv) < 0) return; - switch (get_options()->command) { - case CMD_RUN_TOR: - run_tor_main_loop(); - break; - case CMD_LIST_FINGERPRINT: - case CMD_HASH_PASSWORD: - case CMD_VERIFY_CONFIG: - case CMD_DUMP_CONFIG: - case CMD_KEYGEN: - case CMD_KEY_EXPIRATION: - log_err(LD_CONFIG, "Unsupported command (--list-fingerprint, " - "--hash-password, --keygen, --dump-config, --verify-config, " - "or --key-expiration) in NT service."); - break; - case CMD_RUN_UNITTESTS: - case CMD_IMMEDIATE: - default: - log_err(LD_CONFIG, "Illegal command number %d: internal error.", - get_options()->command); - } - tor_cleanup(); + + tor_run_main(cfg); + tor_main_configuration_free(cfg); } } } diff --git a/src/feature/api/tor_api_internal.h b/src/feature/api/tor_api_internal.h index 115d33d58e..40c64e1ab8 100644 --- a/src/feature/api/tor_api_internal.h +++ b/src/feature/api/tor_api_internal.h @@ -29,6 +29,11 @@ struct tor_main_configuration_t { /** Socket that Tor will use as an owning control socket. Owned. */ tor_socket_t owning_controller_socket; + + /** Disable commands other than "run tor". Not for use from outside Tor + * itself; if you need to use this for embedding, please contact the tor + * developers. */ + int run_tor_only; }; #endif /* !defined(TOR_API_INTERNAL_H) */ From b6f099672b90cb606d89bfcb0cae01ea49e46b9c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 6 Jan 2020 09:19:09 -0500 Subject: [PATCH 3/3] Make ntmain.h functions stubs when NT_SERVICE not enabled. This lets us simplify main.c a little, and avoid a practracker exception. Followup from #32883. --- src/app/main/main.c | 15 ++++----------- src/app/main/ntmain.h | 3 ++- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/app/main/main.c b/src/app/main/main.c index 2a5cb4aa07..9c60d3c0f5 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -1238,15 +1238,10 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) memcpy(argv + tor_cfg->argc, tor_cfg->argv_owned, tor_cfg->argc_owned*sizeof(char*)); -#ifdef NT_SERVICE - { - int done = 0; - result = nt_service_parse_options(argc, argv, &done); - if (done) { - goto done; - } - } -#endif /* defined(NT_SERVICE) */ + int done = 0; + result = nt_service_parse_options(argc, argv, &done); + if (done) + goto done; pubsub_install(); @@ -1288,9 +1283,7 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) switch (get_options()->command) { case CMD_RUN_TOR: -#ifdef NT_SERVICE nt_service_set_state(SERVICE_RUNNING); -#endif result = run_tor_main_loop(); break; case CMD_KEYGEN: diff --git a/src/app/main/ntmain.h b/src/app/main/ntmain.h index c39386c054..e23641bffa 100644 --- a/src/app/main/ntmain.h +++ b/src/app/main/ntmain.h @@ -22,7 +22,8 @@ int nt_service_is_stopping(void); void nt_service_set_state(DWORD state); #else #define nt_service_is_stopping() 0 +#define nt_service_parse_options(a, b, c) (0) +#define nt_service_set_state(s) STMT_NIL #endif /* defined(NT_SERVICE) */ #endif /* !defined(TOR_NTMAIN_H) */ -