mod_ssl: Add "no_crl_for_cert_ok" flag to SSLCARevocationCheck directive

to opt-in previous behaviour (2.2) with CRLs verification when checking
certificate(s) with no corresponding CRL.

Submitted by: ylavic
Reviewed by: icing, minfrin

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1748338 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 7173c827a11b93075bac5cc96527e7c78064610d..2efb5ae721b3284df1f9af7c1ad233c8404e458d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 
 Changes with Apache 2.4.21
 
+  *) mod_ssl: Add "no_crl_for_cert_ok" flag to SSLCARevocationCheck directive
+     to opt-in previous behaviour (2.2) with CRLs verification when checking
+     certificate(s) with no corresponding CRL.  [Yann Ylavic]
+
   *) mpm_event, mpm_worker: Fix computation of MinSpareThreads' lower bound
      according the number of listeners buckets.  [Yann Ylavic]
 

diff --git a/STATUS b/STATUS
index 9c402e961692b8f91b4140ecb288585ea1d70b8e..d4dd27333717057cd5937d46d7ae27ce8482cd6f 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -114,18 +114,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_ssl: Add "no_crl_for_cert_ok" flag to SSLCARevocationCheck directive
-     to opt-in previous behaviour (2.2) with CRLs verification when checking
-     certificate(s) with no corresponding CRL.
-     trunk patch: http://svn.apache.org/r1734561
-                  http://svn.apache.org/r1734807
-                  http://svn.apache.org/r1735159
-                  http://svn.apache.org/r1735337
-                  http://svn.apache.org/r1737265
-     2.4.x patch: trunk works (modulo CHANGES) or
-                  http://home.apache.org/~ylavic/patches/httpd-2.4.x-no_crl_for_cert_ok-v2.patch
-     +1: ylavic, icing, minfrin
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/docs/manual/mod/mod_ssl.xml b/docs/manual/mod/mod_ssl.xml
index 08348df192e8f39be6b7676414484161e2ddd03c..8c1305f9f1727b0482ae3b8d569a2f4580ad7578 100644 (file)
--- a/docs/manual/mod/mod_ssl.xml
+++ b/docs/manual/mod/mod_ssl.xml
@@ -1210,10 +1210,12 @@ SSLCARevocationFile "/usr/local/apache2/conf/ssl.crl/ca-bundle-client.crl"
 <directivesynopsis>
 <name>SSLCARevocationCheck</name>
 <description>Enable CRL-based revocation checking</description>
-<syntax>SSLCARevocationCheck chain|leaf|none</syntax>
+<syntax>SSLCARevocationCheck chain|leaf|none <em>flag</em>s</syntax>
 <default>SSLCARevocationCheck none</default>
 <contextlist><context>server config</context>
 <context>virtual host</context></contextlist>
+<compatibility>Optional <em>flag</em>s available in httpd 2.5-dev or
+later</compatibility>
 
 <usage>
 <p>
@@ -1224,25 +1226,38 @@ configured. When set to <code>chain</code> (recommended setting),
 CRL checks are applied to all certificates in the chain, while setting it to
 <code>leaf</code> limits the checks to the end-entity cert.
 </p>
-<note>
-<title>When set to <code>chain</code> or <code>leaf</code>,
-CRLs <em>must</em> be available for successful validation</title>
-<p>
-Prior to version 2.3.15, CRL checking in mod_ssl also succeeded when
-no CRL(s) were found in any of the locations configured with
-<directive module="mod_ssl">SSLCARevocationFile</directive>
-or <directive module="mod_ssl">SSLCARevocationPath</directive>.
-With the introduction of this directive, the behavior has been changed:
-when checking is enabled, CRLs <em>must</em> be present for the validation
-to succeed - otherwise it will fail with an
-<code>"unable to get certificate CRL"</code> error.
-</p>
-</note>
+<p>The available <em>flag</em>s are:</p>
+<ul>
+<li><code>no_crl_for_cert_ok</code>
+    <p>
+    Prior to version 2.3.15, CRL checking in mod_ssl also succeeded when
+    no CRL(s) for the checked certificate(s) were found in any of the locations
+    configured with <directive module="mod_ssl">SSLCARevocationFile</directive>
+    or <directive module="mod_ssl">SSLCARevocationPath</directive>.
+    </p>
+    <p>
+    With the introduction of <directive>SSLCARevocationFile</directive>,
+    the behavior has been changed: by default with <code>chain</code> or
+    <code>leaf</code>, CRLs <strong>must</strong> be present for the
+    validation to succeed - otherwise it will fail with an
+    <code>"unable to get certificate CRL"</code> error.
+    </p>
+    <p>
+    The <em>flag</em> <code>no_crl_for_cert_ok</code> allows to restore
+    previous behaviour.
+    </p>
+</li>
+</ul>
 <example><title>Example</title>
 <highlight language="config">
 SSLCARevocationCheck chain
 </highlight>
 </example>
+<example><title>Compatibility with versions 2.2</title>
+<highlight language="config">
+SSLCARevocationCheck chain no_crl_for_cert_ok
+</highlight>
+</example>
 </usage>
 </directivesynopsis>
 

diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c
index 219e33376f138ff1eb021eac4628089902682f67..f8e71b33a53755fdc363b6136dc3573f6bea73ca 100644 (file)
--- a/modules/ssl/mod_ssl.c
+++ b/modules/ssl/mod_ssl.c
@@ -119,7 +119,7 @@ static const command_rec ssl_config_cmds[] = {
     SSL_CMD_SRV(CARevocationFile, TAKE1,
                 "SSL CA Certificate Revocation List (CRL) file "
                 "('/path/to/file' - PEM encoded)")
-    SSL_CMD_SRV(CARevocationCheck, TAKE1,
+    SSL_CMD_SRV(CARevocationCheck, RAW_ARGS,
                 "SSL CA Certificate Revocation List (CRL) checking mode")
     SSL_CMD_ALL(VerifyClient, TAKE1,
                 "SSL Client verify type "
@@ -197,7 +197,7 @@ static const command_rec ssl_config_cmds[] = {
     SSL_CMD_SRV(ProxyCARevocationFile, TAKE1,
                 "SSL Proxy: CA Certificate Revocation List (CRL) file "
                 "('/path/to/file' - PEM encoded)")
-    SSL_CMD_SRV(ProxyCARevocationCheck, TAKE1,
+    SSL_CMD_SRV(ProxyCARevocationCheck, RAW_ARGS,
                 "SSL Proxy: CA Certificate Revocation List (CRL) checking mode")
     SSL_CMD_SRV(ProxyMachineCertificateFile, TAKE1,
                "SSL Proxy: file containing client certificates "

diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c
index a3d5af526692fdb1452f2f381aebc985ae45534b..129a01ff545525b43e71c02fefed5a8838a49714 100644 (file)
--- a/modules/ssl/ssl_engine_config.c
+++ b/modules/ssl/ssl_engine_config.c
@@ -121,7 +121,7 @@ static void modssl_ctx_init(modssl_ctx_t *mctx, apr_pool_t *p)
 
     mctx->crl_path            = NULL;
     mctx->crl_file            = NULL;
-    mctx->crl_check_mode      = SSL_CRLCHECK_UNSET;
+    mctx->crl_check_mask      = UNSET;
 
     mctx->auth.ca_cert_path   = NULL;
     mctx->auth.ca_cert_file   = NULL;
@@ -271,7 +271,7 @@ static void modssl_ctx_cfg_merge(apr_pool_t *p,
 
     cfgMerge(crl_path, NULL);
     cfgMerge(crl_file, NULL);
-    cfgMerge(crl_check_mode, SSL_CRLCHECK_UNSET);
+    cfgMergeInt(crl_check_mask);
 
     cfgMergeString(auth.ca_cert_path);
     cfgMergeString(auth.ca_cert_file);
@@ -1000,23 +1000,38 @@ const char *ssl_cmd_SSLCARevocationFile(cmd_parms *cmd,
 
 static const char *ssl_cmd_crlcheck_parse(cmd_parms *parms,
                                           const char *arg,
-                                          ssl_crlcheck_t *mode)
+                                          int *mask)
 {
-    if (strcEQ(arg, "none")) {
-        *mode = SSL_CRLCHECK_NONE;
+    const char *w;
+
+    w = ap_getword_conf(parms->temp_pool, &arg);
+    if (strcEQ(w, "none")) {
+        *mask = SSL_CRLCHECK_NONE;
     }
-    else if (strcEQ(arg, "leaf")) {
-        *mode = SSL_CRLCHECK_LEAF;
+    else if (strcEQ(w, "leaf")) {
+        *mask = SSL_CRLCHECK_LEAF;
     }
-    else if (strcEQ(arg, "chain")) {
-        *mode = SSL_CRLCHECK_CHAIN;
+    else if (strcEQ(w, "chain")) {
+        *mask = SSL_CRLCHECK_CHAIN;
     }
     else {
         return apr_pstrcat(parms->temp_pool, parms->cmd->name,
-                           ": Invalid argument '", arg, "'",
+                           ": Invalid argument '", w, "'",
                            NULL);
     }
 
+    while (*arg) {
+        w = ap_getword_conf(parms->temp_pool, &arg);
+        if (strcEQ(w, "no_crl_for_cert_ok")) {
+            *mask |= SSL_CRLCHECK_NO_CRL_FOR_CERT_OK;
+        }
+        else {
+            return apr_pstrcat(parms->temp_pool, parms->cmd->name,
+                               ": Invalid argument '", w, "'",
+                               NULL);
+        }
+    }
+
     return NULL;
 }
 
@@ -1026,7 +1041,7 @@ const char *ssl_cmd_SSLCARevocationCheck(cmd_parms *cmd,
 {
     SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
 
-    return ssl_cmd_crlcheck_parse(cmd, arg, &sc->server->crl_check_mode);
+    return ssl_cmd_crlcheck_parse(cmd, arg, &sc->server->crl_check_mask);
 }
 
 static const char *ssl_cmd_verify_parse(cmd_parms *parms,
@@ -1540,7 +1555,7 @@ const char *ssl_cmd_SSLProxyCARevocationCheck(cmd_parms *cmd,
 {
     SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
 
-    return ssl_cmd_crlcheck_parse(cmd, arg, &sc->proxy->crl_check_mode);
+    return ssl_cmd_crlcheck_parse(cmd, arg, &sc->proxy->crl_check_mask);
 }
 
 const char *ssl_cmd_SSLProxyMachineCertificateFile(cmd_parms *cmd,

diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c
index 797fbd12e04e7784eb7259fbea789ab5c0e94453..270c86cd4096af16088a04f438d9c9e7229567b1 100644 (file)
--- a/modules/ssl/ssl_engine_init.c
+++ b/modules/ssl/ssl_engine_init.c
@@ -787,14 +787,15 @@ static apr_status_t ssl_init_ctx_crl(server_rec *s,
     X509_STORE *store = SSL_CTX_get_cert_store(mctx->ssl_ctx);
     unsigned long crlflags = 0;
     char *cfgp = mctx->pkp ? "SSLProxy" : "SSL";
+    int crl_check_mode = mctx->crl_check_mask & ~SSL_CRLCHECK_FLAGS;
 
     /*
      * Configure Certificate Revocation List (CRL) Details
      */
 
     if (!(mctx->crl_file || mctx->crl_path)) {
-        if (mctx->crl_check_mode == SSL_CRLCHECK_LEAF ||
-            mctx->crl_check_mode == SSL_CRLCHECK_CHAIN) {
+        if (crl_check_mode == SSL_CRLCHECK_LEAF ||
+            crl_check_mode == SSL_CRLCHECK_CHAIN) {
             ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01899)
                          "Host %s: CRL checking has been enabled, but "
                          "neither %sCARevocationFile nor %sCARevocationPath "
@@ -816,7 +817,7 @@ static apr_status_t ssl_init_ctx_crl(server_rec *s,
         return ssl_die(s);
     }
 
-    switch (mctx->crl_check_mode) {
+    switch (crl_check_mode) {
        case SSL_CRLCHECK_LEAF:
            crlflags = X509_V_FLAG_CRL_CHECK;
            break;

diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c
index 17fd7db39216146e0ed585388f7dc6857cb42543..4fa0b12889f965336441998b16d30420e11cdcb0 100644 (file)
--- a/modules/ssl/ssl_engine_kernel.c
+++ b/modules/ssl/ssl_engine_kernel.c
@@ -1553,22 +1553,24 @@ int ssl_callback_SSLVerify(int ok, X509_STORE_CTX *ctx)
     SSLDirConfigRec *dc = r ? myDirConfig(r) : NULL;
     SSLConnRec *sslconn = myConnConfig(conn);
     modssl_ctx_t *mctx  = myCtxConfig(sslconn, sc);
+    int crl_check_mode  = mctx->crl_check_mask & ~SSL_CRLCHECK_FLAGS;
 
     /* Get verify ingredients */
     int errnum   = X509_STORE_CTX_get_error(ctx);
     int errdepth = X509_STORE_CTX_get_error_depth(ctx);
     int depth, verify;
 
+
     /*
      * Log verification information
      */
     ssl_log_cxerror(SSLLOG_MARK, APLOG_DEBUG, 0, conn,
                     X509_STORE_CTX_get_current_cert(ctx), APLOGNO(02275)
                     "Certificate Verification, depth %d, "
-                    "CRL checking mode: %s", errdepth,
-                    mctx->crl_check_mode == SSL_CRLCHECK_CHAIN ?
-                    "chain" : (mctx->crl_check_mode == SSL_CRLCHECK_LEAF ?
-                               "leaf" : "none"));
+                    "CRL checking mode: %s (%x)", errdepth,
+                    crl_check_mode == SSL_CRLCHECK_CHAIN ? "chain" :
+                    crl_check_mode == SSL_CRLCHECK_LEAF  ? "leaf"  : "none",
+                    mctx->crl_check_mask);
 
     /*
      * Check for optionally acceptable non-verifiable issuer situation
@@ -1617,6 +1619,17 @@ int ssl_callback_SSLVerify(int ok, X509_STORE_CTX *ctx)
         X509_STORE_CTX_set_error(ctx, -1);
     }
 
+    if (!ok && errnum == X509_V_ERR_UNABLE_TO_GET_CRL
+            && (mctx->crl_check_mask & SSL_CRLCHECK_NO_CRL_FOR_CERT_OK)) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, conn,
+                      "Certificate Verification: Temporary error (%d): %s: "
+                      "optional therefore we're accepting the certificate",
+                      errnum, X509_verify_cert_error_string(errnum));
+        X509_STORE_CTX_set_error(ctx, X509_V_OK);
+        errnum = X509_V_OK;
+        ok = TRUE;
+    }
+
 #ifndef OPENSSL_NO_OCSP
     /*
      * Perform OCSP-based revocation checks

diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index 70b3ac22d0a92b5bdbdea033732438c46fc59a20..7f6f9fd95ace40880610587b9db920e3cd5fc8db 100644 (file)
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -343,13 +343,15 @@ typedef enum {
     || (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))
 
 /**
-  * CRL checking modes
+  * CRL checking mask (mode | flags)
   */
 typedef enum {
-    SSL_CRLCHECK_UNSET = UNSET,
-    SSL_CRLCHECK_NONE  = 0,
-    SSL_CRLCHECK_LEAF  = 1,
-    SSL_CRLCHECK_CHAIN = 2
+    SSL_CRLCHECK_NONE  = (0),
+    SSL_CRLCHECK_LEAF  = (1 << 0),
+    SSL_CRLCHECK_CHAIN = (1 << 1),
+
+#define SSL_CRLCHECK_FLAGS (~0x3)
+    SSL_CRLCHECK_NO_CRL_FOR_CERT_OK = (1 << 2)
 } ssl_crlcheck_t;
 
 /**
@@ -607,7 +609,7 @@ typedef struct {
     /** certificate revocation list */
     const char    *crl_path;
     const char    *crl_file;
-    ssl_crlcheck_t crl_check_mode;
+    int            crl_check_mask;
 
 #ifdef HAVE_OCSP_STAPLING
     /** OCSP stapling options */