From 54c71ab51b6c8d94492168e9cf6cf6045d7794f3 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Mon, 21 Jan 2019 14:01:53 +0100 Subject: Fix OpenSSLContext to work correctly with OpenSSL 1.1.1 The previous code only worked with 1.1.0j or older. Now the code works with 1.1.0j and OpenSSL 1.1.1. Adjusted ClientServerTest to be more graceful in case of errors, i.e. failing tests instead of crashing. Test-Information: Tested that without the changes, the tests pass with OpenSSL 1.1.0j and test fail or crash with OpenSSL 1.1.1 and OpenSSL 1.1.1a. Tested that with the changes, the tests pass with OpenSSL 1.1.0j, OpenSSL 1.1.1, and OpenSSL 1.1.1a. Tested on macOS 10.14.2 with system clang. Change-Id: Ic63774049727f6d949153166f63a8545e9a24892 diff --git a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp index e9889bc..5692e74 100644 --- a/Swiften/TLS/OpenSSL/OpenSSLContext.cpp +++ b/Swiften/TLS/OpenSSL/OpenSSLContext.cpp @@ -229,6 +229,7 @@ void OpenSSLContext::doAccept() { onConnected(); // The following call is important so the client knowns the handshake is finished. sendPendingDataToNetwork(); + sendPendingDataToApplication(); break; } case SSL_ERROR_WANT_READ: @@ -254,6 +255,9 @@ void OpenSSLContext::doConnect() { //const char* comp = SSL_get_current_compression(handle_.get()); //std::cout << "Compression: " << SSL_COMP_get_name(comp) << std::endl; onConnected(); + // The following is needed since OpenSSL 1.1.1 for the server to be able to calculate the + // TLS finish message. + sendPendingDataToNetwork(); break; } case SSL_ERROR_WANT_READ: diff --git a/Swiften/TLS/UnitTest/ClientServerTest.cpp b/Swiften/TLS/UnitTest/ClientServerTest.cpp index 24bd7c5..a356dcf 100644 --- a/Swiften/TLS/UnitTest/ClientServerTest.cpp +++ b/Swiften/TLS/UnitTest/ClientServerTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2018 Isode Limited. + * Copyright (c) 2010-2019 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -408,7 +408,6 @@ class TLSEventToSafeByteArrayVisitor : public boost::static_visitor { @@ -459,6 +458,23 @@ class TLSClientServerEventHistory { } } + template + boost::optional getEvent(const std::string& peer, size_t number = 0) { + for (const auto& pair : events) { + if (pair.first == peer) { + if (pair.second.type() == typeid(TLSEventType)) { + if (number == 0) { + return boost::optional(boost::get(pair.second)); + } + else { + number--; + } + } + } + } + return {}; + } + private: void connectContext(const std::string& name, TLSContext* context) { connections_.push_back(context->onDataForNetwork.connect([=](const SafeByteArray& data) { @@ -602,12 +618,12 @@ TEST(ClientServerTest, testClientServerBasicCommunication) { clientContext->handleDataFromApplication(createSafeByteArray("This is a test message from the client.")); serverContext->handleDataFromApplication(createSafeByteArray("This is a test message from the server.")); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "server" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "client" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); + auto firstMessageFromClient = events.getEvent("server"); + ASSERT_EQ(true, firstMessageFromClient.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(firstMessageFromClient->data)); + auto firstMessageFromServer = events.getEvent("client"); + ASSERT_EQ(true, firstMessageFromServer.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(firstMessageFromServer->data)); } TEST(ClientServerTest, testClientServerBasicCommunicationEncryptedPrivateKeyRightPassword) { @@ -632,12 +648,12 @@ TEST(ClientServerTest, testClientServerBasicCommunicationEncryptedPrivateKeyRigh clientContext->handleDataFromApplication(createSafeByteArray("This is a test message from the client.")); serverContext->handleDataFromApplication(createSafeByteArray("This is a test message from the server.")); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "server" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "client" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); + auto firstMessageFromClient = events.getEvent("server"); + ASSERT_EQ(true, firstMessageFromClient.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(firstMessageFromClient->data)); + auto firstMessageFromServer = events.getEvent("client"); + ASSERT_EQ(true, firstMessageFromServer.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(firstMessageFromServer->data)); } TEST(ClientServerTest, testClientServerBasicCommunicationWithChainedCert) { @@ -739,14 +755,15 @@ TEST(ClientServerTest, testClientServerSNIRequestedHostAvailable) { clientContext->handleDataFromApplication(createSafeByteArray("This is a test message from the client.")); serverContext->handleDataFromApplication(createSafeByteArray("This is a test message from the server.")); - ASSERT_EQ("This is a test message from the client.", safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "server" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); - ASSERT_EQ("This is a test message from the server.", safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "client" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); - ASSERT_EQ("/CN=montague.example", boost::get(events.events[5].second).chain[0]->getSubjectName()); + auto firstMessageFromClient = events.getEvent("server"); + ASSERT_EQ(true, firstMessageFromClient.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(firstMessageFromClient->data)); + auto firstMessageFromServer = events.getEvent("client"); + ASSERT_EQ(true, firstMessageFromServer.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(firstMessageFromServer->data)); + + ASSERT_EQ("/CN=montague.example", events.getEvent("client")->chain[0]->getSubjectName()); } TEST(ClientServerTest, testClientServerSNIRequestedHostUnavailable) { @@ -825,12 +842,12 @@ TEST(ClientServerTest, testClientServerBasicCommunicationWith2048BitDHParams) { clientContext->handleDataFromApplication(createSafeByteArray("This is a test message from the client.")); serverContext->handleDataFromApplication(createSafeByteArray("This is a test message from the server.")); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "server" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "client" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); + auto firstMessageFromClient = events.getEvent("server"); + ASSERT_EQ(true, firstMessageFromClient.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(firstMessageFromClient->data)); + auto firstMessageFromServer = events.getEvent("client"); + ASSERT_EQ(true, firstMessageFromServer.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(firstMessageFromServer->data)); } TEST(ClientServerTest, testClientServerBasicCommunicationWith1024BitDHParams) { @@ -857,10 +874,10 @@ TEST(ClientServerTest, testClientServerBasicCommunicationWith1024BitDHParams) { clientContext->handleDataFromApplication(createSafeByteArray("This is a test message from the client.")); serverContext->handleDataFromApplication(createSafeByteArray("This is a test message from the server.")); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "server" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); - ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(boost::apply_visitor(TLSEventToSafeByteArrayVisitor(), std::find_if(events.events.begin(), events.events.end(), [](std::pair& event){ - return event.first == "client" && (event.second.type() == typeid(TLSDataForApplication)); - })->second))); + auto firstMessageFromClient = events.getEvent("server"); + ASSERT_EQ(true, firstMessageFromClient.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the client.")), safeByteArrayToString(firstMessageFromClient->data)); + auto firstMessageFromServer = events.getEvent("client"); + ASSERT_EQ(true, firstMessageFromServer.is_initialized()); + ASSERT_EQ(safeByteArrayToString(createSafeByteArray("This is a test message from the server.")), safeByteArrayToString(firstMessageFromServer->data)); } -- cgit v0.10.2-6-g49f6