[fix] retry matching chain from trust-store and skip expired intermediate certs early

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

@@ -37,14 +37,18 @@
 import java.util.Collection;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Set;
 
 import org.bouncycastle.asn1.ASN1InputStream;
 import org.bouncycastle.asn1.ASN1Integer;
 import org.bouncycastle.asn1.ASN1Sequence;
 import org.jruby.ext.openssl.SecurityHelper;
 
+import static org.jruby.ext.openssl.x509store.X509Utils.*;
+
 /**
  * c: X509_STORE_CTX
  *
@@ -134,7 +138,7 @@ public Object getApplicationData() {
     /**
      * c: X509_STORE_CTX_get1_issuer
      */
-    int getFirstIssuer(final X509AuxCertificate[] issuers, final X509AuxCertificate x) throws Exception {
+    int getFirstIssuer(final X509AuxCertificate[] _issuer, final X509AuxCertificate x) throws Exception {
         final Name xn = new Name( x.getIssuerX500Principal() );
         final X509Object[] s_obj = new X509Object[1];
         int ok = store == null ? 0 : getBySubject(X509Utils.X509_LU_X509, xn, s_obj);
@@ -148,33 +152,107 @@ else if ( ok != X509Utils.X509_LU_FAIL ) {
             }
             return 0;
         }
-
+        /* If certificate matches all OK */
         X509Object obj = s_obj[0];
         if ( checkIssued.call(this, x, ((Certificate) obj).cert) != 0 ) {
-            issuers[0] = ((Certificate) obj).cert;
-            return 1;
+            X509AuxCertificate issuer = ((Certificate) obj).cert;
+            if (x509_check_cert_time(issuer, -1)) {
+                _issuer[0] = issuer;
+                return 1;
+            }
         }
 
         List<X509Object> objects = store.getObjects();
         int idx = X509Object.indexBySubject(objects, X509Utils.X509_LU_X509, xn);
         if ( idx == -1 ) return 0;
 
+        int ret = 0;
         /* Look through all matching certificates for a suitable issuer */
         for ( int i = idx; i < objects.size(); i++ ) {
             final X509Object pobj = objects.get(i);
             if ( pobj.type() != X509Utils.X509_LU_X509 ) {
-                return 0;
+                break; // return 0
             }
             final X509AuxCertificate x509 = ((Certificate) pobj).cert;
             if ( ! xn.equalTo( x509.getSubjectX500Principal() ) ) {
-                return 0;
+                break; // return 0
             }
             if ( checkIssued.call(this, x, x509) != 0 ) {
-                issuers[0] = x509;
-                return 1;
+                _issuer[0] = x509;
+                ret = 1;
+                /*
+                 * If times check, exit with match, otherwise keep looking.
+                 * Leave last match in issuer so we return nearest
+                 * match if no certificate time is OK.
+                 */
+                if (x509_check_cert_time(x509, -1)) break;
             }
         }
-        return 0;
+        return ret;
+    }
+
+    // NOTE: JOSSL specific - semi alternate chain search (drop once OpenSSL 1.1.1 verify ported)
+    private int getValidIssuersOrFirst(final X509AuxCertificate x, final List<X509AuxCertificate> _issuers)
+        throws Exception {
+        final Name xn = new Name( x.getIssuerX500Principal() );
+        final X509Object[] s_obj = new X509Object[1];
+        int ok = store == null ? 0 : getBySubject(X509Utils.X509_LU_X509, xn, s_obj);
+        if ( ok != X509Utils.X509_LU_X509 ) {
+            if ( ok == X509Utils.X509_LU_RETRY ) {
+                X509Error.addError(X509Utils.X509_R_SHOULD_RETRY);
+                return -1;
+            }
+            else if ( ok != X509Utils.X509_LU_FAIL ) {
+                return -1;
+            }
+            return 0;
+        }
+        int ret = 0;
+        /* If certificate matches all OK */
+        X509Object obj = s_obj[0];
+        if ( checkIssued.call(this, x, ((Certificate) obj).cert) != 0 ) {
+            X509AuxCertificate issuer = ((Certificate) obj).cert;
+            if (x509_check_cert_time(issuer, -1)) {
+                _issuers.add(issuer);
+                ret = 1;
+            }
+        }
+
+        List<X509Object> objects = store.getObjects();
+
+        int idx = X509Object.indexBySubject(objects, X509Utils.X509_LU_X509, xn);
+        if ( idx == -1 ) return ret;
+
+        // Leave last match in issuer so we return nearest match if no certificate time is OK.
+        // NOTE: due store.error reporting compatibility (with plain-old getFirstIssuer)
+        X509AuxCertificate firstTimeCheckFailed = null;
+
+        /* Look through all matching certificates for a suitable issuer */
+        for ( int i = idx; i < objects.size(); i++ ) {
+            final X509Object pobj = objects.get(i);
+            if ( pobj.type() != X509Utils.X509_LU_X509 ) {
+                continue;
+            }
+            final X509AuxCertificate x509 = ((Certificate) pobj).cert;
+            if ( ! xn.equalTo( x509.getSubjectX500Principal() ) ) {
+                continue;
+            }
+
+            if ( checkIssued.call(this, x, x509) != 0 ) {
+                if (x509_check_cert_time(x509, -1)) {
+                    _issuers.add(x509);
+                    ret = 1;
+                } else {
+                    if (firstTimeCheckFailed == null) firstTimeCheckFailed = x509;
+                    ret = 1;
+                }
+            }
+        }
+
+        if (firstTimeCheckFailed != null && _issuers.isEmpty()) {
+            _issuers.add(firstTimeCheckFailed);
+        }
+        return ret;
     }
 
     public static List<X509AuxCertificate> ensureAux(final Collection<X509Certificate> input) {
@@ -658,10 +736,8 @@ public int verifyCertificate() throws Exception {
 
         // We use a temporary STACK so we can chop and hack at it
 
-        List<X509AuxCertificate> sktmp = null;
-        if ( untrusted != null ) {
-            sktmp = new ArrayList<X509AuxCertificate>(untrusted);
-        }
+        LinkedList<X509AuxCertificate> sktmp = untrusted != null ? new LinkedList<>(untrusted) : null;
+
         num = chain.size();
         x = chain.get(num - 1);
         depth = verifyParameter.depth;
@@ -671,7 +747,7 @@ public int verifyCertificate() throws Exception {
             if ( checkIssued.call(this, x, x) != 0 ) break;
 
             if ( sktmp != null ) {
-                xtmp = findIssuer(sktmp, x);
+                xtmp = findIssuer(sktmp, x, true);
                 if ( xtmp != null ) {
                     chain.add(xtmp);
                     sktmp.remove(xtmp);
@@ -725,23 +801,42 @@ public int verifyCertificate() throws Exception {
             }
         }
         // We now lookup certs from the certificate store
-        for(;;) {
-            // If we have enough, we break
-            if ( depth < num ) break;
-            //xn = new X509_NAME(x.getIssuerX500Principal());
-            // If we are self signed, we break
-            if ( checkIssued.call(this, x, x) != 0 ) break;
 
-            X509AuxCertificate[] p_xtmp = new X509AuxCertificate[]{ xtmp };
-            int ok = getIssuer.call(this, p_xtmp, x);
-            xtmp = p_xtmp[0];
-
-            if ( ok < 0 ) return ok;
-            if ( ok == 0 ) break;
+        // NOTE: JOSSL specific - semi alternate chain search (drop once OpenSSL 1.1.1 verify ported)
+        if ( checkIssued.call(this, x, x) == 0 ) {
+            final int[] p_num = new int[] { num };
+            if (getIssuer == getFirstIssuer) {
+                ArrayList<X509AuxCertificate> xtmps = new ArrayList<>(4);
+                int ok = getValidIssuersOrFirst(x, xtmps);
+                if ( ok < 0 ) return ok; // error
+                if ( ok != 0 ) { // at least one issuer for given name
+                    for (int t = 0; t < xtmps.size(); t++) {
+                        chain.add(x = xtmps.get(t));
+                        p_num[0] = num + 1;
+
+                        ok = finishChain(x, depth, p_num);
+                        if ( ok < 0 ) return ok; // error
+                        if ( ok == 1 ) {
+                            x = chain.get(chain.size() - 1);
+                            if (x509_check_cert_time(x, -1)) break;
+                        }
+
+                        if (t < xtmps.size() - 1) {
+                            // did not break - going to retry thus reset chain back :
+                            int added = p_num[0] - num;
+                            while (added-- > 0) chain.remove(chain.size() - 1);
+                        }
+                    }
+                }
+            } else {
+                int ok = finishChain(x, depth, p_num);
+                if ( ok < 0 ) return ok; // error
+            }
 
-            x = xtmp;
-            chain.add(x);
-            num++;
+            if (p_num[0] != num) {
+                x = chain.get(chain.size() - 1);
+            }
+            num = p_num[0];
         }
 
         /* we now have our chain, lets check it... */
@@ -801,6 +896,40 @@ public int verifyCertificate() throws Exception {
         return ok;
     }
 
+    // NOTE: JOSSL specific - semi alternate chain search (drop once OpenSSL 1.1.1 verify ported)
+    private int finishChain(X509AuxCertificate x, final int depth, final int[] _num)
+        throws Exception {
+        X509AuxCertificate[] p_xtmp = new X509AuxCertificate[] { null };
+        int num = _num[0];
+        for(;;) {
+            // If we have enough, we break
+            if ( depth < num ) break;
+            // If we are self signed, we break
+            if ( checkIssued.call(this, x, x) != 0 ) break;
+
+            int ok = getIssuer.call(this, p_xtmp, x);
+            if ( ok < 0 ) return ok; // error
+            if ( ok == 0 ) return 0; // no issuer for given name
+
+            x = p_xtmp[0];
+
+            chain.add(x);
+            num++;
+        }
+
+        _num[0] = num;
+        return 1;
+    }
+
+    public X509AuxCertificate findIssuer(final List<X509AuxCertificate> certs,
+        final X509AuxCertificate cert, final boolean check_time) throws Exception {
+        for ( X509AuxCertificate issuer : certs ) {
+            if ( checkIssued.call(this, cert, issuer) != 0 ) {
+                if (!check_time || x509_check_cert_time(issuer, -1)) return issuer;
+            }
+        }
+        return null;
+    }
 
     private final static Set<String> CRITICAL_EXTENSIONS = new HashSet<String>(8);
     static {
@@ -960,32 +1089,46 @@ public int checkTrust() throws Exception {
         return verifyCallback.call(this, ZERO);
     }
 
-    /**
-     * c: check_cert_time
+    /*-
+     * Check certificate validity times.
+     * If depth >= 0, invoke verification callbacks on error, otherwise just return
+     * the validation status.
+     *
+     * Return 1 on success, 0 otherwise.
      */
-    public int checkCertificateTime(X509AuxCertificate x) throws Exception {
+    boolean x509_check_cert_time(X509AuxCertificate x, final int depth) throws Exception {
         final Date pTime;
-        if ( (verifyParameter.flags & X509Utils.V_FLAG_USE_CHECK_TIME) != 0 ) {
-            pTime = this.verifyParameter.checkTime;
+        if ((getParam().flags & V_FLAG_USE_CHECK_TIME) != 0) {
+            pTime = getParam().checkTime;
+        } else if ((getParam().flags & V_FLAG_NO_CHECK_TIME) != 0) {
+            return true;
         } else {
             pTime = Calendar.getInstance().getTime();
         }
 
-        if ( ! x.getNotBefore().before(pTime) ) {
-            error = X509Utils.V_ERR_CERT_NOT_YET_VALID;
-            currentCertificate = x;
-            if ( verifyCallback.call(this, ZERO) == 0 ) {
-                return 0;
-            }
+        int i = x.getNotBefore().compareTo(pTime);
+        if (i >= 0 && depth < 0) {
+            return false;
         }
-        if ( ! x.getNotAfter().after(pTime) ) {
-            error = X509Utils.V_ERR_CERT_HAS_EXPIRED;
-            currentCertificate = x;
-            if ( verifyCallback.call(this, ZERO) == 0 ) {
-                return 0;
-            }
+        if (i == 0 && verify_cb_cert(x, depth, V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD) == 0) {
+            return false;
         }
-        return 1;
+        if (i > 0 && verify_cb_cert(x, depth, V_ERR_CERT_NOT_YET_VALID) == 0) {
+            return false;
+        }
+
+        i = x.getNotAfter().compareTo(pTime);
+        if (i <= 0 && depth < 0) {
+            return false;
+        }
+        if (i == 0 && verify_cb_cert(x, depth, V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD) == 0) {
+            return false;
+        }
+        if (i < 0 && verify_cb_cert(x, depth, V_ERR_CERT_HAS_EXPIRED) == 0) {
+            return false;
+        }
+
+        return true;
     }
 
     /**
@@ -1068,6 +1211,22 @@ public int getCRLStack(X509CRL[] pcrl, Name name, List<X509CRL> crls) throws Exc
         return 0;
     }
 
+    /*
+     * Inform the verify callback of an error.
+     * If B<x> is not NULL it is the error cert, otherwise use the chain cert at
+     * B<depth>.
+     * If B<err> is not X509_V_OK, that's the error value, otherwise leave
+     * unchanged (presumably set by the caller).
+     *
+     * Returns 0 to abort verification with an error, non-zero to continue.
+     */
+    private int verify_cb_cert(X509AuxCertificate x, int depth, int err) throws Exception {
+        this.errorDepth = depth;
+        this.currentCertificate = x != null ? x : chain.get(depth);
+        if (err != V_OK) this.error = err;
+        return verifyCallback.call(this, 0); // ctx->verify_cb(0, ctx)
+    }
+
     final static Store.GetIssuerFunction getFirstIssuer = new Store.GetIssuerFunction() {
         public int call(StoreContext context, X509AuxCertificate[] issuer, X509AuxCertificate cert) throws Exception {
             return context.getFirstIssuer(issuer, cert);
@@ -1079,7 +1238,7 @@ public int call(StoreContext context, X509AuxCertificate[] issuer, X509AuxCertif
      */
     final static Store.GetIssuerFunction getIssuerStack = new Store.GetIssuerFunction() {
         public int call(StoreContext context, X509AuxCertificate[] issuer, X509AuxCertificate x) throws Exception {
-            issuer[0] = context.findIssuer(context.otherContext, x);
+            issuer[0] = context.findIssuer(context.otherContext, x, false);
             if ( issuer[0] != null ) {
                 return 1;
             } else {
@@ -1169,8 +1328,8 @@ public int call(final StoreContext context) throws Exception {
                 }
 
                 xs.setValid(true);
-                ok = context.checkCertificateTime(xs);
-                if ( ok == 0 ) return ok;
+
+                if (!context.x509_check_cert_time(xs, n)) return 0;
 
                 context.currentIssuer = xi;
                 context.currentCertificate = xs;

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

@@ -427,6 +427,10 @@ else if (maybeCertFile != null && new File(maybeCertFile).exists()) {
     public static final int V_FLAG_INHIBIT_MAP = 0x400;
     public static final int V_FLAG_NOTIFY_POLICY = 0x800;
 
+
+    /* Do not check certificate/CRL validity against current time */
+    public static final int V_FLAG_NO_CHECK_TIME = 0x200000;
+
     public static final int VP_FLAG_DEFAULT = 0x1;
     public static final int VP_FLAG_OVERWRITE = 0x2;
     public static final int VP_FLAG_RESET_FLAGS = 0x4;