From 2c2261881d243ebdb59bd9df28b1ccf15e43a013 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Mon, 24 Apr 2017 11:31:10 -0400 Subject: [PATCH] Fix support for elliptic curve certificates in PEM files (elastic/x-pack-elasticsearch#1050) This commit fixes the support for elliptic curve certificates that are specified as a PEM file. These certificates and private keys can now be read properly and a integration test was added to ensure that TLS also functions correctly with these certificates. Original commit: elastic/x-pack-elasticsearch@6d6d579c88b6abc7f660b603d53db3bb0652e1c7 --- .../elasticsearch/xpack/ssl/CertUtils.java | 61 +++++----- .../transport/ssl/EllipticCurveSSLTests.java | 108 ++++++++++++++++++ .../xpack/ssl/CertUtilsTests.java | 35 ++++++ .../ssl/certs/simple/prime256v1-cert.pem | 15 +++ .../certs/simple/prime256v1-key-noparam.pem | 5 + .../ssl/certs/simple/prime256v1-key.pem | 8 ++ 6 files changed, 206 insertions(+), 26 deletions(-) create mode 100644 plugin/src/test/java/org/elasticsearch/xpack/security/transport/ssl/EllipticCurveSSLTests.java create mode 100644 plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem create mode 100644 plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key-noparam.pem create mode 100644 plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertUtils.java b/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertUtils.java index 498cc17b470..6726d6b889a 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertUtils.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ssl/CertUtils.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.ssl; +import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers; import org.bouncycastle.asn1.pkcs.PrivateKeyInfo; import org.bouncycastle.asn1.x500.X500Name; @@ -14,7 +15,6 @@ import org.bouncycastle.asn1.x509.Extension; import org.bouncycastle.asn1.x509.ExtensionsGenerator; import org.bouncycastle.asn1.x509.GeneralName; import org.bouncycastle.asn1.x509.GeneralNames; -import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo; import org.bouncycastle.asn1.x509.Time; import org.bouncycastle.cert.CertIOException; import org.bouncycastle.cert.X509CertificateHolder; @@ -66,7 +66,6 @@ import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; -import java.security.PublicKey; import java.security.SecureRandom; import java.security.UnrecoverableKeyException; import java.security.cert.Certificate; @@ -106,7 +105,7 @@ public class CertUtils { /** * Returns a {@link X509ExtendedKeyManager} that is built from the provided private key and certificate chain */ - static X509ExtendedKeyManager keyManager(Certificate[] certificateChain, PrivateKey privateKey, char[] password) + public static X509ExtendedKeyManager keyManager(Certificate[] certificateChain, PrivateKey privateKey, char[] password) throws NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException, IOException, CertificateException { KeyStore keyStore = KeyStore.getInstance("jks"); keyStore.load(null, null); @@ -234,38 +233,48 @@ public class CertUtils { /** * Reads the private key from the reader and optionally uses the password supplier to retrieve a password if the key is encrypted */ - static PrivateKey readPrivateKey(Reader reader, Supplier passwordSupplier) throws IOException { + public static PrivateKey readPrivateKey(Reader reader, Supplier passwordSupplier) throws IOException { try (PEMParser parser = new PEMParser(reader)) { - final Object parsed = parser.readObject(); + PrivateKeyInfo privateKeyInfo = innerReadPrivateKey(parser, passwordSupplier); if (parser.readObject() != null) { throw new IllegalStateException("key file contained more that one entry"); } - - PrivateKeyInfo privateKeyInfo; - if (parsed instanceof PEMEncryptedKeyPair) { - char[] keyPassword = passwordSupplier.get(); - if (keyPassword == null) { - throw new IllegalArgumentException("cannot read encrypted key without a password"); - } - // we have an encrypted key pair so we need to decrypt it - PEMEncryptedKeyPair encryptedKeyPair = (PEMEncryptedKeyPair) parsed; - privateKeyInfo = encryptedKeyPair - .decryptKeyPair(new JcePEMDecryptorProviderBuilder().setProvider(BC_PROV).build(keyPassword)) - .getPrivateKeyInfo(); - } else if (parsed instanceof PEMKeyPair) { - privateKeyInfo = ((PEMKeyPair) parsed).getPrivateKeyInfo(); - } else if (parsed instanceof PrivateKeyInfo) { - privateKeyInfo = (PrivateKeyInfo) parsed; - } else { - throw new IllegalArgumentException("parsed an unsupported object [" + - parsed.getClass().getSimpleName() + "]"); - } - JcaPEMKeyConverter converter = new JcaPEMKeyConverter(); + converter.setProvider(BC_PROV); return converter.getPrivateKey(privateKeyInfo); } } + private static PrivateKeyInfo innerReadPrivateKey(PEMParser parser, Supplier passwordSupplier) throws IOException { + final Object parsed = parser.readObject(); + if (parsed == null) { + throw new IllegalStateException("key file did not contain a supported key"); + } + + PrivateKeyInfo privateKeyInfo; + if (parsed instanceof PEMEncryptedKeyPair) { + char[] keyPassword = passwordSupplier.get(); + if (keyPassword == null) { + throw new IllegalArgumentException("cannot read encrypted key without a password"); + } + // we have an encrypted key pair so we need to decrypt it + PEMEncryptedKeyPair encryptedKeyPair = (PEMEncryptedKeyPair) parsed; + privateKeyInfo = encryptedKeyPair + .decryptKeyPair(new JcePEMDecryptorProviderBuilder().setProvider(BC_PROV).build(keyPassword)) + .getPrivateKeyInfo(); + } else if (parsed instanceof PEMKeyPair) { + privateKeyInfo = ((PEMKeyPair) parsed).getPrivateKeyInfo(); + } else if (parsed instanceof PrivateKeyInfo) { + privateKeyInfo = (PrivateKeyInfo) parsed; + } else if (parsed instanceof ASN1ObjectIdentifier) { + // skip this object and recurse into this method again to read the next object + return innerReadPrivateKey(parser, passwordSupplier); + } else { + throw new IllegalArgumentException("parsed an unsupported object [" + parsed.getClass().getSimpleName() + "]"); + } + + return privateKeyInfo; + } /** * Generates a CA certificate */ diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/transport/ssl/EllipticCurveSSLTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/transport/ssl/EllipticCurveSSLTests.java new file mode 100644 index 00000000000..4a1d562fc42 --- /dev/null +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/transport/ssl/EllipticCurveSSLTests.java @@ -0,0 +1,108 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.transport.ssl; + +import com.unboundid.util.ssl.TrustAllTrustManager; +import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.test.SecurityIntegTestCase; +import org.elasticsearch.xpack.ssl.CertUtils; + +import javax.net.ssl.HandshakeCompletedEvent; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509ExtendedKeyManager; +import java.io.Reader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.AccessController; +import java.security.PrivateKey; +import java.security.PrivilegedExceptionAction; +import java.security.SecureRandom; +import java.security.cert.Certificate; +import java.util.Collections; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import static org.hamcrest.Matchers.containsString; + +public class EllipticCurveSSLTests extends SecurityIntegTestCase { + + @Override + protected Settings nodeSettings(int nodeOrdinal) { + final Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem"); + final Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem"); + return Settings.builder() + .put(super.nodeSettings(nodeOrdinal).filter(s -> s.startsWith("xpack.ssl") == false)) + .put("xpack.ssl.key", keyPath) + .put("xpack.ssl.certificate", certPath) + .put("xpack.ssl.certificate_authorities", certPath) + .put("xpack.ssl.verification_mode", "certificate") // disable hostname verificate since these certs aren't setup for that + .build(); + } + + @Override + protected Settings transportClientSettings() { + final Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem"); + final Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem"); + return Settings.builder() + .put(super.transportClientSettings().filter(s -> s.startsWith("xpack.ssl") == false)) + .put("xpack.ssl.key", keyPath) + .put("xpack.ssl.certificate", certPath) + .put("xpack.ssl.certificate_authorities", certPath) + .put("xpack.ssl.verification_mode", "certificate") // disable hostname verification since these certs aren't setup for that + .build(); + } + + @Override + protected boolean useGeneratedSSLConfig() { + return false; + } + + public void testConnection() throws Exception { + final Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem"); + final Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem"); + PrivateKey privateKey; + try (Reader reader = Files.newBufferedReader(keyPath)) { + privateKey = CertUtils.readPrivateKey(reader, () -> null); + } + Certificate[] certs = CertUtils.readCertificates(Collections.singletonList(certPath.toString()), null); + X509ExtendedKeyManager x509ExtendedKeyManager = CertUtils.keyManager(certs, privateKey, new char[0]); + SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(new X509ExtendedKeyManager[] { x509ExtendedKeyManager }, + new TrustManager[] { new TrustAllTrustManager(false) }, new SecureRandom()); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + NodesInfoResponse response = client().admin().cluster().prepareNodesInfo().setTransport(true).get(); + TransportAddress address = randomFrom(response.getNodes()).getTransport().getAddress().publishAddress(); + + final CountDownLatch latch = new CountDownLatch(1); + try (SSLSocket sslSocket = AccessController.doPrivileged(new PrivilegedExceptionAction() { + @Override + public SSLSocket run() throws Exception { + return (SSLSocket) socketFactory.createSocket(address.address().getAddress(), address.address().getPort()); + }})) { + final AtomicReference reference = new AtomicReference<>(); + sslSocket.addHandshakeCompletedListener((event) -> { + reference.set(event); + latch.countDown(); + }); + sslSocket.startHandshake(); + latch.await(); + + HandshakeCompletedEvent event = reference.get(); + assertNotNull(event); + SSLSession session = event.getSession(); + Certificate[] peerChain = session.getPeerCertificates(); + assertEquals(1, peerChain.length); + assertEquals(certs[0], peerChain[0]); + assertThat(session.getCipherSuite(), containsString("ECDSA")); + } + } +} diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertUtilsTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertUtilsTests.java index c314da82d60..55c864ab4ec 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertUtilsTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ssl/CertUtilsTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.ssl; import org.bouncycastle.asn1.x509.GeneralName; import org.bouncycastle.asn1.x509.GeneralNames; +import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec; +import org.bouncycastle.jce.spec.ECNamedCurveSpec; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.network.NetworkAddress; @@ -26,10 +28,13 @@ import java.security.PrivateKey; import java.security.cert.Certificate; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import java.security.interfaces.ECPrivateKey; +import java.security.interfaces.ECPublicKey; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -151,4 +156,34 @@ public class CertUtilsTests extends ESTestCase { verify(address).isAnyLocalAddress(); verifyNoMoreInteractions(address); } + + public void testReadEllipticCurveCertificateAndKey() throws Exception { + Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem"); + try (Reader reader = Files.newBufferedReader(keyPath)) { + verifyPrime256v1ECKey(reader); + } + + Path keyNoSpecPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key-noparam.pem"); + try (Reader reader = Files.newBufferedReader(keyNoSpecPath)) { + verifyPrime256v1ECKey(reader); + } + + Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem"); + Certificate[] certs = CertUtils.readCertificates(Collections.singletonList(certPath.toString()), null); + assertEquals(1, certs.length); + Certificate cert = certs[0]; + assertNotNull(cert); + assertEquals("EC", cert.getPublicKey().getAlgorithm()); + } + + private void verifyPrime256v1ECKey(Reader reader) throws Exception { + PrivateKey privateKey = CertUtils.readPrivateKey(reader, () -> null); + assertNotNull(privateKey); + assertEquals("ECDSA", privateKey.getAlgorithm()); + assertThat(privateKey, instanceOf(ECPrivateKey.class)); + ECPrivateKey ecPrivateKey = (ECPrivateKey) privateKey; + assertThat(ecPrivateKey.getParams(), instanceOf(ECNamedCurveSpec.class)); + ECNamedCurveSpec namedCurveSpec = (ECNamedCurveSpec) ecPrivateKey.getParams(); + assertEquals("prime256v1", namedCurveSpec.getName()); + } } diff --git a/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem b/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem new file mode 100644 index 00000000000..b85edb30b02 --- /dev/null +++ b/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-cert.pem @@ -0,0 +1,15 @@ +-----BEGIN CERTIFICATE----- +MIICYjCCAgmgAwIBAgIJAPBTfsMrh6VTMAkGByqGSM49BAEwWDELMAkGA1UEBhMC +VVMxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdp +dHMgUHR5IEx0ZDERMA8GA1UEAxMISmF5cy1NQlAwHhcNMTcwNDExMTM1MDQ3WhcN +MTkwNDExMTM1MDQ3WjBYMQswCQYDVQQGEwJVUzETMBEGA1UECBMKU29tZS1TdGF0 +ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMREwDwYDVQQDEwhK +YXlzLU1CUDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABMMRTMV1HqtZPH7dwtWI +q/kLw8cYV8vK7bN7Mi09q9JQogbvwRkVir8b1/3DgUEvLv+8u8zgcIcx2iaWtaLz +rfmjgbwwgbkwHQYDVR0OBBYEFGgv2RGxkfH8fDYUMiAcLgSZ2el2MIGJBgNVHSME +gYEwf4AUaC/ZEbGR8fx8NhQyIBwuBJnZ6XahXKRaMFgxCzAJBgNVBAYTAlVTMRMw +EQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0 +eSBMdGQxETAPBgNVBAMTCEpheXMtTUJQggkA8FN+wyuHpVMwDAYDVR0TBAUwAwEB +/zAJBgcqhkjOPQQBA0gAMEUCIBI2zkYo8aZImnlXxIS+7cILdx8AKo6VNvGykn3X +k/n1AiEAp5O/xswzb35GZbAnNCbXDYi2Ny2mv1S9WypHC6Y5/qk= +-----END CERTIFICATE----- diff --git a/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key-noparam.pem b/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key-noparam.pem new file mode 100644 index 00000000000..2cb86113d41 --- /dev/null +++ b/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key-noparam.pem @@ -0,0 +1,5 @@ +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIN74E4fO1Pq89hh7NYjUBFu7akoHC36ZvlnHfcCASq5ToAoGCCqGSM49 +AwEHoUQDQgAEwxFMxXUeq1k8ft3C1Yir+QvDxxhXy8rts3syLT2r0lCiBu/BGRWK +vxvX/cOBQS8u/7y7zOBwhzHaJpa1ovOt+Q== +-----END EC PRIVATE KEY----- diff --git a/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem b/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem new file mode 100644 index 00000000000..51a3f018558 --- /dev/null +++ b/plugin/src/test/resources/org/elasticsearch/xpack/security/transport/ssl/certs/simple/prime256v1-key.pem @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIN74E4fO1Pq89hh7NYjUBFu7akoHC36ZvlnHfcCASq5ToAoGCCqGSM49 +AwEHoUQDQgAEwxFMxXUeq1k8ft3C1Yir+QvDxxhXy8rts3syLT2r0lCiBu/BGRWK +vxvX/cOBQS8u/7y7zOBwhzHaJpa1ovOt+Q== +-----END EC PRIVATE KEY-----