diff options
| -rw-r--r-- | changes/bridgepassword | 11 | ||||
| -rw-r--r-- | src/or/config.c | 19 | ||||
| -rw-r--r-- | src/or/directory.c | 11 | ||||
| -rw-r--r-- | 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 000000000..5f0e250ff --- /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 2a8c54096..2ce930bd7 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -813,6 +813,7 @@ or_options_free(or_options_t *options) rs, routerset_free(rs)); smartlist_free(options->NodeFamilySets); } + tor_free(options->BridgePassword_AuthDigest); config_free(&options_format, options); } @@ -1547,6 +1548,24 @@ options_act(const 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 cc5f1bd97..18122c394 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3217,22 +3217,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 4996fd33a..0a835029a 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3045,10 +3045,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. */ |