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 22:11:48 UTC

[mesos] branch 1.9.x updated (b84b0a4 -> 4d2db68)

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a change to branch 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from b84b0a4  Added MESOS-10096 to the 1.9.1 CHANGELOG.
     new e43ec1b  Added logging of peer address in LibeventSSLSocket accept failures.
     new 4d2db68  Added MESOS-10112 to the 1.9.1 CHANGELOG.

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 +--
 CHANGELOG                                          |  1 +
 4 files changed, 68 insertions(+), 27 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 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit e43ec1b3652aa3c2eda8644ec0a05ac011bd9dbe
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 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


[mesos] 02/02: Added MESOS-10112 to the 1.9.1 CHANGELOG.

Posted by bm...@apache.org.
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 4d2db68d5f7c488c1511ba77e8fc83af280e3479
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Fri Apr 17 18:10:56 2020 -0400

    Added MESOS-10112 to the 1.9.1 CHANGELOG.
---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index bb36831..d07f6ce 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -20,6 +20,7 @@ Release Notes - Mesos - Version 1.9.1 (WIP)
   * [MESOS-9948] - master::Slave::hasExecutor occupies 37% of a 150 second perf sample.
   * [MESOS-10017] - Log all reverse DNS lookup failures in 'legacy' TLS (SSL) hostname validation scheme.
   * [MESOS-10095] - Agent draining logging makes it hard to tell which tasks did not terminate.
+  * [MESOS-10112] - Log peer address during TLS handshake failures.
 
 
 Release Notes - Mesos - Version 1.9.0