You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by me...@apache.org on 2016/03/24 05:07:46 UTC

[7/9] mesos git commit: Added authentication to agent HTTP endpoints.

Added authentication to agent HTTP endpoints.

This patch adds HTTP authentication to the `/state`, `/state.json`,
and `/flags` endpoints. Tests are also updated to use authentication
when hitting these endpoints, and a new test was added to probe these
endpoints' behavior when bad credentials are supplied:
`SlaveTest.HTTPEndpointsBadAuthentication`.

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


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

Branch: refs/heads/master
Commit: c2927b88cbd85fac0d618b9eaaf5c05477d2c8ce
Parents: 919bb71
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Mar 23 20:02:37 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Wed Mar 23 21:07:23 2016 -0700

----------------------------------------------------------------------
 src/slave/http.cpp                              |   8 +-
 src/slave/slave.cpp                             |  18 ++--
 src/slave/slave.hpp                             |   6 +-
 .../docker_containerizer_tests.cpp              |   7 +-
 src/tests/containerizer/isolator_tests.cpp      |  12 ++-
 src/tests/fault_tolerance_tests.cpp             |  18 +++-
 src/tests/health_check_tests.cpp                |  24 ++++-
 src/tests/slave_tests.cpp                       | 101 +++++++++++++++++--
 8 files changed, 165 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 0117827..5b7bc5e 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -350,7 +350,9 @@ string Slave::Http::FLAGS_HELP()
 }
 
 
-Future<Response> Slave::Http::flags(const Request& request) const
+Future<Response> Slave::Http::flags(
+    const Request& request,
+    const Option<string>& /* principal */) const
 {
   JSON::Object object;
 
@@ -480,7 +482,9 @@ string Slave::Http::STATE_HELP() {
 }
 
 
-Future<Response> Slave::Http::state(const Request& request) const
+Future<Response> Slave::Http::state(
+    const Request& request,
+    const Option<string>& /* principal */) const
 {
   if (slave->state == Slave::RECOVERING) {
     return ServiceUnavailable("Agent has not finished recovery");

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 80a4ef5..6954f86 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -690,22 +690,28 @@ void Slave::initialize()
   // TODO(ijimenez): Remove this endpoint at the end of the
   // deprecation cycle on 0.26.
   route("/state.json",
+        DEFAULT_HTTP_AUTHENTICATION_REALM,
         Http::STATE_HELP(),
-        [http](const process::http::Request& request) {
+        [http](const process::http::Request& request,
+               const Option<string>& principal) {
           Http::log(request);
-          return http.state(request);
+          return http.state(request, principal);
         });
   route("/state",
+        DEFAULT_HTTP_AUTHENTICATION_REALM,
         Http::STATE_HELP(),
-        [http](const process::http::Request& request) {
+        [http](const process::http::Request& request,
+               const Option<string>& principal) {
           Http::log(request);
-          return http.state(request);
+          return http.state(request, principal);
         });
   route("/flags",
+        DEFAULT_HTTP_AUTHENTICATION_REALM,
         Http::FLAGS_HELP(),
-        [http](const process::http::Request& request) {
+        [http](const process::http::Request& request,
+               const Option<string>& principal) {
           Http::log(request);
-          return http.flags(request);
+          return http.flags(request, principal);
         });
   route("/health",
         Http::HEALTH_HELP(),

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 7520cc3..bc68cf1 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -433,7 +433,8 @@ private:
 
     // /slave/flags
     process::Future<process::http::Response> flags(
-        const process::http::Request& request) const;
+        const process::http::Request& request,
+        const Option<std::string>& /* principal */) const;
 
     // /slave/health
     process::Future<process::http::Response> health(
@@ -441,7 +442,8 @@ private:
 
     // /slave/state
     process::Future<process::http::Response> state(
-        const process::http::Request& request) const;
+        const process::http::Request& request,
+        const Option<std::string>& /* principal */) const;
 
     static std::string EXECUTOR_HELP();
     static std::string FLAGS_HELP();

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/tests/containerizer/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_containerizer_tests.cpp b/src/tests/containerizer/docker_containerizer_tests.cpp
index f6fce7d..b9c26b9 100644
--- a/src/tests/containerizer/docker_containerizer_tests.cpp
+++ b/src/tests/containerizer/docker_containerizer_tests.cpp
@@ -526,7 +526,12 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch)
   EXPECT_SOME_EQ(false, find);
 
   // Check if container information is exposed through slave's state endpoint.
-  response = http::get(slave.get()->pid, "state");
+  response = http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(process::http::OK().status, response);
 
   parse = JSON::parse<JSON::Object>(response.get().body);

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 6a2e25b..df506fc 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -1083,8 +1083,8 @@ TEST_F(NetClsIsolatorTest, ROOT_CGROUPS_NetClsIsolate)
 }
 
 
-// This test verifies that are able to retrieve the `net_cls` handle
-// from `state.json`.
+// This test verifies that we are able to retrieve the `net_cls` handle
+// from `/state`.
 TEST_F(NetClsIsolatorTest, ROOT_CGROUPS_ContainerStatus)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
@@ -1142,8 +1142,12 @@ TEST_F(NetClsIsolatorTest, ROOT_CGROUPS_ContainerStatus)
   AWAIT_READY(status);
   ASSERT_EQ(TASK_RUNNING, status.get().state());
 
-  // Task is ready. Verify `ContainerStatus` is present in slave state.json.
-  Future<Response> response = http::get(slave.get()->pid, "state.json");
+  // Task is ready. Verify `ContainerStatus` is present in slave state.
+  Future<Response> response = http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/tests/fault_tolerance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fault_tolerance_tests.cpp b/src/tests/fault_tolerance_tests.cpp
index f99413f..6723887 100644
--- a/src/tests/fault_tolerance_tests.cpp
+++ b/src/tests/fault_tolerance_tests.cpp
@@ -250,7 +250,11 @@ TEST_F(FaultToleranceTest, ReregisterCompletedFrameworks)
       1u,
       masterJSON.values["frameworks"].as<JSON::Array>().values.size());
 
-  Future<Response> slaveState = process::http::get(slave.get()->pid, "state");
+  Future<Response> slaveState = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, slaveState);
 
@@ -301,7 +305,11 @@ TEST_F(FaultToleranceTest, ReregisterCompletedFrameworks)
       1u,
       masterJSON.values["frameworks"].as<JSON::Array>().values.size());
 
-  slaveState = process::http::get(slave.get()->pid, "state");
+  slaveState = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, slaveState);
 
@@ -332,7 +340,11 @@ TEST_F(FaultToleranceTest, ReregisterCompletedFrameworks)
   AWAIT_READY(executorLost);
 
   // Verify slave sees completed framework.
-  slaveState = process::http::get(slave.get()->pid, "state");
+  slaveState = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, slaveState);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/tests/health_check_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/health_check_tests.cpp b/src/tests/health_check_tests.cpp
index d32164a..eace7c3 100644
--- a/src/tests/health_check_tests.cpp
+++ b/src/tests/health_check_tests.cpp
@@ -279,7 +279,11 @@ TEST_F(HealthCheckTest, HealthyTask)
 
   // Verify that task health is exposed in the slave's state endpoint.
   {
-    Future<http::Response> response = http::get(slave.get()->pid, "state");
+    Future<http::Response> response = http::get(
+        slave.get()->pid,
+        "state",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(process::http::OK().status, response);
 
@@ -582,7 +586,11 @@ TEST_F(HealthCheckTest, HealthStatusChange)
 
   // Verify that task health is exposed in the slave's state endpoint.
   {
-    Future<http::Response> response = http::get(slave.get()->pid, "state");
+    Future<http::Response> response = http::get(
+        slave.get()->pid,
+        "state",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(process::http::OK().status, response);
 
@@ -620,7 +628,11 @@ TEST_F(HealthCheckTest, HealthStatusChange)
   // Verify that the task health change is reflected in the slave's
   // state endpoint.
   {
-    Future<http::Response> response = http::get(slave.get()->pid, "state");
+    Future<http::Response> response = http::get(
+        slave.get()->pid,
+        "state",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(process::http::OK().status, response);
 
@@ -658,7 +670,11 @@ TEST_F(HealthCheckTest, HealthStatusChange)
   // Verify through slave's state endpoint that the task is back to a
   // healthy state.
   {
-    Future<http::Response> response = http::get(slave.get()->pid, "state");
+    Future<http::Response> response = http::get(
+        slave.get()->pid,
+        "state",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(process::http::OK().status, response);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/c2927b88/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index ea1d776..5f47a71 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -78,6 +78,7 @@ using process::UPID;
 using process::http::OK;
 using process::http::Response;
 using process::http::ServiceUnavailable;
+using process::http::Unauthorized;
 
 using std::map;
 using std::shared_ptr;
@@ -1213,7 +1214,11 @@ TEST_F(SlaveTest, StateEndpoint)
   AWAIT_READY(__recover);
   Clock::settle();
 
-  Future<Response> response = process::http::get(slave.get()->pid, "state");
+  Future<Response> response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
@@ -1322,7 +1327,11 @@ TEST_F(SlaveTest, StateEndpoint)
   AWAIT_READY(status);
   EXPECT_EQ(TASK_RUNNING, status.get().state());
 
-  response = http::get(slave.get()->pid, "state");
+  response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
@@ -1442,7 +1451,11 @@ TEST_F(SlaveTest, StateEndpointUnavailableDuringRecovery)
   // Ensure slave has setup the route for "/state".
   AWAIT_READY(_recover);
 
-  Future<Response> response = process::http::get(slave.get()->pid, "state");
+  Future<Response> response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(ServiceUnavailable().status, response);
 
@@ -1451,6 +1464,64 @@ TEST_F(SlaveTest, StateEndpointUnavailableDuringRecovery)
 }
 
 
+// Tests that a client will receive an `Unauthorized` response when agent HTTP
+// authentication is enabled and requests for the `/state` and `/flags`
+// endpoints include invalid credentials or no credentials at all.
+TEST_F(SlaveTest, HTTPEndpointsBadAuthentication)
+{
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  // A credential that will not be accepted by the agent.
+  Credential badCredential;
+  badCredential.set_principal("bad-principal");
+  badCredential.set_secret("bad-secret");
+
+  // Capture the start time deterministically.
+  Clock::pause();
+
+  Future<Nothing> recover = FUTURE_DISPATCH(_, &Slave::__recover);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  // HTTP authentication is enabled by default in `StartSlave`.
+  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+  ASSERT_SOME(slave);
+
+  // Ensure slave has finished recovery.
+  AWAIT_READY(recover);
+  Clock::settle();
+
+  // Requests containing invalid credentials.
+  {
+    Future<Response> response = process::http::get(
+        slave.get()->pid,
+        "state",
+        None(),
+        createBasicAuthHeaders(badCredential));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+
+    response = process::http::get(
+        slave.get()->pid,
+        "flags",
+        None(),
+        createBasicAuthHeaders(badCredential));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+  }
+
+  // Requests containing no authentication headers.
+  {
+    Future<Response> response = process::http::get(slave.get()->pid, "state");
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+
+    response = process::http::get(slave.get()->pid, "flags");
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+  }
+}
+
+
 // This test ensures that when a slave is shutting down, it will not
 // try to re-register with the master.
 TEST_F(SlaveTest, DISABLED_TerminatingSlaveDoesNotReregister)
@@ -2375,7 +2446,11 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
   AWAIT_READY(launchTask);
 
   // Verify label key and value in slave state endpoint.
-  Future<Response> response = process::http::get(slave.get()->pid, "state");
+  Future<Response> response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
@@ -2481,7 +2556,11 @@ TEST_F(SlaveTest, TaskLabels)
   AWAIT_READY(update);
 
   // Verify label key and value in slave state endpoint.
-  Future<Response> response = process::http::get(slave.get()->pid, "state");
+  Future<Response> response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
@@ -2580,7 +2659,11 @@ TEST_F(SlaveTest, TaskStatusLabels)
   AWAIT_READY(status);
 
   // Verify label key and value in master state endpoint.
-  Future<Response> response = process::http::get(slave.get()->pid, "state");
+  Future<Response> response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
@@ -2674,7 +2757,11 @@ TEST_F(SlaveTest, TaskStatusContainerStatus)
       status.get().container_status().network_infos(0).ip_address());
 
   // Now do the same validation with state endpoint.
-  Future<Response> response = process::http::get(slave.get()->pid, "state");
+  Future<Response> response = process::http::get(
+      slave.get()->pid,
+      "state",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
   AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);