[or-cvs] r11377: There is no good reason to make hashedcontrolpassword and co (in tor/trunk: . src/or)

[nickm at seul.org]

Author: nickm
Date: 2007-09-04 20:31:07 -0400 (Tue, 04 Sep 2007)
New Revision: 11377

Modified:
   tor/trunk/
   tor/trunk/ChangeLog
   tor/trunk/src/or/config.c
   tor/trunk/src/or/control.c
Log:
 r14328 at 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.



Property changes on: tor/trunk
___________________________________________________________________
 svk:merge ticket from /tor/trunk [r14328] on c95137ef-5f19-0410-b913-86e773d04f59

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2007-09-05 00:31:01 UTC (rev 11376)
+++ tor/trunk/ChangeLog	2007-09-05 00:31:07 UTC (rev 11377)
@@ -2,6 +2,8 @@
   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

Modified: tor/trunk/src/or/config.c
===================================================================
--- tor/trunk/src/or/config.c	2007-09-05 00:31:01 UTC (rev 11376)
+++ tor/trunk/src/or/config.c	2007-09-05 00:31:07 UTC (rev 11377)
@@ -2903,8 +2903,6 @@
     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;

Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c	2007-09-05 00:31:01 UTC (rev 11376)
+++ tor/trunk/src/or/control.c	2007-09-05 00:31:07 UTC (rev 11377)
@@ -953,6 +953,7 @@
   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 @@
     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;
+      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.";
+      bad_password = 1;
+      if (!also_cookie)
+        goto err;
     }
-    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;
   }
 
+  /** 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)