HTTP authentication flag for basic/digest HTTP authentication.

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

include/mesos/mesos.proto

@@ -675,6 +675,16 @@ message Credential {
 }
 
 
+/**
+ * Credentials used for authentication
+ *
+ */
+message Credentials {
+  repeated Credential registration = 1;
+  repeated Credential http = 2;
+}
+
+
 /**
  * ACLs used for authorization.
  */

src/Makefile.am

@@ -996,6 +996,7 @@ mesos_tests_SOURCES =				\
   tests/authorization_tests.cpp		        \
   tests/containerizer.cpp			\
   tests/containerizer_tests.cpp			\
+  tests/credentials_tests.cpp			\
   tests/environment.cpp				\
   tests/examples_tests.cpp			\
   tests/exception_tests.cpp			\

src/credentials/credentials.hpp

@@ -24,13 +24,14 @@
 
 #include <stout/option.hpp>
 #include <stout/os.hpp>
+#include <stout/protobuf.hpp>
 #include <stout/try.hpp>
 
 namespace mesos {
 namespace internal {
 namespace credentials {
 
-inline Result<std::vector<Credential> > read(const std::string& path)
+inline Result<Credentials> read(const std::string& path)
 {
   LOG(INFO) << "Loading credentials for authentication from '" << path << "'";
 
@@ -52,24 +53,78 @@ inline Result<std::vector<Credential> > read(const std::string& path)
                  << "credentials file is NOT accessible by others.";
   }
 
-  std::vector<Credential> credentials;
+  // TODO(ijimenez) deprecate text support only JSON like acls
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+  if (!json.isError()) {
+    Try<Credentials> credentials = ::protobuf::parse<Credentials>(json.get());
+    if (!credentials.isError()) {
+      return credentials.get();
+    }
+  }
+
+  Credentials credentials;
   foreach (const std::string& line, strings::tokenize(read.get(), "\n")) {
     const std::vector<std::string>& pairs = strings::tokenize(line, " ");
     if (pairs.size() != 2) {
-      return Error("Invalid credential format at line: " +
-                   stringify(credentials.size() + 1));
+        return Error("Invalid credential format at line " +
+                     credentials.registration().size() + 1);
     }
 
     // Add the credential.
-    Credential credential;
-    credential.set_principal(pairs[0]);
-    credential.set_secret(pairs[1]);
-    credentials.push_back(credential);
+    Credential *credential = credentials.add_registration();
+    credential->set_principal(pairs[0]);
+    credential->set_secret(pairs[1]);
   }
-
   return credentials;
 }
 
+
+inline Result<Credential> readCredential(const std::string& path)
+{
+  LOG(INFO) << "Loading credential for authentication from '" << path << "'";
+
+  Try<std::string> read = os::read(path);
+  if (read.isError()) {
+    return Error("Failed to read credential file '" + path +
+                 "': " + read.error());
+  } else if (read.get().empty()) {
+    return None();
+  }
+
+  Try<os::Permissions> permissions = os::permissions(path);
+  if (permissions.isError()) {
+    LOG(WARNING) << "Failed to stat credential file '" << path
+                 << "': " << permissions.error();
+  } else if (permissions.get().others.rwx) {
+    LOG(WARNING) << "Permissions on credential file '" << path
+                 << "' are too open. It is recommended that your "
+                 << "credential file is NOT accessible by others.";
+  }
+
+  // TODO(ijimenez) deprecate text support only JSON like acls
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+  if (!json.isError()) {
+    Try<Credential> credential = ::protobuf::parse<Credential>(json.get());
+    if (!credential.isError()) {
+      return credential.get();
+    }
+  }
+
+  Credential credential;
+  const std::vector<std::string>& line = strings::tokenize(read.get(), "\n");
+  if (line.size() != 1) {
+    return Error("Expecting only one credential");
+  }
+  const std::vector<std::string>& pairs = strings::tokenize(line[0], " ");
+  if (pairs.size() != 2) {
+    return Error("Invalid credential format");
+  }
+  // Add the credential.
+  credential.set_principal(pairs[0]);
+  credential.set_secret(pairs[1]);
+  return credential;
+}
+
 } // namespace credentials {
 } // namespace internal {
 } // namespace mesos {

src/master/flags.hpp

@@ -209,9 +209,31 @@ class Flags : public logging::Flags
 
     add(&Flags::credentials,
         "credentials",
-        "Path to a file with a list of credentials.\n"
-        "Each line contains 'principal' and 'secret' separated by whitespace.\n"
-        "Path could be of the form 'file:///path/to/file' or '/path/to/file'.");
+        "Either a path to a text file with a list of credentials,\n"
+        "each line containing 'principal' and 'secret' separated by "
+        "whitespace,\n"
+        "or, a path to a JSON-formatted file containing credentials\n"
+        "for identification/registration and http authentication."
+        "Path could be of the form 'file:///path/to/file' or '/path/to/file'."
+        "\n"
+        "JSON file Example:\n"
+        "{\n"
+        "  \"http\": [\n"
+        "             {\n"
+        "                \"principal\": \"username\",\n"
+        "                \"secret\": \"secret\",\n"
+        "             }\n"
+        "            ],\n"
+        "  \"identification\": [\n"
+        "                       {\n"
+        "                          \"principal\": \"username\",\n"
+        "                          \"secret\": \"secret\",\n"
+        "                       }\n"
+        "                      ]\n"
+        "}\n"
+        "Text file Example:\n"
+        "username:secret\n"
+        );
 
     add(&Flags::acls,
         "acls",

src/master/master.cpp

@@ -337,16 +337,20 @@ void Master::initialize()
     const string& path =
       strings::remove(flags.credentials.get(), "file://", strings::PREFIX);
 
-    Result<vector<Credential> > credentials = credentials::read(path);
-    if (credentials.isError()) {
-      EXIT(1) << credentials.error() << " (see --credentials flag)";
-    } else if (credentials.isNone()) {
+    Result<Credentials> _credentials = credentials::read(path);
+    if (_credentials.isError()) {
+      EXIT(1) << _credentials.error() << " (see --credentials flag)";
+    } else if (_credentials.isNone()) {
       EXIT(1) << "Credentials file must contain at least one credential"
               << " (see --credentials flag)";
     }
 
     // Give Authenticator access to credentials.
-    sasl::secrets::load(credentials.get());
+    sasl::secrets::load(_credentials.get());
+
+    // Store credentials in master
+    this->credentials = _credentials.get();
+
   } else if (flags.authenticate_frameworks || flags.authenticate_slaves) {
     EXIT(1) << "Authentication requires a credentials file"
             << " (see --credentials flag)";

src/master/master.hpp

@@ -370,6 +370,8 @@ class Master : public ProtobufProcess<Master>
   OfferID newOfferId();
   SlaveID newSlaveId();
 
+  Option<Credentials> credentials;
+
 private:
   // Inner class used to namespace HTTP route handlers (see
   // master/http.cpp for implementations).

src/sasl/authenticator.hpp

@@ -466,17 +466,12 @@ void load(const std::map<std::string, std::string>& secrets)
   InMemoryAuxiliaryPropertyPlugin::load(properties);
 }
 
-
-// Load credentials into the in-memory auxiliary propery plugin
-// that is used by the authenticators.
-void load(const std::vector<Credential>& credentials)
+void load(const Credentials& c)
 {
   std::map<std::string, std::string> secrets;
-
-  foreach (const Credential& credential, credentials) {
-    secrets[credential.principal()] = credential.secret();
+  foreach(const Credential& registration, c.registration()) {
+    secrets[registration.principal()] = registration.secret();
   }
-
   load(secrets);
 }
 

src/slave/flags.hpp

@@ -243,9 +243,18 @@ class Flags : public logging::Flags
 
     add(&Flags::credential,
         "credential",
-        "Path to a file containing a single line with\n"
-        "the 'principal' and 'secret' separated by whitespace.\n"
-        "Path could be of the form 'file:///path/to/file' or '/path/to/file'");
+        "Either a path to a text with a single line\n"
+        "containing 'principal' and 'secret' separated by "
+        "whitespace.\n"
+        "Or a path containing the JSON "
+        "formatted information used for one credential.\n"
+        "Path could be of the form 'file:///path/to/file' or '/path/to/file'."
+        "\n"
+        "Example:\n"
+        "{\n"
+        "    \"principal\": \"username\",\n"
+        "    \"secret\": \"secret\",\n"
+        "}");
 
     add(&Flags::containerizer_path,
         "containerizer_path",

src/slave/slave.cpp

@@ -255,16 +255,14 @@ void Slave::initialize()
     const string& path =
       strings::remove(flags.credential.get(), "file://", strings::PREFIX);
 
-    Result<vector<Credential> > credentials = credentials::read(path);
-    if (credentials.isError()) {
-      EXIT(1) << credentials.error() << " (see --credential flag)";
-    } else if (credentials.isNone()) {
+    Result<Credential> _credential = credentials::readCredential(path);
+    if (_credential.isError()) {
+      EXIT(1) << _credential.error() << " (see --credential flag)";
+    } else if (_credential.isNone()) {
       EXIT(1) << "Empty credential file '" << path
               << "' (see --credential flag)";
-    } else if (credentials.get().size() != 1) {
-      EXIT(1) << "Not expecting multiple credentials (see --credential flag)";
     } else {
-      credential = credentials.get()[0];
+      credential = _credential.get();
       LOG(INFO) << "Slave using credential for: "
                 << credential.get().principal();
     }

src/tests/credentials_tests.cpp

@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gmock/gmock.h>
+
+#include <string>
+
+#include <process/gmock.hpp>
+#include <process/pid.hpp>
+
+#include "master/flags.hpp"
+#include "master/master.hpp"
+#include "tests/mesos.hpp"
+#include "tests/utils.hpp"
+
+using std::map;
+using std::string;
+using std::vector;
+
+using namespace mesos;
+using namespace mesos::internal;
+using namespace mesos::internal::slave;
+using namespace mesos::internal::tests;
+
+using mesos::internal::master::Master;
+using mesos::internal::slave::Slave;
+
+using process::PID;
+
+using testing::_;
+using testing::Eq;
+using testing::Return;
+
+class CredentialsTest : public MesosTest {};
+
+
+// This test verifies that an authenticated slave is
+// granted registration by the master.
+TEST_F(CredentialsTest, authenticatedSlave)
+{
+  Try<PID<Master> > master = StartMaster();
+  ASSERT_SOME(master);
+
+  Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+  Try<PID<Slave> > slave = StartSlave();
+  ASSERT_SOME(slave);
+
+  AWAIT_READY(slaveRegisteredMessage);
+  ASSERT_NE("", slaveRegisteredMessage.get().slave_id().value());
+
+  Shutdown();
+}
+
+
+// Test verifing well executed credential authentication
+// using text formatted credentials so as to test
+// backwards compatibility
+TEST_F(CredentialsTest, authenticatedSlaveText)
+{
+  master::Flags flags = CreateMasterFlags();
+
+  const string& path =  path::join(os::getcwd(), "credentials");
+
+  Try<int> fd = os::open(
+      path,
+      O_WRONLY | O_CREAT | O_TRUNC,
+      S_IRUSR | S_IWUSR | S_IRGRP);
+
+  CHECK_SOME(fd);
+
+  const std::string& credentials =
+    DEFAULT_CREDENTIAL.principal() + " " + DEFAULT_CREDENTIAL.secret();
+  CHECK_SOME(os::write(fd.get(), credentials))
+     << "Failed to write credentials to '" << path << "'";
+  CHECK_SOME(os::close(fd.get()));
+
+  flags.credentials = "file://" + path;
+
+  Try<PID<Master> > master = StartMaster(flags);
+  ASSERT_SOME(master);
+
+  Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+  slave::Flags slaveFlags = CreateSlaveFlags();
+
+  slaveFlags.credential = "file://" + path;
+
+  Try<PID<Slave> > slave = StartSlave(slaveFlags);
+  ASSERT_SOME(slave);
+
+  AWAIT_READY(slaveRegisteredMessage);
+  ASSERT_NE("", slaveRegisteredMessage.get().slave_id().value());
+
+  Shutdown();
+}