From 73023291e9a22a2467288c6f71db63af71be0898 Mon Sep 17 00:00:00 2001 From: Willi Goesgens Date: Thu, 29 Jan 2015 09:52:08 +0100 Subject: [PATCH] Add SSL error handling inspired by curl; make shure the user gets the message. --- lib/SimpleHttpClient/ClientConnection.cpp | 1 + .../GeneralClientConnection.h | 14 +++ lib/SimpleHttpClient/SimpleHttpClient.cpp | 10 +- lib/SimpleHttpClient/SimpleHttpClient.h | 6 ++ lib/SimpleHttpClient/SslClientConnection.cpp | 99 +++++++++++++++---- 5 files changed, 111 insertions(+), 19 deletions(-) diff --git a/lib/SimpleHttpClient/ClientConnection.cpp b/lib/SimpleHttpClient/ClientConnection.cpp index fa858ff9c2..f1b78f4937 100644 --- a/lib/SimpleHttpClient/ClientConnection.cpp +++ b/lib/SimpleHttpClient/ClientConnection.cpp @@ -122,6 +122,7 @@ bool ClientConnection::connectSocket () { _socket = _endpoint->connect(_connectTimeout, _requestTimeout); if (! TRI_isvalidsocket(_socket)) { + _errorDetails = std::string("failed to connect : ") + std::string(strerror(errno)); return false; } diff --git a/lib/SimpleHttpClient/GeneralClientConnection.h b/lib/SimpleHttpClient/GeneralClientConnection.h index 3420a7c9c2..50b5f11342 100644 --- a/lib/SimpleHttpClient/GeneralClientConnection.h +++ b/lib/SimpleHttpClient/GeneralClientConnection.h @@ -167,6 +167,14 @@ namespace triagens { bool handleRead (double, triagens::basics::StringBuffer&, bool& connectionClosed); +//////////////////////////////////////////////////////////////////////////////// +/// @brief return the endpoint +//////////////////////////////////////////////////////////////////////////////// + + const std::string& getErrorDetails () const { + return _errorDetails; + } + // ----------------------------------------------------------------------------- // --SECTION-- protected virtual methods // ----------------------------------------------------------------------------- @@ -215,6 +223,12 @@ namespace triagens { protected: +//////////////////////////////////////////////////////////////////////////////// +/// @brief details to errors +//////////////////////////////////////////////////////////////////////////////// + + std::string _errorDetails; + //////////////////////////////////////////////////////////////////////////////// /// @brief endpoint to connect to //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/SimpleHttpClient/SimpleHttpClient.cpp b/lib/SimpleHttpClient/SimpleHttpClient.cpp index abff05c548..af05d33060 100644 --- a/lib/SimpleHttpClient/SimpleHttpClient.cpp +++ b/lib/SimpleHttpClient/SimpleHttpClient.cpp @@ -288,7 +288,11 @@ namespace triagens { TRI_ASSERT(_connection != nullptr); if (! _connection->connect()) { - setErrorMessage("Could not connect to '" + _connection->getEndpoint()->getSpecification() + "'", errno); + setErrorMessage("Could not connect to '" + + _connection->getEndpoint()->getSpecification() + + "' '" + + _connection->getErrorDetails() + + "' '"); _state = DEAD; } else { @@ -348,7 +352,9 @@ namespace triagens { case IN_CONNECT: default: { _result->setResultType(SimpleHttpResult::COULD_NOT_CONNECT); - setErrorMessage("Could not connect"); + if (!haveErrorMessage()) { + setErrorMessage("Could not connect"); + } break; } } diff --git a/lib/SimpleHttpClient/SimpleHttpClient.h b/lib/SimpleHttpClient/SimpleHttpClient.h index 9357581d5a..2f83ada2e3 100644 --- a/lib/SimpleHttpClient/SimpleHttpClient.h +++ b/lib/SimpleHttpClient/SimpleHttpClient.h @@ -186,6 +186,12 @@ namespace triagens { } } +//////////////////////////////////////////////////////////////////////////////// +/// @brief checks whether an error message is already there +//////////////////////////////////////////////////////////////////////////////// + + bool haveErrorMessage () { return _errorMessage.size() > 0;} + private: //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/SimpleHttpClient/SslClientConnection.cpp b/lib/SimpleHttpClient/SslClientConnection.cpp index d2c1103014..391fdfe306 100644 --- a/lib/SimpleHttpClient/SslClientConnection.cpp +++ b/lib/SimpleHttpClient/SslClientConnection.cpp @@ -146,17 +146,21 @@ bool SslClientConnection::connectSocket () { _socket = _endpoint->connect(_connectTimeout, _requestTimeout); if (! TRI_isvalidsocket(_socket) || _ctx == nullptr) { + _errorDetails = std::string("failed to connect : ") + std::string(strerror(errno)); return false; } _ssl = SSL_new(_ctx); if (_ssl == nullptr) { + _errorDetails = std::string("failed to create ssl context"); _endpoint->disconnect(); TRI_invalidatesocket(&_socket); return false; } if (SSL_set_fd(_ssl, (int) TRI_get_fd_or_handle_of_socket(_socket)) != 1) { + _errorDetails = std::string("SSL: failed to create context ") + + ERR_error_string(ERR_get_error(), NULL); _endpoint->disconnect(); SSL_free(_ssl); _ssl = nullptr; @@ -166,8 +170,49 @@ bool SslClientConnection::connectSocket () { SSL_set_verify(_ssl, SSL_VERIFY_NONE, NULL); + ERR_clear_error(); int ret = SSL_connect(_ssl); if (ret != 1) { + int errorDetail; + int certError; + + errorDetail = SSL_get_error(_ssl, ret); + if ( (errorDetail == SSL_ERROR_WANT_READ) || + (errorDetail == SSL_ERROR_WANT_WRITE)) { + return true; + } + errorDetail = ERR_get_error(); /* Gets the earliest error code from the + thread's error queue and removes the + entry. */ + switch(errorDetail) { + case 0x1407E086: + /* 1407E086: + SSL routines: + SSL2_SET_CERTIFICATE: + certificate verify failed */ + /* fall-through */ + case 0x14090086: + /* 14090086: + SSL routines: + SSL3_GET_SERVER_CERTIFICATE: + certificate verify failed */ + + certError = SSL_get_verify_result(_ssl); + if(certError != X509_V_OK) { + _errorDetails = std::string("SSL: certificate problem: ") + + X509_verify_cert_error_string(certError); + } + else { + _errorDetails = std::string("SSL: certificate problem, verify that the CA cert is OK."); + } + break; + default: + char errorBuffer[256]; + ERR_error_string_n(errorDetail, errorBuffer, sizeof(errorBuffer)); + _errorDetails = std::string("SSL: ") + errorBuffer; + break; + } + _endpoint->disconnect(); SSL_free(_ssl); _ssl = 0; @@ -238,28 +283,42 @@ bool SslClientConnection::writeClientConnection (void* buffer, size_t length, si if (_ssl == 0) { return false; } - + int errorDetail; int written = SSL_write(_ssl, buffer, (int) length); - switch (SSL_get_error(_ssl, written)) { - case SSL_ERROR_NONE: - *bytesWritten = written; + int err = SSL_get_error(_ssl, written); + switch (err) { + case SSL_ERROR_NONE: + *bytesWritten = written; - return true; + return true; - case SSL_ERROR_ZERO_RETURN: - SSL_shutdown(_ssl); - break; + case SSL_ERROR_ZERO_RETURN: + SSL_shutdown(_ssl); + break; - case SSL_ERROR_WANT_READ: - case SSL_ERROR_WANT_WRITE: - case SSL_ERROR_WANT_CONNECT: - case SSL_ERROR_SYSCALL: - default: { - /* fall through */ - } - } + case SSL_ERROR_WANT_READ: + case SSL_ERROR_WANT_WRITE: + case SSL_ERROR_WANT_CONNECT: + break; + case SSL_ERROR_SYSCALL: + _errorDetails = std::string("SSL: while writing: SYSCALL returned errno = ") + + std::to_string(errno) + std::string(" - ") + strerror(errno); + break; + case SSL_ERROR_SSL: + /* A failure in the SSL library occurred, usually a protocol error. + The OpenSSL error queue contains more information on the error. */ + errorDetail = ERR_get_error(); + char errorBuffer[256]; + ERR_error_string_n(errorDetail, errorBuffer, sizeof(errorBuffer)); + _errorDetails = std::string("SSL: while writing: ") + errorBuffer; + + break; + default: + /* a true error */ + _errorDetails = std::string("SSL: while writing: error ") + std::to_string(err); +} - return false; +return false; } //////////////////////////////////////////////////////////////////////////////// @@ -308,6 +367,12 @@ again: case SSL_ERROR_WANT_CONNECT: case SSL_ERROR_SYSCALL: default: + int errorDetail = ERR_get_error(); + char errorBuffer[256]; + ERR_error_string_n(errorDetail, errorBuffer, sizeof(errorBuffer)); + _errorDetails = std::string("SSL: while reading: error '") + std::to_string(errno) + + std::string("' - ") + errorBuffer; + /* unexpected */ connectionClosed = true; return false;