You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/04/13 00:25:03 UTC

kudu git commit: KUDU-1965: Allow user provided TLS certificates to work with KRPC

Repository: kudu
Updated Branches:
  refs/heads/master 548721090 -> 642e01190


KUDU-1965: Allow user provided TLS certificates to work with KRPC

This patch adds a bool in the TlsContext class to keep track of
whether the certificates provided are externally provided or not.

If it is externally provided, then authenticated TLS is not
negotiated, causing a fallback to SASL authentication. The
certificates are used only for encryption.

This adds some tests in negotiation-test

Change-Id: Idd44770a1fe85e9934a657ee79d93139b9a86dff
Reviewed-on: http://gerrit.cloudera.org:8080/6594
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/642e0119
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/642e0119
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/642e0119

Branch: refs/heads/master
Commit: 642e01190ca19882b5052739bae5508b0b38b32e
Parents: 5487210
Author: Sailesh Mukil <sa...@apache.org>
Authored: Fri Apr 7 11:53:54 2017 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Thu Apr 13 00:24:42 2017 +0000

----------------------------------------------------------------------
 docs/design-docs/rpc.md                 |   4 +-
 src/kudu/rpc/client_negotiation.cc      |   4 +-
 src/kudu/rpc/negotiation-test.cc        | 138 +++++++++++++++++++++++++++
 src/kudu/rpc/server_negotiation.cc      |   6 +-
 src/kudu/security/security-test-util.cc |  13 +++
 src/kudu/security/security-test-util.h  |   4 +-
 src/kudu/security/tls_context.cc        |   5 +-
 src/kudu/security/tls_context.h         |   3 +
 8 files changed, 171 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/docs/design-docs/rpc.md
----------------------------------------------------------------------
diff --git a/docs/design-docs/rpc.md b/docs/design-docs/rpc.md
index af03164..474962d 100644
--- a/docs/design-docs/rpc.md
+++ b/docs/design-docs/rpc.md
@@ -486,8 +486,8 @@ the server depends on configuration and the available credentials:
 +----------------------+       +----------------------+
 |                      |       |                      |       +----------------------+
 | Does the server have |  yes  | Does the client have |  yes  |                      |
-| a CA-signed TLS      +-------> a CA-signed TLS      +-------> Authenticate via TLS |
-| certificate?         |       | certificate?         |       |                      |
+| an internally        +-------> an internally        +-------> Authenticate via TLS |
+| CA-signed TLS cert?  |       | CA-signed TLS cert?  |       |                      |
 |                      |       |                      |       +----------------------+
 +------+---------------+       +---------+------------+
        |                                 |

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/rpc/client_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.cc b/src/kudu/rpc/client_negotiation.cc
index 35bafd6..e083658 100644
--- a/src/kudu/rpc/client_negotiation.cc
+++ b/src/kudu/rpc/client_negotiation.cc
@@ -311,7 +311,9 @@ Status ClientNegotiation::SendNegotiate() {
   if (!helper_.EnabledMechs().empty()) {
     msg.add_authn_types()->mutable_sasl();
   }
-  if (tls_context_->has_signed_cert()) {
+  if (tls_context_->has_signed_cert() && !tls_context_->is_external_cert()) {
+    // We only provide authenticated TLS if the certificates are generated
+    // by the internal CA.
     msg.add_authn_types()->mutable_certificate();
   }
   if (authn_token_ && tls_context_->has_trusted_cert()) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/rpc/negotiation-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/negotiation-test.cc b/src/kudu/rpc/negotiation-test.cc
index 38c8ed9..329a2b5 100644
--- a/src/kudu/rpc/negotiation-test.cc
+++ b/src/kudu/rpc/negotiation-test.cc
@@ -723,6 +723,144 @@ INSTANTIATE_TEST_CASE_P(NegotiationCombinations,
           AuthenticationType::SASL,
           SaslMechanism::PLAIN,
           true,
+        },
+
+        // client: GSSAPI, TLS required, externally-signed cert
+        // server: GSSAPI, TLS required, externally-signed cert
+        NegotiationDescriptor {
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          false,
+          Status::OK(),
+          Status::OK(),
+          AuthenticationType::SASL,
+          SaslMechanism::GSSAPI,
+          true,
+        },
+
+        // client: GSSAPI, TLS optional, externally-signed cert
+        // server: GSSAPI, TLS required, signed cert
+        NegotiationDescriptor {
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::OPTIONAL,
+          },
+          EndpointConfig {
+            PkiConfig::SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          false,
+          Status::OK(),
+          Status::OK(),
+          AuthenticationType::SASL,
+          SaslMechanism::GSSAPI,
+          true,
+        },
+
+        // client: GSSAPI, TLS required
+        // server: GSSAPI, TLS required, externally-signed cert
+        NegotiationDescriptor {
+          EndpointConfig {
+            PkiConfig::NONE,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          false,
+          Status::OK(),
+          Status::OK(),
+          AuthenticationType::SASL,
+          SaslMechanism::GSSAPI,
+          true,
+        },
+
+        // client: GSSAPI, PLAIN, TLS required, externally-signed cert
+        // server: PLAIN, TLS required, externally-signed cert
+        NegotiationDescriptor {
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI, SaslMechanism::PLAIN },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::PLAIN },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          false,
+          Status::OK(),
+          Status::OK(),
+          AuthenticationType::SASL,
+          SaslMechanism::PLAIN,
+          true,
+        },
+
+        // client: GSSAPI, TLS disabled, signed cert
+        // server: GSSAPI, TLS required, externally-signed cert
+        NegotiationDescriptor {
+          EndpointConfig {
+            PkiConfig::SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::DISABLED,
+          },
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          false,
+          Status::NotAuthorized(".*client does not support required TLS encryption"),
+          Status::NotAuthorized(".*client does not support required TLS encryption"),
+          AuthenticationType::SASL,
+          SaslMechanism::GSSAPI,
+          true,
+        },
+
+        // client: GSSAPI, TLS required, signed cert
+        // server: GSSAPI, TLS required, externally-signed cert
+        NegotiationDescriptor {
+          EndpointConfig {
+            PkiConfig::SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          EndpointConfig {
+            PkiConfig::EXTERNALLY_SIGNED,
+            { SaslMechanism::GSSAPI },
+            false,
+            RpcEncryption::REQUIRED,
+          },
+          false,
+          Status::OK(),
+          Status::OK(),
+          AuthenticationType::SASL,
+          SaslMechanism::GSSAPI,
+          true,
         }
 ));
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index 17a3cac..7f5e8ee 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -364,7 +364,11 @@ Status ServerNegotiation::HandleNegotiate(const NegotiatePB& request) {
           authn_types.insert(AuthenticationType::TOKEN);
           break;
         case AuthenticationTypePB::kCertificate:
-          authn_types.insert(AuthenticationType::CERTIFICATE);
+          // We only provide authenticated TLS if the certificates are generated
+          // by the internal CA.
+          if (!tls_context_->is_external_cert()) {
+            authn_types.insert(AuthenticationType::CERTIFICATE);
+          }
           break;
         case AuthenticationTypePB::TYPE_NOT_SET: {
           Sockaddr addr;

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/security/security-test-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/security-test-util.cc b/src/kudu/security/security-test-util.cc
index 2043b5c..b2c8020 100644
--- a/src/kudu/security/security-test-util.cc
+++ b/src/kudu/security/security-test-util.cc
@@ -23,6 +23,8 @@
 #include "kudu/security/cert.h"
 #include "kudu/security/crypto.h"
 #include "kudu/security/tls_context.h"
+#include "kudu/util/path_util.h"
+#include "kudu/util/test_util.h"
 
 namespace kudu {
 namespace security {
@@ -49,6 +51,7 @@ std::ostream& operator<<(std::ostream& o, PkiConfig c) {
       case PkiConfig::SELF_SIGNED: o << "SELF_SIGNED"; break;
       case PkiConfig::TRUSTED: o << "TRUSTED"; break;
       case PkiConfig::SIGNED: o << "SIGNED"; break;
+      case PkiConfig::EXTERNALLY_SIGNED: o << "EXTERNALLY_SIGNED"; break;
     }
     return o;
 }
@@ -73,6 +76,16 @@ Status ConfigureTlsContext(PkiConfig config,
       RETURN_NOT_OK(tls_context->AdoptSignedCert(cert));
       break;
     };
+    case PkiConfig::EXTERNALLY_SIGNED: {
+      // Write certificate to file.
+      std::string cert_path = JoinPathSegments(GetTestDataDirectory(), "kudu-test-cert.pem");
+      RETURN_NOT_OK(CreateSSLServerCert(cert_path));
+      // Write private key to file.
+      std::string key_path = JoinPathSegments(GetTestDataDirectory(), "kudu-test-key.pem");
+      RETURN_NOT_OK(CreateSSLPrivateKey(key_path));
+      RETURN_NOT_OK(tls_context->LoadCertificateAndKey(cert_path, key_path));
+      RETURN_NOT_OK(tls_context->LoadCertificateAuthority(cert_path));
+    };
   }
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/security/security-test-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/security-test-util.h b/src/kudu/security/security-test-util.h
index e4851f8..f0b8977 100644
--- a/src/kudu/security/security-test-util.h
+++ b/src/kudu/security/security-test-util.h
@@ -112,8 +112,10 @@ enum class PkiConfig {
   SELF_SIGNED,
   // The TLS context has no TLS cert and a trusted cert.
   TRUSTED,
-  // The TLS context has a signed TLS cert and trusts the corresonding signing cert.
+  // The TLS context has a signed TLS cert and trusts the corresponding signing cert.
   SIGNED,
+  // The TLS context has a externally signed TLS cert and trusts the corresponding signing cert.
+  EXTERNALLY_SIGNED,
 };
 
 // PkiConfig pretty-printer.

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/security/tls_context.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_context.cc b/src/kudu/security/tls_context.cc
index 402c0e8..93256fe 100644
--- a/src/kudu/security/tls_context.cc
+++ b/src/kudu/security/tls_context.cc
@@ -91,7 +91,8 @@ template<> struct SslTypeTraits<X509_STORE_CTX> {
 TlsContext::TlsContext()
     : lock_(RWMutex::Priority::PREFER_READING),
       trusted_cert_count_(0),
-      has_cert_(false) {
+      has_cert_(false),
+      is_external_cert_(false) {
   security::InitializeOpenSSL();
 }
 
@@ -413,11 +414,13 @@ Status TlsContext::LoadCertificateAndKey(const string& certificate_path,
   RETURN_NOT_OK(c.FromFile(certificate_path, DataFormat::PEM));
   PrivateKey k;
   RETURN_NOT_OK(k.FromFile(key_path, DataFormat::PEM));
+  is_external_cert_ = true;
   return UseCertificateAndKey(c, k);
 }
 
 Status TlsContext::LoadCertificateAuthority(const string& certificate_path) {
   SCOPED_OPENSSL_NO_PENDING_ERRORS;
+  if (has_cert_) DCHECK(is_external_cert_);
   Cert c;
   RETURN_NOT_OK(c.FromFile(certificate_path, DataFormat::PEM));
   return AddTrustedCertificate(c);

http://git-wip-us.apache.org/repos/asf/kudu/blob/642e0119/src/kudu/security/tls_context.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_context.h b/src/kudu/security/tls_context.h
index 09f35a6..b278d9c 100644
--- a/src/kudu/security/tls_context.h
+++ b/src/kudu/security/tls_context.h
@@ -159,6 +159,8 @@ class TlsContext {
     return trusted_cert_count_;
   }
 
+  bool is_external_cert() const { return is_external_cert_; }
+
  private:
 
   Status VerifyCertChainUnlocked(const Cert& cert) WARN_UNUSED_RESULT;
@@ -172,6 +174,7 @@ class TlsContext {
   c_unique_ptr<SSL_CTX> ctx_;
   int32_t trusted_cert_count_;
   bool has_cert_;
+  bool is_external_cert_;
   boost::optional<CertSignRequest> csr_;
 };