You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ab...@apache.org on 2020/11/03 22:37:41 UTC
[kudu] 02/02: KUDU-3210 Increase key size in tests and EMC
This is an automated email from the ASF dual-hosted git repository.
abukor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 75b113eafbf93a93fc1ec0c8e15d29fccd65b477
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Tue Oct 20 12:21:51 2020 +0200
KUDU-3210 Increase key size in tests and EMC
The cryptographic key sizes were lowered in tests to speed up execution.
As FIPS approved mode requires larger key sizes, these tests fail with
the small key size. This commit introduces a
KUDU_USE_LARGE_KEYS_IN_TESTS environment variable for tests. If it's set
to true, the test-only key size restrictions are removed.
Change-Id: I45b96e0b4499ea1d80db5871a529c732ad41220e
Reviewed-on: http://gerrit.cloudera.org:8080/16658
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
src/kudu/integration-tests/authz_token-itest.cc | 3 +-
.../master_cert_authority-itest.cc | 3 +-
.../security-unknown-tsk-itest.cc | 5 +-
src/kudu/master/master-test.cc | 7 ++-
src/kudu/mini-cluster/external_mini_cluster.cc | 53 ++++++++++++----------
src/kudu/security/ca/cert_management-test.cc | 3 +-
src/kudu/security/token-test.cc | 2 +-
.../tserver/tablet_server_authorization-test.cc | 3 +-
src/kudu/util/test_util.cc | 23 ++++++----
src/kudu/util/test_util.h | 4 ++
10 files changed, 64 insertions(+), 42 deletions(-)
diff --git a/src/kudu/integration-tests/authz_token-itest.cc b/src/kudu/integration-tests/authz_token-itest.cc
index 395702c..ecec994 100644
--- a/src/kudu/integration-tests/authz_token-itest.cc
+++ b/src/kudu/integration-tests/authz_token-itest.cc
@@ -369,7 +369,8 @@ TEST_F(AuthzTokenTest, TestUnknownTsk) {
// the server.
TokenSigningPrivateKeyPB tsk;
PrivateKey private_key;
- ASSERT_OK(GeneratePrivateKey(/*num_bits=*/512, &private_key));
+ int key_size = UseLargeKeys() ? 2048 : 512;
+ ASSERT_OK(GeneratePrivateKey(key_size, &private_key));
string private_key_str_der;
ASSERT_OK(private_key.ToString(&private_key_str_der, DataFormat::DER));
tsk.set_rsa_key_der(private_key_str_der);
diff --git a/src/kudu/integration-tests/master_cert_authority-itest.cc b/src/kudu/integration-tests/master_cert_authority-itest.cc
index f933fb3..088bba9 100644
--- a/src/kudu/integration-tests/master_cert_authority-itest.cc
+++ b/src/kudu/integration-tests/master_cert_authority-itest.cc
@@ -260,7 +260,8 @@ TEST_F(MasterCertAuthorityTest, CertAuthorityOnLeaderRoleSwitch) {
void GenerateCSR(const CertRequestGenerator::Config& gen_config,
string* csr_str) {
PrivateKey key;
- ASSERT_OK(security::GeneratePrivateKey(512, &key));
+ int key_size = UseLargeKeys() ? 2048 : 512;
+ ASSERT_OK(security::GeneratePrivateKey(key_size, &key));
CertRequestGenerator gen(gen_config);
ASSERT_OK(gen.Init());
CertSignRequest csr;
diff --git a/src/kudu/integration-tests/security-unknown-tsk-itest.cc b/src/kudu/integration-tests/security-unknown-tsk-itest.cc
index f270db9..35f90aa 100644
--- a/src/kudu/integration-tests/security-unknown-tsk-itest.cc
+++ b/src/kudu/integration-tests/security-unknown-tsk-itest.cc
@@ -122,9 +122,10 @@ class SecurityUnknownTskTest : public KuduTest {
}
// Generate custom TSK.
- Status GenerateTsk(TokenSigningPrivateKeyPB* tsk, int64_t seq_num = 100) {
+ static Status GenerateTsk(TokenSigningPrivateKeyPB* tsk, int64_t seq_num = 100) {
PrivateKey private_key;
- RETURN_NOT_OK(GeneratePrivateKey(512, &private_key));
+ int key_size = UseLargeKeys() ? 2048 : 512;
+ RETURN_NOT_OK(GeneratePrivateKey(key_size, &private_key));
string private_key_str_der;
RETURN_NOT_OK(private_key.ToString(&private_key_str_der, DataFormat::DER));
tsk->set_rsa_key_der(private_key_str_der);
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 3ecf4ed..0caf538 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -1967,8 +1967,11 @@ TEST_F(MasterTest, TestConnectToMaster) {
ASSERT_EQ(1, resp.ca_cert_der_size()) << "should have one cert";
EXPECT_GT(resp.ca_cert_der(0).size(), 100) << "CA cert should be at least 100 bytes";
ASSERT_TRUE(resp.has_authn_token()) << "should return an authn token";
- // Using 512 bit RSA key and SHA256 digest results in 64 byte signature.
- EXPECT_EQ(64, resp.authn_token().signature().size());
+ // Using 512 bit RSA key and SHA256 digest results in 64 byte signature. If
+ // large keys are used, we use 2048 bit RSA keys so the signature is 256
+ // bytes.
+ int signature_size = UseLargeKeys() ? 256 : 64;
+ EXPECT_EQ(signature_size, resp.authn_token().signature().size());
ASSERT_TRUE(resp.authn_token().has_signing_key_seq_num());
EXPECT_GT(resp.authn_token().signing_key_seq_num(), -1);
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index 3da89a7..4c81ef7 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -975,20 +975,6 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
// rely on forcefully cutting power to a machine or equivalent.
"--never_fsync",
- // Generate smaller RSA keys -- generating a 768-bit key is faster
- // than generating the default 2048-bit key, and we don't care about
- // strong encryption in tests. Setting it lower (e.g. 512 bits) results
- // in OpenSSL errors RSA_sign:digest too big for rsa key:rsa_sign.c:122
- // since we are using strong/high TLS v1.2 cipher suites, so the minimum
- // size of TLS-related RSA key is 768 bits (due to the usage of
- // the ECDHE-RSA-AES256-GCM-SHA384 suite).
- "--ipki_server_key_size=768",
-
- // The RSA key of 768 bits is too short if OpenSSL security level is set to
- // 1 or higher (applicable for OpenSSL 1.1.0 and newer). Lowering the
- // security level to 0 makes possible ot use shorter keys in such cases.
- "--openssl_security_level_override=0",
-
// Disable minidumps by default since many tests purposely inject faults.
"--enable_minidumps=false",
@@ -1022,6 +1008,23 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
argv.emplace_back("--logbuflevel=-1");
}
+ // If large keys are not enabled.
+ if (!UseLargeKeys()) {
+ // Generate smaller RSA keys -- generating a 768-bit key is faster
+ // than generating the default 2048-bit key, and we don't care about
+ // strong encryption in tests. Setting it lower (e.g. 512 bits) results
+ // in OpenSSL errors RSA_sign:digest too big for rsa key:rsa_sign.c:122
+ // since we are using strong/high TLS v1.2 cipher suites, so the minimum
+ // size of TLS-related RSA key is 768 bits (due to the usage of
+ // the ECDHE-RSA-AES256-GCM-SHA384 suite).
+ argv.emplace_back("--ipki_server_key_size=768");
+
+ // The RSA key of 768 bits is too short if OpenSSL security level is set to
+ // 1 or higher (applicable for OpenSSL 1.1.0 and newer). Lowering the
+ // security level to 0 makes possible ot use shorter keys in such cases.
+ argv.emplace_back("--openssl_security_level_override=0");
+ }
+
// Add all the flags coming from the minicluster framework.
argv.insert(argv.end(), user_flags.begin(), user_flags.end());
@@ -1447,19 +1450,21 @@ Status ExternalMaster::WaitForCatalogManager(WaitMode wait_mode) {
}
const vector<string>& ExternalMaster::GetCommonFlags() {
- static const vector<string> kFlags = {
- // See the in-line comment for "--ipki_server_key_size" flag in
- // ExternalDaemon::StartProcess() method.
- "--ipki_ca_key_size=768",
-
- // As for the TSK keys, 512 bits is the minimum since we are using
- // SHA256 digest for token signing/verification.
- "--tsk_num_rsa_bits=512",
- };
+ static vector<string> kFlags;
+ if (!UseLargeKeys()) {
+ kFlags = {
+ // See the in-line comment for "--ipki_server_key_size" flag in
+ // ExternalDaemon::StartProcess() method.
+ "--ipki_ca_key_size=768",
+
+ // As for the TSK keys, 512 bits is the minimum since we are using
+ // SHA256 digest for token signing/verification.
+ "--tsk_num_rsa_bits=512",
+ };
+ }
return kFlags;
}
-
//------------------------------------------------------------
// ExternalTabletServer
//------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management-test.cc b/src/kudu/security/ca/cert_management-test.cc
index e91d8c0..32a3cb2 100644
--- a/src/kudu/security/ca/cert_management-test.cc
+++ b/src/kudu/security/ca/cert_management-test.cc
@@ -72,7 +72,8 @@ class CertManagementTest : public KuduTest {
template<class CSRGen = CertRequestGenerator>
CertSignRequest PrepareTestCSR(typename CSRGen::Config config,
PrivateKey* key) {
- CHECK_OK(GeneratePrivateKey(512, key));
+ int key_size = UseLargeKeys() ? 2048 : 512;
+ CHECK_OK(GeneratePrivateKey(key_size, key));
CSRGen gen(std::move(config));
CHECK_OK(gen.Init());
CertSignRequest req;
diff --git a/src/kudu/security/token-test.cc b/src/kudu/security/token-test.cc
index a4d7804..00e4f61 100644
--- a/src/kudu/security/token-test.cc
+++ b/src/kudu/security/token-test.cc
@@ -57,7 +57,7 @@ namespace security {
namespace {
// Dummy variables to use when their values don't matter much.
-const int kNumBits = 512;
+const int kNumBits = UseLargeKeys() ? 2048 : 512;
const int64_t kTokenValiditySeconds = 10;
const char kUser[] = "user";
diff --git a/src/kudu/tserver/tablet_server_authorization-test.cc b/src/kudu/tserver/tablet_server_authorization-test.cc
index 0f8ab03..5713176 100644
--- a/src/kudu/tserver/tablet_server_authorization-test.cc
+++ b/src/kudu/tserver/tablet_server_authorization-test.cc
@@ -120,7 +120,8 @@ void CheckInvalidAuthzToken(const Status& s, const RpcController& rpc) {
TokenSigningPrivateKeyPB GetTokenSigningPrivateKey(int seq_num) {
TokenSigningPrivateKeyPB tsk;
PrivateKey private_key;
- CHECK_OK(GeneratePrivateKey(/*num_bits=*/512, &private_key));
+ int key_size = UseLargeKeys() ? 2048 : 512;
+ CHECK_OK(GeneratePrivateKey(key_size, &private_key));
string private_key_str_der;
CHECK_OK(private_key.ToString(&private_key_str_der, security::DataFormat::DER));
tsk.set_rsa_key_der(private_key_str_der);
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index 13a5e3d..dd6eb1b 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -75,6 +75,7 @@ namespace kudu {
const char* kInvalidPath = "/dev/invalid-path-for-kudu-tests";
static const char* const kSlowTestsEnvVar = "KUDU_ALLOW_SLOW_TESTS";
+static const char* const kLargeKeysEnvVar = "KUDU_USE_LARGE_KEYS_IN_TESTS";
static const uint64_t kTestBeganAtMicros = Env::Default()->NowMicros();
@@ -99,24 +100,26 @@ KuduTest::KuduTest()
{"never_fsync", "true"},
// Disable redaction.
{"redact", "none"},
+ // For a generic Kudu test, the local wall-clock time is good enough even
+ // if it's not synchronized by NTP. All test components are run at the same
+ // node, so there aren't multiple time sources to synchronize.
+ {"time_source", "system_unsync"},
+ };
+ if (!UseLargeKeys()) {
// Reduce default RSA key length for faster tests. We are using strong/high
// TLS v1.2 cipher suites, so minimum possible for TLS-related RSA keys is
// 768 bits. Java security policies in tests tweaked appropriately to allow
// for using smaller RSA keys in certificates. As for the TSK keys, 512 bits
// is the minimum since the SHA256 digest is used for token
// signing/verification.
- {"ipki_server_key_size", "768"},
- {"ipki_ca_key_size", "768"},
- {"tsk_num_rsa_bits", "512"},
+ flags_for_tests.emplace("ipki_server_key_size", "768");
+ flags_for_tests.emplace("ipki_ca_key_size", "768");
+ flags_for_tests.emplace("tsk_num_rsa_bits", "512");
// Some OS distros set the default security level higher than 0, so it's
// necessary to override it to use the key length specified above (which are
// considered lax and don't work in case of security level 2 or higher).
- {"openssl_security_level_override", "0"},
- // For a generic Kudu test, the local wall-clock time is good enough even
- // if it's not synchronized by NTP. All test components are run at the same
- // node, so there aren't multiple time sources to synchronize.
- {"time_source", "system_unsync"},
- };
+ flags_for_tests.emplace("openssl_security_level_override", "0");
+ }
for (const auto& e : flags_for_tests) {
// We don't check for errors here, because we have some default flags that
// only apply to certain tests. If a flag is defined in a library which
@@ -181,6 +184,8 @@ void KuduTest::OverrideKrb5Environment() {
bool AllowSlowTests() { return GetBooleanEnvironmentVariable(kSlowTestsEnvVar); }
+bool UseLargeKeys() { return GetBooleanEnvironmentVariable(kLargeKeysEnvVar); }
+
void OverrideFlagForSlowTests(const std::string& flag_name,
const std::string& new_value) {
// Ensure that the flag is valid.
diff --git a/src/kudu/util/test_util.h b/src/kudu/util/test_util.h
index a8139f8..6dbb37b 100644
--- a/src/kudu/util/test_util.h
+++ b/src/kudu/util/test_util.h
@@ -87,6 +87,10 @@ class KuduTest : public ::testing::Test {
// Returns true if slow tests are runtime-enabled.
bool AllowSlowTests();
+// Returns true if the KUDU_USE_LARGE_KEYS_IN_TESTS environment variable is set
+// to true. This is required to pass certain tests in FIPS approved mode.
+bool UseLargeKeys();
+
// Override the given gflag to the new value, only in the case that
// slow tests are enabled and the user hasn't otherwise overridden
// it on the command line.