Squashed Asio SSL FUBAR Issue

So, this affects the HTTPS implementation in 0.9.3 as well, where Asio is not exactly being smart about how it deals with SSL errors. We've had to do some acrobatics just to determine whether the error code returned by an operation performed on an SSL stream resulted in a short read. The change is simple, but tracking it down is so much of a PITA. We probably should file a bug in Asio to be able to better detect the situation with short reads on SSL streams, and somehow also file a bug for it. At worst the short read should just use an asio::error::eof so that it's not too hard to deal with this issue from the user side. Also, this commit has some header-fudging to reduce dependencies and incremental compile-times. A more extensive effort should be done to reduce dependencies across header-files library-wide.
degiz · Mar 17, 2012 · b26c440 · b26c440
1 parent 7ff00ac
commit b26c440
Show file tree

Hide file tree

Showing 11 changed files with 121 additions and 52 deletions.
diff --git a/boost/network/protocol/http/client/connection/async_normal.hpp b/boost/network/protocol/http/client/connection/async_normal.hpp
@@ -9,15 +9,24 @@
 
 #include <boost/shared_ptr.hpp>
 #include <boost/scoped_ptr.hpp>
-#include <boost/asio/io_service.hpp>
-#include <boost/network/protocol/http/request.hpp>
-#include <boost/network/protocol/http/response.hpp>
 #include <boost/network/protocol/http/client/client_connection.hpp>
-#include <boost/network/protocol/http/client/connection/resolver_delegate.hpp>
-#include <boost/network/protocol/http/client/connection/connection_delegate.hpp>
+#include <boost/enable_shared_from_this.hpp>
+
+namespace boost { namespace asio {
+
+class io_service;
+
+}  // namespace asio
+
+}  // namespace boost
 
 namespace boost { namespace network { namespace http {
 
+struct request;
+struct response;
+struct resolver_delegate;
+struct connection_delegate;
+
 struct http_async_connection_pimpl;
 
 struct http_async_connection : client_connection

diff --git a/boost/network/protocol/http/client/connection/async_normal.ipp b/boost/network/protocol/http/client/connection/async_normal.ipp
@@ -10,10 +10,16 @@
 #include <boost/asio/placeholders.hpp>
 #include <boost/network/protocol/http/client/connection/async_normal.hpp>
 #include <boost/network/protocol/http/request.hpp>
+#include <boost/network/protocol/http/response.hpp>
+#include <boost/network/protocol/http/client/connection/connection_delegate.hpp>
+#include <boost/network/protocol/http/client/connection/resolver_delegate.hpp>
 #include <boost/network/protocol/http/algorithms/linearize.hpp>
 #include <boost/network/protocol/http/impl/access.hpp>
 #include <boost/asio/strand.hpp>
 #include <boost/network/detail/debug.hpp>
+#ifdef BOOST_NETWORK_ENABLE_HTTPS
+#include <boost/asio/ssl/error.hpp>
+#endif
 
 namespace boost { namespace network { namespace http {
 
@@ -59,8 +65,9 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
     BOOST_NETWORK_MESSAGE("method: " << this->method);
     boost::uint16_t port_ = port(request);
     BOOST_NETWORK_MESSAGE("port: " << port_);
+    this->host_ = host(request);
     resolver_delegate_->resolve(
-        host(request),
+        this->host_,
         port_,
         request_strand_.wrap(
             boost::bind(
@@ -131,17 +138,19 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
       BOOST_NETWORK_MESSAGE("trying connection to: "
                             << iter->endpoint().address() << ":" << port);
       asio::ip::tcp::endpoint endpoint(iter->endpoint().address(), port);
-      connection_delegate_->connect(endpoint,
-                         request_strand_.wrap(
-                             boost::bind(
-                                 &this_type::handle_connected,
-                                 this_type::shared_from_this(),
-                                 port,
-                                 get_body,
-                                 callback,
-                                 std::make_pair(++iter,
-                                                resolver_iterator()),
-                                 placeholders::error)));
+      connection_delegate_->connect(
+          endpoint,
+          this->host_,
+          request_strand_.wrap(
+              boost::bind(
+                  &this_type::handle_connected,
+                  this_type::shared_from_this(),
+                  port,
+                  get_body,
+                  callback,
+                  std::make_pair(++iter,
+                                 resolver_iterator()),
+                  placeholders::error)));
     } else {
       BOOST_NETWORK_MESSAGE("error encountered while resolving.");
       set_errors(ec ? ec : boost::asio::error::host_not_found);
@@ -174,6 +183,7 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
         BOOST_NETWORK_MESSAGE("trying: " << iter->endpoint().address() << ":" << port);
         asio::ip::tcp::endpoint endpoint(iter->endpoint().address(), port);
         connection_delegate_->connect(endpoint,
+                           this->host_,
                            request_strand_.wrap(
                                boost::bind(
                                    &this_type::handle_connected,
@@ -218,7 +228,20 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
 
   void handle_received_data(state_t state, bool get_body, body_callback_function_type callback, boost::system::error_code const & ec, std::size_t bytes_transferred) {
     BOOST_NETWORK_MESSAGE("http_async_connection_pimpl::handle_received_data(...)");
-    if (!ec || ec == boost::asio::error::eof) {
+    // Okay, there's some weirdness with Boost.Asio's handling of SSL errors
+    // so we need to do some acrobatics to make sure that we're handling the
+    // short-read errors correctly. This is such a PITA that we have to deal
+    // with this here.
+    static long short_read_error = 335544539;
+    bool is_short_read_error =
+#ifdef BOOST_NETWORK_ENABLE_HTTPS
+        ec.category() == asio::error::ssl_category &&
+        ec.value() == short_read_error
+#else
+        false
+#endif
+        ;
+    if (!ec || ec == boost::asio::error::eof || is_short_read_error) {
       BOOST_NETWORK_MESSAGE("processing data chunk, no error encountered so far...");
       logic::tribool parsed_ok;
       size_t remainder;
@@ -347,7 +370,7 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
           return;
         case body:
           BOOST_NETWORK_MESSAGE("parsing body...");
-          if (ec == boost::asio::error::eof) {
+          if (ec == boost::asio::error::eof || is_short_read_error) {
             BOOST_NETWORK_MESSAGE("end of the line.");
             // Here we're handling the case when the connection has been
             // closed from the server side, or at least that the end of file
@@ -429,6 +452,7 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
       }
     } else {
       boost::system::system_error error(ec);
+      BOOST_NETWORK_MESSAGE("error encountered: " << error.what() << " (" << ec << ")");
       this->source_promise.set_exception(boost::copy_exception(error));
       this->destination_promise.set_exception(boost::copy_exception(error));
       switch (state) {
@@ -773,6 +797,7 @@ struct http_async_connection_pimpl : boost::enable_shared_from_this<http_async_c
   buffer_type part;
   buffer_type::const_iterator part_begin;
   std::string partial_parsed;
+  std::string host_;
 };
 
 // END OF PIMPL DEFINITION

diff --git a/boost/network/protocol/http/client/connection/connection_delegate.hpp b/boost/network/protocol/http/client/connection/connection_delegate.hpp
@@ -15,6 +15,7 @@ namespace boost { namespace network { namespace http {
 
 struct connection_delegate {
   virtual void connect(asio::ip::tcp::endpoint & endpoint,
+                       std::string const & host,
                        function<void(system::error_code const &)> handler) = 0;
   virtual void write(asio::streambuf & command_streambuf,
                      function<void(system::error_code const &, size_t)> handler) = 0;

diff --git a/boost/network/protocol/http/client/connection/normal_delegate.hpp b/boost/network/protocol/http/client/connection/normal_delegate.hpp
@@ -24,6 +24,7 @@ struct normal_delegate : connection_delegate {
   normal_delegate(asio::io_service & service);
 
   virtual void connect(asio::ip::tcp::endpoint & endpoint,
+                       std::string const &host,
                        function<void(system::error_code const &)> handler);
   virtual void write(asio::streambuf & command_streambuf,
                      function<void(system::error_code const &, size_t)> handler);

diff --git a/boost/network/protocol/http/client/connection/normal_delegate.ipp b/boost/network/protocol/http/client/connection/normal_delegate.ipp
@@ -21,6 +21,7 @@ boost::network::http::normal_delegate::normal_delegate(asio::io_service & servic
 
 void boost::network::http::normal_delegate::connect(
     asio::ip::tcp::endpoint & endpoint,
+    std::string const &host,
     function<void(system::error_code const &)> handler) {
   socket_.reset(new asio::ip::tcp::socket(service_));
   socket_->async_connect(endpoint, handler);

diff --git a/boost/network/protocol/http/client/connection/simple_connection_factory.ipp b/boost/network/protocol/http/client/connection/simple_connection_factory.ipp
@@ -16,6 +16,7 @@
 #ifdef BOOST_NETWORK_DEBUG
 #include <boost/network/uri/uri_io.hpp> 
 #endif
+#include <boost/algorithm/string/case_conv.hpp>
 
 #include <boost/network/protocol/http/message/wrappers/uri.hpp>
 

diff --git a/boost/network/protocol/http/client/connection/ssl_delegate.hpp b/boost/network/protocol/http/client/connection/ssl_delegate.hpp
@@ -27,6 +27,7 @@ struct ssl_delegate : connection_delegate, enable_shared_from_this<ssl_delegate>
                client_options const &options);
 
   virtual void connect(asio::ip::tcp::endpoint & endpoint,
+                       std::string const &host,
                        function<void(system::error_code const &)> handler);
   virtual void write(asio::streambuf & command_streambuf,
                      function<void(system::error_code const &, size_t)> handler);

diff --git a/boost/network/protocol/http/client/connection/ssl_delegate.ipp b/boost/network/protocol/http/client/connection/ssl_delegate.ipp
@@ -16,30 +16,39 @@
 boost::network::http::ssl_delegate::ssl_delegate(asio::io_service & service,
                                                  client_options const &options) :
   service_(service),
-  options_(options) {}
+  options_(options) {
+  BOOST_NETWORK_MESSAGE("ssl_delegate::ssl_delegate(...)");
+}
 
 void boost::network::http::ssl_delegate::connect(
     asio::ip::tcp::endpoint & endpoint,
+    std::string const &host,
     function<void(system::error_code const &)> handler) {
   BOOST_NETWORK_MESSAGE("ssl_delegate::connect(...)");
   context_.reset(new asio::ssl::context(
-      service_,
-      asio::ssl::context::sslv23_client));
+      asio::ssl::context::sslv23));
   std::list<std::string> const & certificate_paths = 
       options_.openssl_certificate_paths();
   std::list<std::string> const & verifier_paths =
       options_.openssl_verify_paths();
-  bool verify_peer = !certificate_paths.empty() || !verifier_paths.empty();
-  BOOST_NETWORK_MESSAGE("verify peer setting is: " << verify_peer);
-  if (verify_peer) context_->set_verify_mode(asio::ssl::context::verify_peer);
-  else context_->set_verify_mode(asio::ssl::context::verify_none);
-  for (std::list<std::string>::const_iterator it = certificate_paths.begin();
-       it != certificate_paths.end(); ++it) {
-    context_->load_verify_file(*it);
-  }
-  for (std::list<std::string>::const_iterator it = verifier_paths.begin();
-       it != verifier_paths.begin(); ++it) {
-    context_->add_verify_path(*it);
+  bool use_default_verification = certificate_paths.empty() && verifier_paths.empty();
+  if (!use_default_verification) {
+    for (std::list<std::string>::const_iterator it = certificate_paths.begin();
+         it != certificate_paths.end(); ++it) {
+      context_->load_verify_file(*it);
+    }
+    for (std::list<std::string>::const_iterator it = verifier_paths.begin();
+         it != verifier_paths.begin(); ++it) {
+      context_->add_verify_path(*it);
+    }
+    BOOST_NETWORK_MESSAGE("verifying peer: " << host);
+    context_->set_verify_mode(asio::ssl::context::verify_peer);
+    context_->set_verify_callback(asio::ssl::rfc2818_verification(host));
+  } else {
+    BOOST_NETWORK_MESSAGE("not verifying peer");
+    context_->set_default_verify_paths();
+    context_->set_verify_mode(asio::ssl::context::verify_peer);
+    context_->set_verify_callback(asio::ssl::rfc2818_verification(host));
   }
   socket_.reset(new asio::ssl::stream<asio::ip::tcp::socket>(service_, *context_));
   BOOST_NETWORK_MESSAGE("scheduling asynchronous connection...");
@@ -51,12 +60,24 @@ void boost::network::http::ssl_delegate::connect(
            handler));
 }
 
-void boost::network::http::ssl_delegate::handle_connected(system::error_code const & ec,
-                                           function<void(system::error_code const &)> handler) {
+
+void boost::network::http::ssl_delegate::handle_connected(
+    system::error_code const & ec,
+    function<void(system::error_code const &)> handler) {
   BOOST_NETWORK_MESSAGE("ssl_delegate::handle_connected(...)");
   if (!ec) {
     BOOST_NETWORK_MESSAGE("connected to endpoint.");
-    socket_->async_handshake(asio::ssl::stream_base::client, handler);
+    // Here we check if there's an existing session for the connection.
+    SSL_SESSION *existing_session = SSL_get1_session(socket_->impl()->ssl);
+    if (existing_session == NULL) {
+      BOOST_NETWORK_MESSAGE("found no existing session, performing handshake.");
+      socket_->async_handshake(asio::ssl::stream_base::client, handler);
+    } else {
+      BOOST_NETWORK_MESSAGE("found existing session, bypassing handshake.");
+      SSL_set_session(socket_->impl()->ssl, existing_session);
+      SSL_connect(socket_->impl()->ssl);
+      handler(ec);
+    }
   } else {
     BOOST_NETWORK_MESSAGE("encountered error: " << ec);
     handler(ec);

diff --git a/boost/network/protocol/http/message/wrappers/port.ipp b/boost/network/protocol/http/message/wrappers/port.ipp
@@ -19,6 +19,14 @@ port_wrapper::operator boost::uint16_t () const {
   uri::uri uri_;
   request_.get_uri(uri_);
   optional<boost::uint16_t> optional_port = port_us(uri_);
+  if (!optional_port) {
+    std::string scheme_ = scheme(uri_);
+    if (scheme_ == "http") {
+      return 80u;
+    } else if (scheme_ == "https") {
+      return 443u;
+    }
+  }
   return optional_port ? *optional_port : 80u;
 }
 

diff --git a/boost/network/protocol/http/response/response.ipp b/boost/network/protocol/http/response/response.ipp
@@ -23,7 +23,7 @@ struct response_pimpl {
     promise<std::string> destination_promise;
     destination_promise.set_value(destination);
     unique_future<std::string> tmp_future = destination_promise.get_future();
-    destination_future_ = move(tmp_future);
+    destination_future_ = boost::move(tmp_future);
   }
 
   void get_destination(std::string &destination) {
@@ -38,7 +38,7 @@ struct response_pimpl {
     promise<std::string> source_promise;
     source_promise.set_value(source);
     unique_future<std::string> tmp_future = source_promise.get_future();
-    source_future_ = move(tmp_future);
+    source_future_ = boost::move(tmp_future);
   }
 
   void get_source(std::string &source) {
@@ -66,7 +66,7 @@ struct response_pimpl {
           headers_promise.get_future();
       std::multimap<std::string, std::string>().swap(added_headers_);
       std::set<std::string>().swap(removed_headers_);
-      headers_future_ = move(tmp);
+      headers_future_ = boost::move(tmp);
     }
   }
 
@@ -102,7 +102,7 @@ struct response_pimpl {
     promise<std::string> body_promise;
     body_promise.set_value(body);
     unique_future<std::string> tmp_future = body_promise.get_future();
-    body_future_ = move(tmp_future);
+    body_future_ = boost::move(tmp_future);
   }
 
   void append_body(std::string const & data) { /* FIXME: Do something! */ }
@@ -123,7 +123,7 @@ struct response_pimpl {
     promise<boost::uint16_t> status_promise;
     status_promise.set_value(status);
     unique_future<boost::uint16_t> tmp_future = status_promise.get_future();
-    status_future_ = move(tmp_future);
+    status_future_ = boost::move(tmp_future);
   }
 
   void get_status(boost::uint16_t &status) {
@@ -138,7 +138,7 @@ struct response_pimpl {
     promise<std::string> status_message_promise_;
     status_message_promise_.set_value(status_message);
     unique_future<std::string> tmp_future = status_message_promise_.get_future();
-    status_message_future_ = move(tmp_future);
+    status_message_future_ = boost::move(tmp_future);
   }
 
   void get_status_message(std::string &status_message) {
@@ -153,7 +153,7 @@ struct response_pimpl {
     promise<std::string> version_promise;
     version_promise.set_value(version);
     unique_future<std::string> tmp_future = version_promise.get_future();
-    version_future_ = move(tmp_future);
+    version_future_ = boost::move(tmp_future);
   }
 
   void get_version(std::string &version) {
@@ -166,37 +166,37 @@ struct response_pimpl {
 
   void set_source_promise(promise<std::string> &promise_) {
     unique_future<std::string> tmp_future = promise_.get_future();
-    source_future_ = move(tmp_future);
+    source_future_ = boost::move(tmp_future);
   }
 
   void set_destination_promise(promise<std::string> &promise_) {
     unique_future<std::string> tmp_future = promise_.get_future();
-    destination_future_ = move(tmp_future);
+    destination_future_ = boost::move(tmp_future);
   }
 
   void set_headers_promise(promise<std::multimap<std::string, std::string> > &promise_) {
     unique_future<std::multimap<std::string, std::string> > tmp_future = promise_.get_future();
-    headers_future_ = move(tmp_future);
+    headers_future_ = boost::move(tmp_future);
   }
 
   void set_status_promise(promise<boost::uint16_t> &promise_) {
     unique_future<boost::uint16_t> tmp_future = promise_.get_future();
-    status_future_ = move(tmp_future);
+    status_future_ = boost::move(tmp_future);
   }
 
   void set_status_message_promise(promise<std::string> &promise_) {
     unique_future<std::string> tmp_future = promise_.get_future();
-    status_message_future_ = move(tmp_future);
+    status_message_future_ = boost::move(tmp_future);
   }
 
   void set_version_promise(promise<std::string> &promise_) {
     unique_future<std::string> tmp_future = promise_.get_future();
-    version_future_ = move(tmp_future);
+    version_future_ = boost::move(tmp_future);
   }
 
   void set_body_promise(promise<std::string> &promise_) {
     unique_future<std::string> tmp_future = promise_.get_future();
-    body_future_ = move(tmp_future);
+    body_future_ = boost::move(tmp_future);
   }
 
   bool equals(response_pimpl const &other) {