Bug 45054 - SSLVerifyClient optional_no_ca is broken
Summary: SSLVerifyClient optional_no_ca is broken
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_ssl (show other bugs)
Version: 2.5-HEAD
Hardware: PC Linux
: P2 normal with 7 votes (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
Keywords: PatchAvailable
Depends on:
Reported: 2008-05-21 02:29 UTC by Benjamin Dauvergne
Modified: 2013-11-21 11:22 UTC (History)
4 users (show)

Patch for 2.2.x (615 bytes, patch)
2010-04-13 14:01 UTC, Paul Donohue
Details | Diff
Patch for trunk (615 bytes, patch)
2010-04-13 14:01 UTC, Paul Donohue
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Dauvergne 2008-05-21 02:29:41 UTC
The documented behavious for the option 'optional_no_ca' is that if the return code
of SSL_Accept valid this predicate:
ssl_private.h:#define ssl_verify_error_is_optional(errnum) \
ssl_private.h-   ((errnum == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT) \
ssl_private.h-    || (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN) \
ssl_private.h-    || (errnum == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY) \
ssl_private.h-    || (errnum == X509_V_ERR_CERT_UNTRUSTED) \
ssl_private.h-    || (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))
then the error code in SSL_VERIFY_CLIENT should be "FAILED:(an error message)" but "GENEROUS".

This functionality is very useful to use certificate as an user-centric
authentication token, why forbid it with this code from ssl_engine_io.c:
        if (ssl_verify_error_is_optional(verify_result) &&
            (sc->server->auth.verify_mode == SSL_CVERIFY_OPTIONAL_NO_CA))
            /* leaving this log message as an error for the moment,
             * according to the mod_ssl docs:
             * "level optional_no_ca is actually against the idea
             *  of authentication (but can be used to establish
             * SSL test pages, etc.)"
             * optional_no_ca doesn't appear to work as advertised
             * in 1.x
            ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, c,
                          "SSL client authentication failed, "
                          "accepting certificate based on "
                          "\"SSLVerifyClient optional_no_ca\" "
            ssl_log_ssl_error(APLOG_MARK, APLOG_INFO, c->base_server);
look that we don't reset the result to something good like X509_V_OK.

And this code that return the value of SSL_VERIFY_CLIENT:
    if (vrc == X509_V_OK && verr == NULL && vinfo == NULL && xs == NULL)
        /* no client verification done at all */
        result = "NONE";
    else if (vrc == X509_V_OK && verr == NULL && vinfo == NULL && xs != NULL)
        /* client verification done successful */
        result = "SUCCESS";
    else if (vrc == X509_V_OK && vinfo != NULL && strEQ(vinfo, "GENEROUS"))
        /* client verification done in generous way */
        result = "GENEROUS";
        /* client verification failed */
        result = apr_psprintf(p, "FAILED:%s", verr);

    if (xs)

The third condition can never happen as vrc will be OK and vinfo == GENEROUS at the same time.
Two approaches:
- reset the result code (we loss information), with a
  SSL_set_verify_result(ssl, X509_V_OK); inside the first extracted code.
- change third condition with this patch:
        -    else if (vrc == X509_V_OK && vinfo != NULL && strEQ(vinfo, "GENEROUS"))
        +    else if (ssl_verify_error_is_optional(vrc) && vinfo != NULL && strEQ(vinfo, "GENEROUS"))
 - bonus point would be to return the result code inside another variable and
   completely removing the optional_no_ca option, web app could just use
   optional and choose actions in function of SSL_VERIFY_CLIENT == FAILED and
   the error code.

What do you think ?
Comment 1 Paul Donohue 2010-04-13 13:52:53 UTC
I've run into the same problem.  If 'SSLVerifyClient optional_no_ca' is used, the SSL_CLIENT_VERIFY environment variable will contain either 'SUCCESS' or 'FAILED:' (with no error string). It will never contain 'GENEROUS', as stated by the documentation.

As Benjamin stated, this happens because ssl_callback_SSLVerify() in ssl_engine_kernel.c only sets sslconn->verify_info = "GENEROUS" if validation failed with an "optional" error, and only sets sslconn->verify_error if optional_no_ca is used and validation failed with a non-optional error. Thus, the 'if (vrc == X509_V_OK && vinfo != NULL && strEQ(vinfo, "GENEROUS"))' statement in ssl_var_lookup_ssl_cert_verify() in ssl_engine_vars.c will never match because vinfo will not have been set if vrc is set to X509_V_OK, and the FAILED message will not contain an error string since sslconn->verify_error was never set.

I get the sense that the original mistake was in the if statement in ssl_engine_vars.c, so the second approach mentioned by Benjamin is probably the best fix.  However, the ssl_verify_error_is_optional(vrc) check that Benjamin used isn't actually needed, since sslconn->verify_info will not be set if the error is optional, so you only really need to check for sslconn->verify_info == GENEROUS.

I'm attaching two patches (one for 2.2.x, and one for trunk) to correct this.  Hopefully someone will apply them, as this is a simple but annoying bug.
Comment 2 Paul Donohue 2010-04-13 14:01:22 UTC
Created attachment 25278 [details]
Patch for 2.2.x
Comment 3 Paul Donohue 2010-04-13 14:01:54 UTC
Created attachment 25279 [details]
Patch for trunk
Comment 4 Paul Donohue 2010-04-14 11:35:18 UTC
BTW, I've attached a patch to bug #45922 which includes this fix and also provides the verification error message (only one error message if multiple verification failures occurred) when returning a GENEROUS result.
Comment 5 Arnis UT 2013-11-21 11:22:25 UTC
Current possible SSL_CLIENT_VERIFY values when SSLVerifyClient set to optional_no_ca:
NONE: if empty client certificate message sent.
SUCCESS: successful authentication when client certificate chains up to a trusted root.
GENEROUS: initial negotiation when client certificate does not chain up to a trusted root.
SUCCESS: when resuming GENEROUS session (bug #53193).
FAILED:(null): renegotiation when client certificate does not chain up to a trusted root.