From ae1594eaf33e2cfb529190507405acf26e583200 Mon Sep 17 00:00:00 2001 From: Anirudh Ramachandran Date: Tue, 6 Jun 2017 18:24:35 -0700 Subject: [PATCH] Add a version of async_test using openssl 1.1.0 Summary: Now that it's possible, let's add some 1.1.0 tests to avoid regressing 1.1.0 support Reviewed By: yfeldblum Differential Revision: D5167246 fbshipit-source-id: ba12414504131697d4e0757c9c340a66f810acd4 --- folly/io/async/test/AsyncSSLSocketTest.cpp | 48 ++++++++++++++++++---- folly/io/async/test/AsyncSSLSocketTest.h | 6 ++- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/folly/io/async/test/AsyncSSLSocketTest.cpp b/folly/io/async/test/AsyncSSLSocketTest.cpp index 36a68027..1ad8caa6 100644 --- a/folly/io/async/test/AsyncSSLSocketTest.cpp +++ b/folly/io/async/test/AsyncSSLSocketTest.cpp @@ -409,6 +409,7 @@ class NextProtocolTest : public testing::TestWithParam { } void expectProtocol(const std::string& proto) { + expectHandshakeSuccess(); EXPECT_NE(client->nextProtoLength, 0); EXPECT_EQ(client->nextProtoLength, server->nextProtoLength); EXPECT_EQ( @@ -419,6 +420,7 @@ class NextProtocolTest : public testing::TestWithParam { } void expectNoProtocol() { + expectHandshakeSuccess(); EXPECT_EQ(client->nextProtoLength, 0); EXPECT_EQ(server->nextProtoLength, 0); EXPECT_EQ(client->nextProto, nullptr); @@ -426,6 +428,7 @@ class NextProtocolTest : public testing::TestWithParam { } void expectProtocolType() { + expectHandshakeSuccess(); if (GetParam().first == SSLContext::NextProtocolType::ANY && GetParam().second == SSLContext::NextProtocolType::ANY) { EXPECT_EQ(client->protocolType, server->protocolType); @@ -438,10 +441,25 @@ class NextProtocolTest : public testing::TestWithParam { } void expectProtocolType(NextProtocolTypePair expected) { + expectHandshakeSuccess(); EXPECT_EQ(client->protocolType, expected.first); EXPECT_EQ(server->protocolType, expected.second); } + void expectHandshakeSuccess() { + EXPECT_FALSE(client->except.hasValue()) + << "client handshake error: " << client->except->what(); + EXPECT_FALSE(server->except.hasValue()) + << "server handshake error: " << server->except->what(); + } + + void expectHandshakeError() { + EXPECT_TRUE(client->except.hasValue()) + << "Expected client handshake error!"; + EXPECT_TRUE(server->except.hasValue()) + << "Expected server handshake error!"; + } + EventBase eventBase; std::shared_ptr clientCtx{std::make_shared()}; std::shared_ptr serverCtx{std::make_shared()}; @@ -505,27 +523,32 @@ TEST_P(NextProtocolTest, NpnTestNoOverlap) { clientCtx->setAdvertisedNextProtocols({"blub"}, GetParam().first); serverCtx->setAdvertisedNextProtocols({"foo", "bar", "baz"}, GetParam().second); - connect(); if (GetParam().first == SSLContext::NextProtocolType::ALPN || GetParam().second == SSLContext::NextProtocolType::ALPN) { // This is arguably incorrect behavior since RFC7301 states an ALPN protocol - // mismatch should result in a fatal alert, but this is OpenSSL's current - // behavior and we want to know if it changes. + // mismatch should result in a fatal alert, but this is the current behavior + // on all OpenSSL versions/variants, and we want to know if it changes. expectNoProtocol(); } -#if defined(OPENSSL_IS_BORINGSSL) - // BoringSSL also doesn't fatal on mismatch but behaves slightly differently - // from OpenSSL 1.0.2h+ - it doesn't select a protocol if both ends support - // NPN *and* ALPN +#if FOLLY_OPENSSL_IS_110 || defined(OPENSSL_IS_BORINGSSL) else if ( GetParam().first == SSLContext::NextProtocolType::ANY && GetParam().second == SSLContext::NextProtocolType::ANY) { +# if FOLLY_OPENSSL_IS_110 + // OpenSSL 1.1.0 sends a fatal alert on mismatch, which is probavbly the + // correct behavior per RFC7301 + expectHandshakeError(); +# else + // BoringSSL also doesn't fatal on mismatch but behaves slightly differently + // from OpenSSL 1.0.2h+ - it doesn't select a protocol if both ends support + // NPN *and* ALPN expectNoProtocol(); +# endif } #endif - else { + else { expectProtocol("blub"); expectProtocolType( {SSLContext::NextProtocolType::NPN, SSLContext::NextProtocolType::NPN}); @@ -1647,9 +1670,13 @@ TEST(AsyncSSLSocketTest, ConnEOFErrorString) { socket->close(); handshakeCallback.waitForHandshake(); +#if FOLLY_OPENSSL_IS_110 + EXPECT_NE( + handshakeCallback.errorString_.find("Network error"), std::string::npos); +#else EXPECT_NE( handshakeCallback.errorString_.find("Connection EOF"), std::string::npos); - EXPECT_NE(handshakeCallback.errorString_.find("EOF"), std::string::npos); +#endif } TEST(AsyncSSLSocketTest, ConnOpenSSLErrorString) { @@ -1675,6 +1702,9 @@ TEST(AsyncSSLSocketTest, ConnOpenSSLErrorString) { EXPECT_NE( handshakeCallback.errorString_.find("ENCRYPTED_LENGTH_TOO_LONG"), std::string::npos); +#elif FOLLY_OPENSSL_IS_110 + EXPECT_NE(handshakeCallback.errorString_.find("packet length too long"), + std::string::npos); #else EXPECT_NE(handshakeCallback.errorString_.find("unknown protocol"), std::string::npos); diff --git a/folly/io/async/test/AsyncSSLSocketTest.h b/folly/io/async/test/AsyncSSLSocketTest.h index d2acbf43..816971d6 100644 --- a/folly/io/async/test/AsyncSSLSocketTest.h +++ b/folly/io/async/test/AsyncSSLSocketTest.h @@ -968,6 +968,7 @@ class NpnClient : const unsigned char* nextProto; unsigned nextProtoLength; SSLContext::NextProtocolType protocolType; + folly::Optional except; private: void handshakeSuc(AsyncSSLSocket*) noexcept override { @@ -977,7 +978,7 @@ class NpnClient : void handshakeErr( AsyncSSLSocket*, const AsyncSocketException& ex) noexcept override { - ADD_FAILURE() << "client handshake error: " << ex.what(); + except = ex; } void writeSuccess() noexcept override { socket_->close(); @@ -1004,6 +1005,7 @@ class NpnServer : const unsigned char* nextProto; unsigned nextProtoLength; SSLContext::NextProtocolType protocolType; + folly::Optional except; private: void handshakeSuc(AsyncSSLSocket*) noexcept override { @@ -1013,7 +1015,7 @@ class NpnServer : void handshakeErr( AsyncSSLSocket*, const AsyncSocketException& ex) noexcept override { - ADD_FAILURE() << "server handshake error: " << ex.what(); + except = ex; } void getReadBuffer(void** /* bufReturn */, size_t* lenReturn) override { *lenReturn = 0; -- 2.34.1