Repository: kudu Updated Branches: refs/heads/master 578bf84bb -> 475920a21
[security] added TokenSigner::IsCurrentKeyValid() method Introduced TokenSigner::IsCurrentKeyValid() method to check whether TokenSigner's current key is valid. Added a unit test as well. Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9 Reviewed-on: http://gerrit.cloudera.org:8080/6246 Tested-by: Alexey Serbin <[email protected]> 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/475920a2 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/475920a2 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/475920a2 Branch: refs/heads/master Commit: 475920a21e9f8c13f48c453eefa3b7ad9be18e15 Parents: 578bf84 Author: Alexey Serbin <[email protected]> Authored: Fri Mar 3 09:07:44 2017 -0800 Committer: Alexey Serbin <[email protected]> Committed: Tue Mar 7 01:51:41 2017 +0000 ---------------------------------------------------------------------- src/kudu/security/token-test.cc | 26 ++++++++++++++++++++++++++ src/kudu/security/token_signer.cc | 10 +++++++++- src/kudu/security/token_signer.h | 4 ++++ 3 files changed, 39 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/475920a2/src/kudu/security/token-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/token-test.cc b/src/kudu/security/token-test.cc index 7f27c71..31bb7a2 100644 --- a/src/kudu/security/token-test.cc +++ b/src/kudu/security/token-test.cc @@ -122,6 +122,31 @@ TEST_F(TokenTest, TestGenerateAuthToken) { ASSERT_STR_CONTAINS(s.ToString(), "no username provided for authn token"); } +TEST_F(TokenTest, TestIsCurrentKeyValid) { + static const int64_t kAuthnTokenValiditySeconds = 1; + static const int64_t kKeyRotationSeconds = 1; + static const int64_t kKeyValiditySeconds = + kAuthnTokenValiditySeconds + kKeyRotationSeconds; + + TokenSigner signer(kAuthnTokenValiditySeconds, kKeyRotationSeconds); + EXPECT_FALSE(signer.IsCurrentKeyValid()); + { + std::unique_ptr<TokenSigningPrivateKey> key; + ASSERT_OK(signer.CheckNeedKey(&key)); + // No keys are available yet, so should be able to add. + ASSERT_NE(nullptr, key.get()); + ASSERT_OK(signer.AddKey(std::move(key))); + } + EXPECT_TRUE(signer.IsCurrentKeyValid()); + SleepFor(MonoDelta::FromSeconds(kKeyValiditySeconds)); + // The key should expire after its validity interval. + EXPECT_FALSE(signer.IsCurrentKeyValid()); + + // Anyway, current implementation allows to use an expired key to sign tokens. + SignedTokenPB token = MakeUnsignedToken(WallTime_Now()); + EXPECT_OK(signer.SignToken(&token)); +} + TEST_F(TokenTest, TestTokenSignerAddKeys) { { TokenSigner signer(10, 10); @@ -385,6 +410,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { } SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600); + // Current implementation allows to use an expired key to sign tokens. ASSERT_OK(signer.SignToken(&signed_token)); TokenPB token; ASSERT_EQ(VerificationResult::EXPIRED_SIGNING_KEY, http://git-wip-us.apache.org/repos/asf/kudu/blob/475920a2/src/kudu/security/token_signer.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/token_signer.cc b/src/kudu/security/token_signer.cc index 563721e..5b4fcd6 100644 --- a/src/kudu/security/token_signer.cc +++ b/src/kudu/security/token_signer.cc @@ -151,11 +151,19 @@ Status TokenSigner::SignToken(SignedTokenPB* token) const { if (tsk_deque_.empty()) { return Status::IllegalState("no token signing key"); } - TokenSigningPrivateKey* key = tsk_deque_.front().get(); + const TokenSigningPrivateKey* key = tsk_deque_.front().get(); RETURN_NOT_OK_PREPEND(key->Sign(token), "could not sign authn token"); return Status::OK(); } +bool TokenSigner::IsCurrentKeyValid() const { + shared_lock<RWMutex> l(lock_); + if (tsk_deque_.empty()) { + return false; + } + return (tsk_deque_.front()->expire_time() > WallTime_Now()); +} + Status TokenSigner::CheckNeedKey(unique_ptr<TokenSigningPrivateKey>* tsk) const { CHECK(tsk); const int64_t now = WallTime_Now(); http://git-wip-us.apache.org/repos/asf/kudu/blob/475920a2/src/kudu/security/token_signer.h ---------------------------------------------------------------------- diff --git a/src/kudu/security/token_signer.h b/src/kudu/security/token_signer.h index 698bf45..b7e637f 100644 --- a/src/kudu/security/token_signer.h +++ b/src/kudu/security/token_signer.h @@ -236,6 +236,10 @@ class TokenSigner { const TokenVerifier& verifier() const { return *verifier_; } + // Check if the current TSK is valid: return 'true' if current key is present + // and it's not yet expired, return 'false' otherwise. + bool IsCurrentKeyValid() const; + private: FRIEND_TEST(TokenTest, TestEndToEnd_InvalidCases);
