[kudu] branch master updated: [security] handle a few unexpected authn token conditions

[alexey]

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


The following commit(s) were added to refs/heads/master by this push:
     new 24f85ce  [security] handle a few unexpected authn token conditions
24f85ce is described below

commit 24f85cedcd47e585ea9c308c317da963c5ab8fa9
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Wed Feb 23 21:22:23 2022 -0800

    [security] handle a few unexpected authn token conditions
    
    This patch addresses a few unexpected error conditions in the
    server-side negotiation code when authenticating a client with
    its authn token.
    
    I didn't add any tests since there is no mock for the corresponding
    client side and implementing it from scratch just to verify these
    two simple cases looked like an overkill.
    
    Change-Id: Ic05ff6a9b289877d8440b94f00b2375da938c901
    Reviewed-on: http://gerrit.cloudera.org:8080/18271
    Tested-by: Alexey Serbin <aser...@cloudera.com>
    Reviewed-by: Attila Bukor <abu...@apache.org>
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/rpc/server_negotiation.cc | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/kudu/rpc/server_negotiation.cc 
b/src/kudu/rpc/server_negotiation.cc
index bcbebf6..025c36a 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -688,12 +688,17 @@ Status ServerNegotiation::AuthenticateByToken(faststring* 
recv_buf) {
   NegotiatePB pb;
   RETURN_NOT_OK(RecvNegotiatePB(&pb, recv_buf));
 
-  if (pb.step() != NegotiatePB::TOKEN_EXCHANGE) {
-    Status s =  Status::NotAuthorized("expected TOKEN_EXCHANGE step",
-                                      
NegotiatePB::NegotiateStep_Name(pb.step()));
+  if (PREDICT_FALSE(pb.step() != NegotiatePB::TOKEN_EXCHANGE)) {
+    const auto s = Status::NotAuthorized("expected TOKEN_EXCHANGE step",
+                                         
NegotiatePB::NegotiateStep_Name(pb.step()));
+    RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_UNAUTHORIZED, s));
+    return s;
   }
-  if (!pb.has_authn_token()) {
-    Status s = Status::NotAuthorized("TOKEN_EXCHANGE message must include an 
authentication token");
+  if (PREDICT_FALSE(!pb.has_authn_token())) {
+    const auto s = Status::NotAuthorized(
+        "TOKEN_EXCHANGE message must include an authentication token");
+    RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_UNAUTHORIZED, s));
+    return s;
   }
 
   // TODO(KUDU-1924): propagate the specific token verification failure back 
to the client,
@@ -701,9 +706,10 @@ Status ServerNegotiation::AuthenticateByToken(faststring* 
recv_buf) {
   security::TokenPB token;
   auto verification_result = 
token_verifier_->VerifyTokenSignature(pb.authn_token(), &token);
   ErrorStatusPB::RpcErrorCodePB error;
-  Status s = ParseTokenVerificationResult(verification_result,
-      ErrorStatusPB::FATAL_INVALID_AUTHENTICATION_TOKEN, &error);
-  if (!s.ok()) {
+  if (const auto s = ParseTokenVerificationResult(
+        verification_result,
+        ErrorStatusPB::FATAL_INVALID_AUTHENTICATION_TOKEN,
+        &error); !s.ok()) {
     RETURN_NOT_OK(SendError(error, s));
     return s;
   }