[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 <[email protected]> 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 <[email protected]> Authored: Sat Feb 18 01:19:04 2017 -0800 Committer: Alexey Serbin <[email protected]> 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_;
