diff --git a/src/main/java/org/jruby/ext/openssl/SSLContext.java b/src/main/java/org/jruby/ext/openssl/SSLContext.java index a9e34625..9f32bc45 100644 --- a/src/main/java/org/jruby/ext/openssl/SSLContext.java +++ b/src/main/java/org/jruby/ext/openssl/SSLContext.java @@ -1373,29 +1373,30 @@ public java.security.cert.X509Certificate[] getCertificateChain(String alias) { } final ArrayList 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()] ); diff --git a/src/main/java/org/jruby/ext/openssl/SimpleSecretKey.java b/src/main/java/org/jruby/ext/openssl/SimpleSecretKey.java index 0c5743ea..1557df85 100644 --- a/src/main/java/org/jruby/ext/openssl/SimpleSecretKey.java +++ b/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() { diff --git a/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java b/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java index b132bef5..94ff8a7d 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java +++ b/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; } diff --git a/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java b/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java index 1ed7e1aa..e0a2c829 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java +++ b/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 Ola Bini */ 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 @@ public X509AuxCertificate(X509Certificate wrap) { 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 @@ final X509AuxCertificate cloneForCache() { } 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; } @@ -290,17 +284,14 @@ public byte[] getEncoded() throws CertificateEncodingException { 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 diff --git a/src/test/java/org/jruby/ext/openssl/SimpleSecretKeyTest.java b/src/test/java/org/jruby/ext/openssl/SimpleSecretKeyTest.java new file mode 100644 index 00000000..bb23cbf4 --- /dev/null +++ b/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); + } + +} diff --git a/src/test/java/org/jruby/ext/openssl/x509store/StoreContextTest.java b/src/test/java/org/jruby/ext/openssl/x509store/StoreContextTest.java index 46e07419..84f27c92 100644 --- a/src/test/java/org/jruby/ext/openssl/x509store/StoreContextTest.java +++ b/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))); + } } diff --git a/test/ssl/test_ssl.rb b/test/ssl/test_ssl.rb index face51bc..9887b5a0 100644 --- a/test/ssl/test_ssl.rb +++ b/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)