[or-cvs] r8574: Improve error messages from AUTHENTICATE attempts to control (in tor/trunk: . doc src/or)

[nickm at seul.org]

Author: nickm
Date: 2006-10-02 14:08:46 -0400 (Mon, 02 Oct 2006)
New Revision: 8574

Modified:
   tor/trunk/
   tor/trunk/ChangeLog
   tor/trunk/doc/control-spec.txt
   tor/trunk/src/or/control.c
Log:
 r8835 at totoro:  nickm | 2006-10-02 12:54:41 -0400
 Improve error messages from AUTHENTICATE attempts to controller.



Property changes on: tor/trunk
___________________________________________________________________
 svk:merge ticket from /tor/trunk [r8835] on 96637b51-b116-0410-a10e-9941ebb49b64

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2006-10-01 22:16:55 UTC (rev 8573)
+++ tor/trunk/ChangeLog	2006-10-02 18:08:46 UTC (rev 8574)
@@ -50,6 +50,7 @@
     - Avoid choosing Exit nodes for entry or middle hops when the bandwidth
       available in non-Exit nodes is much higher then the bandwidth available
       in Exit nodes. (Fixes bug 200.)
+    - Give more meaningful errors on control authentication failure.
 
   o Security Fixes, minor:
     - If a client asked for a server by name, and we didn't have a

Modified: tor/trunk/doc/control-spec.txt
===================================================================
--- tor/trunk/doc/control-spec.txt	2006-10-01 22:16:55 UTC (rev 8573)
+++ tor/trunk/doc/control-spec.txt	2006-10-02 18:08:46 UTC (rev 8574)
@@ -788,13 +788,8 @@
 
   If the 'CookieAuthentication' option is true, Tor writes a "magic cookie"
   file named "control_auth_cookie" into its data directory.  To authenticate,
-  the controller must send the contents of this file.
+  the controller must send the contents of this file, encoded in hexadecimal.
 
-  [With the v1 controller protocol, what we really mean is that you should
-   send the base16 of the contents of this file. Is this it, or is there
-   more to it? Should we write a control_auth_cookie.asc file too that
-   makes this step easier for people doing it manually? -RD]
-
   If the 'HashedControlPassword' option is set, it must contain the salted
   hash of a secret password.  The salted hash is computed according to the
   S2K algorithm in RFC 2440 (OpenPGP), and prefixed with the s2k specifier.

Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c	2006-10-01 22:16:55 UTC (rev 8573)
+++ tor/trunk/src/or/control.c	2006-10-02 18:08:46 UTC (rev 8574)
@@ -1010,6 +1010,7 @@
 {
   int used_quoted_string = 0;
   or_options_t *options = get_options();
+  const char *errstr = NULL;
   char *password;
   size_t password_len;
   if (STATE_IS_V0(conn->_base.state)) {
@@ -1043,8 +1044,16 @@
     }
   }
   if (options->CookieAuthentication) {
-    if (password_len == AUTHENTICATION_COOKIE_LEN &&
-        !memcmp(authentication_cookie, password, password_len)) {
+    if (password_len != AUTHENTICATION_COOKIE_LEN) {
+      log_warn(LD_CONTROL, "Got authentication cookie with wrong length (%d)",
+               password_len);
+      errstr = "Wrong length on authentication cookie.";
+      goto err;
+    } 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;
+    } else {
       goto ok;
     }
   } else if (options->HashedControlPassword) {
@@ -1053,11 +1062,20 @@
     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 (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 you put it in double quotes.";
     goto err;
   } else {
     /* if Tor doesn't demand any stronger authentication, then
@@ -1071,12 +1089,10 @@
                         "Authentication failed");
   else {
     tor_free(password);
-    if (used_quoted_string)
-      connection_write_str_to_buf("515 Authentication failed\r\n", conn);
-    else
-      connection_write_str_to_buf(
-         "515 Authentication failed.  Maybe you tried a plain text password?  "
-         "If so, the standard requires you put it in double quotes.\r\n",conn);
+    if (!errstr)
+      errstr = "Unknown reason.";
+    connection_printf_to_buf(conn, "515 Authentication failed: %s\r\n",
+                             errstr);
   }
   return 0;
  ok: