kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject kudu git commit: KUDU-1981 Kudu should run at hosts len(FQDN) > 64
Date Fri, 05 May 2017 03:43:48 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.3.x 1232c893e -> 49c9d6eb8


KUDU-1981 Kudu should run at hosts len(FQDN) > 64

This is a fix for KUDU-1981: with security enabled, Kudu servers cannot
start at machines with len(FQDN) > 64.  Prior to this fix, the host FQDN
was put into the CSR's CN (common name) field while generating
self-signed certificate for server RPC messenger. Per RFC5280, the CN
field cannot contain strings longer than 64 characters long, and it
seems OpenSSL enforces that limit as required.

The idea is to put FQDNs into the SAN X509v3 extension field as 'DNS'
fields.  That makes it possible to have names in the SAN which are even
longer than 255 characters.  This patch returns back a part of the
SAN-related functionality which had been implemented initially in
cert_management.cc and then removed since it was not used back then.

This patch also adds a couple of unit tests to cover the new
functionality and to make sure it's possible to set CN field of CSR to
64-chars length value and have corresponding X509 certificate generated
with no issues.

Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19
Reviewed-on: http://gerrit.cloudera.org:8080/6734
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>
(cherry picked from commit eccafbcfbd41324164f7df10219a2b9c3d161269)
Reviewed-on: http://gerrit.cloudera.org:8080/6805
Reviewed-by: Jean-Daniel Cryans <jdcryans@apache.org>


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

Branch: refs/heads/branch-1.3.x
Commit: 49c9d6eb8fb1e995b728f47adfcbfe75f216d421
Parents: 1232c89
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Tue Apr 25 19:21:28 2017 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Fri May 5 03:42:49 2017 +0000

----------------------------------------------------------------------
 .../master_cert_authority-itest.cc              | 15 +++-
 src/kudu/security/ca/cert_management-test.cc    | 84 ++++++++++++++------
 src/kudu/security/ca/cert_management.cc         | 73 +++++++++--------
 src/kudu/security/ca/cert_management.h          | 43 +++++++---
 src/kudu/security/cert-test.cc                  | 24 ++++++
 src/kudu/security/cert.cc                       | 25 ++++++
 src/kudu/security/cert.h                        |  4 +
 src/kudu/security/test/test_certs.cc            | 28 +++++++
 src/kudu/security/test/test_certs.h             |  2 +
 src/kudu/security/tls_context.cc                |  8 +-
 10 files changed, 232 insertions(+), 74 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/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 c6873de..6d6016a 100644
--- a/src/kudu/integration-tests/master_cert_authority-itest.cc
+++ b/src/kudu/integration-tests/master_cert_authority-itest.cc
@@ -241,7 +241,7 @@ TEST_F(MasterCertAuthorityTest, RefuseToSignInvalidCSR) {
   string csr_str;
   {
     CertRequestGenerator::Config gen_config;
-    gen_config.cn = "ts.foo.com";
+    gen_config.hostname = "ts.foo.com";
     gen_config.user_id = "joe-impostor";
     NO_FATALS(GenerateCSR(gen_config, &csr_str));
   }
@@ -265,7 +265,18 @@ TEST_F(MasterCertAuthorityTest, MasterLeaderSignsCSR) {
   string csr_str;
   {
     CertRequestGenerator::Config gen_config;
-    gen_config.cn = "ts.foo.com";
+    // The hostname is longer than permitted 255 characters and breaks other
+    // restrictions on valid DNS hostnames, but it should not be an issue for
+    // both the requestor and the signer.
+    gen_config.hostname =
+        "toooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo."
+        "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+        "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+        "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+        "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+        "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+        "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+        "ng.hostname.io";
     string test_user;
     ASSERT_OK(GetLoggedInUser(&test_user));
     gen_config.user_id = test_user;

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/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 d209f5f..6e4662d 100644
--- a/src/kudu/security/ca/cert_management-test.cc
+++ b/src/kudu/security/ca/cert_management-test.cc
@@ -54,14 +54,18 @@ class CertManagementTest : public KuduTest {
 
  protected:
   CertRequestGenerator::Config PrepareConfig(
-      const string& common_name) {
-    return { common_name };
+      const string& hostname = "localhost.localdomain") {
+    return { hostname };
+  }
+
+  CaCertRequestGenerator::Config PrepareCaConfig(const string& cn) {
+    return { cn };
   }
 
   // Create a new private key in 'key' and return a CSR associated with that
   // key.
   template<class CSRGen = CertRequestGenerator>
-  CertSignRequest PrepareTestCSR(CertRequestGenerator::Config config,
+  CertSignRequest PrepareTestCSR(typename CSRGen::Config config,
                                  PrivateKey* key) {
     CHECK_OK(GeneratePrivateKey(512, key));
     CSRGen gen(std::move(config));
@@ -79,25 +83,21 @@ class CertManagementTest : public KuduTest {
   PrivateKey ca_exp_private_key_;
 };
 
-// Check for basic constraints while initializing
-// CertRequestGenerator objects.
+// Check for basic constraints while initializing CertRequestGenerator objects.
 TEST_F(CertManagementTest, RequestGeneratorConstraints) {
-  // Missing CN
-  {
-    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 CN");
-  }
+  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, "hostname must not be empty");
 }
 
 // Check for the basic functionality of the CertRequestGenerator class:
 // 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("my-cn");
+  const CertRequestGenerator::Config gen_config = PrepareConfig();
 
   PrivateKey key;
   ASSERT_OK(GeneratePrivateKey(1024, &key));
@@ -114,7 +114,7 @@ TEST_F(CertManagementTest, RequestGeneratorBasics) {
 // CA private key and certificate.
 TEST_F(CertManagementTest, SignerInitWithMismatchedCertAndKey) {
   PrivateKey key;
-  const auto& csr = PrepareTestCSR(PrepareConfig("test-cn"), &key);
+  const auto& csr = PrepareTestCSR(PrepareConfig(), &key);
   {
     Cert cert;
     Status s = CertSigner(&ca_cert_, &ca_exp_private_key_)
@@ -137,7 +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("test-cn");
+  const CertRequestGenerator::Config gen_config = PrepareConfig();
   PrivateKey key;
   CertSignRequest req = PrepareTestCSR(gen_config, &key);
 
@@ -147,10 +147,44 @@ TEST_F(CertManagementTest, SignerInitWithExpiredCert) {
   ASSERT_OK(cert.CheckKeyMatch(key));
 }
 
+// Generate X509 CSR and issue corresponding certificate putting the specified
+// hostname into the SAN X509v3 extension field. The fix for KUDU-1981 addresses
+// the issue of enabling Kudu server components on systems with FQDN longer than
+// 64 characters. This test is a regression for KUDU-1981, so let's verify that
+// CSRs and the result X509 cerificates with long hostnames in SAN are handled
+// properly.
+TEST_F(CertManagementTest, SignCertLongHostnameInSan) {
+  for (auto const& hostname :
+      {
+        "foo.bar.com",
+
+        "222222222222222222222222222222222222222222222222222222222222222."
+        "555555555555555555555555555555555555555555555555555555555555555."
+        "555555555555555555555555555555555555555555555555555555555555555."
+        "chaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaars",
+      }) {
+    CertRequestGenerator::Config gen_config;
+    gen_config.hostname = hostname;
+    gen_config.user_id = "test-uid";
+    PrivateKey key;
+    const auto& csr = PrepareTestCSR(gen_config, &key);
+    Cert cert;
+    ASSERT_OK(CertSigner(&ca_cert_, &ca_private_key_).Sign(csr, &cert));
+    ASSERT_OK(cert.CheckKeyMatch(key));
+
+    EXPECT_EQ("C = US, ST = CA, O = MyCompany, CN = MyName, emailAddress = my@email.com",
+              cert.IssuerName());
+    EXPECT_EQ("UID = test-uid", cert.SubjectName());
+    vector<string> hostnames = cert.Hostnames();
+    ASSERT_EQ(1, hostnames.size());
+    EXPECT_EQ(hostname, hostnames[0]);
+  }
+}
+
 // Generate X509 CSR and issues corresponding certificate.
 TEST_F(CertManagementTest, SignCert) {
   CertRequestGenerator::Config gen_config;
-  gen_config.cn = "test-cn";
+  gen_config.hostname = "foo.bar.com";
   gen_config.user_id = "test-uid";
   gen_config.kerberos_principal = "kudu/foo.bar.com@bar.com";
   PrivateKey key;
@@ -161,15 +195,17 @@ TEST_F(CertManagementTest, SignCert) {
 
   EXPECT_EQ("C = US, ST = CA, O = MyCompany, CN = MyName, emailAddress = my@email.com",
             cert.IssuerName());
-  EXPECT_EQ("CN = test-cn, UID = test-uid", cert.SubjectName());
+  EXPECT_EQ("UID = test-uid", cert.SubjectName());
   EXPECT_EQ(gen_config.user_id, *cert.UserId());
   EXPECT_EQ(gen_config.kerberos_principal, *cert.KuduKerberosPrincipal());
+  vector<string> hostnames = cert.Hostnames();
+  ASSERT_EQ(1, hostnames.size());
+  EXPECT_EQ("foo.bar.com", hostnames[0]);
 }
 
 // Generate X509 CA CSR and sign the result certificate.
 TEST_F(CertManagementTest, SignCaCert) {
-  const CertRequestGenerator::Config gen_config(
-      PrepareConfig("8C084CF6-A30B-4F5B-9673-A73E62E29A9D"));
+  const CaCertRequestGenerator::Config gen_config(PrepareCaConfig("self-ca"));
   PrivateKey key;
   const auto& csr = PrepareTestCSR<CaCertRequestGenerator>(gen_config, &key);
   Cert cert;
@@ -185,7 +221,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");
+  const auto& config = PrepareConfig();
   PrivateKey ts_key;
   CertSignRequest ts_csr = PrepareTestCSR(config, &ts_key);
 
@@ -203,7 +239,7 @@ TEST_F(CertManagementTest, X509CsrFromAndToString) {
 
   PrivateKey key;
   ASSERT_OK(GeneratePrivateKey(1024, &key));
-  CertRequestGenerator gen(PrepareConfig("test-cn"));
+  CertRequestGenerator gen(PrepareConfig());
   ASSERT_OK(gen.Init());
   CertSignRequest req_ref;
   ASSERT_OK(gen.GenerateRequest(key, &req_ref));
@@ -228,7 +264,7 @@ TEST_F(CertManagementTest, X509FromAndToString) {
 
   PrivateKey key;
   ASSERT_OK(GeneratePrivateKey(1024, &key));
-  CertRequestGenerator gen(PrepareConfig("test-cn"));
+  CertRequestGenerator gen(PrepareConfig());
   ASSERT_OK(gen.Init());
   CertSignRequest req;
   ASSERT_OK(gen.GenerateRequest(key, &req));

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/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 978dd54..a6d7f7b 100644
--- a/src/kudu/security/ca/cert_management.cc
+++ b/src/kudu/security/ca/cert_management.cc
@@ -60,10 +60,22 @@ template<> struct SslTypeTraits<BIGNUM> {
 
 namespace ca {
 
-CertRequestGeneratorBase::CertRequestGeneratorBase(Config config)
-    : config_(move(config)) {
+namespace {
+
+Status SetSubjectNameField(X509_NAME* name,
+                           const char* field_code,
+                           const string& field_value) {
+  CHECK(name);
+  CHECK(field_code);
+  OPENSSL_RET_NOT_OK(X509_NAME_add_entry_by_txt(
+      name, field_code, MBSTRING_ASC,
+      reinterpret_cast<const unsigned char*>(field_value.c_str()), -1, -1, 0),
+      Substitute("error setting subject field $0", field_code));
+  return Status::OK();
 }
 
+} // anonymous namespace
+
 CertRequestGenerator::~CertRequestGenerator() {
   sk_X509_EXTENSION_pop_free(extensions_, X509_EXTENSION_free);
 }
@@ -76,25 +88,9 @@ Status CertRequestGeneratorBase::GenerateRequest(const PrivateKey&
key,
   auto req = ssl_make_unique(X509_REQ_new());
   OPENSSL_RET_NOT_OK(X509_REQ_set_pubkey(req.get(), key.GetRawData()),
       "error setting X509 public key");
-  X509_NAME* name = X509_REQ_get_subject_name(req.get());
-  CHECK(name);
 
-#define CERT_SET_SUBJ_FIELD(field, code, err_msg) \
-  do { \
-    const string& f = (field); \
-    if (!f.empty()) { \
-      OPENSSL_RET_NOT_OK(X509_NAME_add_entry_by_txt(\
-          name, (code), MBSTRING_ASC, \
-          reinterpret_cast<const unsigned char*>(f.c_str()), -1, -1, 0), \
-          ("error setting subject " # err_msg)); \
-    } \
-  } while (false)
-
-  CERT_SET_SUBJ_FIELD(config_.cn, "CN", "common name");
-  if (config_.user_id) {
-    CERT_SET_SUBJ_FIELD(*config_.user_id, "UID", "userId");
-  }
-#undef CERT_SET_SUBJ_FIELD
+  // Populate the subject field of the request.
+  RETURN_NOT_OK(SetSubject(req.get()));
 
   // Set necessary extensions into the request.
   RETURN_NOT_OK(SetExtensions(req.get()));
@@ -119,7 +115,8 @@ Status CertRequestGeneratorBase::PushExtension(stack_st_X509_EXTENSION*
st,
 }
 
 CertRequestGenerator::CertRequestGenerator(Config config)
-    : CertRequestGeneratorBase(config) {
+    : CertRequestGeneratorBase(),
+      config_(std::move(config)) {
 }
 
 Status CertRequestGenerator::Init() {
@@ -127,9 +124,13 @@ Status CertRequestGenerator::Init() {
   SCOPED_OPENSSL_NO_PENDING_ERRORS;
 
   CHECK(!is_initialized_);
-  if (config_.cn.empty()) {
-    return Status::InvalidArgument("missing end-entity CN");
+
+  // Build the SAN field using the specified hostname. In general, it might be
+  // multiple DNS hostnames in the field, but in our use-cases it's always one.
+  if (config_.hostname.empty()) {
+    return Status::InvalidArgument("hostname must not be empty");
   }
+  const string san_hosts = Substitute("DNS.0:$0", config_.hostname);
 
   extensions_ = sk_X509_EXTENSION_new_null();
 
@@ -165,6 +166,7 @@ Status CertRequestGenerator::Init() {
     RETURN_NOT_OK(PushExtension(extensions_, nid,
                                 Substitute("ASN1:UTF8:$0", *config_.kerberos_principal)));
   }
+  RETURN_NOT_OK(PushExtension(extensions_, NID_subject_alt_name, san_hosts));
 
   is_initialized_ = true;
 
@@ -175,6 +177,14 @@ bool CertRequestGenerator::Initialized() const {
   return is_initialized_;
 }
 
+Status CertRequestGenerator::SetSubject(X509_REQ* req) const {
+  if (config_.user_id) {
+    RETURN_NOT_OK(SetSubjectNameField(X509_REQ_get_subject_name(req),
+                                      "UID", *config_.user_id));
+  }
+  return Status::OK();
+}
+
 Status CertRequestGenerator::SetExtensions(X509_REQ* req) const {
   OPENSSL_RET_NOT_OK(X509_REQ_add_extensions(req, extensions_),
       "error setting X509 request extensions");
@@ -182,7 +192,7 @@ Status CertRequestGenerator::SetExtensions(X509_REQ* req) const {
 }
 
 CaCertRequestGenerator::CaCertRequestGenerator(Config config)
-    : CertRequestGeneratorBase(config),
+    : config_(std::move(config)),
       extensions_(nullptr),
       is_initialized_(false) {
 }
@@ -226,6 +236,10 @@ bool CaCertRequestGenerator::Initialized() const {
   return is_initialized_;
 }
 
+Status CaCertRequestGenerator::SetSubject(X509_REQ* req) const {
+  return SetSubjectNameField(X509_REQ_get_subject_name(req), "CN", config_.cn);
+}
+
 Status CaCertRequestGenerator::SetExtensions(X509_REQ* req) const {
   OPENSSL_RET_NOT_OK(X509_REQ_add_extensions(req, extensions_),
       "error setting X509 request extensions");
@@ -245,11 +259,9 @@ Status CertSigner::SelfSignCA(const PrivateKey& key,
   }
 
   // Self-sign the CA's CSR.
-  RETURN_NOT_OK(CertSigner(nullptr, &key)
-                .set_expiration_interval(
-                    MonoDelta::FromSeconds(cert_expiration_seconds))
-                .Sign(ca_csr, cert));
-  return Status::OK();
+  return CertSigner(nullptr, &key)
+      .set_expiration_interval(MonoDelta::FromSeconds(cert_expiration_seconds))
+      .Sign(ca_csr, cert);
 }
 
 Status CertSigner::SelfSignCert(const PrivateKey& key,
@@ -265,8 +277,7 @@ Status CertSigner::SelfSignCert(const PrivateKey& key,
   }
 
   // Self-sign the CSR with the key.
-  RETURN_NOT_OK(CertSigner(nullptr, &key).Sign(csr, cert));
-  return Status::OK();
+  return CertSigner(nullptr, &key).Sign(csr, cert);
 }
 
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/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 0121513..919caff 100644
--- a/src/kudu/security/ca/cert_management.h
+++ b/src/kudu/security/ca/cert_management.h
@@ -54,17 +54,7 @@ namespace ca {
 // Base utility class for issuing X509 CSRs.
 class CertRequestGeneratorBase {
  public:
-  // Properties for the generated X509 CSR.
-  struct Config {
-    // Common Name (CN)
-    std::string cn;
-    // userId (UID)
-    boost::optional<std::string> user_id;
-    // Our custom extension which stores the full Kerberos principal for IPKI certs.
-    boost::optional<std::string> kerberos_principal;
-  };
-
-  explicit CertRequestGeneratorBase(Config config);
+  CertRequestGeneratorBase() = default;
   virtual ~CertRequestGeneratorBase() = default;
 
   virtual Status Init() = 0;
@@ -79,11 +69,13 @@ class CertRequestGeneratorBase {
   static Status PushExtension(stack_st_X509_EXTENSION* st,
                               int32_t nid,
                               StringPiece value) WARN_UNUSED_RESULT;
+
+  // Set the certificate-specific subject fields into the specified request.
+  virtual Status SetSubject(X509_REQ* req) const = 0;
+
   // Set the certificate-specific extensions into the specified request.
   virtual Status SetExtensions(X509_REQ* req) const = 0;
 
-  const Config config_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(CertRequestGeneratorBase);
 };
@@ -92,6 +84,18 @@ class CertRequestGeneratorBase {
 // (a.k.a. X509 CSRs).
 class CertRequestGenerator : public CertRequestGeneratorBase {
  public:
+  // Properties for the generated X509 CSR. The 'hostname' is for the name of
+  // the machine the requestor is to use the certificate at. Valid configuration
+  // should contain non-empty 'hostname' field.
+  struct Config {
+    // FQDN name to put into the 'DNS' fields of the subjectAltName extension.
+    std::string hostname;
+    // userId (UID)
+    boost::optional<std::string> user_id;
+    // Our custom extension which stores the full Kerberos principal for IPKI certs.
+    boost::optional<std::string> kerberos_principal;
+  };
+
   // 'config' contains the properties to fill into the X509 attributes of the
   // CSR.
   explicit CertRequestGenerator(Config config);
@@ -107,9 +111,11 @@ class CertRequestGenerator : public CertRequestGeneratorBase {
   }
 
  protected:
+  Status SetSubject(X509_REQ* req) const override WARN_UNUSED_RESULT;
   Status SetExtensions(X509_REQ* req) const override WARN_UNUSED_RESULT;
 
  private:
+  const Config config_;
   stack_st_X509_EXTENSION* extensions_ = nullptr;
   bool is_initialized_ = false;
   bool for_self_signing_ = false;
@@ -119,6 +125,15 @@ class CertRequestGenerator : public CertRequestGeneratorBase {
 // signing requests.
 class CaCertRequestGenerator : public CertRequestGeneratorBase {
  public:
+  // Properties for the generated X509 CA CSR.
+  struct Config {
+    // Common name (CN); e.g. 'master 239D6D2F-BDD2-4463-8933-78D9559C2124'.
+    // Don't put hostname/FQDN in here: for CA cert it does not make sense and
+    // it might be longer than 64 characters which is the limit specified
+    // by RFC5280. The limit is enforced by the OpenSSL library.
+    std::string cn;
+  };
+
   explicit CaCertRequestGenerator(Config config);
   ~CaCertRequestGenerator();
 
@@ -126,9 +141,11 @@ class CaCertRequestGenerator : public CertRequestGeneratorBase {
   bool Initialized() const override;
 
  protected:
+  Status SetSubject(X509_REQ* req) const override WARN_UNUSED_RESULT;
   Status SetExtensions(X509_REQ* req) const override WARN_UNUSED_RESULT;
 
  private:
+  const Config config_;
   stack_st_X509_EXTENSION* extensions_;
   mutable simple_spinlock lock_;
   bool is_initialized_; // protected by lock_

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/src/kudu/security/cert-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert-test.cc b/src/kudu/security/cert-test.cc
index 91774bd..275a60b 100644
--- a/src/kudu/security/cert-test.cc
+++ b/src/kudu/security/cert-test.cc
@@ -110,5 +110,29 @@ TEST_F(CertTest, TestGetKuduSpecificFieldsWhenMissing) {
   EXPECT_EQ(boost::none, ca_cert_.KuduKerberosPrincipal());
 }
 
+TEST_F(CertTest, DnsHostnameInSanField) {
+  const string hostname_foo_bar = "foo.bar.com";
+  const string hostname_mega_giga = "mega.giga.io";
+  const string hostname_too_long =
+      "toooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo."
+      "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+      "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+      "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+      "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+      "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
+      "ng.hostname.io";
+
+  Cert cert;
+  ASSERT_OK(cert.FromString(kCertDnsHostnamesInSan, DataFormat::PEM));
+
+  EXPECT_EQ("C = US, ST = CA, O = MyCompany, CN = MyName, emailAddress = my@email.com",
+            cert.IssuerName());
+  vector<string> hostnames = cert.Hostnames();
+  ASSERT_EQ(3, hostnames.size());
+  EXPECT_EQ(hostname_mega_giga, hostnames[0]);
+  EXPECT_EQ(hostname_foo_bar, hostnames[1]);
+  EXPECT_EQ(hostname_too_long, hostnames[2]);
+}
+
 } // namespace security
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/src/kudu/security/cert.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc
index 8bccc03..a47c7b9 100644
--- a/src/kudu/security/cert.cc
+++ b/src/kudu/security/cert.cc
@@ -37,6 +37,10 @@ using std::string;
 namespace kudu {
 namespace security {
 
+template<> struct SslTypeTraits<GENERAL_NAMES> {
+  static constexpr auto free = &GENERAL_NAMES_free;
+};
+
 // This OID is generated via the UUID method.
 static const char* kKuduKerberosPrincipalOidStr = "2.25.243346677289068076843480765133256509912";
 
@@ -88,6 +92,27 @@ boost::optional<string> Cert::UserId() const {
   return string(buf, len);
 }
 
+vector<string> Cert::Hostnames() const {
+  vector<string> result;
+  auto gens = ssl_make_unique(reinterpret_cast<GENERAL_NAMES*>(X509_get_ext_d2i(
+      data_.get(), NID_subject_alt_name, nullptr, nullptr)));
+  if (gens) {
+    for (int i = 0; i < sk_GENERAL_NAME_num(gens.get()); ++i) {
+      GENERAL_NAME* gen = sk_GENERAL_NAME_value(gens.get(), i);
+      if (gen->type != GEN_DNS) {
+        continue;
+      }
+      const ASN1_STRING* cstr = gen->d.dNSName;
+      if (cstr->type != V_ASN1_IA5STRING || cstr->data == nullptr) {
+        LOG(DFATAL) << "invalid DNS name in the SAN field";
+        return {};
+      }
+      result.emplace_back(reinterpret_cast<char*>(cstr->data), cstr->length);
+    }
+  }
+  return result;
+}
+
 boost::optional<string> Cert::KuduKerberosPrincipal() const {
   int idx = X509_get_ext_by_NID(data_.get(), GetKuduKerberosPrincipalOidNid(), -1);
   if (idx < 0) return boost::none;

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/src/kudu/security/cert.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert.h b/src/kudu/security/cert.h
index 02765d2..2facb2c 100644
--- a/src/kudu/security/cert.h
+++ b/src/kudu/security/cert.h
@@ -18,6 +18,7 @@
 #pragma once
 
 #include <string>
+#include <vector>
 
 #include <boost/optional/optional_fwd.hpp>
 
@@ -48,6 +49,9 @@ class Cert : public RawDataWrapper<X509> {
   std::string SubjectName() const;
   std::string IssuerName() const;
 
+  // Return DNS names from the SAN extension field.
+  std::vector<std::string> Hostnames() const;
+
   // Return the 'userId' extension of this cert, if set.
   boost::optional<std::string> UserId() const;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/src/kudu/security/test/test_certs.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/test/test_certs.cc b/src/kudu/security/test/test_certs.cc
index 5432906..cdc20b9 100644
--- a/src/kudu/security/test/test_certs.cc
+++ b/src/kudu/security/test/test_certs.cc
@@ -240,6 +240,34 @@ OwIDAQAB
 -----END PUBLIC KEY-----
 )***";
 
+const char kCertDnsHostnamesInSan[] = R"***(
+-----BEGIN CERTIFICATE-----
+MIIEPzCCAyegAwIBAgIJAJoczuNKGspGMA0GCSqGSIb3DQEBCwUAMFwxCzAJBgNV
+BAYTAlVTMQswCQYDVQQIDAJDQTESMBAGA1UECgwJTXlDb21wYW55MQ8wDQYDVQQD
+DAZNeU5hbWUxGzAZBgkqhkiG9w0BCQEWDG15QGVtYWlsLmNvbTAeFw0xNzA0Mjgx
+OTUwNTVaFw0yNzA0MjYxOTUwNTVaMAAwXDANBgkqhkiG9w0BAQEFAANLADBIAkEA
+rpJhLdS/Euf2cu0hPXkvkocLO0XbNtFwXNjkOOjuJZd65FHqLb6TmmxxDpL7fB94
+Mq1fD20fqdAgSVzljOyvuwIDAQABo4ICJjCCAiIwDgYDVR0PAQH/BAQDAgWgMCAG
+A1UdJQEB/wQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAMBgNVHRMBAf8EAjAAMIIB
+3gYDVR0RBIIB1TCCAdGCDG1lZ2EuZ2lnYS5pb4ILZm9vLmJhci5jb22CggGydG9v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vLmxvb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29v
+b29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vbmcuaG9zdG5hbWUuaW8w
+DQYJKoZIhvcNAQELBQADggEBAIKVVABj3nTyqDEnDKDfvS6QNEQSa1mw7WdFTpzH
+6cbMfiixVyyLqfAV4NZ+PnIa4mpsWP5LrsrWFVK/HtiTX7Y8oW0qdA04WtYd9VUT
+BgWKHyLygbA+PSZ6GdXFjZd8wDthK0qlT2MfZJUwD36eYnBxuonU8a4lmxaUG2zC
+L8FplhNJUEt6XfJ0zZGx1VHe12LLjgMz3ShDAmD9DlHHFjJ1aQ/17OGmmjWmbWnm
+an4ys5seqeHuK2WzP3NAx7LOwe/R1kHpEAX/Al6xyLIY3h7BBzurpgfrO6hTTECF
+561gUMp+cAvogw074thF5j4b+uEK5Bl8nzN2h8BwwxwGzUo=
+-----END CERTIFICATE-----
+)***";
+
 //
 // The reference signatures were obtained by using the following sequence:
 //  0. The reference private key was saved into /tmp/ca.pkey.pem file.

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/src/kudu/security/test/test_certs.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/test/test_certs.h b/src/kudu/security/test/test_certs.h
index 763d9ed..ea4e7a6 100644
--- a/src/kudu/security/test/test_certs.h
+++ b/src/kudu/security/test/test_certs.h
@@ -43,6 +43,8 @@ extern const char kCaExpiredCert[];
 extern const char kCaExpiredPrivateKey[];
 // The public part of the abovementioned private key.
 extern const char kCaExpiredPublicKey[];
+// Certificate with multiple DNS hostnames in the SAN field.
+extern const char kCertDnsHostnamesInSan[];
 
 extern const char kDataTiny[];
 extern const char kSignatureTinySHA512[];

http://git-wip-us.apache.org/repos/asf/kudu/blob/49c9d6eb/src/kudu/security/tls_context.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_context.cc b/src/kudu/security/tls_context.cc
index 09ab577..d5253ff 100644
--- a/src/kudu/security/tls_context.cc
+++ b/src/kudu/security/tls_context.cc
@@ -291,7 +291,7 @@ Status TlsContext::DumpTrustedCerts(vector<string>* cert_ders) const
{
 namespace {
 Status SetCertAttributes(CertRequestGenerator::Config* config) {
   SCOPED_OPENSSL_NO_PENDING_ERRORS;
-  RETURN_NOT_OK_PREPEND(GetFQDN(&config->cn), "could not determine FQDN for CSR");
+  RETURN_NOT_OK_PREPEND(GetFQDN(&config->hostname), "could not determine FQDN for
CSR");
 
   // If the server has logged in from a keytab, then we have a 'real' identity,
   // and our desired CN should match the local username mapped from the Kerberos
@@ -315,8 +315,6 @@ Status SetCertAttributes(CertRequestGenerator::Config* config) {
 
 Status TlsContext::GenerateSelfSignedCertAndKey() {
   SCOPED_OPENSSL_NO_PENDING_ERRORS;
-  CertRequestGenerator::Config config;
-  RETURN_NOT_OK(SetCertAttributes(&config));
   // Step 1: generate the private key to be self signed.
   PrivateKey key;
   RETURN_NOT_OK_PREPEND(GeneratePrivateKey(FLAGS_ipki_server_key_size,
@@ -325,9 +323,11 @@ Status TlsContext::GenerateSelfSignedCertAndKey() {
 
   // Step 2: generate a CSR so that the self-signed cert can eventually be
   // replaced with a CA-signed cert.
+  CertRequestGenerator::Config config;
+  RETURN_NOT_OK(SetCertAttributes(&config));
   CertRequestGenerator gen(config);
-  CertSignRequest csr;
   RETURN_NOT_OK_PREPEND(gen.Init(), "could not initialize CSR generator");
+  CertSignRequest csr;
   RETURN_NOT_OK_PREPEND(gen.GenerateRequest(key, &csr), "could not generate CSR");
 
   // Step 3: generate a self-signed cert that we can use for terminating TLS


Mime
View raw message