You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2012/08/17 13:59:45 UTC

svn commit: r1374214 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c

Author: jorton
Date: Fri Aug 17 11:59:45 2012
New Revision: 1374214

URL: http://svn.apache.org/viewvc?rev=1374214&view=rev
Log:
* modules/ssl/ssl_engine_init.c (ssl_init_proxy_certs): Fix test for
  missing decrypted private keys, and ensure that the keypair matches.

PR: 52212
Submitted by: Keith Burdis <keith burdis.org>, jorton

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1374214&r1=1374213&r2=1374214&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 17 11:59:45 2012
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_ssl: Catch missing or mismatched client cert/key pairs with
+     SSLProxyCACertificateFile/Path directives.  PR 52212.  
+     [Keith Burdis <keith burdis.org>, Joe Orton]
+
   *) mod_lua: Allow scripts handled by the lua-script handler to return 
      a status code to the client (such as a 302 or a 500) [Daniel Gruno]
 

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?rev=1374214&r1=1374213&r2=1374214&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Fri Aug 17 11:59:45 2012
@@ -1381,7 +1381,7 @@ static void ssl_init_proxy_certs(server_
     for (n = 0; n < ncerts; n++) {
         X509_INFO *inf = sk_X509_INFO_value(sk, n);
 
-        if (!inf->x509 || !inf->x_pkey) {
+        if (!inf->x509 || !inf->x_pkey || !inf->x_pkey->dec_pkey) {
             sk_X509_INFO_free(sk);
             ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, s, APLOGNO(02252)
                          "incomplete client cert configured for SSL proxy "
@@ -1389,6 +1389,15 @@ static void ssl_init_proxy_certs(server_
             ssl_die(s);
             return;
         }
+        
+        if (X509_check_private_key(inf->x509, inf->x_pkey->dec_pkey) != 1) {
+            ssl_log_xerror(SSLLOG_MARK, APLOG_STARTUP, 0, ptemp, s, inf->x509,
+                           APLOGNO(02326) "proxy client certificate and "
+                           "private key do not match");
+            ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, s);
+            ssl_die(s);
+            return;
+        }
     }
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02207)
@@ -1412,6 +1421,8 @@ static void ssl_init_proxy_certs(server_
         ssl_die(s);
     }
 
+    /* ### Why is all the following done?  Why is it necessary or
+     * useful for the server to try to verify its own client cert? */
     X509_STORE_load_locations(store, pkp->ca_cert_file, NULL);
 
     for (n = 0; n < ncerts; n++) {



Re: svn commit: r1374214 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c

Posted by Joe Orton <jo...@redhat.com>.
On Sat, Aug 18, 2012 at 09:00:00AM +0200, Kaspar Brand wrote:
> On 17.8.12 13:59, jorton@apache.org wrote:
> > @@ -1412,6 +1421,8 @@ static void ssl_init_proxy_certs(server_
> >          ssl_die(s);
> >      }
> >  
> > +    /* ### Why is all the following done?  Why is it necessary or
> > +     * useful for the server to try to verify its own client cert? */
> 
> It's the somewhat surprising way to let OpenSSL build the chain of the
> client cert, cf.
> 
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E620C5E.3050802@opensslfoundation.com%3E
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E61C3CE.4020500@velox.ch%3E
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E625521.3000905@opensslfoundation.com%3E

Ah, I see.  Thanks Kaspar.  I've updated the comment.

Regards, Joe

Re: svn commit: r1374214 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c

Posted by Dr Stephen Henson <sh...@opensslfoundation.com>.
On 18/08/2012 08:00, Kaspar Brand wrote:
> On 17.8.12 13:59, jorton@apache.org wrote:
>> Author: jorton
>> Date: Fri Aug 17 11:59:45 2012
>> New Revision: 1374214
>>
>> URL: http://svn.apache.org/viewvc?rev=1374214&view=rev
>> Log:
>> * modules/ssl/ssl_engine_init.c (ssl_init_proxy_certs): Fix test for
>>   missing decrypted private keys, and ensure that the keypair matches.
> 
> [...]
> 
>> @@ -1412,6 +1421,8 @@ static void ssl_init_proxy_certs(server_
>>          ssl_die(s);
>>      }
>>  
>> +    /* ### Why is all the following done?  Why is it necessary or
>> +     * useful for the server to try to verify its own client cert? */
> 
> It's the somewhat surprising way to let OpenSSL build the chain of the
> client cert, cf.
> 
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E620C5E.3050802@opensslfoundation.com%3E
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E61C3CE.4020500@velox.ch%3E
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E625521.3000905@opensslfoundation.com%3E
> 

Though this quirk has been addressed in the current development version of
OpenSSL. It's now possible to set certificate chains on a per-type and per-SSL
context basis, instead of one per parent SSL_CTX. The functionality is likely to
be back ported to OpenSSL 1.0.2.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shenson@opensslfoundation.com

Re: svn commit: r1374214 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c

Posted by Kaspar Brand <ht...@velox.ch>.
On 17.8.12 13:59, jorton@apache.org wrote:
> Author: jorton
> Date: Fri Aug 17 11:59:45 2012
> New Revision: 1374214
> 
> URL: http://svn.apache.org/viewvc?rev=1374214&view=rev
> Log:
> * modules/ssl/ssl_engine_init.c (ssl_init_proxy_certs): Fix test for
>   missing decrypted private keys, and ensure that the keypair matches.

[...]

> @@ -1412,6 +1421,8 @@ static void ssl_init_proxy_certs(server_
>          ssl_die(s);
>      }
>  
> +    /* ### Why is all the following done?  Why is it necessary or
> +     * useful for the server to try to verify its own client cert? */

It's the somewhat surprising way to let OpenSSL build the chain of the
client cert, cf.

http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E620C5E.3050802@opensslfoundation.com%3E
http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E61C3CE.4020500@velox.ch%3E
http://mail-archives.apache.org/mod_mbox/httpd-dev/201109.mbox/%3C4E625521.3000905@opensslfoundation.com%3E

Kaspar