I don't believe we should write something specific for your module (eg checking
for the exact name of your module).

If we want to provide this, I think we should look at integrating that module
with the core mod_ssl.  The other alternative is to just provide a configuration
directive to not return.  This might be the best way to go.

Something like:

SSLFatalErrors Off

Thoughts?

The best would obviously to integrate it with the core mod_ssl.

The main goal was to avoid a misconfiguration that would accept wrong
certificates; this could be the case with the directive.

If we want to keep the compatibility with the curretn version (SSL error not
remapped), maybe the best would be to put my module in the standard distribution
and check if it is loaded, no ? This ensures that non fatal errors are only
passing when a module is loaded to trap them later.

Maybe I'm too paranoiac ... or experienced ;-)

Addition to trap when no certif is sent (user hits Esc in browser):

+ orig_verify_mode = filter_ctx->pssl->verify_mode;
+ if ( sslErrorRedirected )
+    filter_ctx->pssl->verify_mode &= ~SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
 if ((n = SSL_accept(filter_ctx->pssl)) <= 0) {

...

 verify_result = SSL_get_verify_result(filter_ctx->pssl);

+ if ( orig_verify_mode != filter_ctx->pssl->verify_mode ) {
+    verify_result = X509_V_ERR_CERT_REJECTED;
+    sslconn->verify_error = X509_verify_cert_error_string(verify_result);
+ }

 if ((verify_result != X509_V_OK) ||


The choice of X509_V_ERR_CERT_REJECTED as error is purely arbitrary, any better
one ?

Sorry, I forgot something.
Here is the correct version:

+ orig_verify_mode = filter_ctx->pssl->verify_mode;
+ if ( sslErrorRedirected )
+    filter_ctx->pssl->verify_mode &= ~SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
 if ((n = SSL_accept(filter_ctx->pssl)) <= 0) {

...

 verify_result = SSL_get_verify_result(filter_ctx->pssl);

+ if ( (orig_verify_mode != filter_ctx->pssl->verify_mode) &&
+      !filter_ctx->pssl->session->peer ) {
+    verify_result = X509_V_ERR_CERT_REJECTED;
+    sslconn->verify_error = X509_verify_cert_error_string(verify_result);
+ }

 if ((verify_result != X509_V_OK) ||

(In reply to comment #2)
> The best would obviously to integrate it with the core mod_ssl.
> 

I agree strongly.  The current behavior of mod_ssl is significantly broken for
client certificates.  

I'm not convinced this is necessary.  mod_ssl should be reporting errors at
TLS-level for TLS-level problems like bad certs.  If browsers handle that badly
then fix the browsers.

If you do want custom HTML responses for cert validation problems, it should be
possible already using "SSLVerifyClient optional" with SSLRequire and a custom
403 errordoc.  What about the current code prevents that?  Possibly more
information needs to be exported to get to the "certificate has expired" stuff.
 2.1's SSL_CLIENT_V_REMAIN might be sufficient for that.

Joe,

1. I do not think we could blame the browsers (at least not totally), because
they do not receive enough information to display a error complete mesage.

2. Although Apache strictly follows the standards, its main goal is to be used
by B2B-like clients, AND BROWSERS, no ?
There is no one browser that implements SSL error trapping in another way that
giving a technical error message or number

3. In a lot of environments, the Web admin would like to give more information
to the user about how to solve the connection problem. This is especially true
in governmental sites: most of European countries (and several other ones) are
deploying identity smart card with national certificates that are intended to
bare citizens that do not understand anything about certificates. The error
message to be given is highly specific in this case.

4. Although not stressed in the module description, the module also allows some
reporting of errors in a way governmental applications like a lot: their
application can receive (and so handle/log) any connection failure with the
exact reason. Again, for governmental certificates, this is highly important
because of the OCSP SLA, etc.
This could be achieved in a separate application/script by analysing the logs,
although the needed log level would probably generate to much data for a
production environment.

> If you do want custom HTML responses for cert validation problems, it should
> be possible already using "SSLVerifyClient optional" with SSLRequire and
> a custom 403 errordoc.
> What about the current code prevents that?
> Possibly more information needs to be exported to get to the
> "certificate has expired" stuff.
> 2.1's SSL_CLIENT_V_REMAIN might be sufficient for that.
I tried, but couldn't do that (at least in 2.0.54). Revoked certificates are
accepted and ther's no way to trap it.
If we could trap the errors without modifying med_ssl core, then it's obviously
perfect. My module could thus be reimplemented to use that new
SSL_CLIENT_V_REMAIN, is that correct ?

The specific configuration I'm talking about would be:

  SSLVerifyClient optional_no_ca
  SSLRequire %{SSL_VERIFY_CLIENT} eq "SUCCESS"
  ErrorDocument 403 /bzzt.html

but, I guess for the case where the cert has been revoked by a CRL, or the cert
has expired, this is not sufficient, since the handshake will fail in those cases.

So the minimal enhancement that I think is acceptable is to have more
fine-grained failure modes for SSLVerifyClient.  e.g.

  SSLVerifyCLient optional_revoked
  SSLVerifyClient optional_expired

or something like that.  Maybe ideally it would be possible to combine such
options perhaps, allowing

  SSLVerifyClient optional no_ca revoked expired

or using a separate directive:

  SSLVerifyClient optional
  SSLVerifyIgnoreFailures no_ca revoked expired
  
I'm not sure about the best UI here.

So I think a patch for something like this would be acceptable and is the best
way to implement this feature.  This is critical code and has a bad security
history though so it needs to be done carefully.

>   SSLVerifyClient optional_no_ca
>   SSLRequire %{SSL_VERIFY_CLIENT} eq "SUCCESS"
>   ErrorDocument 403 /bzzt.html
How would the application know about the exact status, in order to display a
relevant error message ?
What is the reason for error 403: authorisation failure, certificate revocation,
certificate expiration ?
Can the relevant info systematically be added to an environment variable ?

If you look the mod_ssl_error, it adds all information to GET data.
Maybe a stupid question, but does the "SSLVerifyClient optional" directive add
any added value to mod_ssl_error ? If not, couldn't we imagine to completely
replace this directive by mod_ssl_error which is more general ?

Yes, put any required info into env vars so that an SSI page or whatever can
retrieve that stuff directly.  The redirect-to-custom-URI-with-QUERY_URI thing
used by mod_ssl_error is just a nasty hack; do it with env vars.

I would like to be sure to not introduce a security risk because of a bad
configuration: suppose we use something like "SSLVerifyClient optional_no_ca"
and trap the error by activating another directive; if the administrator removes
the error trapping, the certificate would be accepted.
This is obviously the administrator's fault, but you know about the security
failure principle.
Shouldn't we replace the check for "SSLVerifyClient optional_no_ca ..." by a
check "is error trapping activated" ? Currently, the check controls if the
module is loaded (thus activated), but we could imagine a general directive
"SSLTrapCertifErrors".

Btw, what was the original intend of the "SSLVerifyClient  optional_no_ca"
directive ? Is it really used ? Because, if we use it as a flag to trap the
error, we cannot use the "old way" anymore, so we break the compatibility.

To recap, does the following seem better:
 - we include the whole handling in mod_ssl (no external module)
 - we add a general directive "SSLTrapCertifErrors"
 - we put all extra info (error, DN, serial, ...) in environment variables

If you prefer a separate module, how to activate it in without either explicitly
patching mod_ssl, or accepting all certrificates error in mod_ssl - which I find
dangerous as explained above ?

 - we include the whole handling in mod_ssl (no external module)

I'm not sure what you mean by that.

 - we add a general directive "SSLTrapCertifErrors"

I am OK with a new directive which allows the admin to relax certain SSL
certificate verification errors such as recovation status, expiry status, etc.

 - we put all extra info (error, DN, serial, ...) in environment variables

Yes.  DN and serial are already available in env vars of course.

>  - we include the whole handling in mod_ssl (no external module)
> I'm not sure what you mean by that.
I mean it becomes part of mod_ssl, it is not a separate module.
To be honest, I designed it as a module to have minimal impact on mod_ssl, in
order to speed up acceptance - bad guess.

>  - we add a general directive "SSLTrapCertifErrors"
> I am OK with a new directive which allows the admin to relax certain SSL
> certificate verification errors such as recovation status, expiry status, etc.
This does not relax anything, it just replaces a SSL error by a HTTP error (or a
custom page).
If needed, "SSLVerifyClient optional_no_ca ..." can still be used.
If it has any utility (?)

While waiting for the best integration as requested, here is how to integrate it
correctly with 2.0.54 (and probably above) - I added some other traps during
renegociation.

mod_ssl.h: add
extern int sslErrorRedirected;

ssl_engine_init.c: add
int sslErrorRedirected = -1;

in ssl_engine_io.c:
at the beginning of ssl_io_filter_connect(): add
if ( sslErrorRedirected == -1 ) { /* Check if mod_ssl_error is loaded */
   extern AP_DECLARE_DATA module *ap_top_module;
   module *modp;
   sslErrorRedirected = FALSE;
   for ( modp = ap_top_module; modp; modp = modp->next )
      if ( strcmp(modp->name, "mod_ssl_error.c") == 0 ) {
         sslErrorRedirected = TRUE;
         break;
      }
}

in ssl_engine_io.c:
in ssl_io_filter_connect(), after
   orig_verify_mode = filter_ctx->pssl->verify_mode;
/* Accept if no certs are sent (user hits Esc) */
if ( sslErrorRedirected ) filter_ctx->pssl->verify_mode &=
~SSL_VERIFY_FAIL_IF_NO_PEER_CERT;

in ssl_engine_io.c:
in ssl_io_filter_connect(), replace
if (ssl_verify_error_is_optional(verify_result) && ...) {
   ...
}
else {
   return ssl_filter_io_shutdown(filter_ctx, c, 1);
}
by
if (ssl_verify_error_is_optional(verify_result) && ...) {
   ...
}
else {
   if ( sslErrorRedirected ) return APR_SUCCESS;
   return ssl_filter_io_shutdown(filter_ctx, c, 1);
}


in ssl_engine_io.c:
in ssl_io_filter_connect(), replace
if ((sc->server->auth.verify_mode == SSL_CVERIFY_REQUIRE) && ... ){
   return ssl_filter_io_shutdown(filter_ctx, c, 1);
}
by if ((sc->server->auth.verify_mode == SSL_CVERIFY_REQUIRE) && ... ){
   if ( sslErrorRedirected ) return APR_SUCCESS;
   return ssl_filter_io_shutdown(filter_ctx, c, 1);
}

in ssl_engine_kernel.c: replace
SSL_do_handshake(ssl);
if (SSL_get_state(ssl) != SSL_ST_OK) {
by
SSL_do_handshake(ssl);
if (!sslErrorRedirected && SSL_get_state(ssl) != SSL_ST_OK) {

in ssl_engine_kernel.c in ssl_hook_Access(): replace
if (do_verify && (SSL_get_verify_result(ssl) != X509_V_OK)) {
   ...
   return HTTP_FORBIDDEN;
by
if (do_verify && (SSL_get_verify_result(ssl) != X509_V_OK)) {
   ...
   if (!sslErrorRedirected) return HTTP_FORBIDDEN;

in ssl_engine_kernel.c in ssl_callback_SSLVerify():
before
if (!ok) {
...
}
/*
 * And finally signal OpenSSL the (perhaps changed) state
 */
return ok;
add
if (sslErrorRedirected) ok = TRUE; /* MSTERN: SSL errors are trapped later */

I'm not sure what you're waiting for from anyone else, I thought we were agreed
on how to proceed here:

1. add a new directive which allows the admin to selectively ignore SSL cert
verification failures (per comment 8)

2. add new env vars which describe the verification error and allow a custom 403
error page to explain the issue

Creating env variables also solves point 4 from comment 7 as it can be covered
by LogFormat's and conditional CustomLogs and if the application wants it /
needs it, it can fetch these too (at least inside httpd). So Joe's approach also
saves a lot of wheel reinvention here. 

Created attachment 19457 [details]
Patch based on SSL_CVERIFY_OPTIONAL_NO_CA

I extended SSL_CVERIFY_OPTIONAL_NO_CA usage to let the session establish,
whatever validation problems happen. This seems to me consistent with comments
in teh code stating that this option should be used for testing purpose, so
everyt certificate should be accepted. If I correctly understood,
"OPTIONAL_NO_CA" should so be replaced everywhere by something like
"IGNORE_ERRORS"; if not, we should add another option.

What is the status of this request for enhancement ?
I have use for this behaviour of optional_no_ca ?

The patch is waiting for approval.
Glad to see other people manifesting their interest ;-)

I think the only real question before actually implementing it is to see if the "legacy" directive SSL_CVERIFY_OPTIONAL_NO_CA is still meaningful, or if we should replace it with something like SSL_IGNORE_ERRORS.

Otherwise, I never got any problem report.
It runs in production for 3 years (18 months for latest patch) on dozens of Web sites, in a version published and supported by Belgian government for all agencies and private sector.

Note that this is key differentiator compared to IIS and other servers.
Wouldn't it worth to integrate it ?

Hello, is this being incorporated into mod_ssl and, if yes, what is the release schedule? Thanks, Paul

Any update to this? -- it's been 4 years since Stern added it ("pending approval") and 6 years since mod_ssl_error was offered up. 

Any pointers to alternate solutions?

Thanx

I have the very same problem with revoked certificates as explained by comment 7: not being able to "trap" SSL errors in order to redirect to some sort of custom processing is very annoying.

I have a business case where customer's security policy prescribes to log any failed login attempt and this includes revoked certificates. This can not be done.

Tested with 2.2.14 on Linux (+ checked 2.2.21 source code of modules/ssl/ssl_engine_kernel.c: as far as I remember C language, the problem is still there).

Tried with rewrite rules: doesn't work, connection attempt with revoked certificate is shutdown by mod_ssl and rules are not evaluated.

RewriteEngine	on
RewriteCond	%{SSL:SSL_CLIENT_VERIFY} !=SUCCESS
RewriteRule	.? /unsecure/bad-ssl.cgi [R,L]

One alternative would be to run a frontend before Apache, if that frontend is capable of better handling of SSL errors. Quite a heavy solution (+ more admin burden)... Another alternative would be to run SSL controls at the application layer, "after" Apache handles the request to Tomcat or whatever else. This would be ill-architected.

It would be great to be able to detect SSL errors (not only revoked certificates, one can think of out-dated CRLs also) in order to redirect those cases to some custom processing, with rewrite rules for example or whatever else that is under our responsability, not Apache's code.

Willing to help in order to give "additional information before it can be dealt", Erik.



PS: "out-dated CRLs" => see 2.2.21 ssl_engine_kernel.c line 2106: from what I understand (newbie to Apache source code...), if current date is greater than next update date stored in current CRL then all client certificates are rejected. This is kind of "brute force" if the admin forgets to update CRL before the "next update" date stored in the current CRL, or if a cron job fails, or whatever like this happens...

The patch traps all validation errors, thus also expired CRL.
What about the status NEEDINFO? This should be changed. Should I put back to NEW?

(In reply to comment #23)
> The patch traps all validation errors, thus also expired CRL.
> What about the status NEEDINFO? This should be changed. Should I put back to
> NEW?

Sorry, looks like I was not clear enough in my post: the described patch is ok and I wish it is already shipped to a stable version of Apache!

Erik

How does this patch compare to the one attached to #45922 ? This one does not change optional_no_ca behaviour and create a new option_no_verify which gives the permissive behaviour we are all looking for.

After a very quick browse, I think it totally supersedes mine.

Obsolete with TLS1.2 that handles this in the TLS protocol