This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 5542d1db3 KUDU-3448 Add support for encrypting IPKI keys
5542d1db3 is described below
commit 5542d1db30cb67a6b224ae4d4539a4e022856c98
Author: Attila Bukor <abu...@apache.org>
AuthorDate: Mon Mar 13 16:37:02 2023 +0100
KUDU-3448 Add support for encrypting IPKI keys
This patch introduces a new flag, --ipki_private_key_password_cmd. If
set, Kudu's internal PKI's root CA private key will be encrypted with
the password that is output by the command set with this flag.
The key is encrypted with AES-256-CBC and encoded in PKCS#8 format. The
behavior is similar to --webserver_private_key_password_cmd, which is
used to provide a command to decrypt the webserver certificate's private
key.
Currently, Kudu doesn't support rotating IPKI keys, so this flag can't
be used on existing clusters, and if it was used on the first startup of
a master, it must be used as long as that master exists, it won't be
able to start without it.
Change-Id: I71f2ec856f018d56efbf6901039eed2676fcbe23
Reviewed-on: http://gerrit.cloudera.org:8080/19616
Reviewed-by: Alexey Serbin <ale...@apache.org>
Reviewed-by: Zoltan Chovan <zcho...@cloudera.com>
Tested-by: Kudu Jenkins
---
src/kudu/master/catalog_manager.cc | 34 +++++++++++++++++++--
src/kudu/master/master-test.cc | 17 +++++++++--
src/kudu/master/sys_catalog-test.cc | 61 ++++++++++++++++++++++++++++++++++++-
src/kudu/rpc/messenger.cc | 9 +++---
src/kudu/util/openssl_util.h | 2 +-
src/kudu/util/openssl_util_bio.h | 7 ++++-
6 files changed, 117 insertions(+), 13 deletions(-)
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index c2ff76a92..d54c08343 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -425,6 +425,10 @@ DEFINE_uint32(default_deleted_table_reserve_seconds, 0,
TAG_FLAG(default_deleted_table_reserve_seconds, advanced);
TAG_FLAG(default_deleted_table_reserve_seconds, runtime);
+DEFINE_string(ipki_private_key_password_cmd, "",
+ "A Unix command whose output returns the password to encrypt "
+ "and decrypt the IPKI root CA private key.");
+
DECLARE_string(hive_metastore_uris);
bool ValidateDeletedTableReserveSeconds() {
@@ -1304,8 +1308,21 @@ Status
CatalogManager::LoadCertAuthorityInfo(unique_ptr<PrivateKey>* key,
unique_ptr<PrivateKey> ca_private_key(new PrivateKey);
unique_ptr<Cert> ca_cert(new Cert);
- RETURN_NOT_OK(ca_private_key->FromString(
- info.private_key(), DataFormat::DER));
+ if (FLAGS_ipki_private_key_password_cmd.empty()) {
+ RETURN_NOT_OK(ca_private_key->FromString(
+ info.private_key(), DataFormat::DER));
+ } else {
+ RETURN_NOT_OK_PREPEND(ca_private_key->FromEncryptedString(
+ info.private_key(), DataFormat::DER,
+ [&](string* password){
+ RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
+ FLAGS_ipki_private_key_password_cmd, password),
+ "could not get IPKI private key password from configured
command");
+ return Status::OK();
+ }
+ ), "could not decrypt private key with the password returned by the
configured command");
+ }
+
RETURN_NOT_OK(ca_cert->FromString(
info.certificate(), DataFormat::DER));
// Extra sanity check.
@@ -1335,7 +1352,18 @@ Status CatalogManager::StoreCertAuthorityInfo(const
PrivateKey& key,
leader_lock_.AssertAcquiredForWriting();
SysCertAuthorityEntryPB info;
- RETURN_NOT_OK(key.ToString(info.mutable_private_key(), DataFormat::DER));
+ if (FLAGS_ipki_private_key_password_cmd.empty()) {
+ RETURN_NOT_OK(key.ToString(info.mutable_private_key(), DataFormat::DER));
+ } else {
+ RETURN_NOT_OK(key.ToEncryptedString(info.mutable_private_key(),
DataFormat::DER,
+ [&](string* password){
+ RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
+ FLAGS_ipki_private_key_password_cmd, password),
+ "could not get IPKI private key password from configured
command");
+ return Status::OK();
+ }
+ ));
+ }
RETURN_NOT_OK(cert.ToString(info.mutable_certificate(), DataFormat::DER));
RETURN_NOT_OK(sys_catalog_->AddCertAuthorityEntry(info));
LOG(INFO) << "Generated new certificate authority record";
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 5fdd2c664..ae290a3f7 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -146,6 +146,7 @@ DECLARE_int32(max_table_comment_length);
DECLARE_int32(rpc_service_queue_length);
DECLARE_int64(live_row_count_for_testing);
DECLARE_int64(on_disk_size_for_testing);
+DECLARE_string(ipki_private_key_password_cmd);
DECLARE_string(location_mapping_cmd);
DECLARE_string(log_filename);
DECLARE_string(webserver_doc_root);
@@ -3032,9 +3033,20 @@ TEST_F(MasterTest,
TestConcurrentRenameAndDeleteOfSameTable) {
ASSERT_TRUE(renamed ^ deleted);
}
+class MasterWithEncryptedKeysTest : public MasterTest,
+ public ::testing::WithParamInterface<bool>
{
+ public:
+ void SetUp() override {
+ if (GetParam()) {
+ FLAGS_ipki_private_key_password_cmd = "echo test";
+ }
+ MasterTest::SetUp();
+ }
+};
+
// Unit tests for the ConnectToMaster() RPC:
// should issue authentication tokens and the master CA cert.
-TEST_F(MasterTest, TestConnectToMaster) {
+TEST_P(MasterWithEncryptedKeysTest, TestConnectToMaster) {
ConnectToMasterRequestPB req;
ConnectToMasterResponsePB resp;
RpcController rpc;
@@ -3072,6 +3084,7 @@ TEST_F(MasterTest, TestConnectToMaster) {
ASSERT_TRUE(!cluster_id.empty());
ASSERT_EQ(cluster_id, resp.cluster_id());
}
+INSTANTIATE_TEST_SUITE_P(SupportsEncryptedKeys, MasterWithEncryptedKeysTest,
::testing::Bool());
TEST_F(MasterTest, TestConnectToMasterAndAssignLocation) {
// Test first with a valid location mapping command.
@@ -3126,7 +3139,7 @@ TEST_F(MasterTest, TestConnectToMasterAndAssignLocation) {
}
}
-// Test that the master signs its on server certificate when it becomes the
leader,
+// Test that the master signs its own server certificate when it becomes the
leader,
// and also that it loads TSKs into the messenger's verifier.
TEST_F(MasterTest, TestSignOwnCertAndLoadTSKs) {
ASSERT_EVENTUALLY([&]() {
diff --git a/src/kudu/master/sys_catalog-test.cc
b/src/kudu/master/sys_catalog-test.cc
index e28a10625..3841f1bcb 100644
--- a/src/kudu/master/sys_catalog-test.cc
+++ b/src/kudu/master/sys_catalog-test.cc
@@ -17,11 +17,13 @@
#include "kudu/master/sys_catalog.h"
+#include <functional>
#include <memory>
#include <string>
#include <utility>
#include <vector>
+#include <gflags/gflags_declare.h>
#include <gtest/gtest.h>
#include "kudu/common/common.pb.h"
@@ -54,7 +56,8 @@ using kudu::security::PrivateKey;
using std::shared_ptr;
using std::string;
using std::unique_ptr;
-using std::vector;
+
+DECLARE_string(ipki_private_key_password_cmd);
namespace google {
namespace protobuf {
@@ -386,6 +389,62 @@ TEST_F(SysCatalogTest, AttemptOverwriteCertAuthorityInfo) {
ASSERT_EQ("Corruption: failed to write one or more rows", s.ToString());
}
+class SysCatalogEncryptedCertTest : public SysCatalogTest {
+ public:
+ const string kPassword = "test";
+ void SetUp() override {
+ FLAGS_ipki_private_key_password_cmd = "echo " + kPassword;
+ SysCatalogTest::SetUp();
+ }
+};
+
+TEST_F(SysCatalogEncryptedCertTest, LoadCertAuthorityInfoEncrypted) {
+ SysCertAuthorityEntryPB ca_entry;
+ ASSERT_OK(master_->catalog_manager()->sys_catalog()->
+ GetCertAuthorityEntry(&ca_entry));
+
+ // Decrypting key without password should not work.
+ {
+ PrivateKey pkey;
+ Status s = pkey.FromString(ca_entry.private_key(), DataFormat::DER);
+ ASSERT_TRUE(s.IsRuntimeError());
+ }
+
+ // Decrypting key with incorrect password should not work.
+ {
+ PrivateKey pkey;
+ ASSERT_TRUE(pkey.FromEncryptedString(ca_entry.private_key(),
+ DataFormat::DER,
+ [&](string* password) {
+ *password = "not " + kPassword;
+ return Status::OK();
+ }).IsRuntimeError());
+ }
+
+ // Decrypting key with correct password should work.
+ {
+ PrivateKey pkey;
+ ASSERT_OK(pkey.FromEncryptedString(ca_entry.private_key(),
+ DataFormat::DER,
+ [&](string* password) {
+ *password = kPassword;
+ return Status::OK();
+ }));
+ }
+
+ // Password callback returning an error should cause the decryption to fail.
+ {
+ PrivateKey pkey;
+ ASSERT_TRUE(pkey.FromEncryptedString(ca_entry.private_key(),
+ DataFormat::DER,
+ [&](string* password) {
+ *password = kPassword;
+ return Status::RuntimeError("Something
went wrong");
+ }).IsRuntimeError());
+ }
+
+}
+
// Check loading the auto-generated cluster ID upon startup.
TEST_F(SysCatalogTest, LoadClusterID) {
// The system catalog should already contain a generated cluster ID:
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 1ce94fa39..afda86c77 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -123,12 +123,11 @@ Status MessengerBuilder::Build(shared_ptr<Messenger>*
msgr) {
} else {
RETURN_NOT_OK(tls_context->LoadCertificateAndPasswordProtectedKey(
rpc_certificate_file_, rpc_private_key_file_,
- [&](){
- string ret;
- WARN_NOT_OK(security::GetPasswordFromShellCommand(
- rpc_private_key_password_cmd_, &ret),
+ [&](string* password){
+ RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
+ rpc_private_key_password_cmd_, password),
"could not get RPC password from configured command");
- return ret;
+ return Status::OK();
}
));
}
diff --git a/src/kudu/util/openssl_util.h b/src/kudu/util/openssl_util.h
index d1e4b16a3..97daf7121 100644
--- a/src/kudu/util/openssl_util.h
+++ b/src/kudu/util/openssl_util.h
@@ -80,7 +80,7 @@ typedef struct x509_st X509;
namespace kudu {
namespace security {
-using PasswordCallback = std::function<std::string(void)>;
+using PasswordCallback = std::function<Status(std::string*)>;
// Disable initialization of OpenSSL. Must be called before
// any call to InitializeOpenSSL().
diff --git a/src/kudu/util/openssl_util_bio.h b/src/kudu/util/openssl_util_bio.h
index 92861ac82..1baf82dd2 100644
--- a/src/kudu/util/openssl_util_bio.h
+++ b/src/kudu/util/openssl_util_bio.h
@@ -55,7 +55,12 @@ Status ToBIO(BIO* bio, DataFormat format, TYPE* obj,
// a password protected private key.
inline int TLSPasswordCB(char* buf, int size, int /* rwflag */, void*
userdata) {
const auto* cb = reinterpret_cast<const PasswordCallback*>(userdata);
- std::string pw = (*cb)();
+ std::string pw;
+ Status s = (*cb)(&pw);
+ if (!s.ok()) {
+ LOG(ERROR) << "Failed to obtain password: " << s.ToString();
+ return -1;
+ }
if (pw.size() >= size) {
LOG(ERROR) << "Provided key password is longer than maximum length "
<< size;
