commit 9a69c24150965e54322ed9616638d4f1939b1289
Author: Nick Mathewson <nickm(a)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. */