This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 205710c9246ea847b19d6d923297bcf5002cd70b Author: Alexey Serbin <ale...@apache.org> AuthorDate: Fri Feb 4 18:28:47 2022 -0800 [security] VerificationResult --> TokenVerificationResult In this patch I renamed a few authn token-related functions and types in preparation to JWT-related changes in follow-up changelists. This patch doesn't contain any functional modifications. Change-Id: I1c3eaf4f41800735620aca7869addd38181a4d52 Reviewed-on: http://gerrit.cloudera.org:8080/18201 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong <aw...@cloudera.com> --- .../security-unknown-tsk-itest.cc | 5 +-- src/kudu/integration-tests/token_signer-itest.cc | 16 +++++----- src/kudu/rpc/rpc_verification_util.cc | 24 +++++++------- src/kudu/rpc/rpc_verification_util.h | 9 +++--- src/kudu/rpc/server_negotiation.cc | 14 ++++---- src/kudu/security/token-test.cc | 37 ++++++++++++---------- src/kudu/security/token_verifier.cc | 37 +++++++++++----------- src/kudu/security/token_verifier.h | 10 +++--- src/kudu/tserver/tablet_service.cc | 4 +-- 9 files changed, 80 insertions(+), 76 deletions(-) diff --git a/src/kudu/integration-tests/security-unknown-tsk-itest.cc b/src/kudu/integration-tests/security-unknown-tsk-itest.cc index 57d61a6..bb729d3 100644 --- a/src/kudu/integration-tests/security-unknown-tsk-itest.cc +++ b/src/kudu/integration-tests/security-unknown-tsk-itest.cc @@ -85,7 +85,7 @@ using security::TokenPB; using security::TokenSigner; using security::TokenSigningPrivateKeyPB; using security::TokenVerifier; -using security::VerificationResult; +using security::TokenVerificationResult; class SecurityUnknownTskTest : public KuduTest { public: @@ -151,7 +151,8 @@ class SecurityUnknownTskTest : public KuduTest { TokenSigner* signer = cluster_->mini_master()->master()->token_signer(); TokenPB token; const TokenVerifier& verifier = signer->verifier(); - if (verifier.VerifyTokenSignature(*authn_token, &token) != VerificationResult::VALID) { + if (verifier.VerifyTokenSignature(*authn_token, &token) != + TokenVerificationResult::VALID) { return Status::RuntimeError("current client authn token is not valid"); } diff --git a/src/kudu/integration-tests/token_signer-itest.cc b/src/kudu/integration-tests/token_signer-itest.cc index be07721..cfffddc 100644 --- a/src/kudu/integration-tests/token_signer-itest.cc +++ b/src/kudu/integration-tests/token_signer-itest.cc @@ -54,7 +54,7 @@ using kudu::cluster::InternalMiniClusterOptions; using kudu::security::TokenPB; using kudu::security::TokenSigningPublicKeyPB; using kudu::security::SignedTokenPB; -using kudu::security::VerificationResult; +using kudu::security::TokenVerificationResult; namespace kudu { namespace master { @@ -235,9 +235,9 @@ TEST_F(TokenSignerITest, AuthnTokenLifecycle) { // There might be some delay due to parallel OS activity, but the public // part of TSK should reach all tablet servers in a few heartbeat intervals. AssertEventually([&] { - const VerificationResult res = ts->messenger()->token_verifier(). + const TokenVerificationResult res = ts->messenger()->token_verifier(). VerifyTokenSignature(stoken, &token); - ASSERT_EQ(VerificationResult::VALID, res); + ASSERT_EQ(TokenVerificationResult::VALID, res); }, MonoDelta::FromMilliseconds(5L * FLAGS_heartbeat_interval_ms)); NO_PENDING_FATALS(); } @@ -276,9 +276,9 @@ TEST_F(TokenSignerITest, AuthnTokenLifecycle) { ASSERT_NE(nullptr, ts); const int64_t time_pre = WallTime_Now(); TokenPB token; - const VerificationResult res = ts->messenger()->token_verifier(). - VerifyTokenSignature(stoken_eotai, &token); - if (res == VerificationResult::VALID) { + const auto res = ts->messenger()->token_verifier().VerifyTokenSignature( + stoken_eotai, &token); + if (res == TokenVerificationResult::VALID) { // Both authn token and its TSK should be valid. valid_at_tserver[i] = true; ASSERT_GE(token.expire_unix_epoch_seconds(), time_pre); @@ -286,7 +286,7 @@ TEST_F(TokenSignerITest, AuthnTokenLifecycle) { } else { expired_at_tserver[i] = true; // The only expected error here is EXPIRED_TOKEN. - ASSERT_EQ(VerificationResult::EXPIRED_TOKEN, res); + ASSERT_EQ(TokenVerificationResult::EXPIRED_TOKEN, res); const int64_t time_post = WallTime_Now(); ASSERT_LT(token.expire_unix_epoch_seconds(), time_post); } @@ -312,7 +312,7 @@ TEST_F(TokenSignerITest, AuthnTokenLifecycle) { const tserver::TabletServer* ts = cluster_->mini_tablet_server(i)->server(); ASSERT_NE(nullptr, ts); TokenPB token; - ASSERT_EQ(VerificationResult::EXPIRED_TOKEN, + ASSERT_EQ(TokenVerificationResult::EXPIRED_TOKEN, ts->messenger()->token_verifier(). VerifyTokenSignature(stoken_eotai, &token)); } diff --git a/src/kudu/rpc/rpc_verification_util.cc b/src/kudu/rpc/rpc_verification_util.cc index c0bf8b1..27c8af4 100644 --- a/src/kudu/rpc/rpc_verification_util.cc +++ b/src/kudu/rpc/rpc_verification_util.cc @@ -26,33 +26,33 @@ namespace kudu { -using security::VerificationResult; +using security::TokenVerificationResult; namespace rpc { -Status ParseVerificationResult(const VerificationResult& result, - ErrorStatusPB::RpcErrorCodePB retry_error, - ErrorStatusPB::RpcErrorCodePB* error) { +Status ParseTokenVerificationResult(const TokenVerificationResult& result, + ErrorStatusPB::RpcErrorCodePB retry_error, + ErrorStatusPB::RpcErrorCodePB* error) { DCHECK(error); switch (result) { - case VerificationResult::VALID: return Status::OK(); + case TokenVerificationResult::VALID: return Status::OK(); - case VerificationResult::INVALID_TOKEN: - case VerificationResult::INVALID_SIGNATURE: - case VerificationResult::EXPIRED_TOKEN: - case VerificationResult::EXPIRED_SIGNING_KEY: { + case TokenVerificationResult::INVALID_TOKEN: + case TokenVerificationResult::INVALID_SIGNATURE: + case TokenVerificationResult::EXPIRED_TOKEN: + case TokenVerificationResult::EXPIRED_SIGNING_KEY: { // These errors indicate the client should get a new token and try again. *error = retry_error; break; } - case VerificationResult::UNKNOWN_SIGNING_KEY: { + case TokenVerificationResult::UNKNOWN_SIGNING_KEY: { // The server doesn't recognize the signing key. This indicates that the // server has not been updated with the most recent TSKs, so tell the // client to try again later. *error = ErrorStatusPB::ERROR_UNAVAILABLE; break; } - case VerificationResult::INCOMPATIBLE_FEATURE: { + case TokenVerificationResult::INCOMPATIBLE_FEATURE: { // These error types aren't recoverable by having the client get a new token. *error = ErrorStatusPB::FATAL_UNAUTHORIZED; break; @@ -60,7 +60,7 @@ Status ParseVerificationResult(const VerificationResult& result, default: LOG(FATAL) << "Unknown verification result: " << static_cast<int>(result); } - return Status::NotAuthorized(VerificationResultToString(result)); + return Status::NotAuthorized(TokenVerificationResultToString(result)); } } // namespace rpc diff --git a/src/kudu/rpc/rpc_verification_util.h b/src/kudu/rpc/rpc_verification_util.h index 0c15b9c..cc626be 100644 --- a/src/kudu/rpc/rpc_verification_util.h +++ b/src/kudu/rpc/rpc_verification_util.h @@ -23,7 +23,7 @@ namespace kudu { namespace security { -enum class VerificationResult; +enum class TokenVerificationResult; } // namespace security namespace rpc { @@ -33,9 +33,10 @@ namespace rpc { // otherwise returns non-OK and sets 'error' appropriately. // 'retry_error' is the error code to be returned to denote that verification // should be retried after retrieving a new token. -Status ParseVerificationResult(const security::VerificationResult& result, - ErrorStatusPB::RpcErrorCodePB retry_error, - ErrorStatusPB::RpcErrorCodePB* error); +Status ParseTokenVerificationResult( + const security::TokenVerificationResult& result, + ErrorStatusPB::RpcErrorCodePB retry_error, + ErrorStatusPB::RpcErrorCodePB* error); } // namespace rpc } // namespace kudu diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc index 0f373e5..bcbebf6 100644 --- a/src/kudu/rpc/server_negotiation.cc +++ b/src/kudu/rpc/server_negotiation.cc @@ -701,7 +701,7 @@ Status ServerNegotiation::AuthenticateByToken(faststring* recv_buf) { security::TokenPB token; auto verification_result = token_verifier_->VerifyTokenSignature(pb.authn_token(), &token); ErrorStatusPB::RpcErrorCodePB error; - Status s = ParseVerificationResult(verification_result, + Status s = ParseTokenVerificationResult(verification_result, ErrorStatusPB::FATAL_INVALID_AUTHENTICATION_TOKEN, &error); if (!s.ok()) { RETURN_NOT_OK(SendError(error, s)); @@ -722,26 +722,26 @@ Status ServerNegotiation::AuthenticateByToken(faststring* recv_buf) { } if (PREDICT_FALSE(FLAGS_rpc_inject_invalid_authn_token_ratio > 0)) { - security::VerificationResult res; + security::TokenVerificationResult res; int sel = rand() % 4; switch (sel) { case 0: - res = security::VerificationResult::INVALID_TOKEN; + res = security::TokenVerificationResult::INVALID_TOKEN; break; case 1: - res = security::VerificationResult::INVALID_SIGNATURE; + res = security::TokenVerificationResult::INVALID_SIGNATURE; break; case 2: - res = security::VerificationResult::EXPIRED_TOKEN; + res = security::TokenVerificationResult::EXPIRED_TOKEN; break; case 3: - res = security::VerificationResult::EXPIRED_SIGNING_KEY; + res = security::TokenVerificationResult::EXPIRED_SIGNING_KEY; break; default: LOG(FATAL) << "unreachable"; } if (kudu::fault_injection::MaybeTrue(FLAGS_rpc_inject_invalid_authn_token_ratio)) { - Status s = Status::NotAuthorized(VerificationResultToString(res)); + Status s = Status::NotAuthorized(TokenVerificationResultToString(res)); RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_INVALID_AUTHENTICATION_TOKEN, s)); return s; } diff --git a/src/kudu/security/token-test.cc b/src/kudu/security/token-test.cc index 3f1cd32..158cb91 100644 --- a/src/kudu/security/token-test.cc +++ b/src/kudu/security/token-test.cc @@ -412,7 +412,7 @@ TEST_F(TokenTest, TestGenerateAuthzToken) { &signed_token_pb)); ASSERT_TRUE(signed_token_pb.has_token_data()); TokenPB token_pb; - ASSERT_EQ(VerificationResult::VALID, + ASSERT_EQ(TokenVerificationResult::VALID, verifier->VerifyTokenSignature(signed_token_pb, &token_pb)); ASSERT_TRUE(token_pb.has_authz()); ASSERT_EQ(kAuthorized, token_pb.authz().username()); @@ -619,11 +619,11 @@ TEST_F(TokenTest, TestEndToEnd_Valid) { TokenVerifier verifier; ASSERT_OK(verifier.ImportKeys(signer.verifier().ExportKeys())); TokenPB token; - ASSERT_EQ(VerificationResult::VALID, verifier.VerifyTokenSignature(signed_token, &token)); + ASSERT_EQ(TokenVerificationResult::VALID, verifier.VerifyTokenSignature(signed_token, &token)); } // Test all of the possible cases covered by token verification. -// See VerificationResult. +// See TokenVerificationResult. TEST_F(TokenTest, TestEndToEnd_InvalidCases) { // Key rotation interval 0 allows adding 2 keys in a row with no delay. TokenSigner signer(kTokenValiditySeconds, kTokenValiditySeconds, 0); @@ -643,7 +643,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { ASSERT_OK(signer.SignToken(&signed_token)); signed_token.set_token_data("xyz"); TokenPB token; - ASSERT_EQ(VerificationResult::INVALID_TOKEN, + ASSERT_EQ(TokenVerificationResult::INVALID_TOKEN, verifier.VerifyTokenSignature(signed_token, &token)); } @@ -653,7 +653,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { ASSERT_OK(signer.SignToken(&signed_token)); signed_token.set_signature("xyz"); TokenPB token; - ASSERT_EQ(VerificationResult::INVALID_SIGNATURE, + ASSERT_EQ(TokenVerificationResult::INVALID_SIGNATURE, verifier.VerifyTokenSignature(signed_token, &token)); } @@ -662,7 +662,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() - 10); ASSERT_OK(signer.SignToken(&signed_token)); TokenPB token; - ASSERT_EQ(VerificationResult::EXPIRED_TOKEN, + ASSERT_EQ(TokenVerificationResult::EXPIRED_TOKEN, verifier.VerifyTokenSignature(signed_token, &token)); } @@ -671,7 +671,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { SignedTokenPB signed_token = MakeIncompatibleToken(); ASSERT_OK(signer.SignToken(&signed_token)); TokenPB token; - ASSERT_EQ(VerificationResult::INCOMPATIBLE_FEATURE, + ASSERT_EQ(TokenVerificationResult::INCOMPATIBLE_FEATURE, verifier.VerifyTokenSignature(signed_token, &token)); } @@ -690,7 +690,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600); ASSERT_OK(signer.SignToken(&signed_token)); TokenPB token; - ASSERT_EQ(VerificationResult::UNKNOWN_SIGNING_KEY, + ASSERT_EQ(TokenVerificationResult::UNKNOWN_SIGNING_KEY, verifier.VerifyTokenSignature(signed_token, &token)); } @@ -713,7 +713,7 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) { // 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, + ASSERT_EQ(TokenVerificationResult::EXPIRED_SIGNING_KEY, verifier.VerifyTokenSignature(signed_token, &token)); } } @@ -771,20 +771,23 @@ TEST_F(TokenTest, TestVaryingTokenValidityIntervals) { ASSERT_OK(signer.GenerateAuthzToken(kUser, table_privilege, &signed_authz)); TokenPB authn_token; TokenPB authz_token; - ASSERT_EQ(VerificationResult::VALID, verifier->VerifyTokenSignature(signed_authn, &authn_token)); - ASSERT_EQ(VerificationResult::VALID, verifier->VerifyTokenSignature(signed_authz, &authz_token)); + ASSERT_EQ(TokenVerificationResult::VALID, + verifier->VerifyTokenSignature(signed_authn, &authn_token)); + ASSERT_EQ(TokenVerificationResult::VALID, + verifier->VerifyTokenSignature(signed_authz, &authz_token)); // Wait for the authz validity interval to pass and verify its expiration. SleepFor(MonoDelta::FromSeconds(1 + kShortValiditySeconds)); - EXPECT_EQ(VerificationResult::VALID, verifier->VerifyTokenSignature(signed_authn, &authn_token)); - EXPECT_EQ(VerificationResult::EXPIRED_TOKEN, + EXPECT_EQ(TokenVerificationResult::VALID, + verifier->VerifyTokenSignature(signed_authn, &authn_token)); + EXPECT_EQ(TokenVerificationResult::EXPIRED_TOKEN, verifier->VerifyTokenSignature(signed_authz, &authz_token)); // Wait for the authn validity interval to pass and verify its expiration. SleepFor(MonoDelta::FromSeconds(kLongValiditySeconds - kShortValiditySeconds)); - EXPECT_EQ(VerificationResult::EXPIRED_TOKEN, + EXPECT_EQ(TokenVerificationResult::EXPIRED_TOKEN, verifier->VerifyTokenSignature(signed_authn, &authn_token)); - EXPECT_EQ(VerificationResult::EXPIRED_TOKEN, + EXPECT_EQ(TokenVerificationResult::EXPIRED_TOKEN, verifier->VerifyTokenSignature(signed_authz, &authz_token)); } @@ -837,9 +840,9 @@ TEST_F(TokenTest, TestKeyValidity) { TokenPB token_pb; const auto result = verifier->VerifyTokenSignature(signed_token, &token_pb); const auto expire_secs = token_pb.expire_unix_epoch_seconds(); - ASSERT_EQ(VerificationResult::EXPIRED_TOKEN, result) + ASSERT_EQ(TokenVerificationResult::EXPIRED_TOKEN, result) << Substitute("validation result '$0': $1 token expires at $2, now $3", - VerificationResultToString(result), token_type, + TokenVerificationResultToString(result), token_type, expire_secs, WallTime_Now()); }; diff --git a/src/kudu/security/token_verifier.cc b/src/kudu/security/token_verifier.cc index e3aef4c..373c272 100644 --- a/src/kudu/security/token_verifier.cc +++ b/src/kudu/security/token_verifier.cc @@ -103,29 +103,29 @@ std::vector<TokenSigningPublicKeyPB> TokenVerifier::ExportKeys( } // Verify the signature on the given token. -VerificationResult TokenVerifier::VerifyTokenSignature(const SignedTokenPB& signed_token, - TokenPB* token) const { +TokenVerificationResult TokenVerifier::VerifyTokenSignature( + const SignedTokenPB& signed_token, TokenPB* token) const { if (!signed_token.has_signature() || !signed_token.has_signing_key_seq_num() || !signed_token.has_token_data()) { - return VerificationResult::INVALID_TOKEN; + return TokenVerificationResult::INVALID_TOKEN; } if (!token->ParseFromString(signed_token.token_data()) || !token->has_expire_unix_epoch_seconds()) { - return VerificationResult::INVALID_TOKEN; + return TokenVerificationResult::INVALID_TOKEN; } int64_t now = WallTime_Now(); if (token->expire_unix_epoch_seconds() < now) { - return VerificationResult::EXPIRED_TOKEN; + return TokenVerificationResult::EXPIRED_TOKEN; } for (auto flag : token->incompatible_features()) { if (!TokenPB::Feature_IsValid(flag)) { KLOG_EVERY_N_SECS(WARNING, 60) << "received token with unknown feature; " "server needs to be updated"; - return VerificationResult::INCOMPATIBLE_FEATURE; + return TokenVerificationResult::INCOMPATIBLE_FEATURE; } } @@ -133,34 +133,34 @@ VerificationResult TokenVerifier::VerifyTokenSignature(const SignedTokenPB& sign shared_lock<RWMutex> l(lock_); auto* tsk = FindPointeeOrNull(keys_by_seq_, signed_token.signing_key_seq_num()); if (!tsk) { - return VerificationResult::UNKNOWN_SIGNING_KEY; + return TokenVerificationResult::UNKNOWN_SIGNING_KEY; } if (tsk->pb().expire_unix_epoch_seconds() < now) { - return VerificationResult::EXPIRED_SIGNING_KEY; + return TokenVerificationResult::EXPIRED_SIGNING_KEY; } if (!tsk->VerifySignature(signed_token)) { - return VerificationResult::INVALID_SIGNATURE; + return TokenVerificationResult::INVALID_SIGNATURE; } } - return VerificationResult::VALID; + return TokenVerificationResult::VALID; } -const char* VerificationResultToString(VerificationResult r) { +const char* TokenVerificationResultToString(TokenVerificationResult r) { switch (r) { - case security::VerificationResult::VALID: + case security::TokenVerificationResult::VALID: return "valid"; - case security::VerificationResult::INVALID_TOKEN: + case security::TokenVerificationResult::INVALID_TOKEN: return "invalid token"; - case security::VerificationResult::INVALID_SIGNATURE: + case security::TokenVerificationResult::INVALID_SIGNATURE: return "invalid token signature"; - case security::VerificationResult::EXPIRED_TOKEN: + case security::TokenVerificationResult::EXPIRED_TOKEN: return "token expired"; - case security::VerificationResult::EXPIRED_SIGNING_KEY: + case security::TokenVerificationResult::EXPIRED_SIGNING_KEY: return "token signing key expired"; - case security::VerificationResult::UNKNOWN_SIGNING_KEY: + case security::TokenVerificationResult::UNKNOWN_SIGNING_KEY: return "token signed with unknown key"; - case security::VerificationResult::INCOMPATIBLE_FEATURE: + case security::TokenVerificationResult::INCOMPATIBLE_FEATURE: return "token uses incompatible feature"; default: LOG(FATAL) << "unexpected VerificationResult value: " @@ -170,4 +170,3 @@ const char* VerificationResultToString(VerificationResult r) { } // namespace security } // namespace kudu - diff --git a/src/kudu/security/token_verifier.h b/src/kudu/security/token_verifier.h index 8d5d176..36a6e09 100644 --- a/src/kudu/security/token_verifier.h +++ b/src/kudu/security/token_verifier.h @@ -35,7 +35,7 @@ class SignedTokenPB; class TokenPB; class TokenSigningPublicKey; class TokenSigningPublicKeyPB; -enum class VerificationResult; +enum class TokenVerificationResult; // Class responsible for verifying tokens provided to a server. // @@ -85,8 +85,8 @@ class TokenVerifier { // Verify the signature on the given signed token, and deserialize the // contents into 'token'. - VerificationResult VerifyTokenSignature(const SignedTokenPB& signed_token, - TokenPB* token) const; + TokenVerificationResult VerifyTokenSignature( + const SignedTokenPB& signed_token, TokenPB* token) const; private: typedef std::map<int64_t, std::unique_ptr<TokenSigningPublicKey>> KeysMap; @@ -100,7 +100,7 @@ class TokenVerifier { // Result of a token verification. // Values added to this enum must also be added to VerificationResultToString(). -enum class VerificationResult { +enum class TokenVerificationResult { // The signature is valid and the token is not expired. VALID, // The token itself is invalid (e.g. missing its signature or data, @@ -120,7 +120,7 @@ enum class VerificationResult { INCOMPATIBLE_FEATURE }; -const char* VerificationResultToString(VerificationResult r); +const char* TokenVerificationResultToString(TokenVerificationResult r); } // namespace security } // namespace kudu diff --git a/src/kudu/tserver/tablet_service.cc b/src/kudu/tserver/tablet_service.cc index 53c78a1..d37bf08 100644 --- a/src/kudu/tserver/tablet_service.cc +++ b/src/kudu/tserver/tablet_service.cc @@ -222,7 +222,7 @@ using kudu::fault_injection::MaybeTrue; using kudu::pb_util::SecureDebugString; using kudu::pb_util::SecureShortDebugString; using kudu::rpc::ErrorStatusPB; -using kudu::rpc::ParseVerificationResult; +using kudu::rpc::ParseTokenVerificationResult; using kudu::rpc::RpcContext; using kudu::rpc::RpcSidecar; using kudu::security::TokenPB; @@ -621,7 +621,7 @@ static bool VerifyAuthzTokenOrRespond(const TokenVerifier& token_verifier, TokenPB token_pb; const auto result = token_verifier.VerifyTokenSignature(req.authz_token(), &token_pb); ErrorStatusPB::RpcErrorCodePB error; - Status s = ParseVerificationResult(result, + Status s = ParseTokenVerificationResult(result, ErrorStatusPB::ERROR_INVALID_AUTHORIZATION_TOKEN, &error); if (!s.ok()) { context->RespondRpcFailure(error, s.CloneAndPrepend("authz token verification failure"));