You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1)" <ma...@hp.com> on 2002/10/29 04:26:36 UTC

[PATCH] fix memory leaks in mod_ssl

Hi,
	Fixes the memory leaks introduced by mod_ssl due to inappropriate
use of SSL_get_peer_certificate() and X509_get_pubkey(). The patch is based
on the following mails in the mod_ssl community.
http://marc.theaimsgroup.com/?l=apache-modssl&m=103571508214721&w=2
http://marc.theaimsgroup.com/?l=apache-modssl&m=103558366114293&w=2

-Madhu


Index: mod_ssl.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/mod_ssl.c,v
retrieving revision 1.72
diff -u -r1.72 mod_ssl.c
--- mod_ssl.c   14 Oct 2002 04:15:57 -0000      1.72
+++ mod_ssl.c   29 Oct 2002 03:14:13 -0000
@@ -531,6 +531,7 @@
         if ((cert = SSL_get_peer_certificate(filter->pssl))) {
             sslconn->client_cert = cert;
             sslconn->client_dn = NULL;
+            X509_free(cert);
         }

         /*
Index: ssl_engine_init.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/ssl_engine_init.c,v
retrieving revision 1.104
diff -u -r1.104 ssl_engine_init.c
--- ssl_engine_init.c   14 Oct 2002 04:15:58 -0000      1.104
+++ ssl_engine_init.c   29 Oct 2002 03:14:13 -0000
@@ -807,6 +807,7 @@
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
                     "Copying DSA parameters from private key to
certificate");
             ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, s);
+            EVP_PKEY_free(pubkey);
         }
     }
Index: ssl_engine_kernel.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/ssl_engine_kernel.c,v
retrieving revision 1.80
diff -u -r1.80 ssl_engine_kernel.c
--- ssl_engine_kernel.c 27 Oct 2002 03:43:03 -0000      1.80
+++ ssl_engine_kernel.c 29 Oct 2002 03:14:13 -0000
@@ -545,9 +545,10 @@

                 if ((dc->nOptions & SSL_OPT_OPTRENEGOTIATE) &&
                     (verify_old == SSL_VERIFY_NONE) &&
-                    SSL_get_peer_certificate(ssl))
+                    ((cert = SSL_get_peer_certificate(ssl)) != NULL))
                 {
                     renegotiate_quick = TRUE;
+                    X509_free(cert);
                 }

                 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
@@ -817,6 +818,7 @@
         if ((cert = SSL_get_peer_certificate(ssl))) {
             sslconn->client_cert = cert;
             sslconn->client_dn = NULL;
+            X509_free(cert);
         }

         /*
@@ -833,7 +835,8 @@
                 return HTTP_FORBIDDEN;
             }

-            if (do_verify && !SSL_get_peer_certificate(ssl)) {
+            if (do_verify &&
+                ((cert = SSL_get_peer_certificate(ssl)) == NULL)) {
                 ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                              "Re-negotiation handshake failed: "
                              "Client certificate missing");
@@ -1399,6 +1402,7 @@
     X509_NAME *subject, *issuer;
     X509 *cert;
     X509_CRL *crl;
+    EVP_PKEY *pubkey;
     int i, n, rc;

     /*
@@ -1485,15 +1489,21 @@
         /*
          * Verify the signature on this CRL
          */
-        if (X509_CRL_verify(crl, X509_get_pubkey(cert)) <= 0) {
+        pubkey = X509_get_pubkey(cert);
+        if (X509_CRL_verify(crl, pubkey) <= 0) {
             ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
                          "Invalid signature on CRL");

             X509_STORE_CTX_set_error(ctx,
X509_V_ERR_CRL_SIGNATURE_FAILURE);
             X509_OBJECT_free_contents(&obj);
+            if (pubkey)
+                EVP_PKEY_free(pubkey);

             return FALSE;
         }
+
+        if (pubkey)
+            EVP_PKEY_free(pubkey);

         /*
          * Check date of CRL to make sure it's not expired
Index: ssl_engine_vars.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/ssl_engine_vars.c,v
retrieving revision 1.21
diff -u -r1.21 ssl_engine_vars.c
--- ssl_engine_vars.c   25 Oct 2002 21:44:28 -0000      1.21
+++ ssl_engine_vars.c   29 Oct 2002 03:14:13 -0000
@@ -296,8 +296,10 @@
         result = ssl_var_lookup_ssl_cert_verify(p, c);
     }
     else if (ssl != NULL && strlen(var) > 7 && strcEQn(var, "CLIENT_", 7))
{
-        if ((xs = SSL_get_peer_certificate(ssl)) != NULL)
+        if ((xs = SSL_get_peer_certificate(ssl)) != NULL) {
             result = ssl_var_lookup_ssl_cert(p, xs, var+7);
+            X509_free(xs);
+        }
     }
     else if (ssl != NULL && strlen(var) > 7 && strcEQn(var, "SERVER_", 7))
{
         if ((xs = SSL_get_certificate(ssl)) != NULL)
@@ -536,6 +538,9 @@
     else
         /* client verification failed */
         result = apr_psprintf(p, "FAILED:%s", verr);
+
+    if (xs)
+        X509_free(xs);
     return result;
 }

Re: [PATCH] fix memory leaks in mod_ssl

Posted by Cliff Woolley <jw...@virginia.edu>.
On Tue, 29 Oct 2002, Cliff Woolley wrote:

> On Mon, 28 Oct 2002, MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1) wrote:
>
> > 	Fixes the memory leaks introduced by mod_ssl due to inappropriate
> > use of SSL_get_peer_certificate() and X509_get_pubkey(). The patch is based
> > on the following mails in the mod_ssl community.
> > http://marc.theaimsgroup.com/?l=apache-modssl&m=103571508214721&w=2
> > http://marc.theaimsgroup.com/?l=apache-modssl&m=103558366114293&w=2

Uh oops, OtherBill already beat me to it.  Thanks Bill!  :)


Re: [PATCH] fix memory leaks in mod_ssl

Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 28 Oct 2002, MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1) wrote:

> 	Fixes the memory leaks introduced by mod_ssl due to inappropriate
> use of SSL_get_peer_certificate() and X509_get_pubkey(). The patch is based
> on the following mails in the mod_ssl community.
> http://marc.theaimsgroup.com/?l=apache-modssl&m=103571508214721&w=2
> http://marc.theaimsgroup.com/?l=apache-modssl&m=103558366114293&w=2

Thanks for the reminder... this is on my list of things to do for
tomorrow.  (I'd have done it today but I ended up having to be in Richmond
all day unexpectedly.)

--Cliff