This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch 1.9.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit e43ec1b3652aa3c2eda8644ec0a05ac011bd9dbe Author: Benjamin Mahler <[email protected]> AuthorDate: Thu Apr 9 19:36:31 2020 -0400 Added logging of peer address in LibeventSSLSocket accept failures. The caller of LibeventSSLSocket::accept() cannot see who tried to connect when accept fails, since the accepted socket is not returned. This adds logging of the peer address when the SSL handshake fails, in order to improve debugging. Review: https://reviews.apache.org/r/72348 --- 3rdparty/libprocess/include/process/address.hpp | 37 ++++++++++++++++ .../src/posix/libevent/libevent_ssl_socket.cpp | 51 ++++++++++++---------- .../src/posix/libevent/libevent_ssl_socket.hpp | 6 +-- 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/3rdparty/libprocess/include/process/address.hpp b/3rdparty/libprocess/include/process/address.hpp index 7494980..365e03a 100644 --- a/3rdparty/libprocess/include/process/address.hpp +++ b/3rdparty/libprocess/include/process/address.hpp @@ -312,6 +312,43 @@ public: } } + // The `length` is the size of the data pointed to by `struct sockaddr`. + static Try<Address> create( + const sockaddr* address, + size_t length) + { + switch (address->sa_family) { +#ifndef __WINDOWS__ + case AF_UNIX: + // We need to know the length in addition to the address, to + // distinguish between e.g. an unnamed socket and an abstract + // socket whose name is a single null byte. + if (length > sizeof(struct sockaddr_un)) { + return Error("Invalid size for AF_UNIX sockaddr: " + + stringify(length) + " actual vs " + + stringify(sizeof(struct sockaddr_un)) + " expected"); + } + return unix::Address(*((const sockaddr_un*)address)); +#endif // __WINDOWS__ + case AF_INET: + if (length < sizeof(struct sockaddr_in)) { + return Error("Invalid size for AF_INET sockaddr: " + + stringify(length) + " actual vs " + + stringify(sizeof(struct sockaddr_in)) + " expected"); + } + return inet4::Address(*((const sockaddr_in*)address)); + case AF_INET6: + if (length < sizeof(struct sockaddr_in6)) { + return Error("Invalid size for AF_INET6 sockaddr: " + + stringify(length) + " actual vs " + + stringify(sizeof(struct sockaddr_in6)) + " expected"); + } + return inet6::Address(*((const sockaddr_in6*)address)); + default: + return Error("Unsupported family: " + stringify(address->sa_family)); + } + } + // Helper constructor for converting an `inet::Address`. Address(const inet::Address& address) : Address([](const Try<Address>& address) { diff --git a/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp index dcb6d8e..d1f00ed 100644 --- a/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp +++ b/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp @@ -571,7 +571,8 @@ Future<Nothing> LibeventSSLSocketImpl::connect( if (address.family() == Address::Family::INET4 || address.family() == Address::Family::INET6) { - inet::Address inetAddress = network::convert<inet::Address>(address).get(); + inet::Address inetAddress = + CHECK_NOTERROR(network::convert<inet::Address>(address)); // Determine the 'peer_ip' from the address we're connecting to in // order to properly verify the certificate later. @@ -967,11 +968,6 @@ Try<Nothing> LibeventSSLSocketImpl::listen(int backlog) #endif // __WINDOWS__ if (impl != nullptr) { - Try<net::IP> ip = net::IP::create(*addr); - if (ip.isError()) { - VLOG(2) << "Could not convert sockaddr to net::IP: " << ip.error(); - } - // We pass the 'listener' into the 'AcceptRequest' because // this function could be executed before 'this->listener' // is set. @@ -983,7 +979,7 @@ Try<Nothing> LibeventSSLSocketImpl::listen(int backlog) // Windows. int_fd(socket), listener, - ip.isSome() ? Option<net::IP>(ip.get()) : None()); + CHECK_NOTERROR(network::Address::create(addr, addr_length))); impl->accept_callback(request); } @@ -1133,14 +1129,8 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) // Set up SSL object. SSL* ssl = SSL_new(openssl::context()); if (ssl == nullptr) { - request->promise.fail("Accept failed, SSL_new"); - delete request; - return; - } - - Try<Address> peer_address = network::peer(request->socket); - if (!peer_address.isSome()) { - request->promise.fail("Could not determine peer IP for connection."); + // TODO(bmahler): Log the error reason. + request->promise.fail("Failed to SSL_new"); delete request; return; } @@ -1149,10 +1139,12 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) // mode, but we still pass the correct peer address to enable modules to // implement application-level logic in the future. Try<Nothing> configured = openssl::configure_socket( - ssl, openssl::Mode::SERVER, peer_address.get(), None()); + ssl, openssl::Mode::SERVER, request->address, None()); if (configured.isError()) { - request->promise.fail("Could not configure socket: " + configured.error()); + request->promise.fail( + "Failed to openssl::configure_socket" + " for " + stringify(request->address) + ": " + configured.error()); delete request; return; } @@ -1172,7 +1164,9 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) BEV_OPT_THREADSAFE); if (bev == nullptr) { - request->promise.fail("Accept failed: bufferevent_openssl_socket_new"); + // TODO(bmahler): Log the error reason. + request->promise.fail("Failed to bufferevent_openssl_socket_new" + " for " + stringify(request->address)); SSL_free(ssl); delete request; return; @@ -1192,16 +1186,23 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) reinterpret_cast<AcceptRequest*>(CHECK_NOTNULL(arg)); if (events & BEV_EVENT_EOF) { - request->promise.fail("Failed accept: connection closed"); + request->promise.fail( + "Connection closed for " + stringify(request->address)); } else if (events & BEV_EVENT_CONNECTED) { SSL* ssl = bufferevent_openssl_get_ssl(bev); CHECK_NOTNULL(ssl); + Try<net::IP> ip = net::IP::create(request->address); + Try<Nothing> verify = openssl::verify( - ssl, Mode::SERVER, None(), request->ip); + ssl, + Mode::SERVER, + None(), + ip.isSome() ? Option<net::IP>(*ip) : None()); if (verify.isError()) { - VLOG(1) << "Failed accept, verification error: " << verify.error(); + VLOG(1) << "Failed accept for " << request->address + << ", verification error: " << verify.error(); request->promise.fail(verify.error()); SSL_free(ssl); bufferevent_free(bev); @@ -1211,7 +1212,7 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) Try<Nothing> close = os::close(request->socket); if (close.isError()) { LOG(FATAL) - << "Failed to close socket " << stringify(request->socket) + << "Failed to close socket " << request->socket << ": " << close.error(); } delete request; @@ -1254,7 +1255,8 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) } // Fail the accept request and log the error. - VLOG(1) << "Socket error: " << stream.str(); + VLOG(1) << "Failed accept for " << request->address + << ": " << stream.str(); SSL* ssl = bufferevent_openssl_get_ssl(CHECK_NOTNULL(bev)); SSL_free(ssl); @@ -1270,7 +1272,8 @@ void LibeventSSLSocketImpl::accept_SSL_callback(AcceptRequest* request) << ": " << close.error(); } request->promise.fail( - "Failed accept: connection error: " + stream.str()); + "Failed to complete SSL connection for " + + stringify(request->address) + ": " + stream.str()); } delete request; diff --git a/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp b/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp index 7bcc66f..153ceb3 100644 --- a/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp +++ b/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp @@ -77,16 +77,16 @@ private: AcceptRequest( int_fd _socket, evconnlistener* _listener, - const Option<net::IP>& _ip) + const Address& _address) : peek_event(nullptr), listener(_listener), socket(_socket), - ip(_ip) {} + address(_address) {} event* peek_event; Promise<std::shared_ptr<SocketImpl>> promise; evconnlistener* listener; int_fd socket; - Option<net::IP> ip; + Address address; }; struct RecvRequest
