kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/3] kudu git commit: [security] use shorter RSA keys in tests
Date Wed, 01 Mar 2017 20:02:09 GMT
[security] use shorter RSA keys in tests

When running tests, use the following RSA private keys:
  * TSK: 512 bit
  * certificate authority: 1024 bit
  * server: 1024 bit

The 512 bit length is the minimum for TSK keys since we use SHA256
for signing/verification of tokens.  The 768 bit length is the minimum
for TLS-related keys since we use stronger cipher suites from TLS v1.2
(one of the suites in use is ECDHE-RSA-AES256-GCM-SHA384).  However,
Java default security policies require at least 1024-bit RSA keys for
certificates used in validation chains, so using that for the external
mini-cluster.  For uniformity, minimum 1024 bit keys are used for
RSA keys throughout C++-only tests.

Change-Id: I180908763a1c520d8a6d8bbaaf981add9396db99
Reviewed-on: http://gerrit.cloudera.org:8080/6194
Reviewed-by: Dan Burkert <danburkert@apache.org>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: 0ee93e31a1febb987c72e7392a69b2584e6f38ed
Parents: 5fdad79
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Tue Feb 28 17:36:53 2017 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Mar 1 20:00:24 2017 +0000

----------------------------------------------------------------------
 .../org/apache/kudu/client/MiniKuduCluster.java |  4 ++
 .../integration-tests/external_mini_cluster.cc  | 49 ++++++++++++--------
 .../integration-tests/external_mini_cluster.h   |  2 +
 src/kudu/master/master-test.cc                  |  3 +-
 src/kudu/rpc/negotiation-test.cc                |  1 -
 src/kudu/security/token-test.cc                 |  6 ---
 src/kudu/security/token_signer.cc               |  5 +-
 src/kudu/util/test_util.cc                      | 11 ++++-
 8 files changed, 49 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java b/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
index 3a60ba0..006057c 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
@@ -173,6 +173,7 @@ public class MiniKuduCluster implements AutoCloseable {
           "--fs_wal_dir=" + dataDirPath,
           "--fs_data_dirs=" + dataDirPath,
           "--flush_threshold_mb=1",
+          "--ipki_server_key_size=1024",
           "--tserver_master_addrs=" + masterAddresses,
           "--webserver_interface=" + bindHost,
           "--local_ip_for_outbound_sockets=" + bindHost,
@@ -251,6 +252,9 @@ public class MiniKuduCluster implements AutoCloseable {
           "--log_dir=" + logDirPath,
           "--fs_wal_dir=" + dataDirPath,
           "--fs_data_dirs=" + dataDirPath,
+          "--ipki_ca_key_size=1024",
+          "--ipki_server_key_size=1024",
+          "--tsk_num_rsa_bits=512",
           "--webserver_interface=" + bindHost,
           "--local_ip_for_outbound_sockets=" + bindHost,
           "--rpc_bind_addresses=" + bindHost + ":" + port,

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/integration-tests/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.cc b/src/kudu/integration-tests/external_mini_cluster.cc
index 617e5e9..5ead31b 100644
--- a/src/kudu/integration-tests/external_mini_cluster.cc
+++ b/src/kudu/integration-tests/external_mini_cluster.cc
@@ -634,7 +634,12 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags)
{
   // Generate smaller RSA keys -- generating a 1024-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.
+  // 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). However, to work with Java
+  // client it's necessary to have at least 1024 bits for certificate RSA key
+  // due to Java security policies.
   argv.push_back("--ipki_server_key_size=1024");
 
   // Disable minidumps by default since many tests purposely inject faults.
@@ -1016,18 +1021,10 @@ ExternalMaster::~ExternalMaster() {
 }
 
 Status ExternalMaster::Start() {
-  vector<string> flags;
-
-  // Generate smaller RSA keys. See note above for server_rsa_key_length_bits.
-  flags.push_back("--ipki_ca_key_size=1024");
-
-  flags.push_back("--fs_wal_dir=" + data_dir_);
-  flags.push_back("--fs_data_dirs=" + data_dir_);
-  flags.push_back("--rpc_bind_addresses=" + get_rpc_bind_address());
-  flags.push_back("--webserver_interface=localhost");
+  vector<string> flags(GetCommonFlags());
   flags.push_back("--webserver_port=0");
-  RETURN_NOT_OK(StartProcess(flags));
-  return Status::OK();
+  flags.push_back("--rpc_bind_addresses=" + get_rpc_bind_address());
+  return StartProcess(flags);
 }
 
 Status ExternalMaster::Restart() {
@@ -1035,14 +1032,12 @@ Status ExternalMaster::Restart() {
   if (bound_rpc_.port() == 0) {
     return Status::IllegalState("Master cannot be restarted. Must call Shutdown() first.");
   }
-  vector<string> flags;
-  flags.push_back("--fs_wal_dir=" + data_dir_);
-  flags.push_back("--fs_data_dirs=" + data_dir_);
-  flags.push_back("--rpc_bind_addresses=" + bound_rpc_.ToString());
-  flags.push_back("--webserver_interface=localhost");
+
+  vector<string> flags(GetCommonFlags());
   flags.push_back(Substitute("--webserver_port=$0", bound_http_.port()));
-  RETURN_NOT_OK(StartProcess(flags));
-  return Status::OK();
+  flags.push_back("--rpc_bind_addresses=" + bound_rpc_.ToString());
+
+  return StartProcess(flags);
 }
 
 Status ExternalMaster::WaitForCatalogManager() {
@@ -1087,6 +1082,22 @@ Status ExternalMaster::WaitForCatalogManager() {
   return Status::OK();
 }
 
+vector<string> ExternalMaster::GetCommonFlags() const {
+  return {
+    "--fs_wal_dir=" + data_dir_,
+    "--fs_data_dirs=" + data_dir_,
+    "--webserver_interface=localhost",
+
+    // See the in-line comment for "--ipki_server_key_size" flag in
+    // ExternalDaemon::StartProcess() method.
+    "--ipki_ca_key_size=1024",
+
+    // As for the TSK keys, 512 bits is the minimum since we are using the SHA256
+    // digest for token signing/verification.
+    "--tsk_num_rsa_bits=512",
+  };
+}
+
 
 //------------------------------------------------------------
 // ExternalTabletServer

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/integration-tests/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.h b/src/kudu/integration-tests/external_mini_cluster.h
index cc5022b..c4518c3 100644
--- a/src/kudu/integration-tests/external_mini_cluster.h
+++ b/src/kudu/integration-tests/external_mini_cluster.h
@@ -515,6 +515,8 @@ class ExternalMaster : public ExternalDaemon {
   Status WaitForCatalogManager() WARN_UNUSED_RESULT;
 
  private:
+  std::vector<std::string> GetCommonFlags() const;
+
   friend class RefCountedThreadSafe<ExternalMaster>;
   virtual ~ExternalMaster();
 };

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/master/master-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 71333d2..3777513 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -1326,7 +1326,8 @@ 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";
-  EXPECT_EQ(256, resp.authn_token().signature().size());
+  // Using 512 bit RSA key and SHA256 digest results in 64 byte signature.
+  EXPECT_EQ(64, 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);
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/rpc/negotiation-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/negotiation-test.cc b/src/kudu/rpc/negotiation-test.cc
index 5265e28..67f8d1f 100644
--- a/src/kudu/rpc/negotiation-test.cc
+++ b/src/kudu/rpc/negotiation-test.cc
@@ -68,7 +68,6 @@ DEFINE_bool(is_test_child, false,
             "Used by tests which require clean processes. "
             "See TestDisableInit.");
 DECLARE_bool(rpc_encrypt_loopback_connections);
-DECLARE_int32(server_rsa_key_length_bits);
 
 using std::string;
 using std::thread;

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/security/token-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/token-test.cc b/src/kudu/security/token-test.cc
index 84a0105..7f27c71 100644
--- a/src/kudu/security/token-test.cc
+++ b/src/kudu/security/token-test.cc
@@ -84,12 +84,6 @@ Status GenerateTokenSigningKey(int64_t seq_num,
 } // anonymous namespace
 
 class TokenTest : public KuduTest {
- public:
-  void SetUp() override {
-    KuduTest::SetUp();
-    // Set the key length smaller to make tests run faster.
-    FLAGS_tsk_num_rsa_bits = 512;
-  }
 };
 
 TEST_F(TokenTest, TestInit) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/security/token_signer.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/token_signer.cc b/src/kudu/security/token_signer.cc
index 20dc8a6..563721e 100644
--- a/src/kudu/security/token_signer.cc
+++ b/src/kudu/security/token_signer.cc
@@ -37,9 +37,8 @@
 #include "kudu/util/status.h"
 
 DEFINE_int32(tsk_num_rsa_bits, 2048,
-             "Number of bits used for token signing keys");
-// TODO(PKI) is 1024 enough for TSKs since they rotate frequently?
-// maybe it would verify faster?
+             "Number of bits in RSA keys used for token signing.");
+TAG_FLAG(tsk_num_rsa_bits, experimental);
 
 using std::lock_guard;
 using std::map;

http://git-wip-us.apache.org/repos/asf/kudu/blob/0ee93e31/src/kudu/util/test_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index 1fa8c8b..50c80c6 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -71,9 +71,16 @@ KuduTest::KuduTest()
     {"never_fsync", "true"},
     // Disable log redaction.
     {"redact", "flag"},
-    // Reduce default RSA key length for faster tests.
+    // 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. However, for the external mini cluster we use 1024 bits because
+    // Java default security policies require at least 1024 bits for RSA keys
+    // used in certificates. For uniformity, here 1024 RSA bit keys are used
+    // as well. As for the TSK keys, 512 bits is the minimum since the SHA256
+    // digest is used for token signing/verification.
     {"ipki_server_key_size", "1024"},
-    {"ipki_ca_key_size", "1024"}
+    {"ipki_ca_key_size", "1024"},
+    {"tsk_num_rsa_bits", "512"},
   };
   for (const auto& e : flags_for_tests) {
     // We don't check for errors here, because we have some default flags that


Mime
View raw message