Cleanup of how we use BIO/BIO_METHODs

Summary:
AsyncSSLSocket's eorAwareBioWrite does some invasive stuff like
reaching into a BIO and replacing its method (and the 'write' funcptr). This
approach won't work with OpenSSL 1.1.0 or BoringSSL due to API changes and
structs being made opaque. This diff adds a layer of wrappers for some BIO
operations. Note that this is still only tested on 1.0.2

Reviewed By: siyengar

Differential Revision: D3338861

fbshipit-source-id: 2ac9318b0df1709873511bfde0fa85d87c5dd29a

diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp
index f91416ed61ac3c0dcff529b329a4165ebcf43719..572d8800a330ceb0449afb76dab6af395321ea7e 100644 (file)
--- a/folly/io/async/AsyncSSLSocket.cpp
+++ b/folly/io/async/AsyncSSLSocket.cpp
@@ -54,6 +54,7 @@ using folly::AsyncSocketException;
 using folly::AsyncSSLSocket;
 using folly::Optional;
 using folly::SSLContext;
+using folly::ssl::OpenSSLUtils;
 
 // We have one single dummy SSL context so that we can implement attach
 // and detach methods in a thread safe fashion without modifying opnessl.
@@ -228,7 +229,8 @@ BIO_METHOD sslWriteBioMethod;
 void* initsslWriteBioMethod(void) {
   memcpy(&sslWriteBioMethod, BIO_s_socket(), sizeof(sslWriteBioMethod));
   // override the bwrite method for MSG_EOR support
-  sslWriteBioMethod.bwrite = AsyncSSLSocket::bioWrite;
+  OpenSSLUtils::setCustomBioWriteMethod(
+      &sslWriteBioMethod, AsyncSSLSocket::bioWrite);
 
   // Note that the sslWriteBioMethod.type and sslWriteBioMethod.name are not
   // set here. openssl code seems to be checking ".type == BIO_TYPE_SOCKET" and
@@ -434,10 +436,10 @@ size_t AsyncSSLSocket::getRawBytesReceived() const {
 void AsyncSSLSocket::invalidState(HandshakeCB* callback) {
   LOG(ERROR) << "AsyncSSLSocket(this=" << this << ", fd=" << fd_
              << ", state=" << int(state_) << ", sslState=" << sslState_ << ", "
-             << "events=" << eventFlags_ << ", server=" << short(server_) << "): "
-             << "sslAccept/Connect() called in invalid "
-             << "state, handshake callback " << handshakeCallback_ << ", new callback "
-             << callback;
+             << "events=" << eventFlags_ << ", server=" << short(server_)
+             << "): " << "sslAccept/Connect() called in invalid "
+             << "state, handshake callback " << handshakeCallback_
+             << ", new callback " << callback;
   assert(!handshakeTimeout_.isScheduled());
   sslState_ = STATE_ERROR;
 
@@ -688,7 +690,7 @@ bool AsyncSSLSocket::setupSSLBio() {
     return false;
   }
 
-  BIO_set_app_data(wb, this);
+  OpenSSLUtils::setBioAppData(wb, this);
   BIO_set_fd(wb, fd_, BIO_NOCLOSE);
   SSL_set_bio(ssl_, wb, wb);
   return true;
@@ -961,16 +963,15 @@ void AsyncSSLSocket::checkForImmediateRead() noexcept {
 void
 AsyncSSLSocket::restartSSLAccept()
 {
-  VLOG(3) << "AsyncSSLSocket::restartSSLAccept() this=" << this << ", fd=" << fd_
-          << ", state=" << int(state_) << ", "
+  VLOG(3) << "AsyncSSLSocket::restartSSLAccept() this=" << this
+          << ", fd=" << fd_ << ", state=" << int(state_) << ", "
           << "sslState=" << sslState_ << ", events=" << eventFlags_;
   DestructorGuard dg(this);
   assert(
     sslState_ == STATE_CACHE_LOOKUP ||
     sslState_ == STATE_ASYNC_PENDING ||
     sslState_ == STATE_ERROR ||
-    sslState_ == STATE_CLOSED
-  );
+    sslState_ == STATE_CLOSED);
   if (sslState_ == STATE_CLOSED) {
     // I sure hope whoever closed this socket didn't delete it already,
     // but this is not strictly speaking an error
@@ -1520,7 +1521,7 @@ int AsyncSSLSocket::bioWrite(BIO* b, const char* in, int inl) {
   msg.msg_iov = &iov;
   msg.msg_iovlen = 1;
 
-  auto appData = BIO_get_app_data(b);
+  auto appData = OpenSSLUtils::getBioAppData(b);
   CHECK(appData);
 
   tsslSock = reinterpret_cast<AsyncSSLSocket*>(appData);
@@ -1535,7 +1536,7 @@ int AsyncSSLSocket::bioWrite(BIO* b, const char* in, int inl) {
       tsslSock->sendSocketMessage(BIO_get_fd(b, nullptr), &msg, flags);
   BIO_clear_retry_flags(b);
   if (!result.exception && result.writeReturn <= 0) {
-    if (BIO_sock_should_retry(result.writeReturn)) {
+    if (OpenSSLUtils::getBioShouldRetryWrite(result.writeReturn)) {
       BIO_set_retry_write(b);
     }
   }

diff --git a/folly/io/async/ssl/OpenSSLUtils.cpp b/folly/io/async/ssl/OpenSSLUtils.cpp
index 4df8b3b027e04c405be720bfe2203ac851acda2f..01aa8548a4ec0d198dd450979f2e37285adc6815 100644 (file)
--- a/folly/io/async/ssl/OpenSSLUtils.cpp
+++ b/folly/io/async/ssl/OpenSSLUtils.cpp
@@ -17,6 +17,7 @@
 #include <folly/ScopeGuard.h>
 #include <folly/portability/Sockets.h>
 
+#include <openssl/bio.h>
 #include <openssl/err.h>
 #include <openssl/rand.h>
 #include <openssl/ssl.h>
@@ -24,6 +25,21 @@
 
 #include <glog/logging.h>
 
+#define OPENSSL_IS_101 (OPENSSL_VERSION_NUMBER >= 0x1000105fL && \
+                         OPENSSL_VERSION_NUMBER < 0x1000200fL)
+#define OPENSSL_IS_102 (OPENSSL_VERSION_NUMBER >= 0x1000200fL && \
+                        OPENSSL_VERSION_NUMBER < 0x10100000L)
+#define OPENSSL_IS_110 (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+
+namespace {
+#if defined(OPENSSL_IS_BORINGSSL)
+// BoringSSL doesn't (as of May 2016) export the equivalent
+// of BIO_sock_should_retry, so this is one way around it :(
+static int boringssl_bio_fd_should_retry(int err);
+#endif
+
+}
+
 namespace folly {
 namespace ssl {
 
@@ -102,5 +118,131 @@ bool OpenSSLUtils::validatePeerCertNames(X509* cert,
   return false;
 }
 
+bool OpenSSLUtils::setCustomBioReadMethod(
+    BIO_METHOD* bioMeth,
+    int (*meth)(BIO*, char*, int)) {
+  bool ret = false;
+#if OPENSSL_IS_110
+  ret = (BIO_meth_set_read(bioMeth, meth) == 1);
+#elif (defined(OPENSSL_IS_BORINGSSL) || OPENSSL_IS_101 || OPENSSL_IS_102)
+  bioMeth->bread = meth;
+  ret = true;
+#endif
+
+  return ret;
+}
+
+bool OpenSSLUtils::setCustomBioWriteMethod(
+    BIO_METHOD* bioMeth,
+    int (*meth)(BIO*, const char*, int)) {
+  bool ret = false;
+#if OPENSSL_IS_110
+  ret = (BIO_meth_set_write(bioMeth, meth) == 1);
+#elif (defined(OPENSSL_IS_BORINGSSL) || OPENSSL_IS_101 || OPENSSL_IS_102)
+  bioMeth->bwrite = meth;
+  ret = true;
+#endif
+
+  return ret;
+}
+
+int OpenSSLUtils::getBioShouldRetryWrite(int r) {
+  int ret = 0;
+#if defined(OPENSSL_IS_BORINGSSL)
+  ret = boringssl_bio_fd_should_retry(r);
+#else
+  ret = BIO_sock_should_retry(r);
+#endif
+  return ret;
+}
+
+void OpenSSLUtils::setBioAppData(BIO* b, void* ptr) {
+#if defined(OPENSSL_IS_BORINGSSL)
+  BIO_set_callback_arg(b, static_cast<char*>(ptr));
+#else
+  BIO_set_app_data(b, ptr);
+#endif
+}
+
+void* OpenSSLUtils::getBioAppData(BIO* b) {
+#if defined(OPENSSL_IS_BORINGSSL)
+  return BIO_get_callback_arg(b);
+#else
+  return BIO_get_app_data(b);
+#endif
+}
+
+void OpenSSLUtils::setCustomBioMethod(BIO* b, BIO_METHOD* meth) {
+#if defined(OPENSSL_IS_BORINGSSL)
+  b->method = meth;
+#else
+  BIO_set(b, meth);
+#endif
+}
+
 } // ssl
 } // folly
+
+namespace {
+#if defined(OPENSSL_IS_BORINGSSL)
+
+static int boringssl_bio_fd_non_fatal_error(int err) {
+  if (
+#ifdef EWOULDBLOCK
+    err == EWOULDBLOCK ||
+#endif
+#ifdef WSAEWOULDBLOCK
+    err == WSAEWOULDBLOCK ||
+#endif
+#ifdef ENOTCONN
+    err == ENOTCONN ||
+#endif
+#ifdef EINTR
+    err == EINTR ||
+#endif
+#ifdef EAGAIN
+    err == EAGAIN ||
+#endif
+#ifdef EPROTO
+    err == EPROTO ||
+#endif
+#ifdef EINPROGRESS
+    err == EINPROGRESS ||
+#endif
+#ifdef EALREADY
+    err == EALREADY ||
+#endif
+    0) {
+    return 1;
+  }
+  return 0;
+}
+
+#if defined(OPENSSL_WINDOWS)
+
+#include <io.h>
+#pragma warning(push, 3)
+#include <windows.h>
+#pragma warning(pop)
+
+int boringssl_bio_fd_should_retry(int i) {
+  if (i == -1) {
+    return boringssl_bio_fd_non_fatal_error((int)GetLastError());
+  }
+  return 0;
+}
+
+#else // !OPENSSL_WINDOWS
+
+#include <unistd.h>
+int boringssl_bio_fd_should_retry(int i) {
+  if (i == -1) {
+    return boringssl_bio_fd_non_fatal_error(errno);
+  }
+  return 0;
+}
+#endif // OPENSSL_WINDOWS
+
+#endif // OEPNSSL_IS_BORINGSSL
+
+}

diff --git a/folly/io/async/ssl/OpenSSLUtils.h b/folly/io/async/ssl/OpenSSLUtils.h
index 7f211bfd1e76c43a79db6ce1db25cdaf6bd65c1f..252785e358d0f2524becd8201e7ac12894bc0e32 100644 (file)
--- a/folly/io/async/ssl/OpenSSLUtils.h
+++ b/folly/io/async/ssl/OpenSSLUtils.h
@@ -54,6 +54,21 @@ class OpenSSLUtils {
   static bool getPeerAddressFromX509StoreCtx(X509_STORE_CTX* ctx,
                                              sockaddr_storage* addrStorage,
                                              socklen_t* addrLen);
+
+  /**
+  * Wrappers for BIO operations that may be different across different
+  * versions/flavors of OpenSSL (including forks like BoringSSL)
+  */
+  static bool setCustomBioReadMethod(
+      BIO_METHOD* bioMeth,
+      int (*meth)(BIO*, char*, int));
+  static bool setCustomBioWriteMethod(
+      BIO_METHOD* bioMeth,
+      int (*meth)(BIO*, const char*, int));
+  static int getBioShouldRetryWrite(int ret);
+  static void setBioAppData(BIO* b, void* ptr);
+  static void* getBioAppData(BIO* b);
+  static void setCustomBioMethod(BIO*, BIO_METHOD*);
 };
 
 } // ssl