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:44 UTC
[mesos] branch master updated (898201b -> 9240c4d)
This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 898201b Updated mesos for os/exec.hpp addition to stout.
new b536849 Added logging of peer address in LibeventSSLSocket accept failures.
new 9240c4d Added logging of peer address in OpenSSLSocket accept failures.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
3rdparty/libprocess/include/process/address.hpp | 37 ++++++++++++++++
.../src/posix/libevent/libevent_ssl_socket.cpp | 51 ++++++++++++----------
.../src/posix/libevent/libevent_ssl_socket.hpp | 6 +--
3rdparty/libprocess/src/ssl/openssl_socket.cpp | 23 ++++++----
4 files changed, 82 insertions(+), 35 deletions(-)
[mesos] 01/02: Added logging of peer address in LibeventSSLSocket
accept failures.
Posted by bm...@apache.org.
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
[mesos] 02/02: Added logging of peer address in OpenSSLSocket
accept failures.
Posted by bm...@apache.org.
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 9240c4d3190f7836cb563971e943a4c7899f33f1
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Thu Apr 9 19:42:32 2020 -0400
Added logging of peer address in OpenSSLSocket accept failures.
The caller of OpenSSLSocket::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/72349
---
3rdparty/libprocess/src/ssl/openssl_socket.cpp | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/3rdparty/libprocess/src/ssl/openssl_socket.cpp b/3rdparty/libprocess/src/ssl/openssl_socket.cpp
index a2ec0a3..3f4dab6 100644
--- a/3rdparty/libprocess/src/ssl/openssl_socket.cpp
+++ b/3rdparty/libprocess/src/ssl/openssl_socket.cpp
@@ -734,7 +734,7 @@ Future<ControlFlow<Nothing>> OpenSSLSocketImpl::handle_accept_callback(
if (!peer_address.isSome()) {
SSL_free(accept_ssl);
accept_queue.put(
- Failure("Could not determine peer IP for connection"));
+ Failure("Failed to determine peer IP: " + peer_address.error()));
return Continue();
}
@@ -747,7 +747,8 @@ Future<ControlFlow<Nothing>> OpenSSLSocketImpl::handle_accept_callback(
if (configured.isError()) {
SSL_free(accept_ssl);
accept_queue.put(
- Failure("Could not configure socket: " + configured.error()));
+ Failure("Failed to openssl::configure_socket for " +
+ stringify(*peer_address) + ": " + configured.error()));
return Continue();
}
@@ -773,16 +774,22 @@ Future<ControlFlow<Nothing>> OpenSSLSocketImpl::handle_accept_callback(
return;
}
+ // For verification purposes, we need to grab the address (again).
+ // We grab it up here (rather than down below) so that we can log
+ // it if the `result` is failed.
+ Try<Address> address = network::address(ssl_socket->get());
+
if (result.isFailed()) {
- self->accept_queue.put(Failure(result.failure()));
+ self->accept_queue.put(
+ Failure("Failed to SSL handshake" +
+ (address.isSome() ? " with " + stringify(*address) : "") +
+ ": " + result.failure()));
return;
}
- // For verification purposes, we need to grab the address (again).
- Try<Address> address = network::address(ssl_socket->get());
if (address.isError()) {
self->accept_queue.put(
- Failure("Failed to get address: " + address.error()));
+ Failure("Failed to determine peer IP: " + address.error()));
return;
}
@@ -798,8 +805,8 @@ Future<ControlFlow<Nothing>> OpenSSLSocketImpl::handle_accept_callback(
: Option<net::IP>::none());
if (verify.isError()) {
- VLOG(1) << "Failed accept, verification error: "
- << verify.error();
+ VLOG(1) << "Failed accept for " << *address
+ << ", verification error: " << verify.error();
self->accept_queue.put(Failure(verify.error()));
return;