Various X509 fixes. Disable broken certificate workarounds

when X509_V_FLAG_X509_STRICT is set. Check for CRLSign in
CRL issuer certificates. Reject CRLs with unhandled (any)
critical extensions.

diff --git a/CHANGES b/CHANGES
index bea8e7683874b91aa19f494bd0123e13ceeea5e8..19803f2935941832d1567ecbbf51d7ca5386f20a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -617,6 +617,14 @@
 
  Changes between 0.9.7c and 0.9.7d  [xx XXX XXXX]
 
+  *) X509 verify fixes. Disable broken certificate workarounds when 
+     X509_V_FLAGS_X509_STRICT is set. Check CRL issuer has cRLSign set if
+     keyUsage extension present. Don't accept CRLs with unhandled critical
+     extensions: since verify currently doesn't process CRL extensions this
+     rejects a CRL with *any* critical extensions. Add new verify error codes
+     for these cases.
+     [Steve Henson]
+
   *) When creating an OCSP nonce use an OCTET STRING inside the extnValue.
      A clarification of RFC2560 will require the use of OCTET STRINGs and 
      some implementations cannot handle the current raw format. Since OpenSSL

diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 5a945a70fb70c737e004e4a2f5e6eee6202cade3..e31ebc6741a01b0a1c4a4346d299e68fd2c436b8 100644 (file)
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -147,6 +147,12 @@ const char *X509_verify_cert_error_string(long n)
        case X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION:
                return("unhandled critical extension");
 
+       case X509_V_ERR_KEYUSAGE_NO_CRL_SIGN:
+               return("key usage does not include CRL signing");
+
+       case X509_V_ERR_UNHANDLED_CRITICAL_CRL_EXTENSION:
+               return("unhandled critical CRL extension");
+
        default:
                BIO_snprintf(buf,sizeof buf,"error number %ld",n);
                return(buf);

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 2bb21b443ec0674b9738935302ad2173a5066e81..2e4d0b823abe16015b7aa449fcb3bdb8562b26d4 100644 (file)
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -383,6 +383,7 @@ static int check_chain_purpose(X509_STORE_CTX *ctx)
        /* Check all untrusted certificates */
        for (i = 0; i < ctx->last_untrusted; i++)
                {
+               int ret;
                x = sk_X509_value(ctx->chain, i);
                if (!(ctx->flags & X509_V_FLAG_IGNORE_CRITICAL)
                        && (x->ex_flags & EXFLAG_CRITICAL))
@@ -393,7 +394,10 @@ static int check_chain_purpose(X509_STORE_CTX *ctx)
                        ok=cb(0,ctx);
                        if (!ok) goto end;
                        }
-               if (!X509_check_purpose(x, ctx->purpose, i))
+               ret = X509_check_purpose(x, ctx->purpose, i);
+               if ((ret == 0)
+                        || ((ctx->flags & X509_V_FLAG_X509_STRICT)
+                               && (ret != 1)))
                        {
                        if (i)
                                ctx->error = X509_V_ERR_INVALID_CA;
@@ -537,6 +541,14 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl)
 
        if(issuer)
                {
+               /* Check for cRLSign bit if keyUsage present */
+               if ((issuer->ex_flags & EXFLAG_KUSAGE) &&
+                       !(issuer->ex_kusage & KU_CRL_SIGN))
+                       {
+                       ctx->error = X509_V_ERR_KEYUSAGE_NO_CRL_SIGN;
+                       ok = ctx->verify_cb(0, ctx);
+                       if(!ok) goto err;
+                       }
 
                /* Attempt to get issuer certificate public key */
                ikey = X509_get_pubkey(issuer);
@@ -611,17 +623,46 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x)
        {
        int idx, ok;
        X509_REVOKED rtmp;
+       STACK_OF(X509_EXTENSION) *exts;
+       X509_EXTENSION *ext;
        /* Look for serial number of certificate in CRL */
        rtmp.serialNumber = X509_get_serialNumber(x);
        idx = sk_X509_REVOKED_find(crl->crl->revoked, &rtmp);
-       /* Not found: OK */
-       if(idx == -1) return 1;
-       /* Otherwise revoked: want something cleverer than
+       /* If found assume revoked: want something cleverer than
         * this to handle entry extensions in V2 CRLs.
         */
-       ctx->error = X509_V_ERR_CERT_REVOKED;
-       ok = ctx->verify_cb(0, ctx);
-       return ok;
+       if(idx >= 0)
+               {
+               ctx->error = X509_V_ERR_CERT_REVOKED;
+               ok = ctx->verify_cb(0, ctx);
+               if (!ok) return 0;
+               }
+
+       if (ctx->flags & X509_V_FLAG_IGNORE_CRITICAL)
+               return 1;
+
+       /* See if we have any critical CRL extensions: since we
+        * currently don't handle any CRL extensions the CRL must be
+        * rejected. 
+        * This code accesses the X509_CRL structure directly: applications
+        * shouldn't do this.
+        */
+
+       exts = crl->crl->extensions;
+
+       for (idx = 0; idx < sk_X509_EXTENSION_num(exts); idx++)
+               {
+               ext = sk_X509_EXTENSION_value(exts, idx);
+               if (ext->critical > 0)
+                       {
+                       ctx->error =
+                               X509_V_ERR_UNHANDLED_CRITICAL_CRL_EXTENSION;
+                       ok = ctx->verify_cb(0, ctx);
+                       if(!ok) return 0;
+                       break;
+                       }
+               }
+       return 1;
        }
 
 static int internal_verify(X509_STORE_CTX *ctx)

diff --git a/crypto/x509/x509_vfy.h b/crypto/x509/x509_vfy.h
index 3a0ccbe48d3ea6a3448c64941b1e173255414af2..7e24ded8102f76896cb36aa88afe9bebdc4257ba 100644 (file)
--- a/crypto/x509/x509_vfy.h
+++ b/crypto/x509/x509_vfy.h
@@ -305,17 +305,26 @@ struct x509_store_ctx_st      /* X509_STORE_CTX */
 
 #define                X509_V_ERR_UNABLE_TO_GET_CRL_ISSUER             33
 #define                X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION         34
+#define                X509_V_ERR_KEYUSAGE_NO_CRL_SIGN                 35
+#define                X509_V_ERR_UNHANDLED_CRITICAL_CRL_EXTENSION     36
 
 /* The application is not happy */
 #define                X509_V_ERR_APPLICATION_VERIFICATION             50
 
 /* Certificate verify flags */
 
-#define        X509_V_FLAG_CB_ISSUER_CHECK             0x1     /* Send issuer+subject checks to verify_cb */
-#define        X509_V_FLAG_USE_CHECK_TIME              0x2     /* Use check time instead of current time */
-#define        X509_V_FLAG_CRL_CHECK                   0x4     /* Lookup CRLs */
-#define        X509_V_FLAG_CRL_CHECK_ALL               0x8     /* Lookup CRLs for whole chain */
-#define        X509_V_FLAG_IGNORE_CRITICAL             0x10    /* Ignore unhandled critical extensions */
+/* Send issuer+subject checks to verify_cb */
+#define        X509_V_FLAG_CB_ISSUER_CHECK             0x1
+/* Use check time instead of current time */
+#define        X509_V_FLAG_USE_CHECK_TIME              0x2
+/* Lookup CRLs */
+#define        X509_V_FLAG_CRL_CHECK                   0x4
+/* Lookup CRLs for whole chain */
+#define        X509_V_FLAG_CRL_CHECK_ALL               0x8
+/* Ignore unhandled critical extensions */
+#define        X509_V_FLAG_IGNORE_CRITICAL             0x10
+/* Disable workarounds for broken certificates */
+#define        X509_V_FLAG_X509_STRICT                 0x20
 
 int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
             X509_NAME *name);

diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c
index b1a6d2632ac96a158cf903466868cd3da8739187..67596862ab3c213de2670975f5ea99b633ee20b0 100644 (file)
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -3,7 +3,7 @@
  * project 2001.
  */
 /* ====================================================================
- * Copyright (c) 1999-2001 The OpenSSL Project.  All rights reserved.
+ * Copyright (c) 1999-2004 The OpenSSL Project.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -415,6 +415,7 @@ static void x509v3_cache_extensions(X509 *x)
  * 1 is a CA
  * 2 basicConstraints absent so "maybe" a CA
  * 3 basicConstraints absent but self signed V1.
+ * 4 basicConstraints absent but keyUsage present and keyCertSign asserted.
  */
 
 #define V1_ROOT (EXFLAG_V1|EXFLAG_SS)
@@ -436,7 +437,7 @@ static int ca_check(const X509 *x)
        } else {
                if((x->ex_flags & V1_ROOT) == V1_ROOT) return 3;
                /* If key usage present it must have certSign so tolerate it */
-               else if (x->ex_flags & EXFLAG_KUSAGE) return 3;
+               else if (x->ex_flags & EXFLAG_KUSAGE) return 4;
                else return 2;
        }
 }