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@6d6d579c88

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<char[]> passwordSupplier) throws IOException {
+    public static PrivateKey readPrivateKey(Reader reader, Supplier<char[]> 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<char[]> 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
      */

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<SSLSocket>() {
+              @Override
+              public SSLSocket run() throws Exception {
+                  return (SSLSocket) socketFactory.createSocket(address.address().getAddress(), address.address().getPort());
+              }})) {
+            final AtomicReference<HandshakeCompletedEvent> 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"));
+        }
+    }
+}

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());
+    }
 }

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-----

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-----

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-----