r17955@catbus: nickm | 2008-02-06 16:53:07 -0500

 The SSL portion of the revised handshake now seems to work: I just finally got a client and a server to negotiate versions.  Now to make sure certificate verification is really happening, connections are getting opened, etc.


svn:r13409

5 files changed, 82 insertions, 79 deletions

diff --git a/doc/TODO b/doc/TODO
index 6036815ca..95e05c5ec 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -64,7 +64,9 @@ N - Before the feature freeze:
         o Servers detect new ciphers, and only send ID cert when they
           get an older cipher list, and only request client cert when
           they get an older cipher list.
-        - Clients only send certificates when asked for them.
+        . Clients only send certificates when asked for them.
+          o Implement
+          - Enable
         o Servers disable callback once negotiation is finished, so
           that renegotiation happens according to the old rules.
         o Clients initiate renegotiation immediately on completing
@@ -73,10 +75,16 @@ N - Before the feature freeze:
           cert, they adust the client ID.
           o Detect.
           o Adjust.
-      - New revised handshake: post-TLS:
-        - start by sending VERSIONS cells
-        - once we have a version, send a netinfo and become open
+      . New revised handshake: post-TLS:
+        o start by sending VERSIONS cells
+        o once we have a version, send a netinfo and become open
         - Ban most cell types on a non-OPEN connection.
+      - Test
+        o Verify version negotiation on client
+        - Verify version negotiation on server
+        - Verify that client->server connection becomes open
+        - Verify that server->server connection becomes open and
+          authenticated.
       - NETINFO fallout
         - Don't extend a circuit over a noncanonical connection with
           mismatched address.
diff --git a/src/common/tortls.c b/src/common/tortls.c
index f29c0fa2f..269984232 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -66,7 +66,7 @@ typedef struct tor_tls_context_t {
  */
 struct tor_tls_t {
   HT_ENTRY(tor_tls_t) node;
-  tor_tls_context_t *context; /**DOCDOC */
+  tor_tls_context_t *context; /** A link to the context object for this tls */
   SSL *ssl; /**< An OpenSSL SSL object. */
   int socket; /**< The underlying file descriptor for this TLS connection. */
   enum {
@@ -75,12 +75,24 @@ struct tor_tls_t {
   } state : 3; /**< The current SSL state, depending on which operations have
                 * completed successfully. */
   unsigned int isServer:1; /**< True iff this is a server-side connection */
-  unsigned int wasV2Handshake:1; /**< DOCDOC */
+  unsigned int wasV2Handshake:1; /**< True iff the original handshake for
+                                  * this connection used the updated version
+                                  * of the connection protocol (client sends
+                                  * different cipher list, server sends only
+                                  * one certificate). */
+  int got_renegotiate:1; /**< True iff we should call negotiated_callback
+                          * when we're done reading. */
   size_t wantwrite_n; /**< 0 normally, >0 if we returned wantwrite last
                        * time. */
+  /** Last values retrieved from BIO_number_read()/write(); see
+   * tor_tls_get_n_raw_bytes() for usage.
+   */
   unsigned long last_write_count;
   unsigned long last_read_count;
+  /** If set, a callback to invoke whenever the client tries to renegotiate
+   * the handshake. */
   void (*negotiated_callback)(tor_tls_t *tls, void *arg);
+  /** Argument to pass to negotiated_callback. */
   void *callback_arg;
 };
 
@@ -199,7 +211,7 @@ tor_errno_to_tls_error(int e)
 #endif
 }
 
-/** DOCDOC */
+/** Given a TOR_TLS_* error code, return a string equivalent. */
 const char *
 tor_tls_err_to_string(int err)
 {
@@ -305,9 +317,8 @@ static int
 always_accept_verify_cb(int preverify_ok,
                         X509_STORE_CTX *x509_ctx)
 {
-  /* avoid "unused parameter" warning. */
-  preverify_ok = 0;
-  x509_ctx = NULL;
+  (void) preverify_ok;
+  (void) x509_ctx;
   return 1;
 }
 
@@ -467,7 +478,8 @@ tor_tls_create_certificate(crypto_pk_env_t *rsa,
                              SSL3_TXT_EDH_RSA_DES_192_CBC3_SHA)
 #endif
 
-/** DOCDOC */
+/** Remove a reference to <b>ctx</b>, and free it if it has no more
+ * references. */
 static void
 tor_tls_context_decref(tor_tls_context_t *ctx)
 {
@@ -481,7 +493,7 @@ tor_tls_context_decref(tor_tls_context_t *ctx)
   }
 }
 
-/** DOCDOC */
+/** Increase the reference count of <b>ctx</b>. */
 static void
 tor_tls_context_incref(tor_tls_context_t *ctx)
 {
@@ -602,7 +614,9 @@ tor_tls_context_new(crypto_pk_env_t *identity, const char *nickname,
 }
 
 #ifdef V2_HANDSHAKE_SERVER
-/** DOCDOC */
+/** Return true iff the cipher list suggested by the client for <b>ssl</b> is
+ * a list that indicates that the client know how to do the v2 TLS connection
+ * handshake. */
 static int
 tor_tls_client_is_using_v2_ciphers(const SSL *ssl)
 {
@@ -651,18 +665,35 @@ tor_tls_client_is_using_v2_ciphers(const SSL *ssl)
   return 1;
 }
 
-/** DOCDOC */
+/** Invoked when we're accepting a connection on <b>ssl</b>, and the connection
+ * changes state. We use this:
+ * <ul><li>To alter the state of the handshake partway through, so we
+ *         do not send or request extra certificates in v2 handshakes.</li>
+ * <li>To detect renegotiation</li></ul>
+ */
 static void
 tor_tls_server_info_callback(const SSL *ssl, int type, int val)
 {
+  tor_tls_t *tls;
   (void) val;
   if (type != SSL_CB_ACCEPT_LOOP)
     return;
   if (ssl->state != SSL3_ST_SW_SRVR_HELLO_A)
     return;
 
+  tls = tor_tls_get_by_ssl(ssl);
+  if (tls) {
+    /* Check whether we're watching for renegotiates.  If so, this is one! */
+    if (tls->negotiated_callback)
+      tls->got_renegotiate = 1;
+  } else {
+    log_warn(LD_BUG, "Couldn't look up the tls for an SSL*. How odd!");
+  }
+
+  /* Now check the cipher list. */
   if (tor_tls_client_is_using_v2_ciphers(ssl)) {
-    tor_tls_t *tls;
+    /*XXXX_TLS keep this from happening more than once! */
+
     /* Yes, we're casting away the const from ssl.  This is very naughty of us.
      * Let's hope openssl doesn't notice! */
 
@@ -671,7 +702,6 @@ tor_tls_server_info_callback(const SSL *ssl, int type, int val)
     /* Don't send a hello request. */
     SSL_set_verify((SSL*) ssl, SSL_VERIFY_NONE, NULL);
 
-    tls = tor_tls_get_by_ssl((SSL*)ssl);
     if (tls) {
       tls->wasV2Handshake = 1;
     } else {
@@ -731,7 +761,10 @@ tor_tls_new(int sock, int isServer)
   return result;
 }
 
-/**DOCDOC*/
+/** Set <b>cb</b> to be called with argument <b>arg</b> whenever <b>tls</b>
+ * next gets a client-side renegotiate in the middle of a read.  Do not
+ * invoke this function untile <em>after</em> initial handshaking is done!
+ */
 void
 tor_tls_set_renegotiate_callback(tor_tls_t *tls,
                                  void (*cb)(tor_tls_t *, void *arg),
@@ -739,6 +772,14 @@ tor_tls_set_renegotiate_callback(tor_tls_t *tls,
 {
   tls->negotiated_callback = cb;
   tls->callback_arg = arg;
+  tls->got_renegotiate = 0;
+#ifdef V2_HANDSHAKE_SERVER
+  if (cb) {
+    SSL_set_info_callback(tls->ssl, tor_tls_server_info_callback);
+  } else {
+    SSL_set_info_callback(tls->ssl, NULL);
+  }
+#endif
 }
 
 /** Return whether this tls initiated the connect (client) or
@@ -785,12 +826,12 @@ tor_tls_read(tor_tls_t *tls, char *cp, size_t len)
   r = SSL_read(tls->ssl, cp, len);
   if (r > 0) {
 #ifdef V2_HANDSHAKE_SERVER
-    if (SSL_num_renegotiations(tls->ssl)) {
-      /* New certificate! */
+    if (tls->got_renegotiate) {
+      /* Renegotiation happened! */
       log_notice(LD_NET, "Got a TLS renegotiation from %p", tls);
       if (tls->negotiated_callback)
         tls->negotiated_callback(tls, tls->callback_arg);
-      SSL_clear_num_renegotiations(tls->ssl);
+      tls->got_renegotiate = 0;
     }
 #endif
     return r;
@@ -1010,57 +1051,6 @@ tor_tls_peer_has_cert(tor_tls_t *tls)
   return 1;
 }
 
-/** DOCDOC */
-int
-tor_tls_get_cert_digests(tor_tls_t *tls,
-                         char *my_digest_out,
-                         char *peer_digest_out)
-{
-  X509 *cert;
-  unsigned int len;
-  tor_assert(tls && tls->context);
-  cert = tls->context->my_cert;
-  if (cert) {
-    X509_digest(cert, EVP_sha1(), (unsigned char*)my_digest_out, &len);
-    if (len != DIGEST_LEN)
-      return -1;
-  }
-  cert = SSL_get_peer_certificate(tls->ssl);
-  if (cert) {
-    X509_digest(cert, EVP_sha1(), (unsigned char*)peer_digest_out, &len);
-    if (len != DIGEST_LEN)
-      return -1;
-  }
-  return 0;
-}
-
-/** DOCDOC */
-crypto_pk_env_t *
-tor_tls_dup_private_key(tor_tls_t *tls)
-{
-  return crypto_pk_dup_key(tls->context->key);
-}
-
-/** DOCDOC */
-char *
-tor_tls_encode_my_certificate(tor_tls_t *tls, size_t *size_out,
-                              int conn_cert)
-{
-  unsigned char *result, *cp;
-  int certlen;
-  X509 *cert;
-  tor_assert(tls && tls->context);
-  cert = conn_cert ? tls->context->my_cert : tls->context->my_id_cert;
-  tor_assert(cert);
-  certlen = i2d_X509(cert, NULL);
-  tor_assert(certlen >= 0);
-  cp = result = tor_malloc(certlen);
-  i2d_X509(cert, &cp);
-  tor_assert(cp-result == certlen);
-  *size_out = (size_t)certlen;
-  return (char*) result;
-}
-
 /** Warn that a certificate lifetime extends through a certain range. */
 static void
 log_cert_lifetime(X509 *cert, const char *problem)
diff --git a/src/common/tortls.h b/src/common/tortls.h
index a7ef9a775..163b0402c 100644
--- a/src/common/tortls.h
+++ b/src/common/tortls.h
@@ -56,11 +56,6 @@ void tor_tls_set_renegotiate_callback(tor_tls_t *tls,
 int tor_tls_is_server(tor_tls_t *tls);
 void tor_tls_free(tor_tls_t *tls);
 int tor_tls_peer_has_cert(tor_tls_t *tls);
-int tor_tls_get_cert_digests(tor_tls_t *tls, char *my_digest_out,
-                             char *peer_digest_out);
-char *tor_tls_encode_my_certificate(tor_tls_t *tls, size_t *size_out,
-                                    int conn_cert);
-crypto_pk_env_t *tor_tls_dup_private_key(tor_tls_t *tls);
 int tor_tls_verify_v1(int severity, tor_tls_t *tls,
                       crypto_pk_env_t **identity);
 int tor_tls_check_lifetime(tor_tls_t *tls, int tolerance);
diff --git a/src/or/command.c b/src/or/command.c
index 6dae387a6..5765431d7 100644
--- a/src/or/command.c
+++ b/src/or/command.c
@@ -476,6 +476,8 @@ command_process_versions_cell(var_cell_t *cell, or_connection_t *conn)
   conn->link_proto = highest_supported_version;
   conn->handshake_state->received_versions = 1;
 
+  // log_notice(LD_OR, "Negotiated version %d", highest_supported_version);
+
   if (highest_supported_version >= 2) {
     if (connection_or_send_netinfo(conn) < 0) {
       connection_mark_for_close(TO_CONN(conn));
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index b8f16a646..3f547a8fc 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -588,6 +588,7 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn)
 
   if (connection_tls_finish_handshake(conn) < 0) {
     /* XXXX_TLS double-check that it's ok to do this from inside read. */
+    /* XXXX_TLS double-check that this verifies certificates. */
     connection_mark_for_close(TO_CONN(conn));
   }
 
@@ -609,11 +610,15 @@ connection_tls_continue_handshake(or_connection_t *conn)
   int result;
   check_no_tls_errors();
  again:
-  if (conn->_base.state == OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING)
+  if (conn->_base.state == OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING) {
+    // log_notice(LD_OR, "Renegotiate with %p", conn->tls);
     result = tor_tls_renegotiate(conn->tls);
-  else {
+    // log_notice(LD_OR, "Result: %d", result);
+  } else {
     tor_assert(conn->_base.state == OR_CONN_STATE_TLS_HANDSHAKING);
+    // log_notice(LD_OR, "Continue handshake with %p", conn->tls);
     result = tor_tls_handshake(conn->tls);
+    // log_notice(LD_OR, "Result: %d", result);
   }
   switch (result) {
     CASE_TOR_TLS_ERROR_ANY:
@@ -624,9 +629,11 @@ connection_tls_continue_handshake(or_connection_t *conn)
       if (! tor_tls_used_v1_handshake(conn->tls)) {
         if (!tor_tls_is_server(conn->tls)) {
           if (conn->_base.state == OR_CONN_STATE_TLS_HANDSHAKING) {
+            // log_notice(LD_OR,"Done. state was TLS_HANDSHAKING.");
             conn->_base.state = OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING;
             goto again;
           }
+          // log_notice(LD_OR,"Done. state was %d.", conn->_base.state);
         } else {
           /* improved handshake, but not a client. */
           tor_tls_set_renegotiate_callback(conn->tls,
@@ -835,7 +842,8 @@ connection_tls_finish_handshake(or_connection_t *conn)
   char digest_rcvd[DIGEST_LEN];
   int started_here = connection_or_nonopen_was_started_here(conn);
 
-  log_debug(LD_OR,"tls handshake done. verifying.");
+  log_debug(LD_OR,"tls handshake with %s done. verifying.",
+            conn->_base.address);
 
   directory_set_dirty();