r14328@Kushana: nickm | 2007-09-04 20:17:34 -0400

 There is no good reason to make hashedcontrolpassword and cookieauthentication mutually exclusive.  So let's not.


svn:r11377

ChangeLog

@@ -2,6 +2,8 @@ Changes in version 0.2.0.7-alpha - 2007-??-??
   o Minor features (security):
     - As a client, do not believe any server that tells us that any address
       maps to an internal address space.
+    - Make it possible to enable HashedControlPassword and
+      CookieAuthentication at the same time.
 
   o Minor features (guard nodes):
     - Tag every guard node in our state file with the version that we believe

src/or/config.c

@@ -2903,8 +2903,6 @@ options_validate(or_options_t *old_options, or_options_t *options,
     if (decode_hashed_password(NULL, options->HashedControlPassword)<0)
       REJECT("Bad HashedControlPassword: wrong length or bad encoding");
   }
-  if (options->HashedControlPassword && options->CookieAuthentication)
-    REJECT("Cannot set both HashedControlPassword and CookieAuthentication");
 
   if (options->ControlListenAddress) {
     int all_are_local = 1;

src/or/control.c

@@ -953,6 +953,7 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
   size_t password_len;
   const char *cp;
   int i;
+  int bad_cookie, bad_password;
 
   if (TOR_ISXDIGIT(body[0])) {
     cp = body;
@@ -984,46 +985,69 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
     used_quoted_string = 1;
   }
 
+  if (!options->CookieAuthentication && !options->HashedControlPassword) {
+    /* if Tor doesn't demand any stronger authentication, then
+     * the controller can get in with anything. */
+    goto ok;
+  }
+
   if (options->CookieAuthentication) {
+    int also_password = options->HashedControlPassword != NULL;
     if (password_len != AUTHENTICATION_COOKIE_LEN) {
-      log_warn(LD_CONTROL, "Got authentication cookie with wrong length (%d)",
-               (int)password_len);
-      errstr = "Wrong length on authentication cookie.";
-      goto err;
+      if (!also_password) {
+        log_warn(LD_CONTROL, "Got authentication cookie with wrong length (%d)",
+                 (int)password_len);
+        errstr = "Wrong length on authentication cookie.";
+        goto err;
+      }
+      bad_cookie = 1;
     } else if (memcmp(authentication_cookie, password, password_len)) {
-      log_warn(LD_CONTROL, "Got mismatched authentication cookie");
-      errstr = "Authentication cookie did not match expected value.";
-      goto err;
+      if (!also_password) {
+        log_warn(LD_CONTROL, "Got mismatched authentication cookie");
+        errstr = "Authentication cookie did not match expected value.";
+        goto err;
+      }
+      bad_cookie = 1;
     } else {
       goto ok;
     }
-  } else if (options->HashedControlPassword) {
+  }
+
+  if (options->HashedControlPassword) {
     char expected[S2K_SPECIFIER_LEN+DIGEST_LEN];
     char received[DIGEST_LEN];
+    int also_cookie = options->CookieAuthentication;
     if (decode_hashed_password(expected, options->HashedControlPassword)<0) {
-      log_warn(LD_CONTROL,
-               "Couldn't decode HashedControlPassword: invalid base16");
-      errstr = "Couldn't decode HashedControlPassword value in configuration.";
-      goto err;
-    }
-    secret_to_key(received,DIGEST_LEN,password,password_len,expected);
-    if (!memcmp(expected+S2K_SPECIFIER_LEN, received, DIGEST_LEN))
-      goto ok;
+      if (!also_cookie) {
+        log_warn(LD_CONTROL,
+                 "Couldn't decode HashedControlPassword: invalid base16");
+        errstr ="Couldn't decode HashedControlPassword value in configuration.";
+      }
+      bad_password = 1;
+    } else {
+      secret_to_key(received,DIGEST_LEN,password,password_len,expected);
+      if (!memcmp(expected+S2K_SPECIFIER_LEN, received, DIGEST_LEN))
+        goto ok;
 
-    if (used_quoted_string)
-      errstr = "Password did not match HashedControlPassword value from "
-        "configuration";
-    else
-      errstr = "Password did not match HashedControlPassword value from "
-        "configuration. Maybe you tried a plain text password? "
-        "If so, the standard requires that you put it in double quotes.";
-    goto err;
-  } else {
-    /* if Tor doesn't demand any stronger authentication, then
-     * the controller can get in with anything. */
-    goto ok;
+      if (used_quoted_string)
+        errstr = "Password did not match HashedControlPassword value from "
+          "configuration";
+      else
+        errstr = "Password did not match HashedControlPassword value from "
+          "configuration. Maybe you tried a plain text password? "
+          "If so, the standard requires that you put it in double quotes.";
+      bad_password = 1;
+      if (!also_cookie)
+        goto err;
+    }
   }
 
+  /** We only get here if both kinds of authentication failed. */
+  tor_assert(bad_password && bad_cookie);
+  log_warn(LD_CONTROL, "Bad password or authentication cookie on controller.");
+  errstr = "Password did not match HashedControlPassword *or* authentication "
+    "cookie.";
+
  err:
   tor_free(password);
   if (!errstr)