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 dfc016ada KUDU-3448 Move password retrieval to master init
dfc016ada is described below
commit dfc016ada35c988d3eb7b96597ba4455c65335bc
Author: Attila Bukor <[email protected]>
AuthorDate: Thu May 11 17:30:58 2023 +0200
KUDU-3448 Move password retrieval to master init
Apparently, forks can be expensive, at least on some systems, which
makes fetching the password to encrypt/decrypt key materials whenever a
TSK or IPKI is generated or loaded into memory not ideal as it may cause
the server process to hang.
This patch moves the password retrieval to the master initialization
phase and the passwords are now stored in memory for later use.
Change-Id: I746e657ae8d295f5f34225d63686beea1dff6b7c
Reviewed-on: http://gerrit.cloudera.org:8080/19873
Tested-by: Attila Bukor <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/master/catalog_manager.cc | 22 ++++++++--------------
src/kudu/master/catalog_manager.h | 13 +++++++++++++
src/kudu/master/master.cc | 32 +++++++++++++++++++++++++++++++-
src/kudu/security/token_signer.cc | 17 +++++++++++------
src/kudu/security/token_signer.h | 6 +++++-
src/kudu/security/token_signing_key.cc | 27 ++++++++++-----------------
src/kudu/security/token_signing_key.h | 6 ++++--
7 files changed, 82 insertions(+), 41 deletions(-)
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 790d130bf..588d20707 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -431,10 +431,6 @@ 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() {
@@ -1027,7 +1023,9 @@ CatalogManager::CatalogManager(Master* master)
state_(kConstructed),
leader_ready_term_(-1),
hms_notification_log_event_id_(-1),
- leader_lock_(RWMutex::Priority::PREFER_WRITING) {
+ leader_lock_(RWMutex::Priority::PREFER_WRITING),
+ ipki_private_key_password_(""),
+ tsk_private_key_password_("") {
if (RangerAuthzProvider::IsEnabled()) {
authz_provider_.reset(new RangerAuthzProvider(master_->fs_manager()->env(),
master_->metric_entity()));
@@ -1314,16 +1312,14 @@ Status
CatalogManager::LoadCertAuthorityInfo(unique_ptr<PrivateKey>* key,
unique_ptr<PrivateKey> ca_private_key(new PrivateKey);
unique_ptr<Cert> ca_cert(new Cert);
- if (FLAGS_ipki_private_key_password_cmd.empty()) {
+ if (ipki_private_key_password_.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");
+ *password = ipki_private_key_password_;
return Status::OK();
}
), "could not decrypt private key with the password returned by the
configured command");
@@ -1358,14 +1354,12 @@ Status CatalogManager::StoreCertAuthorityInfo(const
PrivateKey& key,
leader_lock_.AssertAcquiredForWriting();
SysCertAuthorityEntryPB info;
- if (FLAGS_ipki_private_key_password_cmd.empty()) {
+ if (ipki_private_key_password_.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");
+ *password = ipki_private_key_password_;
return Status::OK();
}
));
@@ -5766,7 +5760,7 @@ Status
CatalogManager::LoadTspkEntries(vector<TokenSigningPublicKeyPB>* keys) {
RETURN_NOT_OK(sys_catalog_->VisitTskEntries(&loader));
for (const auto& private_key : loader.entries()) {
// Extract public parts of the loaded keys for the verifier.
- TokenSigningPrivateKey tsk(private_key);
+ TokenSigningPrivateKey tsk(private_key, tsk_private_key_password_);
TokenSigningPublicKeyPB key;
tsk.ExportPublicKeyPB(&key);
auto key_seq_num = key.key_seq_num();
diff --git a/src/kudu/master/catalog_manager.h
b/src/kudu/master/catalog_manager.h
index b17c29ddd..b15952417 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -929,6 +929,14 @@ class CatalogManager : public
tserver::TabletReplicaLookupIf {
scoped_refptr<TableInfo> FindTableWithNameUnlocked(const std::string&
table_name,
TableInfoMapType map_type
= kAllTableType);
+ void set_ipki_private_key_password(std::string ipki_private_key_password) {
+ ipki_private_key_password_ = std::move(ipki_private_key_password);
+ }
+
+ void set_tsk_private_key_password(std::string tsk_private_key_password) {
+ tsk_private_key_password_ = std::move(tsk_private_key_password);
+ }
+
private:
// These tests calls ElectedAsLeaderCb() directly.
FRIEND_TEST(kudu::client::ClientTest, TestSoftDeleteAndReserveTable);
@@ -1389,6 +1397,11 @@ class CatalogManager : public
tserver::TabletReplicaLookupIf {
// Always acquire this lock before state_lock_.
RWMutex leader_lock_;
+ // Password for the encrypted IPKI and TSK private keys stored in the
+ // sys-catalog table.
+ std::string ipki_private_key_password_;
+ std::string tsk_private_key_password_;
+
// Async operations are accessing some private methods
// (TODO: this stuff should be deferred and done in the background thread)
friend class AsyncAlterTable;
diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 79c83ee7c..07925e727 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -67,6 +67,7 @@
#include "kudu/util/monotime.h"
#include "kudu/util/net/net_util.h"
#include "kudu/util/net/sockaddr.h"
+#include "kudu/util/openssl_util.h"
#include "kudu/util/status.h"
#include "kudu/util/thread.h"
#include "kudu/util/threadpool.h"
@@ -122,6 +123,14 @@ DEFINE_string(master_rpc_proxy_advertised_addresses, "",
"flag for a single master.");
TAG_FLAG(master_rpc_proxy_advertised_addresses, experimental);
+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.");
+
+DEFINE_string(tsk_private_key_password_cmd, "",
+ "A Unix command whose output returns the password to encrypt "
+ "and decrypt the token signing key");
+
DECLARE_bool(txn_manager_lazily_initialized);
DECLARE_bool(txn_manager_enabled);
DECLARE_string(master_addresses);
@@ -254,6 +263,26 @@ Master::~Master() {
Status Master::Init() {
CHECK_EQ(kStopped, state_);
+ // As getting a password from a shell command relies on forking the process,
+ // it's best to do this in the very beginning of Init() to avoid having to
+ // fork unnecessary threads.
+ string ipki_private_key_password;
+ if (!FLAGS_ipki_private_key_password_cmd.empty()) {
+ RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
+ FLAGS_ipki_private_key_password_cmd, &ipki_private_key_password),
+ "could not get IPKI private key password from configured command");
+ }
+
+ string tsk_private_key_password;
+ if (!FLAGS_tsk_private_key_password_cmd.empty()) {
+ RETURN_NOT_OK_PREPEND(security::GetPasswordFromShellCommand(
+ FLAGS_tsk_private_key_password_cmd, &tsk_private_key_password),
+ "could not get TSK private key password from configured command");
+ }
+
+
catalog_manager_->set_ipki_private_key_password(std::move(ipki_private_key_password));
+ catalog_manager_->set_tsk_private_key_password(tsk_private_key_password);
+
cfile::BlockCache::GetSingleton()->StartInstrumentation(
metric_entity(), opts_.block_cache_metrics_policy());
@@ -280,7 +309,8 @@ Status Master::Init() {
FLAGS_authn_token_validity_seconds,
FLAGS_authz_token_validity_seconds,
FLAGS_tsk_rotation_seconds,
- messenger_->shared_token_verifier()));
+ messenger_->shared_token_verifier(),
+ std::move(tsk_private_key_password)));
if (!FLAGS_master_rpc_proxy_advertised_addresses.empty()) {
vector<HostPort> host_ports;
diff --git a/src/kudu/security/token_signer.cc
b/src/kudu/security/token_signer.cc
index aaf1be80c..c01475100 100644
--- a/src/kudu/security/token_signer.cc
+++ b/src/kudu/security/token_signer.cc
@@ -23,6 +23,7 @@
#include <memory>
#include <mutex>
#include <string>
+#include <type_traits>
#include <utility>
#include <vector>
@@ -58,7 +59,8 @@ namespace security {
TokenSigner::TokenSigner(int64_t authn_token_validity_seconds,
int64_t authz_token_validity_seconds,
int64_t key_rotation_seconds,
- shared_ptr<TokenVerifier> verifier)
+ shared_ptr<TokenVerifier> verifier,
+ string private_key_password)
: verifier_(verifier ? std::move(verifier)
: std::make_shared<TokenVerifier>()),
authn_token_validity_seconds_(authn_token_validity_seconds),
@@ -67,7 +69,8 @@ TokenSigner::TokenSigner(int64_t authn_token_validity_seconds,
// The TSK propagation interval is equal to the rotation interval.
key_validity_seconds_(2 * key_rotation_seconds_ +
std::max(authn_token_validity_seconds_,
authz_token_validity_seconds)),
- last_key_seq_num_(-1) {
+ last_key_seq_num_(-1),
+ private_key_password_(std::move(private_key_password)) {
CHECK_GE(key_rotation_seconds_, 0);
CHECK_GE(authn_token_validity_seconds_, 0);
CHECK_GE(authz_token_validity_seconds_, 0);
@@ -91,7 +94,7 @@ Status TokenSigner::ImportKeys(const
vector<TokenSigningPrivateKeyPB>& keys) {
CHECK(key.has_rsa_key_der());
const int64_t key_seq_num = key.key_seq_num();
- unique_ptr<TokenSigningPrivateKey> tsk(new TokenSigningPrivateKey(key));
+ unique_ptr<TokenSigningPrivateKey> tsk(new TokenSigningPrivateKey(key,
private_key_password_));
// Advance the key sequence number, if needed. For the use case when the
// history of keys sequence numbers is important, the generated keys are
@@ -211,7 +214,7 @@ Status
TokenSigner::CheckNeedKey(unique_ptr<TokenSigningPrivateKey>* tsk) const
// Generation of cryptographically strong key takes many CPU cycles;
// do not want to block other parallel activity.
l.unlock();
- return GenerateSigningKey(key_seq_num, key_expiration, tsk);
+ return GenerateSigningKey(key_seq_num, key_expiration,
private_key_password_, tsk);
}
if (tsk_deque_.size() >= 2) {
@@ -244,7 +247,7 @@ Status
TokenSigner::CheckNeedKey(unique_ptr<TokenSigningPrivateKey>* tsk) const
// Generation of cryptographically strong key takes many CPU cycles:
// do not want to block other parallel activity.
l.unlock();
- return GenerateSigningKey(key_seq_num, key_expiration, tsk);
+ return GenerateSigningKey(key_seq_num, key_expiration,
private_key_password_, tsk);
}
// It's not yet time to generate a new key.
@@ -310,6 +313,7 @@ Status TokenSigner::TryRotateKey(bool* has_rotated) {
Status TokenSigner::GenerateSigningKey(int64_t key_seq_num,
int64_t key_expiration,
+ const string& password,
unique_ptr<TokenSigningPrivateKey>*
tsk) {
unique_ptr<PrivateKey> key(new PrivateKey());
RETURN_NOT_OK_PREPEND(
@@ -317,7 +321,8 @@ Status TokenSigner::GenerateSigningKey(int64_t key_seq_num,
"could not generate new RSA token-signing key");
tsk->reset(new TokenSigningPrivateKey(key_seq_num,
key_expiration,
- std::move(key)));
+ std::move(key),
+ password));
return Status::OK();
}
diff --git a/src/kudu/security/token_signer.h b/src/kudu/security/token_signer.h
index 36f5a6552..2d34f58bd 100644
--- a/src/kudu/security/token_signer.h
+++ b/src/kudu/security/token_signer.h
@@ -216,7 +216,8 @@ class TokenSigner {
TokenSigner(int64_t authn_token_validity_seconds,
int64_t authz_token_validity_seconds,
int64_t key_rotation_seconds,
- std::shared_ptr<TokenVerifier> verifier = nullptr);
+ std::shared_ptr<TokenVerifier> verifier = nullptr,
+ std::string private_key_password = "");
~TokenSigner();
// Import token signing keys in PB format, notifying TokenVerifier
@@ -314,6 +315,7 @@ class TokenSigner {
static Status GenerateSigningKey(int64_t key_seq_num,
int64_t key_expiration,
+ const std::string& password,
std::unique_ptr<TokenSigningPrivateKey>*
tsk) WARN_UNUSED_RESULT;
std::shared_ptr<TokenVerifier> verifier_;
@@ -338,6 +340,8 @@ class TokenSigner {
// The sequence number of the last generated/imported key.
int64_t last_key_seq_num_;
+ const std::string private_key_password_;
+
// The currently active key is in the front of the queue,
// the newly added ones are pushed into back of the queue.
std::deque<std::unique_ptr<TokenSigningPrivateKey>> tsk_deque_;
diff --git a/src/kudu/security/token_signing_key.cc
b/src/kudu/security/token_signing_key.cc
index 733cdf832..20667ce30 100644
--- a/src/kudu/security/token_signing_key.cc
+++ b/src/kudu/security/token_signing_key.cc
@@ -22,7 +22,6 @@
#include <string>
#include <utility>
-#include <gflags/gflags.h>
#include <glog/logging.h>
#include "kudu/security/crypto.h"
@@ -30,10 +29,6 @@
#include "kudu/util/openssl_util.h"
#include "kudu/util/status.h"
-DEFINE_string(tsk_private_key_password_cmd, "",
- "A Unix command whose output returns the password to encrypt "
- "and decrypt the token signing key");
-
using kudu::security::PasswordCallback;
using std::unique_ptr;
using std::string;
@@ -64,16 +59,15 @@ bool TokenSigningPublicKey::VerifySignature(const
SignedTokenPB& token) const {
}
TokenSigningPrivateKey::TokenSigningPrivateKey(
- const TokenSigningPrivateKeyPB& pb)
+ const TokenSigningPrivateKeyPB& pb,
+ const string& password)
: key_(new PrivateKey) {
- if (FLAGS_tsk_private_key_password_cmd.empty()) {
+ if (password.empty()) {
CHECK_OK(key_->FromString(pb.rsa_key_der(), DataFormat::DER));
} else {
CHECK_OK(key_->FromEncryptedString(pb.rsa_key_der(), DataFormat::DER,
- [&](string* password) {
- RETURN_NOT_OK_PREPEND(GetPasswordFromShellCommand(
- FLAGS_tsk_private_key_password_cmd, password),
- "could not get TSK private key password from configured
command");
+ [&](string* p) {
+ *p = password;
return Status::OK();
}
));
@@ -88,18 +82,17 @@ TokenSigningPrivateKey::TokenSigningPrivateKey(
}
TokenSigningPrivateKey::TokenSigningPrivateKey(
- int64_t key_seq_num, int64_t expire_time, unique_ptr<PrivateKey> key)
+ int64_t key_seq_num, int64_t expire_time, unique_ptr<PrivateKey> key,
+ const std::string& password)
: key_(std::move(key)),
key_seq_num_(key_seq_num),
expire_time_(expire_time) {
- if (FLAGS_tsk_private_key_password_cmd.empty()) {
+ if (password.empty()) {
CHECK_OK(key_->ToString(&private_key_der_, DataFormat::DER));
} else {
CHECK_OK(key_->ToEncryptedString(&private_key_der_, DataFormat::DER,
- [&](string* password){
- RETURN_NOT_OK_PREPEND(GetPasswordFromShellCommand(
- FLAGS_tsk_private_key_password_cmd, password),
- "could not get TSK private key password from configured
command");
+ [&](string* p){
+ *p = password;
return Status::OK();
}
));
diff --git a/src/kudu/security/token_signing_key.h
b/src/kudu/security/token_signing_key.h
index 67ecf9522..ae89ec24d 100644
--- a/src/kudu/security/token_signing_key.h
+++ b/src/kudu/security/token_signing_key.h
@@ -65,10 +65,12 @@ class TokenSigningPublicKey {
// number and expiration date.
class TokenSigningPrivateKey {
public:
- explicit TokenSigningPrivateKey(const TokenSigningPrivateKeyPB& pb);
+ TokenSigningPrivateKey(const TokenSigningPrivateKeyPB& pb,
+ const std::string& password = "");
TokenSigningPrivateKey(int64_t key_seq_num,
int64_t expire_time,
- std::unique_ptr<PrivateKey> key);
+ std::unique_ptr<PrivateKey> key,
+ const std::string& password = "");
~TokenSigningPrivateKey();
// Sign a token, and store the signature and signing key's sequence number.
