You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2020/04/17 21:19:45 UTC
[mesos] 01/02: Added logging of peer address in LibeventSSLSocket
accept failures.
This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit b5368495a4ccc2438bccb0a50f126e358469bf54
Author: Benjamin Mahler <bm...@apache.org>
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 f23e653..c7a66d3 100644
--- a/3rdparty/libprocess/include/process/address.hpp
+++ b/3rdparty/libprocess/include/process/address.hpp
@@ -365,6 +365,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), length);
+#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 864802d..5f29e16 100644
--- a/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
@@ -573,7 +573,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.
@@ -969,11 +970,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.
@@ -985,7 +981,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);
}
@@ -1135,14 +1131,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;
}
@@ -1151,10 +1141,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;
}
@@ -1174,7 +1166,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;
@@ -1194,16 +1188,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);
@@ -1213,7 +1214,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;
@@ -1256,7 +1257,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);
@@ -1272,7 +1274,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