kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [2/2] kudu git commit: java: implement Channel Bindings
Date Sat, 11 Feb 2017 01:19:09 GMT
java: implement Channel Bindings

This adds a utility function for calculating RFC 5929
"tls-server-endpoint" channel bindings in Java. It also hooks the
channel bindings verification in the Negotiator implementation.

A new simple unit test verifies that channel bindings can be calculated
from a cert.

I also added a Java-format KeyStore with a self-signed cert as a test
resource. This was useful for writing a unit test. I generated it based
on the certs found in the C++ source using the following:

openssl pkcs12 -export -in /tmp/cert.pem -inkey /tmp/key.pem > /tmp/p12
keytool -importkeystore -srckeystore /tmp/p12 -destkeystore java/kudu-client/src/test/resources/test-key-and-cert.jks

No new unit test actually verifies the channel bindings verification,
but I did check that the integrity check is working by temporarily
flipping a byte before unwrapping and making sure that verification
failed.

Change-Id: I8b604ea6a0cff55820f7fbbb3ba4beba3a888a48
Reviewed-on: http://gerrit.cloudera.org:8080/5953
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>


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

Branch: refs/heads/master
Commit: 41dd9d41a1e6a8a0b60104e26ed1f0ece04158e6
Parents: 5833deb
Author: Todd Lipcon <todd@apache.org>
Authored: Wed Feb 8 17:35:48 2017 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Sat Feb 11 00:51:58 2017 +0000

----------------------------------------------------------------------
 .../java/org/apache/kudu/client/Negotiator.java |  64 +++++++++++++++----
 .../java/org/apache/kudu/util/SecurityUtil.java |  57 +++++++++++++++++
 .../org/apache/kudu/client/TestNegotiator.java  |  44 ++++++++++---
 .../src/test/resources/test-key-and-cert.jks    | Bin 0 -> 2212 bytes
 4 files changed, 143 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/41dd9d41/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
index 3b0385b..eba71a3 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
@@ -27,12 +27,14 @@
 package org.apache.kudu.client;
 
 import java.security.PrivilegedExceptionAction;
+import java.security.cert.Certificate;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.CallbackHandler;
@@ -46,11 +48,9 @@ import javax.security.sasl.SaslException;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.common.collect.ObjectArrays;
 import com.google.common.collect.Sets;
 import com.google.protobuf.ByteString;
 import com.google.protobuf.ZeroCopyLiteralByteString;
@@ -140,6 +140,8 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
    */
   private ChannelFuture sslHandshakeFuture;
 
+  private Certificate peerCert;
+
   public Negotiator(Subject subject, String remoteHostname) {
     this.subject = subject;
     this.remoteHostname = remoteHostname;
@@ -211,7 +213,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
         handleChallengeResponse(chan, response);
         break;
       case SASL_SUCCESS:
-        handleSuccessResponse(chan);
+        handleSuccessResponse(chan, response);
         break;
       default:
         throw new IllegalStateException("Wrong negotiation step: " +
@@ -263,9 +265,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
       }
       Map<String, String> props = Maps.newHashMap();
       // If we are using GSSAPI with TLS, enable integrity protection, which we use
-      // to securely transmit the channel bindings. Channel bindings aren't
-      // yet implemented, but if we don't advertise 'auth-int', then the server
-      // won't talk to us.
+      // to securely transmit the channel bindings.
       if ("GSSAPI".equals(clientMech) && willUseTls) {
         props.put(Sasl.QOP, "auth-int");
       }
@@ -303,12 +303,6 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
    */
   private void startTlsHandshake(Channel chan) throws SSLException {
     SSLEngine engine = SecurityUtil.createSslEngine();
-    // TODO(todd): remove usage of this anonymous cipher suite.
-    // It's replaced in the next patch in this patch series by
-    // a self-signed cert used for tests.
-    engine.setEnabledCipherSuites(ObjectArrays.concat(
-        engine.getEnabledCipherSuites(),
-        "TLS_DH_anon_WITH_AES_128_CBC_SHA"));
     engine.setUseClientMode(true);
     SslHandler handler = new SslHandler(engine);
     handler.setEnableRenegotiation(false);
@@ -342,7 +336,17 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
     //
     // NOTE: this takes effect immediately (i.e. the following SASL initiation
     // sequence is encrypted).
-    chan.getPipeline().addFirst("tls", sslEmbedder.getPipeline().getFirst());
+    SslHandler handler = (SslHandler)sslEmbedder.getPipeline().getFirst();
+    try {
+      Certificate[] certs = handler.getEngine().getSession().getPeerCertificates();
+      if (certs.length == 0) {
+        throw new SSLPeerUnverifiedException("no peer cert found");
+      }
+    } catch (SSLPeerUnverifiedException e) {
+      throw Throwables.propagate(e);
+    }
+
+    chan.getPipeline().addFirst("tls", handler);
     sendSaslInitiate(chan);
   }
 
@@ -362,6 +366,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
     }
     ByteString data = ByteString.copyFrom(bufs);
     if (sslHandshakeFuture.isDone()) {
+      // TODO(todd): should check sslHandshakeFuture.isSuccess()
       // TODO(danburkert): is this a correct assumption? would the
       // client ever be "done" but also produce handshake data?
       // if it did, would we want to encrypt the SSL message or no?
@@ -409,7 +414,38 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
     sendSaslMessage(chan, builder.build());
   }
 
-  private void handleSuccessResponse(Channel chan) {
+  /**
+   * Verify the channel bindings included in 'response'. This is used only
+   * for GSSAPI-authenticated connections over TLS.
+   * @throws RuntimeException on failure to verify
+   */
+  private void verifyChannelBindings(NegotiatePB response) {
+    try {
+      byte[] expected = SecurityUtil.getEndpointChannelBindings(peerCert);
+      if (!response.hasChannelBindings()) {
+        throw new RuntimeException("no channel bindings provided by remote peer");
+      }
+      byte[] provided = response.getChannelBindings().toByteArray();
+      // NOTE: the C SASL library's implementation of sasl_encode() actually
+      // includes a length prefix. Java's equivalents do not. So, we have to
+      // chop off the length prefix here before unwrapping.
+      if (provided.length < 4) {
+        throw new RuntimeException("invalid too-short channel bindings");
+      }
+      byte[] unwrapped = saslClient.unwrap(provided, 4, provided.length - 4);
+      if (!Bytes.equals(expected, unwrapped)) {
+        throw new RuntimeException("invalid channel bindings provided by remote peer");
+      }
+    } catch (SaslException se) {
+      throw Throwables.propagate(se);
+    }
+  }
+
+  private void handleSuccessResponse(Channel chan, NegotiatePB response) {
+    if (peerCert != null && "GSSAPI".equals(chosenMech)) {
+      verifyChannelBindings(response);
+    }
+
     state = State.FINISHED;
     chan.getPipeline().remove(this);
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/41dd9d41/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java b/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
index 163cf96..37e4302 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
@@ -19,6 +19,8 @@ package org.apache.kudu.util;
 
 import java.security.AccessControlContext;
 import java.security.AccessController;
+import java.security.MessageDigest;
+import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.util.HashMap;
@@ -37,7 +39,9 @@ import javax.security.auth.login.LoginContext;
 import javax.security.auth.login.LoginException;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableMap;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -50,6 +54,26 @@ public abstract class SecurityUtil {
   public static final String KUDU_TICKETCACHE_PROPERTY = "kudu.krb5ccname";
 
   /**
+   * Map from the names of digest algorithms used in X509 certificates to
+   * the appropriate MessageDigest implementation to use for channel-bindings.
+   */
+  private static final Map<String, String> CERT_DIGEST_TO_MESSAGE_DIGEST =
+      ImmutableMap.<String, String>builder()
+      // RFC 5929: if the certificate's signatureAlgorithm uses a single hash
+      // function, and that hash function is either MD5 [RFC1321] or SHA-1
+      // [RFC3174], then use SHA-256 [FIPS-180-3];
+      .put("MD5", "SHA-256")
+      .put("SHA1", "SHA-256")
+      // For other algorithms, use the provided hash function.
+      .put("SHA224", "SHA-224")
+      .put("SHA256", "SHA-256")
+      .put("SHA384", "SHA-384")
+      .put("SHA512", "SHA-512")
+      // The above list is exhaustive as of JDK8's implementation of
+      // SignatureAndHashAlgorithm.
+      .build();
+
+  /**
    * Return the Subject associated with the current thread's AccessController,
    * if that subject has Kerberos credentials. If there is no such subject, or
    * the subject has no Kerberos credentials, logins in a new subject from the
@@ -110,6 +134,39 @@ public abstract class SecurityUtil {
   }
 
   /**
+   * Compute the "tls-server-endpoint" channel binding data for the given X509
+   * certificate. The algorithm is specified in RFC 5929.
+   *
+   * @return the expected channel bindings for the certificate
+   * @throws RuntimeException if the certificate is not an X509 cert, or if
+   * it uses a signature type for which we cannot compute channel bindings
+   */
+  public static byte[] getEndpointChannelBindings(Certificate cert) {
+    Preconditions.checkArgument(cert instanceof X509Certificate,
+        "can only handle X509 certs");
+    X509Certificate x509 = (X509Certificate)cert;
+    String sigAlg = x509.getSigAlgName();
+
+    // The signature algorithm name is a string like 'SHA256withRSA'.
+    // There's no API available to actually find just the digest algorithm,
+    // so we resort to some hackery.
+    String[] components = sigAlg.split("with");
+    String digestAlg = CERT_DIGEST_TO_MESSAGE_DIGEST.get(components[0].toUpperCase());
+    if (digestAlg == null) {
+      // RFC 5929: if the certificate's signatureAlgorithm uses no hash functions or
+      // uses multiple hash functions, then this channel binding type's channel
+      // bindings are undefined at this time (updates to is channel binding type may
+      // occur to address this issue if it ever arises).
+      throw new RuntimeException("cert uses unknown signature algorithm: " + sigAlg);
+    }
+    try {
+      return MessageDigest.getInstance(digestAlg).digest(cert.getEncoded());
+    } catch (Exception e) {
+      throw Throwables.propagate(e);
+    }
+  }
+
+  /**
    * TrustManager implementation which will trust any certificate.
    * TODO(PKI): this needs to change so that it can be configured with
    * the cluster's CA cert.

http://git-wip-us.apache.org/repos/asf/kudu/blob/41dd9d41/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
index 616937e..880ab5b 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
@@ -22,8 +22,12 @@ import static org.junit.Assert.*;
 import java.nio.ByteBuffer;
 import java.security.AccessControlContext;
 import java.security.AccessController;
+import java.security.KeyStore;
+import java.security.cert.Certificate;
 import java.util.List;
 
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLEngineResult;
 import javax.net.ssl.SSLEngineResult.HandshakeStatus;
@@ -34,9 +38,9 @@ import org.apache.kudu.client.Negotiator.Result;
 import org.apache.kudu.rpc.RpcHeader.ConnectionContextPB;
 import org.apache.kudu.rpc.RpcHeader.NegotiatePB;
 import org.apache.kudu.rpc.RpcHeader.RpcFeatureFlag;
+import org.apache.kudu.util.SecurityUtil;
 import org.apache.kudu.rpc.RpcHeader.NegotiatePB.NegotiateStep;
 import org.apache.kudu.rpc.RpcHeader.NegotiatePB.SaslMechanism;
-import org.apache.kudu.util.SecurityUtil;
 import org.apache.kudu.rpc.RpcHeader.ResponseHeader;
 import org.jboss.netty.buffer.ChannelBuffer;
 import org.jboss.netty.handler.codec.embedder.DecoderEmbedder;
@@ -44,6 +48,7 @@ import org.jboss.netty.handler.ssl.SslHandler;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.google.common.base.Throwables;
 import com.google.common.collect.Lists;
 import com.google.protobuf.ByteString;
 import com.google.protobuf.Message;
@@ -52,6 +57,8 @@ public class TestNegotiator {
   private Negotiator negotiator;
   private DecoderEmbedder<Object> embedder;
 
+  private static final char[] password = "password".toCharArray();
+
   @Before
   public void setup() {
     AccessControlContext context = AccessController.getContext();
@@ -66,6 +73,33 @@ public class TestNegotiator {
     return new CallResponse(buf);
   }
 
+  KeyStore loadTestKeystore() throws Exception {
+    KeyStore ks = KeyStore.getInstance("JKS");
+    ks.load(TestNegotiator.class.getResourceAsStream("/test-key-and-cert.jks"),
+        password);
+    return ks;
+  }
+
+  SSLEngine createServerEngine() {
+    try {
+      KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
+      kmf.init(loadTestKeystore(), password);
+      SSLContext ctx = SSLContext.getInstance("TLS");
+      ctx.init(kmf.getKeyManagers(), null, null);
+      return ctx.createSSLEngine();
+    } catch (Exception e) {
+      throw Throwables.propagate(e);
+    }
+  }
+
+  @Test
+  public void testChannelBinding() throws Exception {
+    KeyStore ks = loadTestKeystore();
+    Certificate cert = ks.getCertificate("1");
+    byte[] bindings = SecurityUtil.getEndpointChannelBindings(cert);
+    assertEquals(32, bindings.length);
+  }
+
   /**
    * Simple test case for a PLAIN negotiation.
    */
@@ -162,14 +196,8 @@ public class TestNegotiator {
 
   @Test
   public void testTlsNegotiation() throws Exception {
-    SSLEngine serverEngine = SecurityUtil.createSslEngine();
+    SSLEngine serverEngine = createServerEngine();
     serverEngine.setUseClientMode(false);
-    // Enable only an anonymous cipher suite, so we don't need to deal with
-    // setting up certs in this test, for now.
-    serverEngine.setEnabledCipherSuites(new String[]{
-        "TLS_DH_anon_WITH_AES_128_CBC_SHA"
-    });
-
     negotiator.sendHello(embedder.getPipeline().getChannel());
 
     // Expect client->server: NEGOTIATE, TLS included.

http://git-wip-us.apache.org/repos/asf/kudu/blob/41dd9d41/java/kudu-client/src/test/resources/test-key-and-cert.jks
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/resources/test-key-and-cert.jks b/java/kudu-client/src/test/resources/test-key-and-cert.jks
new file mode 100644
index 0000000..45d885d
Binary files /dev/null and b/java/kudu-client/src/test/resources/test-key-and-cert.jks differ


Mime
View raw message