r11987@Kushana: nickm | 2007-01-19 14:57:28 -0500

 Implement SOCKS_BAD_HOSTNAME status event. Defer remaining status events.  Clean up control-spec.txt a little, and fill in recommendations for events.


svn:r9374

ChangeLog

@@ -9,6 +9,9 @@ Changes in version 0.1.2.7-alpha - 2007-??-??
   o Minor features (controller):
     - Track reasons for OR connection failure; make these reasons available
       via the controller interface.  (Patch from Mike Perry.)
+    - Add a SOCKS_BAD_HOSTNAME client status event so controllers can learn
+      when clients are sending malformed hostnames to Tor.
+    - Clean up documentation for controller status events.
 
   o Major bugfixes:
     - Fix a crash bug in the presence of DNS hijacking (reported by Andrew

doc/TODO

@@ -32,16 +32,16 @@ N - Test guard unreachable logic; make sure that we actually attempt to
 R - Reconstruct ChangeLog; put rolled-up info in ReleaseNotes or something.
 
 Items for 0.1.2.x:
-N - enumerate events of important things that occur in tor, so vidalia can
+  o enumerate events of important things that occur in tor, so vidalia can
     react.
     o Backend implementation
     o Actually list all the events (notice and warn log messages are a good
       place to look.)  Divide messages into categories, perhaps.
     o Specify general event system
     o Specify actual events.
-    - Implement or defer remaining events
+    o Implement or defer remaining events
-    - Implement or defer GETINFO list of current status events.
+    D Implement or defer GETINFO list of current status events.
-    - Clean up relevant bits of control-spec.txt
+    o Clean up relevant bits of control-spec.txt
 
   . Have (and document) a BEGIN_DIR relay cell that means "Connect to your
     directory port."
@@ -295,6 +295,25 @@ M   - rewrite how libevent does select() on win32 so it's not so very slow.
 
   - Add an option (related to AvoidDiskWrites) to disable directory caching.
 
+  - More status event features:
+    - Missing events:
+      - DIR_REACHABLE
+      - BAD_DIR_RESPONSE (Unexpected directory response; maybe we're behind
+        a firewall.)
+      - BAD_PROXY (Bad http or https proxy)
+      - UNRECOGNIZED_ROUTER (a nickname we asked for is unavailable)
+      - Status events related to hibernation
+      - something about failing to parse our address?
+        from resolve_my_address() in config.c
+      - sketchy OS, sketchy threading
+      - too many onions queued: threading problems or slow CPU?
+    - Missing fields:
+      - TIMEOUT on CHECKING_REACHABILITY
+    - GETINFO status/client, status/server, status/general: There should be
+      some way to learn which status events are currently "in effect."
+      We should specify which these are, what format they appear in, and so
+      on.
+
 Minor items for 0.1.2.x as time permits:
   - getinfo ns/name/moria2 doesn't include a "v" line, even when some
     network-statuses I have show it. I suppose the fix should go in

doc/control-spec.txt

@@ -487,25 +487,6 @@ $Id$
       status events are available as getinfo's currently. Let us know if
       you want more exposed.)
 
-    "status/client"
-    "status/server"
-    "status/general"
-      These provide a (possibly empty) newline-separated list of status
-      events from Section 4.1.10 that Tor believes are still
-      accurate. Controllers can use them to get a summary of any current
-      problems with Tor's operation.
-
-      [Does this mean that Tor must keep state on its side of all the
-      statuses it's sent, and recognize when they're cancelled out,
-      and so on? It's a shame that Tor needs to do this and also Vidalia
-      needs to do this. -RD]
-        [Is there a good alternative? If we want controllers who connect
-        to a running Tor to see its status, I think we need to do this. -NM]
-
-      [What is the format of this list? Is it space-separated,
-      newline-separated?  Does it include keywords, arguments, etc?  Also,
-      what about STATUS_GENERAL? -NM]
-
   Examples:
      C: GETINFO version desc/name/moria1
      S: 250+desc/name/moria=
@@ -967,8 +948,6 @@ $Id$
   or higher.  They differ from log messages in that their format is a
   specified interface.
 
-  [Don't rely on any of these until we work out more of the details. -RD]
-
   Syntax:
      "650" SP StatusType SP StatusSeverity SP StatusAction
                                          [SP StatusArguments] CRLF
@@ -989,9 +968,13 @@ $Id$
      VERBOSE_NAMES; see the explanations in the USEFEATURE section
      for details.
 
-     Controllers MUST tolerate unrecognized actions and arguments, MUST
+     Controllers MUST tolerate unrecognized actions, MUST tolerate
-     tolerate missing arguments, and MUST tolerate arguments that arrive
+     unrecognized arguments, MUST tolerate missing arguments, and MUST
-     in any order.
+     tolerate arguments that arrive in any order.
+
+     Each event description below is accompanied by a recommendation for
+     controllers.  These recommendations are suggestions only; no controller
+     is required to implement them.
 
   Actions for STATUS_GENERAL events can be as follows:
 
@@ -1003,21 +986,12 @@ $Id$
        also happens when the system is swapping so heavily that Tor is
        starving. The "time" argument specifies the number of seconds Tor
        thinks it was unconscious for.
+
        This status event is sent as NOTICE severity normally, but WARN
        severity if Tor is acting as a server currently.
 
-       [Recommendation for controller: ignore it, since we don't really
+       {Recommendation for controller: ignore it, since we don't really
-       know what the user should do anyway. Hm.]
+       know what the user should do anyway. Hm.}
-
-[the basic idea for the rest is to do it like the CLOCK_JUMPED entry
-above: specify a Type, describe what it is and its arguments, and
-describe what Severities to expect and what we suggest the controller
-do for each. -RD]
-
-     DIR_REACHABLE
-     [not implemented yet]
-
-  typically severity WARN events:
 
      DANGEROUS_VERSION
      "CURRENT=version"
@@ -1031,12 +1005,21 @@ do for each. -RD]
        some recommended versions of Tor are newer and some are old than this
        version.
 
+       {Controllers may want to suggest that the user upgrade OLD or
+       UNRECOMMENDED versions.  NEW versions may be known-insecure, or may
+       simply be development versions.}
+
      TOO_MANY_CONNECTIONS
      "CURRENT=NUM"
-       Tor has reached its ulimit -n or whatever the native limit is on
+       Tor has reached its ulimit -n or whatever the native limit is on file
-       file descriptors or sockets. The user should really do something
+       descriptors or sockets.  CURRENT is the number of sockets Tor
-       about this. The "current" argument shows the number of connections
+       currently has open.  The user should really do something about
-       currently open.
+       this. The "current" argument shows the number of connections currently
+       open.
+
+       {Controllers may recommend that the user increase the limit, or
+       increase it for them.  Recommendations should be phrased in an
+       OS-appropriate way and automated when possible.}
 
      BUG
      "REASON=STRING"
@@ -1045,12 +1028,8 @@ do for each. -RD]
        the controller can explain this to the user and encourage her to
        file a bug report?
 
-     [The following two are sent as WARNs if CIRCUIT_ESTABLISHED and
+       {Controllers should log bugs, but shouldn't annoy the user in case a
-     not DIR_ALL_UNREACHABLE, else as ERRs:]
+       bug appears frequently.}
-
-     BAD_DIR_RESPONSE
-     [unimplemented]
-     // unexpected dir response. behind a hotel/airport firewall?
 
      CLOCK_SKEWED
        SKEW="+" / "-" SECONDS
@@ -1061,6 +1040,11 @@ do for each. -RD]
          a NETWORKSTATUS, we decided we're skewed because we got a
          networkstatus from far in the future.
 
+         {Controllers may want to warn the user if the skew is high, or if
+         multiple skew messages appear at severity WARN.  Controllers
+         shouldn't blindly adjust the clock, since the more accurate source
+         of skew info (DIRSERV) is currently unauthenticated.}
+
      BAD_LIBEVENT
      "METHOD=" libevent method
      "VERSION=" libevent version
@@ -1072,11 +1056,10 @@ do for each. -RD]
         fine, but not quickly.  If "RECOVERED" is YES, Tor managed to
         switch to a more reliable (but probably slower!) libevent method.
 
-  Actions for STATUS_GENERAL severity ERR events can be as follows:
+        {Controllers may want to warn the user if this event occurs, though
-
+        generally it's the fault of whoever built the Tor binary and there's
-     BAD_PROXY
+        not much the user can do besides upgrade libevent or upgrade the
-     [unimplemented]
+        binary.}
-     // bad http or https proxy?
 
      DIR_ALL_UNREACHABLE
        Tor believes that none of the known directory servers are
@@ -1084,29 +1067,40 @@ do for each. -RD]
        down or otherwise not working, and might help to explain for the
        user why Tor appears to be broken.
 
-  Actions for STATUS_CLIENT severity NOTICE events can be as follows:
+       {Controllers may want to warn the user if this event occurs; further
+       action is generally not possible.}
+
+  Actions for STATUS_CLIENT events can be as follows:
 
      ENOUGH_DIR_INFO
        Tor now knows enough network-status documents and enough server
        descriptors that it's going to start trying to build circuits now.
 
+       {Controllers may want to use this event to decide when to indicate
+       progress to their users, but should not interrupt the user's browsing
+       to tell them so.}
+
      NOT_ENOUGH_DIR_INFO
        We discarded expired statuses and router descriptors to fall
        below the desired threshold of directory information. We won't
        try to build any circuits until ENOUGH_DIR_INFO occurs again.
 
+       {Controllers may want to use this event to decide when to indicate
+       progress to their users, but should not interrupt the user's browsing
+       to tell them so.}
+
      CIRCUIT_ESTABLISHED
        Tor is able to establish circuits for client use. This event will
        only be sent if we just built a circuit that changed our mind --
        that is, prior to this event we didn't know whether we could
        establish circuits.
 
-       Suggested use: controllers can notify their users that Tor is
+       {Suggested use: controllers can notify their users that Tor is
        ready for use as a client once they see this status event. [Perhaps
        controllers should also have a timeout if too much time passes and
        this event hasn't arrived, to give tips on how to troubleshoot.
        On the other hand, hopefully Tor will send further status events
-       if it can identify the problem.]
+       if it can identify the problem.]}
 
      CIRCUIT_NOT_ESTABLISHED
      "REASON=" "EXTERNAL_ADDRESS" / "DIR_ALL_UNREACHABLE" / "CLOCK_JUMPED"
@@ -1114,11 +1108,11 @@ do for each. -RD]
        keyword provides an explanation: which other status event type caused
        our lack of confidence.
 
-       Suggested use: Vidalia can turn its onion yellow again.
+       {Controllers may want to use this event to decide when to indicate
+       progress to their users, but should not interrupt the user's browsing
+       to do so.}
        [Note: only REASON=CLOCK_JUMPED is implemented currently.]
 
-  Actions for STATUS_CLIENT severity WARN events can be as follows:
-
      DANGEROUS_SOCKS
      "PROTOCOL=SOCKS4/SOCKS5"
      "ADDRESS=IP:port"
@@ -1127,6 +1121,10 @@ do for each. -RD]
        If the client application got this address from gethostbyname(),
        it may be leaking target addresses via DNS.
 
+       {Controllers should warn their users when this occurs, unless they
+       happen to know that the application using Tor is in fact doing so
+       correctly (e.g., because it is part of a distributed bundle).}
+
      SOCKS_UNKNOWN_PROTOCOL
        "DATA=string"
        A connection was made to Tor's SOCKS port that tried to use it
@@ -1134,22 +1132,19 @@ do for each. -RD]
        using Tor as an HTTP proxy?   The DATA is the first few characters
        sent to Tor on the SOCKS port.
 
-     SOCKS_BAD_HOSTNAME
+       {Controllers may want to warn their users when this occurs: it
-     [unimplemented]
+       indicates a misconfigured application.}
-     // Some application gave us a funny-looking hostname. Perhaps
-     // it is broken? In any case it won't work with Tor and the user
-     // should know.
-
-     UNRECOGNIZED_ROUTER
-     [unimplemented]
-     // a nickname we asked for is unavailable. no need for this
-     // quite yet, since no end-user controllers let you configure that.
 
-  Actions for STATUS_CLIENT severity ERR events can be as follows:
+     SOCKS_BAD_HOSTNAME
+      "HOSTNAME=QuotedString"
+       Some application gave us a funny-looking hostname. Perhaps
+       it is broken? In any case it won't work with Tor and the user
+       should know.
 
-     [none yet]
+       {Controllers may want to warn their users when this occurs: it
+       usually indicates a misconfigured application.}
 
-  Actions for STATUS_SERVER severity NOTICE events can be as follows:
+  Actions for STATUS_SERVER can be as follows:
 
      EXTERNAL_ADDRESS
      "ADDRESS=IP"
@@ -1165,36 +1160,32 @@ do for each. -RD]
        the method is 'DIRSERV', a directory server told us a guess for what
        our IP might be.
 
-     // hibernating
+       {Controllers may want to record this info and display it to the user.}
 
      CHECKING_REACHABILITY
      "ORADDRESS=IP:port"
      "DIRADDRESS=IP:port"
-     "TIMEOUT=NUM"     [timeout unimplemented]
        We're going to start testing the reachability of our external OR port
        or directory port.
 
+       {This event could effect the controller's idea of server status, but
+       the controller should not interrupt the user to tell them so.}
+
      REACHABILITY_SUCCEEDED
      "ORADDRESS=IP:port"
      "DIRADDRESS=IP:port"
        We successfully verified the reachability of our external OR port or
        directory port.
 
+       {This event could effect the controller's idea of server status, but
+       the controller should not interrupt the user to tell them so.}
+
      GOOD_SERVER_DESCRIPTOR
        We successfully uploaded our server descriptor to each of the
        directory authorities, with no complaints.
 
-  Actions for STATUS_SERVER severity WARN events can be as follows:
+       {This event could effect the controller's idea of server status, but
-
+       the controller should not interrupt the user to tell them so.}
-     // something about failing to parse our address?
-     // from resolve_my_address() in config.c
-     [unimplemented]
-
-     // sketchy OS, sketchy threading
-     [unimplemented]
-
-     // too many onions queued. threading problem or slow cpu?
-     [unimplemented]
 
      NAMESERVER_STATUS
      "NS=addr"
@@ -1203,17 +1194,32 @@ do for each. -RD]
         One of our nameservers has changed status.
         // actually notice
 
+       {This event could effect the controller's idea of server status, but
+       the controller should not interrupt the user to tell them so.}
+
      NAMESERVER_ALL_DOWN
         All of our nameservers have gone down.
 
+        {This is a problem; if it happens often without the nameservers
+        coming up again, the user needs to configure more or better
+        nameservers.}
+
      DNS_HIJACKED
         Our DNS provider is providing an address when it should be saying
         "NOTFOUND"; Tor will treat the address as a synonym for "NOTFOUND".
 
+        {This is an annoyance; controllers may want to tell admins that their
+        DNS provider is not to be trusted.}
+
      DNS_USELESS
         Our DNS provider is giving a hijacked address instead of well-known
         websites; Tor will not try to be an exit node.
 
+        {Controllers could warn the admin if the server is running as an
+        exit server: the admin needs to configure a good DNS server.
+        Alternatively, this happens a lot in some restrictive environments
+        (hotels, universities, coffeeshops) when the user hasn't registered.}
+
      BAD_SERVER_DESCRIPTOR
      "DIRAUTH=addr:port"
      "REASON=string"
@@ -1221,12 +1227,15 @@ do for each. -RD]
         include malformed descriptors, incorrect keys, highly skewed clocks,
         and so on.
 
+        {Controllers should warn the admin, and try to cope if they can.}
+
      ACCEPTED_SERVER_DESCRIPTOR
      "DIRAUTH=addr:port"
         A single directory authority accepted our descriptor.
         // actually notice
 
-  Actions for STATUS_SERVER severity ERR events can be as follows:
+       {This event could effect the controller's idea of server status, but
+       the controller should not interrupt the user to tell them so.}
 
      REACHABILITY_FAILED
      "ORADDRESS=IP:port"
@@ -1234,8 +1243,8 @@ do for each. -RD]
        We failed to connect to our external OR port or directory port
        successfully.
 
-  Controllers must tolerate hearing about actions that they don't
+       {This event could effect the controller's idea of server status.  The
-  recognize.
+       controller should warn the admin and suggest reasonable steps to take.}
 
 4.1.11. Our set of guard nodes has changed
 

src/common/util.c

@@ -652,8 +652,9 @@ base16_decode(char *dest, size_t destlen, const char *src, size_t srclen)
 /** Allocate and return a new string representing the contents of <b>s</b>,
  * surrounded by quotes and using standard C escapes.
  *
- * Generally, we use this for logging values that come in over the network
+ * Generally, we use this for logging values that come in over the network to
- * to keep them from tricking users.
+ * keep them from tricking users, and for sending certain values to the
+ * controller.
  *
  * We trust values from the resolver, OS, configuration file, and command line
  * to not be maliciously ill-formed.  We validate incoming routerdescs and

src/or/connection_edge.c

@@ -1212,6 +1212,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
 
   if (addresstype == BAD_HOSTNAME) {
     log_warn(LD_APP, "Invalid hostname %s; rejecting", socks->address);
+    control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s",
+                                escaped(socks->address));
     connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
     return -1;
   }
@@ -1227,6 +1229,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
       } else {
         log_warn(LD_APP,"Malformed exit address '%s.exit'. Refusing.",
                  safe_str(socks->address));
+        control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s",
+                                    escaped(socks->address));
         connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
         return -1;
       }
@@ -1249,8 +1253,9 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
 
   if (addresstype != ONION_HOSTNAME) {
     /* not a hidden-service request (i.e. normal or .exit) */
-
     if (address_is_invalid_destination(socks->address, 1)) {
+      control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s",
+                                  escaped(socks->address));
       log_warn(LD_APP,
                "Destination '%s' seems to be an invalid hostname. Failing.",
                safe_str(socks->address));
@@ -1264,6 +1269,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
       /* Reply to resolves immediately if we can. */
       if (strlen(socks->address) > RELAY_PAYLOAD_SIZE) {
         log_warn(LD_APP,"Address to be resolved is too large. Failing.");
+        control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s",
+                                    escaped(socks->address));
         connection_ap_handshake_socks_resolved(conn,RESOLVED_TYPE_ERROR,
                                                0,NULL,-1);
         connection_mark_unattached_ap(conn,