kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [2/2] kudu git commit: [security] derive TSK params from authn token ones
Date Wed, 22 Feb 2017 17:59:28 GMT
[security] derive TSK params from authn token ones

Derive the TSK validity interval from the authn token validity period.
The idea is to have set of parameters which is user-oriented: the authn
token lifetime directly impacts user job lifetimes, etc.

The default validity period for authn tokens is set to 7 days to
mirror other Hadoop ecosystem components (e.g. HBase).

The TSK validity interval is derived from authn token validity period:
tsk_validity = authn_token_validity + tsk_rotation.

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


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

Branch: refs/heads/master
Commit: dc1e45a9a8571b6c93f1357a61322ad0d6b6cea9
Parents: 70ea7fb
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Sat Feb 18 01:19:04 2017 -0800
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Wed Feb 22 17:58:44 2017 +0000

----------------------------------------------------------------------
 .../integration-tests/token_signer-itest.cc     |  4 +--
 src/kudu/master/master.cc                       | 19 ++++++------
 src/kudu/security/token-test.cc                 | 26 ++++++++--------
 src/kudu/security/token_signer.cc               | 22 ++++++--------
 src/kudu/security/token_signer.h                | 32 ++++++++++++++------
 5 files changed, 58 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/dc1e45a9/src/kudu/integration-tests/token_signer-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/token_signer-itest.cc b/src/kudu/integration-tests/token_signer-itest.cc
index 823169c..20e960d 100644
--- a/src/kudu/integration-tests/token_signer-itest.cc
+++ b/src/kudu/integration-tests/token_signer-itest.cc
@@ -38,7 +38,7 @@
 #include "kudu/util/slice.h"
 #include "kudu/util/test_util.h"
 
-DECLARE_int64(tsk_validity_seconds);
+DECLARE_int64(authn_token_validity_seconds);
 DECLARE_int64(tsk_rotation_seconds);
 
 using std::string;
@@ -54,7 +54,7 @@ namespace master {
 class TokenSignerITest : public KuduTest {
  public:
   TokenSignerITest() {
-    FLAGS_tsk_validity_seconds = 60;
+    FLAGS_authn_token_validity_seconds = 60;
     FLAGS_tsk_rotation_seconds = 20;
 
     // Hard-coded ports for the masters. This is safe, as this unit test

http://git-wip-us.apache.org/repos/asf/kudu/blob/dc1e45a9/src/kudu/master/master.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 7c5ed46..753419b 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -53,20 +53,20 @@ DEFINE_int32(master_registration_rpc_timeout_ms, 1500,
              "Timeout for retrieving master registration over RPC.");
 TAG_FLAG(master_registration_rpc_timeout_ms, experimental);
 
-DEFINE_int64(tsk_validity_seconds, 60 * 60 * 24 * 7,
-             "Number of seconds that a TSK (Token Signing Key) is valid for.");
-TAG_FLAG(tsk_validity_seconds, advanced);
-TAG_FLAG(tsk_validity_seconds, experimental);
-
 DEFINE_int64(tsk_rotation_seconds, 60 * 60 * 24 * 1,
              "Number of seconds between consecutive activations of newly "
              "generated TSKs (Token Signing Keys).");
 TAG_FLAG(tsk_rotation_seconds, advanced);
 TAG_FLAG(tsk_rotation_seconds, experimental);
 
+DEFINE_int64(authn_token_validity_seconds, 60 * 60 * 24 * 7,
+             "Period of time for which an issued authentication token is valid.");
+// TODO(PKI): docs for what actual effect this has, given we don't support
+// token renewal.
+TAG_FLAG(authn_token_validity_seconds, experimental);
+
 using std::min;
 using std::shared_ptr;
-using std::unique_ptr;
 using std::vector;
 
 using kudu::consensus::RaftPeerPB;
@@ -119,9 +119,10 @@ Status Master::Init() {
   cert_authority_.reset(new MasterCertAuthority(fs_manager_->uuid()));
 
   // The TokenSigner loads its keys during catalog manager initialization.
-  token_signer_.reset(new TokenSigner(FLAGS_tsk_validity_seconds,
-                                      FLAGS_tsk_rotation_seconds,
-                                      messenger_->shared_token_verifier()));
+  token_signer_.reset(new TokenSigner(
+      FLAGS_authn_token_validity_seconds,
+      FLAGS_tsk_rotation_seconds,
+      messenger_->shared_token_verifier()));
   state_ = kInitialized;
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/dc1e45a9/src/kudu/security/token-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/token-test.cc b/src/kudu/security/token-test.cc
index 5adeb25..d641214 100644
--- a/src/kudu/security/token-test.cc
+++ b/src/kudu/security/token-test.cc
@@ -87,13 +87,13 @@ class TokenTest : public KuduTest {
  public:
   void SetUp() override {
     KuduTest::SetUp();
-    // Set the keylength smaller to make tests run faster.
+    // Set the key length smaller to make tests run faster.
     FLAGS_tsk_num_rsa_bits = 512;
   }
 };
 
 TEST_F(TokenTest, TestInit) {
-  TokenSigner signer(60, 20, make_shared<TokenVerifier>());
+  TokenSigner signer(10, 10);
   const TokenVerifier& verifier(signer.verifier());
 
   SignedTokenPB token = MakeUnsignedToken(WallTime_Now());
@@ -122,7 +122,7 @@ TEST_F(TokenTest, TestInit) {
 
 TEST_F(TokenTest, TestTokenSignerAddKeys) {
   {
-    TokenSigner signer(60, 20, make_shared<TokenVerifier>());
+    TokenSigner signer(10, 10);
     std::unique_ptr<TokenSigningPrivateKey> key;
     ASSERT_OK(signer.CheckNeedKey(&key));
     // No keys are available yet, so should be able to add.
@@ -137,7 +137,7 @@ TEST_F(TokenTest, TestTokenSignerAddKeys) {
   {
     // Special configuration for TokenSigner: rotation interval is zero,
     // so should be able to add two keys right away.
-    TokenSigner signer(60, 0, make_shared<TokenVerifier>());
+    TokenSigner signer(10, 0);
     std::unique_ptr<TokenSigningPrivateKey> key;
     ASSERT_OK(signer.CheckNeedKey(&key));
     // No keys are available yet, so should be able to add.
@@ -156,9 +156,9 @@ TEST_F(TokenTest, TestTokenSignerAddKeys) {
 
   {
     // Special configuration for TokenSigner: key rotation interval
-    // just one second shorter than the validity interval. It should not
-    // need next key right away, but should need next key after 1 second.
-    TokenSigner signer(60, 1, make_shared<TokenVerifier>());
+    // just one second. It should not need next key right away, but should need
+    // next key after 1 second.
+    TokenSigner signer(10, 1);
     std::unique_ptr<TokenSigningPrivateKey> key;
     ASSERT_OK(signer.CheckNeedKey(&key));
     // No keys are available yet, so should be able to add.
@@ -185,7 +185,7 @@ TEST_F(TokenTest, TestTokenSignerAddKeys) {
 // Test how test rotation works.
 TEST_F(TokenTest, TestTokenSignerSignVerifyExport) {
   // Key rotation interval 0 allows adding 2 keys in a row with no delay.
-  TokenSigner signer(60, 0, make_shared<TokenVerifier>());
+  TokenSigner signer(10, 0);
   const TokenVerifier& verifier(signer.verifier());
 
   // Should start off with no signing keys.
@@ -248,8 +248,10 @@ TEST_F(TokenTest, TestTokenSignerSignVerifyExport) {
 TEST_F(TokenTest, TestExportKeys) {
   // Test that the exported public keys don't contain private key material,
   // and have an appropriate expiration.
-  const int64_t key_exp_seconds = 60;
-  TokenSigner signer(key_exp_seconds, 30, make_shared<TokenVerifier>());
+  const int64_t key_exp_seconds = 20;
+  const int64_t key_rotation_seconds = 10;
+  TokenSigner signer(key_exp_seconds - key_rotation_seconds,
+                     key_rotation_seconds);
   int64_t key_seq_num;
   {
     std::unique_ptr<TokenSigningPrivateKey> key;
@@ -273,7 +275,7 @@ TEST_F(TokenTest, TestExportKeys) {
 // Test that the TokenVerifier can import keys exported by the TokenSigner
 // and then verify tokens signed by it.
 TEST_F(TokenTest, TestEndToEnd_Valid) {
-  TokenSigner signer(60, 20, make_shared<TokenVerifier>());
+  TokenSigner signer(10, 10);
   {
     std::unique_ptr<TokenSigningPrivateKey> key;
     ASSERT_OK(signer.CheckNeedKey(&key));
@@ -296,7 +298,7 @@ TEST_F(TokenTest, TestEndToEnd_Valid) {
 // See VerificationResult.
 TEST_F(TokenTest, TestEndToEnd_InvalidCases) {
   // Key rotation interval 0 allows adding 2 keys in a row with no delay.
-  TokenSigner signer(60, 0, make_shared<TokenVerifier>());
+  TokenSigner signer(10, 0);
   {
     std::unique_ptr<TokenSigningPrivateKey> key;
     ASSERT_OK(signer.CheckNeedKey(&key));

http://git-wip-us.apache.org/repos/asf/kudu/blob/dc1e45a9/src/kudu/security/token_signer.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/token_signer.cc b/src/kudu/security/token_signer.cc
index adf6a51..c4a54d5 100644
--- a/src/kudu/security/token_signer.cc
+++ b/src/kudu/security/token_signer.cc
@@ -36,14 +36,6 @@
 #include "kudu/util/locks.h"
 #include "kudu/util/status.h"
 
-DEFINE_int64(authn_token_validity_seconds, 120,
-             "Period of time for which an issued authentication token is valid.");
-// TODO(PKI): docs for what actual effect this has, given we don't support
-// token renewal.
-// TODO(PKI): this is set extremely low, so that we don't forget to come back to
-// this and add rolling and refetching code.
-TAG_FLAG(authn_token_validity_seconds, experimental);
-
 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?
@@ -60,13 +52,17 @@ using std::vector;
 namespace kudu {
 namespace security {
 
-TokenSigner::TokenSigner(int64_t key_validity_seconds,
+TokenSigner::TokenSigner(int64_t authn_token_validity_seconds,
                          int64_t key_rotation_seconds,
                          shared_ptr<TokenVerifier> verifier)
-    : verifier_(std::move(verifier)),
-      key_validity_seconds_(key_validity_seconds),
+    : verifier_(verifier ? std::move(verifier)
+                         : std::make_shared<TokenVerifier>()),
+      authn_token_validity_seconds_(authn_token_validity_seconds),
       key_rotation_seconds_(key_rotation_seconds),
+      key_validity_seconds_(key_rotation_seconds_ + authn_token_validity_seconds_),
       next_key_seq_num_(0) {
+  CHECK_GE(key_rotation_seconds_, 0);
+  CHECK_GE(authn_token_validity_seconds_, 0);
   CHECK(verifier_);
 }
 
@@ -133,7 +129,7 @@ Status TokenSigner::GenerateAuthnToken(string username,
                                        SignedTokenPB* signed_token) const {
   TokenPB token;
   token.set_expire_unix_epoch_seconds(
-      WallTime_Now() + FLAGS_authn_token_validity_seconds);
+      WallTime_Now() + authn_token_validity_seconds_);
   AuthnTokenPB* authn = token.mutable_authn();
   authn->mutable_username()->assign(std::move(username));
 
@@ -177,7 +173,7 @@ Status TokenSigner::CheckNeedKey(unique_ptr<TokenSigningPrivateKey>*
tsk) const
     // It does not make much sense to keep more than two keys in the queue.
     // It's enough to have just one active key and next key ready to be
     // activated when it's time to do so.  However, it does not mean the
-    // process of key refreshement is about to stop once there are two keys
+    // process of key refreshment is about to stop once there are two keys
     // in the queue: the TryRotate() method (which should be called periodically
     // along with CheckNeedKey()/AddKey() pair) will eventually pop the
     // current key out of the keys queue once the key enters its inactive phase.

http://git-wip-us.apache.org/repos/asf/kudu/blob/dc1e45a9/src/kudu/security/token_signer.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/token_signer.h b/src/kudu/security/token_signer.h
index 9f05d71..82aba2b 100644
--- a/src/kudu/security/token_signer.h
+++ b/src/kudu/security/token_signer.h
@@ -135,7 +135,7 @@ class TokenVerifier;
 //
 // Typical usage pattern:
 //
-//    TokenSigner ts;
+//    TokenSigner ts(...);
 //    // Load existing TSKs from the system table.
 //    ...
 //    RETURN_NOT_OK(ts.ImportKeys(...));
@@ -163,14 +163,25 @@ class TokenVerifier;
 //
 class TokenSigner {
  public:
-  // Parameters of the TokenSigner constructor define the TSK rotation schedule.
-  // See the class's comment just above for details.
+  // The 'key_validity_seconds' and 'key_rotation_seconds' parameters define
+  // the schedule of TSK rotation. See the class comment above for details.
   //
   // Any newly imported or generated keys are automatically imported into the
-  // passed 'verifier'.
-  TokenSigner(int64_t key_validity_seconds,
+  // passed 'verifier'. If no verifier passed as a parameter, TokenSigner
+  // creates one on its own. In either case, it's possible to access
+  // the embedded TokenVerifier instance using the verifier() accessor.
+  //
+  // The 'authn_token_validity_seconds' parameter is used to specify validity
+  // interval for the generated authn tokens and with 'key_rotation_seconds'
+  // it defines validity interval of the newly generated TSK:
+  //   key_validity = key_rotation + authn_token_validity.
+  //
+  // That corresponds to the maximum possible token lifetime for the effective
+  // TSK validity and rotation intervals: see the class comment above for
+  // details.
+  TokenSigner(int64_t authn_token_validity_seconds,
               int64_t key_rotation_seconds,
-              std::shared_ptr<TokenVerifier> verifier);
+              std::shared_ptr<TokenVerifier> verifier = nullptr);
   ~TokenSigner();
 
   // Import token signing keys in PB format, notifying TokenVerifier
@@ -236,14 +247,17 @@ class TokenSigner {
 
   std::shared_ptr<TokenVerifier> verifier_;
 
-  // Period of validity for newly created token signing keys. In other words,
-  // the expiration time for a new key is set to (now + key_validity_seconds_).
-  const int64_t key_validity_seconds_;
+  // Validity interval for the generated authn tokens.
+  const int64_t authn_token_validity_seconds_;
 
   // TSK rotation interval: number of seconds between consecutive activations
   // of new token signing keys.
   const int64_t key_rotation_seconds_;
 
+  // Period of validity for newly created token signing keys. In other words,
+  // the expiration time for a new key is set to (now + key_validity_seconds_).
+  const int64_t key_validity_seconds_;
+
   // Protects next_seq_num_ and tsk_deque_ members.
   mutable RWMutex lock_;
 


Mime
View raw message