[or-cvs] r13975: Fix for bug 614: always look at the network BIO for the SSL (in tor/trunk: . src/common)

[nickm at seul.org]

Author: nickm
Date: 2008-03-11 14:22:49 -0400 (Tue, 11 Mar 2008)
New Revision: 13975

Modified:
   tor/trunk/
   tor/trunk/ChangeLog
   tor/trunk/src/common/tortls.c
Log:
 r18751 at catbus:  nickm | 2008-03-11 14:22:43 -0400
 Fix for bug 614: always look at the network BIO for the SSL object, not at the buffering BIO (if one exists because we are renegotiating or something).  Bugfix on 0.1.2.x, oddly enough, though it should be impossible to trigger the problem there.  Backport candidate.  See comments in tortls.c for detailed implementation note.



Property changes on: tor/trunk
___________________________________________________________________
 svk:merge ticket from /tor/trunk [r18751] on 8246c3cf-6607-4228-993b-4d95d33730f1

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2008-03-11 17:21:47 UTC (rev 13974)
+++ tor/trunk/ChangeLog	2008-03-11 18:22:49 UTC (rev 13975)
@@ -13,6 +13,11 @@
       Bugfix on 0.2.0.x.
     - Make sure servers always request certificates from clients during
       TLS renegotiation. Bugfix on 0.2.0.x.
+    - When counting the number of bytes written on a TLS connection, look at
+      the BIO actually used for writing to the network, not at the BIO used
+      (sometimes) to buffer data for the network.  Looking at different BIOs
+      could result in write counts on the order of ULONG_MAX.  Fix for bug
+      614.  Bugfix on 0.1.2.x.
 
   o Minor features:
     - Allow separate log levels to be configured for different logging

Modified: tor/trunk/src/common/tortls.c
===================================================================
--- tor/trunk/src/common/tortls.c	2008-03-11 17:21:47 UTC (rev 13974)
+++ tor/trunk/src/common/tortls.c	2008-03-11 18:22:49 UTC (rev 13975)
@@ -759,6 +759,12 @@
   result->state = TOR_TLS_ST_HANDSHAKE;
   result->isServer = isServer;
   result->wantwrite_n = 0;
+  result->last_write_count = BIO_number_written(bio);
+  result->last_read_count = BIO_number_read(bio);
+  if (result->last_write_count || result->last_read_count) {
+    log_warn(LD_NET, "Newly created BIO has read count %lu, write count %lu",
+             result->last_read_count, result->last_write_count);
+  }
 #ifdef V2_HANDSHAKE_SERVER
   if (isServer) {
     SSL_set_info_callback(result->ssl, tor_tls_server_info_callback);
@@ -1278,18 +1284,33 @@
 void
 tor_tls_get_n_raw_bytes(tor_tls_t *tls, size_t *n_read, size_t *n_written)
 {
+  BIO *wbio, *tmpbio;
   unsigned long r, w;
   r = BIO_number_read(SSL_get_rbio(tls->ssl));
-  w = BIO_number_written(SSL_get_wbio(tls->ssl));
+  /* We want the number of bytes actually for real written.  Unfortunately,
+   * sometimes OpenSSL replaces the wbio on tls->ssl with a buffering bio,
+   * which makes the answer turn out wrong.  Let's cope with that.  Note
+   * that this approach will fail if we ever replace tls->ssl's BIOs with
+   * buffering bios for reasons of our own.  As an alternative, we could
+   * save the original BIO for  tls->ssl in the tor_tls_t structure, but
+   * that would be tempting fate. */
+  wbio = SSL_get_wbio(tls->ssl);
+  if (wbio->method == BIO_f_buffer() && (tmpbio = BIO_next(wbio)) != NULL)
+    wbio = tmpbio;
+  w = BIO_number_written(wbio);
 
   /* We are ok with letting these unsigned ints go "negative" here:
    * If we wrapped around, this should still give us the right answer, unless
    * we wrapped around by more than ULONG_MAX since the last time we called
    * this function.
    */
-
   *n_read = (size_t)(r - tls->last_read_count);
   *n_written = (size_t)(w - tls->last_write_count);
+  if (*n_read > INT_MAX || *n_written > INT_MAX) {
+    log_warn(LD_BUG, "Preposterously large value in tor_tls_get_n_raw_bytes. "
+             "r=%lu, last_read=%lu, w=%lu, last_written=%lu",
+             r, tls->last_read_count, w, tls->last_write_count);
+  }
   tls->last_read_count = r;
   tls->last_write_count = w;
 }