avoid circular (certificate) chains

src/main/java/org/jruby/ext/openssl/SSLContext.java

@@ -1373,29 +1373,30 @@ public java.security.cert.X509Certificate[] getCertificateChain(String alias) {
             }
 
             final ArrayList<java.security.cert.X509Certificate> chain = new ArrayList<>(8);
-            StoreContext storeCtx = internalContext.createStoreContext(null);
-            X509AuxCertificate x = credential.leaf;
-            while (true) {
-
-                chain.add(x);
-
-                if ( x.getIssuerDN().equals(x.getSubjectDN()) ) break;
-
-                try {
-                    final Name name = new Name(x.getIssuerX500Principal());
-                    X509Object[] s_obj = new X509Object[1];
-                    if (storeCtx.getBySubject(X509Utils.X509_LU_X509, name, s_obj) <= 0) {
+            chain.add(credential.leaf);
+            final StoreContext storeCtx = internalContext.createStoreContext(null);
+            if (storeCtx != null) {
+                X509AuxCertificate x = credential.leaf;
+                final int maxChain = Math.max(storeCtx.getParam().getDepth(), 0) + 2;
+                while (chain.size() < maxChain && !x.getIssuerDN().equals(x.getSubjectDN())) {
+                    try {
+                        final Name name = new Name(x.getIssuerX500Principal());
+                        X509Object[] s_obj = new X509Object[1];
+                        if (storeCtx.getBySubject(X509Utils.X509_LU_X509, name, s_obj) <= 0) {
+                            break;
+                        }
+                        x = ((Certificate) s_obj[0]).cert;
+                    }
+                    catch (RuntimeException e) {
+                        LOG.debugStack(e);
                         break;
                     }
-                    x = ((Certificate) s_obj[0]).cert;
-                }
-                catch (RuntimeException e) {
-                    LOG.debugStack(e);
-                    break;
-                }
-                catch (Exception e) {
-                    LOG.debug("KeyManagerImpl bySubject failed", e);
-                    break;
+                    catch (Exception e) {
+                        LOG.debug("KeyManagerImpl bySubject failed", e);
+                        break;
+                    }
+                    if (chain.contains(x)) break; // avoid circular chains
+                    chain.add(x);
                 }
             }
             return chain.toArray( new java.security.cert.X509Certificate[chain.size()] );

src/main/java/org/jruby/ext/openssl/SimpleSecretKey.java

@@ -60,7 +60,7 @@ public String getAlgorithm() {
     }
 
     public byte[] getEncoded() {
-        return value;
+        return value.clone(); // NOTE: providers (SunJCE) wipe the returned bytes after init
     }
 
     public String getFormat() {

src/main/java/org/jruby/ext/openssl/x509store/Certificate.java

@@ -54,8 +54,8 @@ public boolean isName(final Name name) {
     public boolean matches(final X509Object other) {
         if (other instanceof Certificate) {
             final Certificate that = (Certificate) other;
-            return this.cert.cert.hashCode() == that.cert.cert.hashCode()
-                && this.cert.cert.equals(that.cert.cert);
+            return this.cert.hashCode() == that.cert.hashCode()
+                && this.cert.equals(that.cert);
         }
         return false;
     }

src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java

@@ -64,6 +64,7 @@
 import org.bouncycastle.asn1.x509.Certificate;
 import org.bouncycastle.asn1.x509.SubjectKeyIdentifier;
 import org.jruby.ext.openssl.SecurityHelper;
+import org.jruby.ext.openssl.util.ExceptionUtil;
 
 /**
  * Since regular X509Certificate doesn't represent the Aux part of a
@@ -73,14 +74,12 @@
  * @author <a href="mailto:ola.bini@ki.se">Ola Bini</a>
  */
 public class X509AuxCertificate extends X509Certificate implements Cloneable {
-    private static final long serialVersionUID = -909543379295427515L;
 
     final X509Certificate cert;
+    final transient X509Aux aux;
 
-    final X509Aux aux;
-
-    boolean verified = false; // opt: avoids doing internal_verify twice
-    private int ex_flags = -1;
+    transient boolean verified; // opt: avoids doing internal_verify twice
+    private transient int ex_flags = -1;
 
     public X509AuxCertificate(Certificate wrap) throws IOException, CertificateException {
         super();
@@ -100,12 +99,13 @@
         this.aux = aux;
     }
 
-    public final X509AuxCertificate clone() {
+    @Override
+    protected final X509AuxCertificate clone() {
         try {
             return (X509AuxCertificate) super.clone();
         }
         catch (CloneNotSupportedException ex) {
-            throw new IllegalStateException(ex);
+            ExceptionUtil.throwException(ex); return null;
         }
     }
 
@@ -117,13 +117,7 @@
     }
 
     public int getExFlags() throws IOException {
-        if (ex_flags == -1) {
-            //try {
-                ex_flags = computeExFlags();
-            //} catch (IOException e) {
-            //    throw new IllegalStateException(e);
-            //}
-        }
+        if (ex_flags == -1) ex_flags = computeExFlags();
         return ex_flags;
     }
 
@@ -159,12 +153,12 @@
                     akid = AuthorityKeyIdentifier.getInstance(seq);
                 }
 
                 if (akid.getKeyIdentifier() != null) {
                     if (getExtensionValue("2.5.29.14") != null) {
                         DEROctetString der = (DEROctetString) X509Utils.get(getExtensionValue("2.5.29.14"));
                         SubjectKeyIdentifier skid = SubjectKeyIdentifier.getInstance(X509Utils.get(der.getOctets()));
                         if (skid.getKeyIdentifier() != null) {
                             if (Arrays.equals(akid.getKeyIdentifier(), skid.getKeyIdentifier())) { /* SKID matches AKID */
                                 /* .. and the signature alg matches the PUBKEY alg: */
                                 if (getSigAlgName().equals(getPublicKey().getAlgorithm())) {
                                     flags |= X509Utils.EXFLAG_SS; /* indicate self-signed */
@@ -290,17 +284,14 @@
     public boolean equals(Object other) {
         if ( this == other ) return true;
         if ( other instanceof X509AuxCertificate ) {
-            X509AuxCertificate o = (X509AuxCertificate) other;
-            return this.cert.equals(o.cert) && ((this.aux == null) ? o.aux == null : this.aux.equals(o.aux));
+            return this.cert.equals(((X509AuxCertificate) other).cert);
         }
-        return false;
+        return this.cert.equals(other);
     }
 
     @Override
     public int hashCode() {
-        int ret = cert.hashCode();
-        ret += 3 * (aux == null ? 1 : aux.hashCode());
-        return ret;
+        return 11 * cert.hashCode();
     }
 
     @Override

src/test/java/org/jruby/ext/openssl/SimpleSecretKeyTest.java

@@ -0,0 +1,45 @@
+package org.jruby.ext.openssl;
+
+import java.security.Security;
+import java.util.Arrays;
+
+import javax.crypto.Cipher;
+import javax.crypto.Mac;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+public class SimpleSecretKeyTest {
+
+    @Test
+    public void getEncodedReturnsCopy() {
+        final byte[] value = { 1, 2, 3, 4 };
+        final SimpleSecretKey key = new SimpleSecretKey("AES", value);
+        key.getEncoded()[0] = 0;
+        assertArrayEquals(new byte[] { 1, 2, 3, 4 }, key.getEncoded());
+        assertArrayEquals(new byte[] { 1, 2, 3, 4 }, value);
+    }
+
+    // SunJCE wipes the encoded key bytes after init
+    @Test
+    public void keyBytesSurviveSunJCEInit() throws Exception {
+        assumeTrue(Security.getProvider("SunJCE") != null);
+
+        final byte[] value = new byte[16]; Arrays.fill(value, (byte) 'k');
+
+        Cipher cipher = Cipher.getInstance("AES/ECB/NoPadding", "SunJCE");
+        cipher.init(Cipher.ENCRYPT_MODE, new SimpleSecretKey("AES", value));
+        final byte[] out1 = cipher.doFinal(new byte[16]);
+        cipher.init(Cipher.ENCRYPT_MODE, new SimpleSecretKey("AES", value));
+        assertArrayEquals(out1, cipher.doFinal(new byte[16]));
+
+        Mac mac = Mac.getInstance("HmacSHA256", "SunJCE");
+        mac.init(new SimpleSecretKey("HmacSHA256", value));
+
+        final byte[] expected = new byte[16]; Arrays.fill(expected, (byte) 'k');
+        assertArrayEquals(expected, value);
+    }
+
+}

src/test/java/org/jruby/ext/openssl/x509store/StoreContextTest.java

@@ -1,6 +1,11 @@
 
 package org.jruby.ext.openssl.x509store;
 
+import java.io.ByteArrayInputStream;
+import java.nio.charset.StandardCharsets;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509Certificate;
+import java.util.Collections;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.*;
@@ -22,4 +27,38 @@ public void getExtraDataRoundTrips() {
         assertEquals("app", ctx.getExtraData(0));
         assertNull(ctx.getExtraData(3)); // beyond populated range
     }
+
+    @Test
+    public void auxCertificateEqualityIgnoresAux() throws Exception {
+        final String pem = "-----BEGIN CERTIFICATE-----\n" +
+            "MIIC/jCCAmegAwIBAgIBATANBgkqhkiG9w0BAQUFADBNMQswCQYDVQQGEwJKUDER\n" +
+            "MA8GA1UECgwIY3Rvci5vcmcxFDASBgNVBAsMC0RldmVsb3BtZW50MRUwEwYDVQQD\n" +
+            "DAxodHRwLWFjY2VzczIwHhcNMDcwOTExMTM1ODMxWhcNMDkwOTEwMTM1ODMxWjBN\n" +
+            "MQswCQYDVQQGEwJKUDERMA8GA1UECgwIY3Rvci5vcmcxFDASBgNVBAsMC0RldmVs\n" +
+            "b3BtZW50MRUwEwYDVQQDDAxodHRwLWFjY2VzczIwgZ8wDQYJKoZIhvcNAQEBBQAD\n" +
+            "gY0AMIGJAoGBALi66ujWtUCQm5HpMSyr/AAIFYVXC/dmn7C8TR/HMiUuW3waY4uX\n" +
+            "LFqCDAGOX4gf177pX+b99t3mpaiAjJuqc858D9xEECzhDWgXdLbhRqWhUOble4RY\n" +
+            "c1yWYC990IgXJDMKx7VAuZ3cBhdBxtlE9sb1ZCzmHQsvTy/OoRzcJCrTAgMBAAGj\n" +
+            "ge0wgeowDwYDVR0TAQH/BAUwAwEB/zAxBglghkgBhvhCAQ0EJBYiUnVieS9PcGVu\n" +
+            "U1NMIEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNVHQ4EFgQUJNE0GGaRKmN2qhnO\n" +
+            "FyBWVl4Qj6owDgYDVR0PAQH/BAQDAgEGMHUGA1UdIwRuMGyAFCTRNBhmkSpjdqoZ\n" +
+            "zhcgVlZeEI+qoVGkTzBNMQswCQYDVQQGEwJKUDERMA8GA1UECgwIY3Rvci5vcmcx\n" +
+            "FDASBgNVBAsMC0RldmVsb3BtZW50MRUwEwYDVQQDDAxodHRwLWFjY2VzczKCAQEw\n" +
+            "DQYJKoZIhvcNAQEFBQADgYEAH11tstSUuqFpMqoh/vM5l3Nqb8ygblbqEYQs/iG/\n" +
+            "UeQkOZk/P1TxB6Ozn2htJ1srqDpUsncFVZ/ecP19GkeOZ6BmIhppcHhE5WyLBcPX\n" +
+            "It5q1BW0PiAzT9LlEGoaiW0nw39so0Pr1whJDfc1t4fjdk+kSiMIzRHbTDvHWfpV\n" +
+            "nTA=\n" +
+            "-----END CERTIFICATE-----\n";
+        final X509Certificate x509 = (X509Certificate) CertificateFactory.getInstance("X.509")
+            .generateCertificate(new ByteArrayInputStream(pem.getBytes(StandardCharsets.US_ASCII)));
+        final X509AuxCertificate plain = new X509AuxCertificate(x509);
+        final X509Aux aux = new X509Aux("alias", null, Collections.singletonList("1.3.6.1.5.5.7.3.1"), null, null);
+        final X509AuxCertificate withAux = new X509AuxCertificate(x509, aux);
+
+        assertEquals(plain, withAux);
+        assertEquals(withAux, plain);
+        assertEquals(plain.hashCode(), withAux.hashCode());
+        assertEquals(plain, plain.cert);
+        assertTrue(new Certificate(plain).matches(new Certificate(withAux)));
+    }
 }

test/ssl/test_ssl.rb

@@ -1585,6 +1585,43 @@ def test_extra_chain_cert_sends_leaf
     end
   end
 
+  def test_auto_chain_with_circular_cert_store
+    now = Time.now
+    ca_exts = [["basicConstraints", "CA:TRUE", true], ["keyUsage", "cRLSign,keyCertSign", true]]
+    key_a = OpenSSL::PKey::RSA.new(2048)
+    key_b = OpenSSL::PKey::RSA.new(2048)
+    dn_a = OpenSSL::X509::Name.parse("/CN=Cross-CA-A")
+    dn_b = OpenSSL::X509::Name.parse("/CN=Cross-CA-B")
+
+    # Cross-signed cycle: A issued by B, B issued by A
+    stub_b = OpenSSL::X509::Certificate.new
+    stub_b.subject = dn_b
+    cert_a = issue_cert(dn_a, key_a, 20, ca_exts, stub_b, key_b, not_before: now - 60, not_after: now + 3600)
+    cert_b = issue_cert(dn_b, key_b, 21, ca_exts, cert_a, key_a, not_before: now - 60, not_after: now + 3600)
+
+    store = OpenSSL::X509::Store.new
+    store.add_cert(cert_a)
+    store.add_cert(cert_b)
+
+    ctx_proc = Proc.new do |ctx|
+      ctx.cert_store = store
+      ctx.cert = cert_a
+      ctx.key = key_a
+    end
+
+    start_server(OpenSSL::SSL::VERIFY_NONE, true, ctx_proc: ctx_proc) do |server, port|
+      cctx = OpenSSL::SSL::SSLContext.new
+      cctx.verify_mode = OpenSSL::SSL::VERIFY_NONE
+      server_connect(port, cctx) do |ssl|
+        chain = ssl.peer_cert_chain
+        assert_equal ["/CN=Cross-CA-A", "/CN=Cross-CA-B"], chain.first(2).map { |c| c.subject.to_s }
+        assert_equal 2, chain.size if jruby?
+        ssl.puts "hello"
+        assert_equal "hello\n", ssl.gets
+      end
+    end
+  end
+
   def test_sysread_syswrite_raise_before_handshake
     require 'socket'
     server = TCPServer.new("127.0.0.1", 0)