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 2017/05/30 22:37:25 UTC

[1/3] mesos git commit: Make libprocess flags a private global for internal access.

Repository: mesos
Updated Branches:
  refs/heads/master ff78a068c -> 0a8321885


Make libprocess flags a private global for internal access.

Since we will need to check the libprocess flags while receiving
messages, hoist them out of initialize() and make them a static
global.

Review: https://reviews.apache.org/r/59344/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/72b7762b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/72b7762b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/72b7762b

Branch: refs/heads/master
Commit: 72b7762b1c9c0e2e4a2a90dab8ac5e7aa95fbd6f
Parents: ff78a06
Author: James Peach <jp...@apache.org>
Authored: Tue May 30 14:18:10 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Tue May 30 14:19:23 2017 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/72b7762b/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 20ac83d..3d1fba0 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -559,6 +559,7 @@ private:
   std::atomic_bool finalizing;
 };
 
+static internal::Flags* flags = new internal::Flags();
 
 // Synchronization primitives for `initialize`.
 // See documentation in `initialize` for how they are used.
@@ -1097,11 +1098,10 @@ bool initialize(
   __address__ = Address::ANY_ANY();
 
   // Fetch and parse the libprocess environment variables.
-  internal::Flags flags;
-  Try<flags::Warnings> load = flags.load("LIBPROCESS_");
+  Try<flags::Warnings> load = flags->load("LIBPROCESS_");
 
   if (load.isError()) {
-    EXIT(EXIT_FAILURE) << flags.usage(load.error());
+    EXIT(EXIT_FAILURE) << flags->usage(load.error());
   }
 
   // Log any flag warnings.
@@ -1109,12 +1109,12 @@ bool initialize(
     LOG(WARNING) << warning.message;
   }
 
-  if (flags.ip.isSome()) {
-    __address__.ip = flags.ip.get();
+  if (flags->ip.isSome()) {
+    __address__.ip = flags->ip.get();
   }
 
-  if (flags.port.isSome()) {
-    __address__.port = flags.port.get();
+  if (flags->port.isSome()) {
+    __address__.port = flags->port.get();
   }
 
   // Create a "server" socket for communicating.
@@ -1145,12 +1145,12 @@ bool initialize(
   __address__ = bind.get();
 
   // If advertised IP and port are present, use them instead.
-  if (flags.advertise_ip.isSome()) {
-    __address__.ip = flags.advertise_ip.get();
+  if (flags->advertise_ip.isSome()) {
+    __address__.ip = flags->advertise_ip.get();
   }
 
-  if (flags.advertise_port.isSome()) {
-    __address__.port = flags.advertise_port.get();
+  if (flags->advertise_port.isSome()) {
+    __address__.port = flags->advertise_port.get();
   }
 
   // Resolve the hostname if ip is 0.0.0.0 in case we actually have
@@ -1343,6 +1343,9 @@ void finalize(bool finalize_wsa)
   // cannot be cleared until after the `ProcessManager` is deleted.
   __address__ = Address::ANY_ANY();
 
+  // Finally, reset the Flags to defaults.
+  *flags = internal::Flags();
+
 #ifdef __WINDOWS__
   if (finalize_wsa && !net::wsa_cleanup()) {
     EXIT(EXIT_FAILURE) << "Failed to finalize the WSA socket stack";


[2/3] mesos git commit: Optionally verify the source IP address for libprocess messages.

Posted by bm...@apache.org.
Optionally verify the source IP address for libprocess messages.

In general, libprocess does not validate that a peer is a
legitimate owner of the UPID it claims in a libprocess message.
This change adds a check that the IP address in the UPID matches
the peer address. This makes spoofing the UPID harder (e.g. to
send authenticated messages), but also breaks some legitimate
configurations, particularly on multihomed hosts.

Review: https://reviews.apache.org/r/58224/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8fbbebfb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8fbbebfb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8fbbebfb

Branch: refs/heads/master
Commit: 8fbbebfb66e2bc408377031f82c060500be24b19
Parents: 72b7762
Author: James Peach <jp...@apache.org>
Authored: Tue May 30 15:18:17 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Tue May 30 15:34:43 2017 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp | 46 ++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8fbbebfb/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 3d1fba0..4e60231 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -135,6 +135,8 @@ using process::http::authentication::AuthenticatorManager;
 
 using process::http::authorization::AuthorizationCallbacks;
 
+namespace inet = process::network::inet;
+
 using process::network::inet::Address;
 using process::network::inet::Socket;
 
@@ -211,12 +213,27 @@ struct Flags : public virtual flags::FlagsBase
 
           return None();
         });
+
+    add(&Flags::require_peer_address_ip_match,
+        "require_peer_address_ip_match",
+        "If set, the IP address portion of the libprocess UPID in\n"
+        "incoming messages is required to match the IP address of\n"
+        "the socket from which the message was sent. This can be a\n"
+        "security enhancement since it prevents unauthorized senders\n"
+        "impersonating other libprocess actors. This check may\n"
+        "break configurations that require setting LIBPROCESS_IP,\n"
+        "or LIBPROCESS_ADVERTISE_IP. Additionally, multi-homed\n"
+        "configurations may be affected since the address on which\n"
+        "libprocess is listening may not match the address from\n"
+        "which libprocess connects to other actors.\n",
+        false);
   }
 
   Option<net::IP> ip;
   Option<net::IP> advertise_ip;
   Option<int> port;
   Option<int> advertise_port;
+  bool require_peer_address_ip_match;
 };
 
 } // namespace internal {
@@ -2855,6 +2872,35 @@ void ProcessManager::handle(
 
         Message* message = CHECK_NOTNULL(future.get());
 
+        // Verify that the UPID this peer is claiming is on the same IP
+        // address the peer is sending from.
+        if (flags->require_peer_address_ip_match) {
+          CHECK_SOME(request->client);
+
+          // If the client address is not an IP address (e.g. coming
+          // from a domain socket), we also reject the message.
+          Try<inet::Address> client_ip_address =
+            network::convert<inet::Address>(request->client.get());
+
+          if (client_ip_address.isError() ||
+              message->from.address.ip != client_ip_address->ip) {
+            Response response = BadRequest(
+                "UPID IP address validation failed: Message from " +
+                stringify(message->from) + " was sent from IP " +
+                stringify(request->client.get()));
+
+            dispatch(proxy, &HttpProxy::enqueue, response, *request);
+
+            VLOG(1) << "Returning '" << response.status << "'"
+                    << " for '" << request->url.path << "'"
+                    << ": " << response.body;
+
+            delete request;
+            delete message;
+            return;
+          }
+        }
+
         // TODO(benh): Use the sender PID when delivering in order to
         // capture happens-before timing relationships for testing.
         bool accepted = deliver(message->to, new MessageEvent(message));


[3/3] mesos git commit: Documented LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH.

Posted by bm...@apache.org.
Documented LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH.

Review: https://reviews.apache.org/r/59150/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0a832188
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0a832188
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0a832188

Branch: refs/heads/master
Commit: 0a83218855a604fc6c8663950d48cb2f3c93ef93
Parents: 8fbbebf
Author: James Peach <jp...@apache.org>
Authored: Tue May 30 15:35:28 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Tue May 30 15:36:55 2017 -0700

----------------------------------------------------------------------
 docs/configuration.md | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0a832188/docs/configuration.md
----------------------------------------------------------------------
diff --git a/docs/configuration.md b/docs/configuration.md
index 59e1bbe..8c3be23 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -2117,6 +2117,23 @@ quotas for container sandbox directories. Valid project IDs range from
   </tr>
   <tr>
     <td>
+      LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH
+    </td>
+    <td>
+      If set, the IP address portion of the libprocess UPID in
+      incoming messages is required to match the IP address
+      of the socket from which the message was sent. This can be a
+      security enhancement since it prevents unauthorized senders
+      impersonating other libprocess actors. This check may
+      break configurations that require setting LIBPROCESS_IP,
+      or LIBPROCESS_ADVERTISE_IP. Additionally, multi-homed
+      configurations may be affected since the address on
+      which libprocess is listening may not match the address from
+      which libprocess connects to other actors.
+    </td>
+  </tr>
+  <tr>
+    <td>
       LIBPROCESS_ENABLE_PROFILER
     </td>
     <td>