You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2017/02/23 01:02:20 UTC
kudu git commit: IPKI: remove unused fields from cert code
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 <da...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Todd Lipcon <to...@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 <to...@apache.org>
Authored: Wed Feb 22 10:48:35 2017 -0800
Committer: Todd Lipcon <to...@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;