kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject kudu git commit: IPKI: remove unused fields from cert code
Date Thu, 23 Feb 2017 01:02:20 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 3cd434e51 -> 914ed3b98


IPKI: remove unused fields from cert code

This removes all of the X509 attributes that we weren't currently
setting. It's better to not have them than have invalid values.

Change-Id: Ie3e6ff00fa33fe0156a04ead6f72db3432775cdb
Reviewed-on: http://gerrit.cloudera.org:8080/6115
Reviewed-by: Dan Burkert <danburkert@apache.org>
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Tested-by: Todd Lipcon <todd@apache.org>


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

Branch: refs/heads/master
Commit: 914ed3b98533f18513a0ea9a6c47162c8b464100
Parents: 3cd434e
Author: Todd Lipcon <todd@apache.org>
Authored: Wed Feb 22 10:48:35 2017 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Thu Feb 23 01:01:24 2017 +0000

----------------------------------------------------------------------
 .../master_cert_authority-itest.cc              |  3 +-
 src/kudu/master/master_cert_authority.cc        | 23 +----
 src/kudu/security/ca/cert_management-test.cc    | 89 ++++----------------
 src/kudu/security/ca/cert_management.cc         | 61 ++------------
 src/kudu/security/ca/cert_management.h          | 17 +---
 src/kudu/security/security-test-util.cc         |  3 +-
 src/kudu/security/tls_context.cc                | 13 +--
 7 files changed, 28 insertions(+), 181 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/integration-tests/master_cert_authority-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/master_cert_authority-itest.cc b/src/kudu/integration-tests/master_cert_authority-itest.cc
index 8a17373..227c4cd 100644
--- a/src/kudu/integration-tests/master_cert_authority-itest.cc
+++ b/src/kudu/integration-tests/master_cert_authority-itest.cc
@@ -226,8 +226,7 @@ TEST_F(MasterCertAuthorityTest, MasterLeaderSignsCSR) {
   string csr_str;
   {
     CertRequestGenerator::Config gen_config;
-    gen_config.uuid = kFakeTsUUID;
-    gen_config.hostnames =  {"localhost"};
+    gen_config.cn = "ts.foo.com";
     PrivateKey key;
     ASSERT_OK(security::GeneratePrivateKey(512, &key));
     CertRequestGenerator gen(gen_config);

http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/master/master_cert_authority.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master_cert_authority.cc b/src/kudu/master/master_cert_authority.cc
index 75d8b73..f5490f9 100644
--- a/src/kudu/master/master_cert_authority.cc
+++ b/src/kudu/master/master_cert_authority.cc
@@ -58,26 +58,6 @@ TAG_FLAG(ipki_server_cert_expiration_seconds, experimental);
 namespace kudu {
 namespace master {
 
-namespace {
-
-CaCertRequestGenerator::Config PrepareCaConfig(const string& server_uuid) {
-  // TODO(aserbin): do we actually have to set all these fields given we
-  // aren't using a web browser to connect?
-  return {
-    "US",               // country
-    "CA",               // state
-    "San Francisco",    // locality
-    "ASF",              // org
-    "The Kudu Project", // unit
-    server_uuid,        // uuid
-    "Kudu IPKI self-signed root CA certificate",// comment
-    {},                 // hostnames
-    {},                 // ips
-  };
-}
-
-} // anonymous namespace
-
 MasterCertAuthority::MasterCertAuthority(string server_uuid)
     : server_uuid_(std::move(server_uuid)) {
 }
@@ -91,9 +71,10 @@ Status MasterCertAuthority::Generate(security::PrivateKey* key,
   CHECK(key);
   CHECK(cert);
   // Create a key and cert for the self-signed CA.
+  CaCertRequestGenerator::Config config = { "kudu-ipki-ca" };
   RETURN_NOT_OK(GeneratePrivateKey(FLAGS_ipki_ca_key_size, key));
   return CertSigner::SelfSignCA(*key,
-                                PrepareCaConfig(server_uuid_),
+                                config,
                                 FLAGS_ipki_ca_cert_expiration_seconds,
                                 cert);
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/security/ca/cert_management-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management-test.cc b/src/kudu/security/ca/cert_management-test.cc
index e2b2552..b3ec45b 100644
--- a/src/kudu/security/ca/cert_management-test.cc
+++ b/src/kudu/security/ca/cert_management-test.cc
@@ -54,24 +54,8 @@ class CertManagementTest : public KuduTest {
 
  protected:
   CertRequestGenerator::Config PrepareConfig(
-      const string& uuid,
-      const vector<string>& hostnames = {},
-      const vector<string>& ips = {}) const {
-    const ::testing::TestInfo* const test_info =
-        ::testing::UnitTest::GetInstance()->current_test_info();
-    const string comment = string(test_info->test_case_name()) + "." +
-      test_info->name();
-    return {
-      "US",               // country
-      "CA",               // state
-      "San Francisco",    // locality
-      "ASF",              // org
-      "The Kudu Project", // unit
-      uuid,               // uuid
-      comment,            // comment
-      hostnames,          // hostnames
-      ips,                // ips
-    };
+      const string& common_name) {
+    return { common_name };
   }
 
   // Create a new private key in 'key' and return a CSR associated with that
@@ -95,52 +79,17 @@ class CertManagementTest : public KuduTest {
   PrivateKey ca_exp_private_key_;
 };
 
-// Check for basic SAN-related constraints while initializing
+// Check for basic constraints while initializing
 // CertRequestGenerator objects.
-TEST_F(CertManagementTest, RequestGeneratorSanConstraints) {
-  const string kEntityUUID = "D94FBF10-6F40-4F9F-BC82-F96A1C4F2CFB";
-
-  // No hostnames, nor IP addresses are given to populate X509v3 SAN extension.
-  {
-    const CertRequestGenerator::Config gen_config = PrepareConfig(kEntityUUID);
-    CertRequestGenerator gen(gen_config);
-    const Status s = gen.Init();
-    const string err_msg = s.ToString();
-    ASSERT_TRUE(s.IsInvalidArgument()) << err_msg;
-    ASSERT_STR_CONTAINS(err_msg, "SAN: missing DNS names and IP addresses");
-  }
-
-  // An empty hostname
-  {
-    const CertRequestGenerator::Config gen_config =
-        PrepareConfig(kEntityUUID, {"localhost", ""});
-    CertRequestGenerator gen(gen_config);
-    const Status s = gen.Init();
-    const string err_msg = s.ToString();
-    ASSERT_TRUE(s.IsInvalidArgument()) << err_msg;
-    ASSERT_STR_CONTAINS(err_msg, "SAN: an empty hostname");
-  }
-
-  // An empty IP address
-  {
-    const CertRequestGenerator::Config gen_config =
-        PrepareConfig(kEntityUUID, {}, {"127.0.0.1", ""});
-    CertRequestGenerator gen(gen_config);
-    const Status s = gen.Init();
-    const string err_msg = s.ToString();
-    ASSERT_TRUE(s.IsInvalidArgument()) << err_msg;
-    ASSERT_STR_CONTAINS(err_msg, "SAN: an empty IP address");
-  }
-
-  // Missing UUID
+TEST_F(CertManagementTest, RequestGeneratorConstraints) {
+  // Missing CN
   {
-    const CertRequestGenerator::Config gen_config =
-        PrepareConfig("", {"localhost"});
+    const CertRequestGenerator::Config gen_config = PrepareConfig("");
     CertRequestGenerator gen(gen_config);
     const Status s = gen.Init();
     const string err_msg = s.ToString();
     ASSERT_TRUE(s.IsInvalidArgument()) << err_msg;
-    ASSERT_STR_CONTAINS(err_msg, "missing end-entity UUID/name");
+    ASSERT_STR_CONTAINS(err_msg, "missing end-entity CN");
   }
 }
 
@@ -148,8 +97,7 @@ TEST_F(CertManagementTest, RequestGeneratorSanConstraints) {
 // check it's able to generate keys of expected number of bits and that it
 // reports an error if trying to generate a key of unsupported number of bits.
 TEST_F(CertManagementTest, RequestGeneratorBasics) {
-  const CertRequestGenerator::Config gen_config =
-      PrepareConfig("702C1C5E-CF02-4EDC-8883-07ECDEC8CE97", {"localhost"});
+  const CertRequestGenerator::Config gen_config = PrepareConfig("my-cn");
 
   PrivateKey key;
   ASSERT_OK(GeneratePrivateKey(1024, &key));
@@ -166,7 +114,7 @@ TEST_F(CertManagementTest, RequestGeneratorBasics) {
 // CA private key and certificate.
 TEST_F(CertManagementTest, SignerInitWithMismatchedCertAndKey) {
   PrivateKey key;
-  const auto& csr = PrepareTestCSR(PrepareConfig("test-uuid", {"localhost"}), &key);
+  const auto& csr = PrepareTestCSR(PrepareConfig("test-cn"), &key);
   {
     Cert cert;
     Status s = CertSigner(&ca_cert_, &ca_exp_private_key_)
@@ -189,8 +137,7 @@ TEST_F(CertManagementTest, SignerInitWithMismatchedCertAndKey) {
 // Check how CertSigner behaves if given expired CA certificate
 // and corresponding private key.
 TEST_F(CertManagementTest, SignerInitWithExpiredCert) {
-  const CertRequestGenerator::Config gen_config(
-      PrepareConfig("F4466090-BBF8-4042-B72F-BB257500C45A", {"localhost"}));
+  const CertRequestGenerator::Config gen_config = PrepareConfig("test-cn");
   PrivateKey key;
   CertSignRequest req = PrepareTestCSR(gen_config, &key);
 
@@ -202,8 +149,7 @@ TEST_F(CertManagementTest, SignerInitWithExpiredCert) {
 
 // Generate X509 CSR and issues corresponding certificate.
 TEST_F(CertManagementTest, SignCert) {
-  const CertRequestGenerator::Config gen_config(
-      PrepareConfig("test-uuid", {"localhost"}, {"127.0.0.1", "127.0.10.20"}));
+  const CertRequestGenerator::Config gen_config = PrepareConfig("test-cn");
   PrivateKey key;
   const auto& csr = PrepareTestCSR(gen_config, &key);
   Cert cert;
@@ -212,8 +158,7 @@ TEST_F(CertManagementTest, SignCert) {
 
   EXPECT_EQ("C = US, ST = CA, O = MyCompany, CN = MyName, emailAddress = my@email.com",
             cert.IssuerName());
-  EXPECT_EQ("C = US, ST = CA, L = San Francisco, O = ASF, OU = The Kudu Project, "
-            "CN = test-uuid", cert.SubjectName());
+  EXPECT_EQ("CN = test-cn", cert.SubjectName());
 }
 
 // Generate X509 CA CSR and sign the result certificate.
@@ -235,9 +180,7 @@ TEST_F(CertManagementTest, TestSelfSignedCA) {
   ASSERT_OK(GenerateSelfSignedCAForTests(&ca_key, &ca_cert));
 
   // Create a key and CSR for the tablet server.
-  const auto& config = PrepareConfig(
-      "some-tablet-server",
-      {"localhost"}, {"127.0.0.1", "127.0.10.20"});
+  const auto& config = PrepareConfig("some-tablet-server");
   PrivateKey ts_key;
   CertSignRequest ts_csr = PrepareTestCSR(config, &ts_key);
 
@@ -255,8 +198,7 @@ TEST_F(CertManagementTest, X509CsrFromAndToString) {
 
   PrivateKey key;
   ASSERT_OK(GeneratePrivateKey(1024, &key));
-  CertRequestGenerator gen(PrepareConfig(
-      "4C931ADC-3945-4E05-8DB2-447327BF8F62", {"localhost"}));
+  CertRequestGenerator gen(PrepareConfig("test-cn"));
   ASSERT_OK(gen.Init());
   CertSignRequest req_ref;
   ASSERT_OK(gen.GenerateRequest(key, &req_ref));
@@ -281,8 +223,7 @@ TEST_F(CertManagementTest, X509FromAndToString) {
 
   PrivateKey key;
   ASSERT_OK(GeneratePrivateKey(1024, &key));
-  CertRequestGenerator gen(PrepareConfig(
-      "86F676E9-4E77-4DDC-B15C-596E74B03D90", {"localhost"}));
+  CertRequestGenerator gen(PrepareConfig("test-cn"));
   ASSERT_OK(gen.Init());
   CertSignRequest req;
   ASSERT_OK(gen.GenerateRequest(key, &req));

http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/security/ca/cert_management.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management.cc b/src/kudu/security/ca/cert_management.cc
index 5585556..88bb3fb 100644
--- a/src/kudu/security/ca/cert_management.cc
+++ b/src/kudu/security/ca/cert_management.cc
@@ -88,12 +88,7 @@ Status CertRequestGeneratorBase::GenerateRequest(const PrivateKey&
key,
     } \
   } while (false)
 
-  CERT_SET_SUBJ_FIELD(config_.country, "C", "country");
-  CERT_SET_SUBJ_FIELD(config_.state, "ST", "state");
-  CERT_SET_SUBJ_FIELD(config_.locality, "L", "locality/city");
-  CERT_SET_SUBJ_FIELD(config_.org, "O", "organization");
-  CERT_SET_SUBJ_FIELD(config_.unit, "OU", "organizational unit");
-  CERT_SET_SUBJ_FIELD(config_.uuid, "CN", "common name");
+  CERT_SET_SUBJ_FIELD(config_.cn, "CN", "common name");
 #undef CERT_SET_SUBJ_FIELD
 
   // Set necessary extensions into the request.
@@ -127,13 +122,8 @@ Status CertRequestGenerator::Init() {
   InitializeOpenSSL();
 
   CHECK(!is_initialized_);
-  if (config_.uuid.empty()) {
-    return Status::InvalidArgument("missing end-entity UUID/name");
-  }
-  // Check that the config contain at least one entity (DNS name/IP address)
-  // to bind the generated certificate.
-  if (config_.hostnames.empty() && config_.ips.empty()) {
-    return Status::InvalidArgument("SAN: missing DNS names and IP addresses");
+  if (config_.cn.empty()) {
+    return Status::InvalidArgument("missing end-entity CN");
   }
 
   extensions_ = sk_X509_EXTENSION_new_null();
@@ -164,43 +154,7 @@ Status CertRequestGenerator::Init() {
   // (i.e. they cannot be used to sign/issue certificates).
   RETURN_NOT_OK(PushExtension(extensions_, NID_basic_constraints,
                               "critical,CA:FALSE"));
-  ostringstream san_hosts;
-  for (size_t i = 0; i < config_.hostnames.size(); ++i) {
-    const string& hostname = config_.hostnames[i];
-    if (hostname.empty()) {
-      // Basic validation: check for emptyness. Probably, more advanced
-      // validation is needed here.
-      return Status::InvalidArgument("SAN: an empty hostname");
-    }
-    if (i != 0) {
-      san_hosts << ",";
-    }
-    san_hosts << "DNS." << i << ":" << hostname;
-  }
-  ostringstream san_ips;
-  for (size_t i = 0; i < config_.ips.size(); ++i) {
-    const string& ip = config_.ips[i];
-    if (ip.empty()) {
-      // Basic validation: check for emptyness. Probably, more advanced
-      // validation is needed here.
-      return Status::InvalidArgument("SAN: an empty IP address");
-    }
-    if (i != 0) {
-      san_ips << ",";
-    }
-    san_ips << "IP." << i << ":" << ip;
-  }
-  // Encode hostname and IP address into the subjectAlternativeName attribute.
-  const string alt_name = san_hosts.str() +
-      ((!san_hosts.str().empty() && !san_ips.str().empty()) ? "," : "") +
-      san_ips.str();
-  RETURN_NOT_OK(PushExtension(extensions_, NID_subject_alt_name,
-                              alt_name.c_str()));
-  if (!config_.comment.empty()) {
-    // Add the comment if it's not empty.
-    RETURN_NOT_OK(PushExtension(extensions_, NID_netscape_comment,
-                                config_.comment.c_str()));
-  }
+
   is_initialized_ = true;
 
   return Status::OK();
@@ -233,7 +187,7 @@ Status CaCertRequestGenerator::Init() {
   if (is_initialized_) {
     return Status::OK();
   }
-  if (config_.uuid.empty()) {
+  if (config_.cn.empty()) {
     return Status::InvalidArgument("missing CA service UUID/name");
   }
 
@@ -250,11 +204,6 @@ Status CaCertRequestGenerator::Init() {
   // The generated certificates are for the private CA service.
   RETURN_NOT_OK(PushExtension(extensions_, NID_basic_constraints,
                               "critical,CA:TRUE"));
-  if (!config_.comment.empty()) {
-    // Add the comment if it's not empty.
-    RETURN_NOT_OK(PushExtension(extensions_, NID_netscape_comment,
-                                config_.comment.c_str()));
-  }
   is_initialized_ = true;
 
   return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/security/ca/cert_management.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management.h b/src/kudu/security/ca/cert_management.h
index 682989c..f0fd342 100644
--- a/src/kudu/security/ca/cert_management.h
+++ b/src/kudu/security/ca/cert_management.h
@@ -55,15 +55,7 @@ class CertRequestGeneratorBase {
   // Properties for the generated X509 CSR.  Using server UUID for the common
   // name field.
   struct Config {
-    std::string country;  // subject field: C
-    std::string state;    // subject field: ST
-    std::string locality; // subject field: L
-    std::string org;      // subject field: O
-    std::string unit;     // subject field: OU
-    std::string uuid;     // subject field: CN
-    std::string comment;  // custom extension: Netscape Comment
-    std::vector<std::string> hostnames; // subjectAltName extension (DNS:)
-    std::vector<std::string> ips;       // subjectAltName extension (IP:)
+    std::string cn;     // subject field: CN
   };
 
   explicit CertRequestGeneratorBase(Config config);
@@ -93,11 +85,8 @@ class CertRequestGeneratorBase {
 // (a.k.a. X509 CSRs).
 class CertRequestGenerator : public CertRequestGeneratorBase {
  public:
-  // The CertRequestGenerator object is bound to the server UUID, hostnames
-  // and IP addresses specified by the 'config' parameter. The hostnames and
-  // IP addresses are put into the X509v3 SAN extension (subject alternative
-  // name, a.k.a. subjectAltName). The SAN can be used while verifying the
-  // generated certificates during TLS handshake.
+  // 'config' contains the properties to fill into the X509 attributes of the
+  // CSR.
   explicit CertRequestGenerator(Config config);
   ~CertRequestGenerator();
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/security/security-test-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/security-test-util.cc b/src/kudu/security/security-test-util.cc
index bfafb39..dfa9c4c 100644
--- a/src/kudu/security/security-test-util.cc
+++ b/src/kudu/security/security-test-util.cc
@@ -35,8 +35,7 @@ Status GenerateSelfSignedCAForTests(PrivateKey* ca_key, Cert* ca_cert) {
   // Create a key for the self-signed CA.
   RETURN_NOT_OK(GeneratePrivateKey(512, ca_key));
 
-  CaCertRequestGenerator::Config config;
-  config.uuid = "test-ca-uuid";
+  CaCertRequestGenerator::Config config = { "test-ca-cn" };
   RETURN_NOT_OK(CertSigner::SelfSignCA(*ca_key,
                                        config,
                                        kRootCaCertExpirationSeconds,

http://git-wip-us.apache.org/repos/asf/kudu/blob/914ed3b9/src/kudu/security/tls_context.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_context.cc b/src/kudu/security/tls_context.cc
index 2c248ca..0844a70 100644
--- a/src/kudu/security/tls_context.cc
+++ b/src/kudu/security/tls_context.cc
@@ -242,18 +242,7 @@ Status TlsContext::GenerateSelfSignedCertAndKey(const std::string&
server_uuid)
 
   // Step 2: generate a CSR so that the self-signed cert can eventually be
   // replaced with a CA-signed cert.
-  // TODO(aserbin): do these fields actually have to be set?
-  const CertRequestGenerator::Config config = {
-    "US",               // country
-    "CA",               // state
-    "San Francisco",    // locality
-    "ASF",              // org
-    "The Kudu Project", // unit
-    server_uuid,        // uuid
-    "",                 // comment
-    {"localhost"},      // hostnames TODO(PKI): use real hostnames
-    {"127.0.0.1"},      // ips
-  };
+  const CertRequestGenerator::Config config = { server_uuid };
 
   CertRequestGenerator gen(config);
   CertSignRequest csr;


Mime
View raw message