r11966@Kushana: nickm | 2007-01-15 16:12:17 -0500

Tidy up ORCONN reason patch from Mike Perry.  Changes: make some of the handling of TLS error codes less error prone.  Enforce house style wrt spaces.  Make it compile with --enable-gcc-warnings.  Only set or_conn->tls_error in the case of an actual error.  Add a changelog entry.


svn:r9355

ChangeLog

@@ -6,6 +6,10 @@ Changes in version 0.1.2.7-alpha - 2007-??-??
     - Adapt a patch from goodell to let the contrib/exitlist script
       take arguments rather than require direct editing.
 
+ o Minor features (controller):
+    - Track reasons for OR connection failure; make these reasons available
+      via the controller interface.  (Patch from Mike Perry.)
+
  o Major bugfixes:
     - Fix a crash bug in the presence of DNS hijacking  (reported by Andrew
       Del Vecchio).

src/common/tortls.c

@@ -73,8 +73,8 @@ static tor_tls_context_t *global_tls_context = NULL;
 static int tls_library_is_initialized = 0;
 
 /* Module-internal error codes. */
-#define _TOR_TLS_SYSCALL    -10
+#define _TOR_TLS_SYSCALL    (_MIN_TOR_TLS_ERROR_VAL - 2)
-#define _TOR_TLS_ZERORETURN -9
+#define _TOR_TLS_ZERORETURN (_MIN_TOR_TLS_ERROR_VAL - 1)
 
 /* These functions are declared in crypto.c but not exported. */
 EVP_PKEY *_crypto_pk_env_get_evp_pkey(crypto_pk_env_t *env, int private);
@@ -104,9 +104,10 @@ tls_log_errors(int severity, const char *doing)
 }
 
 static int
-tor_errno_to_tls_error(int e) {
+tor_errno_to_tls_error(int e)
+{
 #if defined(MS_WINDOWS) && !defined(USE_BSOCKETS)
-  switch(e) {
+  switch (e) {
     case WSAECONNRESET: // most common
       return TOR_TLS_ERROR_CONNRESET;
     case WSAETIMEDOUT:
@@ -119,8 +120,8 @@ tor_errno_to_tls_error(int e) {
     default:
       return TOR_TLS_ERROR_MISC;
   }
-#else 
+#else
-  switch(e) {
+  switch (e) {
     case ECONNRESET: // most common
       return TOR_TLS_ERROR_CONNRESET;
     case ETIMEDOUT:
@@ -182,6 +183,8 @@ tor_tls_get_error(tor_tls_t *tls, int r, int extra,
         return _TOR_TLS_ZERORETURN;
       log(severity, LD_NET, "TLS error: Zero return");
       tls_log_errors(severity, doing);
+      /* XXXX Actually, a 'zero return' error has a pretty specific meaning:
+       * the connection has been closed cleanly.  */
       return TOR_TLS_ERROR_MISC;
     default:
       tls_log_errors(severity, doing);

src/common/tortls.h

@@ -19,6 +19,7 @@
 typedef struct tor_tls_t tor_tls_t;
 
 /* Possible return values for most tor_tls_* functions. */
+#define _MIN_TOR_TLS_ERROR_VAL     -9
 #define TOR_TLS_ERROR_MISC         -9
 #define TOR_TLS_ERROR_IO           -8
 #define TOR_TLS_ERROR_CONNREFUSED  -7
@@ -30,6 +31,18 @@ typedef struct tor_tls_t tor_tls_t;
 #define TOR_TLS_WANTWRITE          -1
 #define TOR_TLS_DONE                0
 
+/* Use this macro in a switch statement to catch _any_ TLS error.  That way,
+ * if more errors are added, your switches will still work. */
+#define CASE_TOR_TLS_ERROR_ANY                  \
+  case TOR_TLS_ERROR_MISC:                      \
+  case TOR_TLS_ERROR_IO:                        \
+  case TOR_TLS_ERROR_CONNREFUSED:               \
+  case TOR_TLS_ERROR_CONNRESET:                 \
+  case TOR_TLS_ERROR_NO_ROUTE:                  \
+  case TOR_TLS_ERROR_TIMEOUT
+
+#define TOR_TLS_IS_ERROR(rv) ((rv) < TOR_TLS_CLOSE)
+
 void tor_tls_free_all(void);
 int tor_tls_context_new(crypto_pk_env_t *rsa,
                         const char *nickname, unsigned int key_lifetime);

src/or/circuitbuild.c

@@ -897,7 +897,7 @@ circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer)
    *     means that a connection broke or an extend failed. For now,
    *     just give up.
    */
-  circuit_mark_for_close(TO_CIRCUIT(circ), 
+  circuit_mark_for_close(TO_CIRCUIT(circ),
           END_CIRC_REASON_FLAG_REMOTE|END_CIRC_REASON_OR_CONN_CLOSED);
   return 0;
 

src/or/connection.c

@@ -435,7 +435,7 @@ connection_about_to_close_connection(connection_t *conn)
           rep_hist_note_connect_failed(or_conn->identity_digest, now);
           entry_guard_register_connect_status(or_conn->identity_digest,0,now);
           router_set_status(or_conn->identity_digest, 0);
-          control_event_or_conn_status(or_conn, OR_CONN_EVENT_FAILED, 
+          control_event_or_conn_status(or_conn, OR_CONN_EVENT_FAILED,
                   control_tls_error_to_reason(or_conn->tls_error));
         }
         /* Inform any pending (not attached) circs that they should
@@ -1460,7 +1460,8 @@ connection_read_to_buf(connection_t *conn, int *max_to_read)
 
     /* else open, or closing */
     result = read_to_buf_tls(or_conn->tls, at_most, conn->inbuf);
-    or_conn->tls_error = result;
+    if (TOR_TLS_IS_ERROR(result) || result == TOR_TLS_CLOSE)
+      or_conn->tls_error = result;
 
     switch (result) {
       case TOR_TLS_CLOSE:
@@ -1469,12 +1470,7 @@ connection_read_to_buf(connection_t *conn, int *max_to_read)
                  or_conn->nickname ? or_conn->nickname : "not set",
                  conn->address);
         return result;
-      case TOR_TLS_ERROR_IO:
+      CASE_TOR_TLS_ERROR_ANY:
-      case TOR_TLS_ERROR_CONNREFUSED:
-      case TOR_TLS_ERROR_CONNRESET:
-      case TOR_TLS_ERROR_NO_ROUTE:
-      case TOR_TLS_ERROR_TIMEOUT:
-      case TOR_TLS_ERROR_MISC:
         log_info(LD_NET,"tls error. breaking (nickname %s, address %s).",
                  or_conn->nickname ? or_conn->nickname : "not set",
                  conn->address);
@@ -1671,12 +1667,7 @@ connection_handle_write(connection_t *conn, int force)
     result = flush_buf_tls(or_conn->tls, conn->outbuf,
                            max_to_write, &conn->outbuf_flushlen);
     switch (result) {
-      case TOR_TLS_ERROR_IO:
+      CASE_TOR_TLS_ERROR_ANY:
-      case TOR_TLS_ERROR_CONNREFUSED:
-      case TOR_TLS_ERROR_CONNRESET:
-      case TOR_TLS_ERROR_NO_ROUTE:
-      case TOR_TLS_ERROR_TIMEOUT:
-      case TOR_TLS_ERROR_MISC:
       case TOR_TLS_CLOSE:
         log_info(LD_NET,result!=TOR_TLS_CLOSE?
                  "tls error. breaking.":"TLS connection closed on flush");

src/or/connection_or.c

@@ -453,7 +453,7 @@ connection_or_connect(uint32_t addr, uint16_t port, const char *id_digest)
                                             time(NULL));
         router_set_status(conn->identity_digest, 0);
       }
-      control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, 
+      control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED,
               END_OR_CONN_REASON_TCP_REFUSED);
       connection_free(TO_CONN(conn));
       return NULL;
@@ -509,17 +509,11 @@ connection_tls_continue_handshake(or_connection_t *conn)
 {
   check_no_tls_errors();
   switch (tor_tls_handshake(conn->tls)) {
-    case TOR_TLS_ERROR_IO:
+    CASE_TOR_TLS_ERROR_ANY:
-    case TOR_TLS_ERROR_CONNREFUSED:
-    case TOR_TLS_ERROR_CONNRESET:
-    case TOR_TLS_ERROR_NO_ROUTE:
-    case TOR_TLS_ERROR_TIMEOUT:
-    case TOR_TLS_ERROR_MISC:
-    case TOR_TLS_CLOSE:
       log_info(LD_OR,"tls error. breaking connection.");
       return -1;
     case TOR_TLS_DONE:
-     return connection_tls_finish_handshake(conn);
+      return connection_tls_finish_handshake(conn);
     case TOR_TLS_WANTWRITE:
       connection_start_writing(TO_CONN(conn));
       log_debug(LD_OR,"wanted write");
@@ -527,6 +521,9 @@ connection_tls_continue_handshake(or_connection_t *conn)
     case TOR_TLS_WANTREAD: /* handshaking conns are *always* reading */
       log_debug(LD_OR,"wanted read");
       return 0;
+    case TOR_TLS_CLOSE:
+      log_info(LD_OR,"tls closed. breaking connection.");
+      return -1;
   }
   return 0;
 }
@@ -798,16 +795,20 @@ connection_or_send_destroy(uint16_t circ_id, or_connection_t *conn, int reason)
   return 0;
 }
 
+/* XXXX012 This global is getting _too_ global. -NM */
+extern smartlist_t *circuits_pending_or_conns;
+
 /** Count number of pending circs on an or_conn */
-int 
+int
 connection_or_count_pending_circs(or_connection_t *or_conn)
 {
-  extern smartlist_t *circuits_pending_or_conns;
   int cnt = 0;
 
   if (!circuits_pending_or_conns)
     return 0;
 
+  tor_assert(or_conn);
+
   SMARTLIST_FOREACH(circuits_pending_or_conns, circuit_t *, circ,
   {
     if (circ->marked_for_close)

src/or/control.c

@@ -3248,8 +3248,9 @@ orconn_target_get_name(int long_names,
 }
 
 int
-control_tls_error_to_reason(int e) {
+control_tls_error_to_reason(int e)
-  switch(e) {
+{
+  switch (e) {
     case TOR_TLS_ERROR_IO:
       return END_OR_CONN_REASON_TLS_IO_ERROR;
     case TOR_TLS_ERROR_CONNREFUSED:
@@ -3270,9 +3271,10 @@ control_tls_error_to_reason(int e) {
   }
 }
 
-const char *
+static const char *
-or_conn_end_reason_to_string(int r) {
+or_conn_end_reason_to_string(int r)
-  switch(r) {
+{
+  switch (r) {
     case END_OR_CONN_REASON_DONE:
       return "REASON=DONE";
     case END_OR_CONN_REASON_TCP_REFUSED:
@@ -3297,10 +3299,8 @@ or_conn_end_reason_to_string(int r) {
   }
 }
 
-/** Something has happened to the OR connection <b>conn</b>: tell any
- * interested control connections. */
 int
-control_event_or_conn_status(or_connection_t *conn,or_conn_status_event_t tp, 
+control_event_or_conn_status(or_connection_t *conn,or_conn_status_event_t tp,
         int reason)
 {
   char buf[HEX_DIGEST_LEN+3]; /* status, dollar, identity, NUL */
@@ -3333,8 +3333,8 @@ control_event_or_conn_status(or_connection_t *conn,or_conn_status_event_t tp,
       }
     ncircs = connection_or_count_pending_circs(conn);
     ncircs += conn->n_circuits;
-    if(ncircs && (tp == OR_CONN_EVENT_FAILED || tp == OR_CONN_EVENT_CLOSED)) {
+    if (ncircs && (tp == OR_CONN_EVENT_FAILED || tp == OR_CONN_EVENT_CLOSED)) {
-        tor_snprintf(ncircs_buf, sizeof(ncircs_buf), "%sNCIRCS=%d", 
+        tor_snprintf(ncircs_buf, sizeof(ncircs_buf), "%sNCIRCS=%d",
                 reason ? " " : "", ncircs);
     }