From: Anirudh Ramachandran Date: Wed, 6 Jul 2016 16:57:59 +0000 (-0700) Subject: Cleanup of how we use BIO/BIO_METHODs X-Git-Tag: 2016.07.26~74 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=57cdfdf743fd8498680094964e16a81e28858ffe;p=folly.git 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 f91416ed..572d8800 100644 --- 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(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 4df8b3b0..01aa8548 100644 --- a/folly/io/async/ssl/OpenSSLUtils.cpp +++ b/folly/io/async/ssl/OpenSSLUtils.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -24,6 +25,21 @@ #include +#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(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 +#pragma warning(push, 3) +#include +#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 +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 7f211bfd..252785e3 100644 --- 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