kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [1/2] kudu git commit: [security] Negotiate authentication type during RPC setup
Date Fri, 17 Feb 2017 08:17:21 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 0b0781bee -> 4bac53279


http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/security/tls_handshake-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_handshake-test.cc b/src/kudu/security/tls_handshake-test.cc
index 0d8b9a8..6aa019a 100644
--- a/src/kudu/security/tls_handshake-test.cc
+++ b/src/kudu/security/tls_handshake-test.cc
@@ -40,42 +40,28 @@ namespace security {
 
 using ca::CertSigner;
 
-enum class CertType {
-  NONE,
-  SELF_SIGNED,
-  SIGNED,
-};
-
 struct Case {
-  CertType client_cert;
+  PkiConfig client_pki;
   TlsVerificationMode client_verification;
-  CertType server_cert;
+  PkiConfig server_pki;
   TlsVerificationMode server_verification;
   Status expected_status;
 };
 
 // Beautifies CLI test output.
 std::ostream& operator<<(std::ostream& o, Case c) {
-
-  auto cert_type_name = [] (const CertType& cert_type) {
-    switch (cert_type) {
-      case CertType::NONE: return "NONE";
-      case CertType::SELF_SIGNED: return "SELF_SIGNED";
-      case CertType::SIGNED: return "SIGNED";
-    }
-  };
-
   auto verification_mode_name = [] (const TlsVerificationMode& verification_mode) {
     switch (verification_mode) {
       case TlsVerificationMode::VERIFY_NONE: return "NONE";
       case TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST: return "REMOTE_CERT_AND_HOST";
     }
+    return "unreachable";
   };
 
-  o << "{client-cert: " << cert_type_name(c.client_cert) << ", "
+  o << "{client-pki: " << c.client_pki << ", "
     << "client-verification: " << verification_mode_name(c.client_verification)
<< ", "
-    << "server-cert: " << cert_type_name(c.client_cert) << ", "
-    << "server-verification: " << verification_mode_name(c.client_verification)
<< ", "
+    << "server-pki: " << c.server_pki << ", "
+    << "server-verification: " << verification_mode_name(c.server_verification)
<< ", "
     << "expected-status: " << c.expected_status.ToString() << "}";
 
   return o;
@@ -87,11 +73,6 @@ class TestTlsHandshake : public KuduTest,
   void SetUp() override {
     KuduTest::SetUp();
 
-    // Tune down the RSA key length in order to speed up tests. We would tune it
-    // smaller, but at 512 bits OpenSSL returns a "digest too big for rsa key"
-    // error during negotiation.
-    FLAGS_server_rsa_key_length_bits = 1024;
-
     ASSERT_OK(client_tls_.Init());
     ASSERT_OK(server_tls_.Init());
   }
@@ -144,38 +125,14 @@ class TestTlsHandshake : public KuduTest,
   string key_path_;
 };
 
-namespace {
-Status InitTlsContextCert(const PrivateKey& ca_key,
-                          const Cert& ca_cert,
-                          TlsContext* tls_context,
-                          CertType cert_type) {
-  RETURN_NOT_OK(tls_context->AddTrustedCertificate(ca_cert));
-  switch (cert_type) {
-    case CertType::SIGNED: {
-      Cert cert;
-      RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey("test-uuid"));
-      RETURN_NOT_OK(CertSigner(&ca_cert, &ca_key).Sign(*tls_context->GetCsrIfNecessary(),
&cert));
-      RETURN_NOT_OK(tls_context->AdoptSignedCert(cert));
-      break;
-    }
-    case CertType::SELF_SIGNED:
-      RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey("test-uuid"));
-      break;
-    case CertType::NONE:
-      break;
-  }
-  return Status::OK();
-}
-} // anonymous namespace
-
 TEST_F(TestTlsHandshake, TestHandshakeSequence) {
   PrivateKey ca_key;
   Cert ca_cert;
   ASSERT_OK(GenerateSelfSignedCAForTests(&ca_key, &ca_cert));
 
   // Both client and server have certs and CA.
-  ASSERT_OK(InitTlsContextCert(ca_key, ca_cert, &client_tls_, CertType::SIGNED));
-  ASSERT_OK(InitTlsContextCert(ca_key, ca_cert, &server_tls_, CertType::SIGNED));
+  ASSERT_OK(ConfigureTlsContext(PkiConfig::SIGNED, ca_cert, ca_key, &client_tls_));
+  ASSERT_OK(ConfigureTlsContext(PkiConfig::SIGNED, ca_cert, ca_key, &server_tls_));
 
   TlsHandshake server;
   TlsHandshake client;
@@ -278,8 +235,8 @@ TEST_P(TestTlsHandshake, TestHandshake) {
   Cert ca_cert;
   ASSERT_OK(GenerateSelfSignedCAForTests(&ca_key, &ca_cert));
 
-  ASSERT_OK(InitTlsContextCert(ca_key, ca_cert, &client_tls_, test_case.client_cert));
-  ASSERT_OK(InitTlsContextCert(ca_key, ca_cert, &server_tls_, test_case.server_cert));
+  ASSERT_OK(ConfigureTlsContext(test_case.client_pki, ca_cert, ca_key, &client_tls_));
+  ASSERT_OK(ConfigureTlsContext(test_case.server_pki, ca_cert, ca_key, &server_tls_));
 
   Status s = RunHandshake(test_case.client_verification, test_case.server_verification);
 
@@ -295,56 +252,85 @@ INSTANTIATE_TEST_CASE_P(CertCombinations,
         // has a self-signed cert, since we don't expect those to occur in
         // practice.
 
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_NONE,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::OK() },
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::RuntimeError("client error:.*certificate verify failed") },
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_NONE,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::RuntimeError("server error:.*peer did not return a certificate") },
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::RuntimeError("client error:.*certificate verify failed") },
 
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_NONE,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::OK() },
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               Status::RuntimeError("client error:.*certificate verify failed") },
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               Status::RuntimeError("server error:.*peer did not return a certificate") },
+        Case { PkiConfig::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               Status::RuntimeError("client error:.*certificate verify failed") },
+
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::OK() },
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_NONE,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
+               Status::RuntimeError("client error:.*certificate verify failed") },
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::RuntimeError("server error:.*peer did not return a certificate") },
-        Case { CertType::NONE, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               Status::RuntimeError("client error:.*certificate verify failed") },
+
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               Status::OK() },
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               Status::OK() },
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               Status::RuntimeError("server error:.*peer did not return a certificate") },
+        Case { PkiConfig::TRUSTED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::RuntimeError("server error:.*peer did not return a certificate") },
 
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::OK() },
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::RuntimeError("client error:.*certificate verify failed") },
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               Status::OK() },
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               // OpenSSL 1.0.0 returns "no certificate returned" for this case,
+               // which appears to be a bug.
+               Status::RuntimeError("server error:.*(certificate verify failed|"
+                                                    "no certificate returned)") },
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SELF_SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::RuntimeError("client error:.*certificate verify failed") },
 
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::OK() },
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
                Status::OK() },
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_NONE,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_NONE,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::OK() },
-        Case { CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
-               CertType::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+        Case { PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
+               PkiConfig::SIGNED, TlsVerificationMode::VERIFY_REMOTE_CERT_AND_HOST,
                Status::OK() }
 ));
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/security/tls_handshake.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_handshake.cc b/src/kudu/security/tls_handshake.cc
index 26bfa9f..f57f24f 100644
--- a/src/kudu/security/tls_handshake.cc
+++ b/src/kudu/security/tls_handshake.cc
@@ -141,6 +141,8 @@ Status TlsHandshake::Verify(const Socket& socket) const {
     return Status::OK();
   }
 
+  // TODO(PKI): KUDU-1886: Do hostname verification.
+  /*
   TRACE("Verifying peer cert");
 
   // Get the peer's hostname
@@ -167,6 +169,7 @@ Status TlsHandshake::Verify(const Socket& socket) const {
     return Status::RuntimeError("TLS certificate hostname verification error", GetOpenSSLErrors());
   }
   DCHECK_EQ(match, 1);
+  */
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/security/token-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/token-test.cc b/src/kudu/security/token-test.cc
index 0bb751d..ddc6a5c 100644
--- a/src/kudu/security/token-test.cc
+++ b/src/kudu/security/token-test.cc
@@ -131,13 +131,14 @@ TEST_F(TokenTest, TestEndToEnd_Valid) {
   ASSERT_OK(signer.RotateSigningKey());
 
   // Make and sign a token.
-  SignedTokenPB token = MakeUnsignedToken(WallTime_Now() + 600);
-  ASSERT_OK(signer.SignToken(&token));
+  SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600);
+  ASSERT_OK(signer.SignToken(&signed_token));
 
   // Try to verify it.
   TokenVerifier verifier;
   ASSERT_OK(verifier.ImportPublicKeys(signer.GetTokenSigningPublicKeys(0)));
-  ASSERT_EQ(VerificationResult::VALID, verifier.VerifyTokenSignature(token));
+  TokenPB token;
+  ASSERT_EQ(VerificationResult::VALID, verifier.VerifyTokenSignature(signed_token, &token));
 }
 
 // Test all of the possible cases covered by token verification.
@@ -151,32 +152,40 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) {
 
   // Make and sign a token, but corrupt the data in it.
   {
-    SignedTokenPB token = MakeUnsignedToken(WallTime_Now() + 600);
-    ASSERT_OK(signer.SignToken(&token));
-    token.set_token_data("xyz");
-    ASSERT_EQ(VerificationResult::INVALID_TOKEN, verifier.VerifyTokenSignature(token));
+    SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600);
+    ASSERT_OK(signer.SignToken(&signed_token));
+    signed_token.set_token_data("xyz");
+    TokenPB token;
+    ASSERT_EQ(VerificationResult::INVALID_TOKEN,
+              verifier.VerifyTokenSignature(signed_token, &token));
   }
 
   // Make and sign a token, but corrupt the signature.
   {
-    SignedTokenPB token = MakeUnsignedToken(WallTime_Now() + 600);
-    ASSERT_OK(signer.SignToken(&token));
-    token.set_signature("xyz");
-    ASSERT_EQ(VerificationResult::INVALID_SIGNATURE, verifier.VerifyTokenSignature(token));
+    SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600);
+    ASSERT_OK(signer.SignToken(&signed_token));
+    signed_token.set_signature("xyz");
+    TokenPB token;
+    ASSERT_EQ(VerificationResult::INVALID_SIGNATURE,
+              verifier.VerifyTokenSignature(signed_token, &token));
   }
 
   // Make and sign a token, but set it to be already expired.
   {
-    SignedTokenPB token = MakeUnsignedToken(WallTime_Now() - 10);
-    ASSERT_OK(signer.SignToken(&token));
-    ASSERT_EQ(VerificationResult::EXPIRED_TOKEN, verifier.VerifyTokenSignature(token));
+    SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() - 10);
+    ASSERT_OK(signer.SignToken(&signed_token));
+    TokenPB token;
+    ASSERT_EQ(VerificationResult::EXPIRED_TOKEN,
+              verifier.VerifyTokenSignature(signed_token, &token));
   }
 
   // Make and sign a token which uses an incompatible feature flag.
   {
-    SignedTokenPB token = MakeIncompatibleToken();
-    ASSERT_OK(signer.SignToken(&token));
-    ASSERT_EQ(VerificationResult::INCOMPATIBLE_FEATURE, verifier.VerifyTokenSignature(token));
+    SignedTokenPB signed_token = MakeIncompatibleToken();
+    ASSERT_OK(signer.SignToken(&signed_token));
+    TokenPB token;
+    ASSERT_EQ(VerificationResult::INCOMPATIBLE_FEATURE,
+              verifier.VerifyTokenSignature(signed_token, &token));
   }
 
   // Rotate to a new key, but don't inform the verifier of it yet. When we
@@ -184,9 +193,11 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) {
   ASSERT_OK(signer.RotateSigningKey());
   ASSERT_OK(signer.RotateSigningKey());
   {
-    SignedTokenPB token = MakeUnsignedToken(WallTime_Now() + 600);
-    ASSERT_OK(signer.SignToken(&token));
-    ASSERT_EQ(VerificationResult::UNKNOWN_SIGNING_KEY, verifier.VerifyTokenSignature(token));
+    SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600);
+    ASSERT_OK(signer.SignToken(&signed_token));
+    TokenPB token;
+    ASSERT_EQ(VerificationResult::UNKNOWN_SIGNING_KEY,
+              verifier.VerifyTokenSignature(signed_token, &token));
   }
 
   // Rotate to a signing key which is already expired, and inform the verifier
@@ -198,9 +209,11 @@ TEST_F(TokenTest, TestEndToEnd_InvalidCases) {
   ASSERT_OK(verifier.ImportPublicKeys(signer.GetTokenSigningPublicKeys(
       verifier.GetMaxKnownKeySequenceNumber())));
   {
-    SignedTokenPB token = MakeUnsignedToken(WallTime_Now() + 600);
-    ASSERT_OK(signer.SignToken(&token));
-    ASSERT_EQ(VerificationResult::EXPIRED_SIGNING_KEY, verifier.VerifyTokenSignature(token));
+    SignedTokenPB signed_token = MakeUnsignedToken(WallTime_Now() + 600);
+    ASSERT_OK(signer.SignToken(&signed_token));
+    TokenPB token;
+    ASSERT_EQ(VerificationResult::EXPIRED_SIGNING_KEY,
+              verifier.VerifyTokenSignature(signed_token, &token));
   }
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/security/token_signer.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/token_signer.h b/src/kudu/security/token_signer.h
index 2824be0..259b7b5 100644
--- a/src/kudu/security/token_signer.h
+++ b/src/kudu/security/token_signer.h
@@ -98,7 +98,7 @@ class TokenSigner {
   ~TokenSigner();
 
   // Sign the given token using the current TSK.
-  Status SignToken(SignedTokenPB* token) const;
+  Status SignToken(SignedTokenPB* token) const WARN_UNUSED_RESULT;
 
   // Returns the set of valid public keys with sequence numbers greater
   // than 'after_sequence_number'.
@@ -108,7 +108,7 @@ class TokenSigner {
   // Rotate to a new token-signing key.
   //
   // See class documentation for more information.
-  Status RotateSigningKey();
+  Status RotateSigningKey() WARN_UNUSED_RESULT;
 
  private:
   // Protects following fields.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/security/token_verifier.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/token_verifier.cc b/src/kudu/security/token_verifier.cc
index 2150c91..cb1c0a1 100644
--- a/src/kudu/security/token_verifier.cc
+++ b/src/kudu/security/token_verifier.cc
@@ -82,28 +82,25 @@ Status TokenVerifier::ImportPublicKeys(const vector<TokenSigningPublicKeyPB>&
pu
 }
 
 // Verify the signature on the given token.
-VerificationResult TokenVerifier::VerifyTokenSignature(
-    const SignedTokenPB& signed_token) const {
+VerificationResult 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;
   }
 
-  // TODO(perf): should we return the deserialized TokenPB here
-  // since callers are probably going to need it, anyway?
-  TokenPB token;
-  if (!token.ParseFromString(signed_token.token_data()) ||
-      !token.has_expire_unix_epoch_seconds()) {
+  if (!token->ParseFromString(signed_token.token_data()) ||
+      !token->has_expire_unix_epoch_seconds()) {
     return VerificationResult::INVALID_TOKEN;
   }
 
   int64_t now = WallTime_Now();
-  if (token.expire_unix_epoch_seconds() < now) {
+  if (token->expire_unix_epoch_seconds() < now) {
     return VerificationResult::EXPIRED_TOKEN;
   }
 
-  for (auto flag : token.incompatible_features()) {
+  for (auto flag : token->incompatible_features()) {
     if (!TokenPB::Feature_IsValid(flag)) {
       return VerificationResult::INCOMPATIBLE_FEATURE;
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/security/token_verifier.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/token_verifier.h b/src/kudu/security/token_verifier.h
index a35af22..3466fda 100644
--- a/src/kudu/security/token_verifier.h
+++ b/src/kudu/security/token_verifier.h
@@ -25,9 +25,10 @@
 namespace kudu {
 namespace security {
 
+class SignedTokenPB;
+class TokenPB;
 class TokenSigningPublicKey;
 class TokenSigningPublicKeyPB;
-class SignedTokenPB;
 enum class VerificationResult;
 
 // Class responsible for verifying tokens provided to a server.
@@ -64,8 +65,10 @@ class TokenVerifier {
   Status ImportPublicKeys(const std::vector<TokenSigningPublicKeyPB>& public_keys)
     WARN_UNUSED_RESULT;
 
-  // Verify the signature on the given token.
-  VerificationResult VerifyTokenSignature(const SignedTokenPB& signed_token) const;
+  // Verify the signature on the given signed token, and deserialize the
+  // contents into 'token'.
+  VerificationResult VerifyTokenSignature(const SignedTokenPB& signed_token,
+                                          TokenPB* token) const;
 
   // TODO(PKI): should expire out old key versions at some point. eg only
   // keep old key versions until their expiration is an hour or two in the past?

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/server/server_base.h
----------------------------------------------------------------------
diff --git a/src/kudu/server/server_base.h b/src/kudu/server/server_base.h
index 6f17985..a76a338 100644
--- a/src/kudu/server/server_base.h
+++ b/src/kudu/server/server_base.h
@@ -49,6 +49,7 @@ class ServiceIf;
 
 namespace security {
 class TlsContext;
+class TokenVerifier;
 } // namespace security
 
 namespace server {
@@ -76,9 +77,12 @@ class ServerBase {
 
   FsManager* fs_manager() { return fs_manager_.get(); }
 
-  const security::TlsContext& tls_context() { return messenger_->tls_context(); }
+  const security::TlsContext& tls_context() const { return messenger_->tls_context();
}
   security::TlsContext* mutable_tls_context() { return messenger_->mutable_tls_context();
}
 
+  const security::TokenVerifier& token_verifier() const { return messenger_->token_verifier();
}
+  security::TokenVerifier* mutable_token_verifier() { return messenger_->mutable_token_verifier();
}
+
   // Return the instance identifier of this server.
   // This may not be called until after the server is Started.
   const NodeInstancePB& instance_pb() const;

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/tserver/heartbeater.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/heartbeater.cc b/src/kudu/tserver/heartbeater.cc
index f0438ad..512f680 100644
--- a/src/kudu/tserver/heartbeater.cc
+++ b/src/kudu/tserver/heartbeater.cc
@@ -32,6 +32,7 @@
 #include "kudu/master/master.proxy.h"
 #include "kudu/security/cert.h"
 #include "kudu/security/tls_context.h"
+#include "kudu/security/token_verifier.h"
 #include "kudu/server/webserver.h"
 #include "kudu/tserver/tablet_server.h"
 #include "kudu/tserver/tablet_server_options.h"
@@ -372,6 +373,10 @@ Status Heartbeater::Thread::DoHeartbeat() {
   // TODO(PKI): send the version number of the latest CA cert which we know about.
   // The response should include new CA certs.
 
+  // Send the most recently known TSK sequence number so that the master can
+  // send us knew ones if they exist.
+  req.set_latest_tsk_seq_num(server_->token_verifier().GetMaxKnownKeySequenceNumber());
+
   if (send_full_tablet_report_) {
     LOG(INFO) << Substitute(
         "Master $0 was elected leader, sending a full tablet report...",
@@ -437,6 +442,15 @@ Status Heartbeater::Thread::DoHeartbeat() {
         "failed to adopt master-signed X509 cert");
   }
 
+  // Import TSKs.
+  if (!last_hb_response_.tsks().empty()) {
+    vector<security::TokenSigningPublicKeyPB> tsks(last_hb_response_.tsks().begin(),
+                                                   last_hb_response_.tsks().end());
+    RETURN_NOT_OK_PREPEND(
+        server_->mutable_token_verifier()->ImportPublicKeys(tsks),
+        "failed to import token signing public keys from master heartbeat");
+  }
+
   MarkTabletReportAcknowledged(req.tablet_report());
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/4bac5327/src/kudu/util/test_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index 4228e85..217f4dc 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -72,8 +72,8 @@ KuduTest::KuduTest()
     // Disable log redaction.
     {"log_redact_user_data", "false"},
     // Reduce default RSA key length for faster tests.
-    {"server_rsa_key_length_bits", "512"},
-    {"master_ca_rsa_key_length_bits", "512"}
+    {"server_rsa_key_length_bits", "1024"},
+    {"master_ca_rsa_key_length_bits", "1024"}
   };
   for (const auto& e : flags_for_tests) {
     // We don't check for errors here, because we have some default flags that


Mime
View raw message