commit 9a69c24150965e54322ed9616638d4f1939b1289 Author: Nick Mathewson <nickm@torproject.org> Date: Sat Mar 31 22:51:28 2012 -0400 Do not use strcmp() to compare an http authenticator to its expected value This fixes a side-channel attack on the (fortunately unused!) BridgePassword option for bridge authorities. Fix for bug 5543; bugfix on 0.2.0.14-alpha. --- changes/bridgepassword | 11 +++++++++++ src/or/config.c | 19 +++++++++++++++++++ src/or/directory.c | 11 ++++++----- src/or/or.h | 7 ++++--- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/changes/bridgepassword b/changes/bridgepassword new file mode 100644 index 0000000..5f0e250 --- /dev/null +++ b/changes/bridgepassword @@ -0,0 +1,11 @@ + o Security fixes: + - When using the debuging BridgePassword field, a bridge authority + now compares alleged passwords by hashing them, then comparing + the result to a digest of the expected authenticator. This avoids + a potential side-channel attack in the previous code, which + had foolishly used strcmp(). Fortunately, the BridgePassword field + *is not in use*, but if it had been, the timing + behavior of strcmp() might have allowed an adversary to guess the + BridgePassword value, and enumerate the bridges. Bugfix on + 0.2.0.14-alpha. Fixes bug 5543. + diff --git a/src/or/config.c b/src/or/config.c index c3e285d..1e7bd58 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -713,6 +713,7 @@ or_options_free(or_options_t *options) return; routerset_free(options->_ExcludeExitNodesUnion); + tor_free(options->BridgePassword_AuthDigest); config_free(&options_format, options); } @@ -1298,6 +1299,24 @@ options_act(or_options_t *old_options) /* Change the cell EWMA settings */ cell_ewma_set_scale_factor(options, networkstatus_get_latest_consensus()); + /* Update the BridgePassword's hashed version as needed. We store this as a + * digest so that we can do side-channel-proof comparisons on it. + */ + if (options->BridgePassword) { + char *http_authenticator; + http_authenticator = alloc_http_authenticator(options->BridgePassword); + if (!http_authenticator) { + log_warn(LD_BUG, "Unable to allocate HTTP authenticator. Not setting " + "BridgePassword."); + return -1; + } + options->BridgePassword_AuthDigest = tor_malloc(DIGEST256_LEN); + crypto_digest256(options->BridgePassword_AuthDigest, + http_authenticator, strlen(http_authenticator), + DIGEST_SHA256); + tor_free(http_authenticator); + } + /* Check for transitions that need action. */ if (old_options) { int revise_trackexithosts = 0; diff --git a/src/or/directory.c b/src/or/directory.c index e3cc70f..9bc58e5 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3069,22 +3069,23 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, } if (options->BridgeAuthoritativeDir && - options->BridgePassword && + options->BridgePassword_AuthDigest && connection_dir_is_encrypted(conn) && !strcmp(url,"/tor/networkstatus-bridges")) { char *status; - char *secret = alloc_http_authenticator(options->BridgePassword); + char digest[DIGEST256_LEN]; header = http_get_header(headers, "Authorization: Basic "); + if (header) + crypto_digest256(digest, header, strlen(header), DIGEST_SHA256); /* now make sure the password is there and right */ - if (!header || strcmp(header, secret)) { + if (!header || + tor_memneq(digest, options->BridgePassword_AuthDigest, DIGEST256_LEN)) { write_http_status_line(conn, 404, "Not found"); - tor_free(secret); tor_free(header); goto done; } - tor_free(secret); tor_free(header); /* all happy now. send an answer. */ diff --git a/src/or/or.h b/src/or/or.h index eecd375..92592e5 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2489,10 +2489,11 @@ typedef struct { * that aggregates bridge descriptors? */ /** If set on a bridge authority, it will answer requests on its dirport - * for bridge statuses -- but only if the requests use this password. - * If set on a bridge user, request bridge statuses, and use this password - * when doing so. */ + * for bridge statuses -- but only if the requests use this password. */ char *BridgePassword; + /** If BridgePassword is set, this is a SHA256 digest of the basic http + * authenticator for it. */ + char *BridgePassword_AuthDigest; int UseBridges; /**< Boolean: should we start all circuits with a bridge? */ config_line_t *Bridges; /**< List of bootstrap bridge addresses. */