httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rj...@apache.org
Subject svn commit: r943879 - in /httpd/httpd/branches/2.0.x: CHANGES STATUS modules/ssl/mod_ssl.h modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_io.c modules/ssl/ssl_engine_kernel.c
Date Thu, 13 May 2010 13:27:04 GMT
Author: rjung
Date: Thu May 13 13:27:03 2010
New Revision: 943879

URL: http://svn.apache.org/viewvc?rev=943879&view=rev
Log:
Merge r833582, r833593, r881222 from trunk:

SECURITY: Partial fix for CVE-2009-3555:

Reject client-initiated renegotiations; this is sufficient to prevent
the attack for any configuration which does not require renegotiation
due to per-directory/per-location access control configuration.

Configuration with per-directory/per-location access control
requirements (such as "SSLVerifyClient require") are still vulnerable
to CVE-2009-3555 with this patch applied (if using OpenSSL != 0.9.8l).

* modules/ssl/ssl_private.h (SSLConnRec): Add reneg_state field.
  (ssl_callback_Info): Renamed from ssl_callback_LogTracingState.

* modules/ssl/ssl_engine_init.c (ssl_init_ctx_callbacks): Install
  the (renamed) info callback unconditionally.

* modules/ssl/ssl_engine_io.c (ssl_filter_ctx_t): Add config pointer
  to SSLConnRec.
  (bio_filter_out_write, bio_filter_in_read): Fail with
  APR_ECONNABORTED if the reneg state is set to RENEG_ABORT.

* modules/ssl/ssl_engine_kernel.c (log_tracing_state): Factored out
  of ssl_callback_LogTracingState.
  (ssl_callback_Info): New function.

Submitted by: jorton, rpluem, rjung
Reviewed by: rjung, rpluem, pgollucci

Modified:
    httpd/httpd/branches/2.0.x/CHANGES
    httpd/httpd/branches/2.0.x/STATUS
    httpd/httpd/branches/2.0.x/modules/ssl/mod_ssl.h
    httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_init.c
    httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_io.c
    httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_kernel.c

Modified: httpd/httpd/branches/2.0.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.0.x/CHANGES?rev=943879&r1=943878&r2=943879&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.0.x/CHANGES [utf-8] Thu May 13 13:27:03 2010
@@ -1,6 +1,14 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.0.64
 
+  *) SECURITY: CVE-2009-3555 (cve.mitre.org)
+     mod_ssl: A partial fix for the TLS renegotiation prefix injection attack
+     for OpenSSL versions prior to 0.9.8l; reject any client-initiated
+     renegotiations. Any configuration which requires renegotiation for
+     per-directory/location access control is still vulnerable, unless using
+     OpenSSL 0.9.8l or later.
+     [Joe Orton, Ruediger Pluem, Rainer Jung]
+
   *) SECURITY: CVE-2010-0434 (cve.mitre.org)
      Ensure each subrequest has a shallow copy of headers_in so that the
      parent request headers are not corrupted.  Elimiates a problematic

Modified: httpd/httpd/branches/2.0.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.0.x/STATUS?rev=943879&r1=943878&r2=943879&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/STATUS (original)
+++ httpd/httpd/branches/2.0.x/STATUS Thu May 13 13:27:03 2010
@@ -124,16 +124,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
     Backport version for 2.0.x of patch:
        http://people.apache.org/~fuankg/diffs/httpd-2.0.x-ap_vhost_iterate_given_conn.diff
     +1: fuankg, wrowe, pgollucci
-  * mod_ssl: Partial fix for CVE-2009-3555
-    Trunk version of patch:
-      http://svn.apache.org/viewvc?rev=833582&view=rev
-      http://svn.apache.org/viewvc?rev=833593&view=rev
-      http://svn.apache.org/viewvc?rev=881222&view=rev
-    Patch in 2.2.x branch:
-      http://svn.apache.org/viewvc?rev=833622&view=rev
-    Backport version for 2.0.x of patch (Updated with backport of r881222):
-      http://people.apache.org/~rjung/patches/cve-2009-3555_httpd_2_0_x-v2.patch
-    +1: rjung, rpluem, pgollucci (+1 2.0.64 w/ this)
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ please place SVN revisions from trunk here, so it is easy to

Modified: httpd/httpd/branches/2.0.x/modules/ssl/mod_ssl.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.0.x/modules/ssl/mod_ssl.h?rev=943879&r1=943878&r2=943879&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/modules/ssl/mod_ssl.h (original)
+++ httpd/httpd/branches/2.0.x/modules/ssl/mod_ssl.h Thu May 13 13:27:03 2010
@@ -389,6 +389,19 @@ typedef struct {
     int is_proxy;
     int disabled;
     int non_ssl_request;
+
+    /* Track the handshake/renegotiation state for the connection so
+     * that all client-initiated renegotiations can be rejected, as a
+     * partial fix for CVE-2009-3555. */
+    enum {
+        RENEG_INIT = 0, /* Before initial handshake */
+        RENEG_REJECT, /* After initial handshake; any client-initiated
+                       * renegotiation should be rejected */
+        RENEG_ALLOW, /* A server-initated renegotiation is taking
+                      * place (as dictated by configuration) */
+        RENEG_ABORT /* Renegotiation initiated by client, abort the
+                     * connection */
+    } reneg_state;
 } SSLConnRec;
 
 typedef struct {
@@ -585,7 +598,7 @@ int          ssl_callback_proxy_cert(SSL
 int          ssl_callback_NewSessionCacheEntry(SSL *, SSL_SESSION *);
 SSL_SESSION *ssl_callback_GetSessionCacheEntry(SSL *, unsigned char *, int, int *);
 void         ssl_callback_DelSessionCacheEntry(SSL_CTX *, SSL_SESSION *);
-void         ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE, int, int);
+void         ssl_callback_Info(MODSSL_INFO_CB_ARG_TYPE, int, int);
 
 /*  Session Cache Support  */
 void         ssl_scache_init(server_rec *, apr_pool_t *);

Modified: httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_init.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_init.c?rev=943879&r1=943878&r2=943879&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_init.c Thu May 13 13:27:03 2010
@@ -464,10 +464,7 @@ static void ssl_init_ctx_callbacks(serve
     SSL_CTX_set_tmp_rsa_callback(ctx, ssl_callback_TmpRSA);
     SSL_CTX_set_tmp_dh_callback(ctx,  ssl_callback_TmpDH);
 
-    if (s->loglevel >= APLOG_DEBUG) {
-        /* this callback only logs if LogLevel >= info */
-        SSL_CTX_set_info_callback(ctx, ssl_callback_LogTracingState);
-    }
+    SSL_CTX_set_info_callback(ctx, ssl_callback_Info);
 }
 
 static void ssl_init_ctx_verify(server_rec *s,

Modified: httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_io.c?rev=943879&r1=943878&r2=943879&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_io.c Thu May 13 13:27:03 2010
@@ -102,6 +102,7 @@ typedef struct {
     ap_filter_t        *pInputFilter;
     ap_filter_t        *pOutputFilter;
     int                nobuffer; /* non-zero to prevent buffering */
+    SSLConnRec         *config;
 } ssl_filter_ctx_t;
 
 typedef struct {
@@ -192,7 +193,13 @@ static int bio_filter_out_read(BIO *bio,
 static int bio_filter_out_write(BIO *bio, const char *in, int inl)
 {
     bio_filter_out_ctx_t *outctx = (bio_filter_out_ctx_t *)(bio->ptr);
-
+    
+    /* Abort early if the client has initiated a renegotiation. */
+    if (outctx->filter_ctx->config->reneg_state == RENEG_ABORT) {
+        outctx->rc = APR_ECONNABORTED;
+        return -1;
+    }
+    
     /* when handshaking we'll have a small number of bytes.
      * max size SSL will pass us here is about 16k.
      * (16413 bytes to be exact)
@@ -465,6 +472,12 @@ static int bio_filter_in_read(BIO *bio, 
     if (!in)
         return 0;
 
+    /* Abort early if the client has initiated a renegotiation. */
+    if (inctx->filter_ctx->config->reneg_state == RENEG_ABORT) {
+        inctx->rc = APR_ECONNABORTED;
+        return -1;
+    }
+
     /* XXX: flush here only required for SSLv2;
      * OpenSSL calls BIO_flush() at the appropriate times for
      * the other protocols.
@@ -1585,6 +1598,8 @@ void ssl_io_filter_init(conn_rec *c, SSL
 
     filter_ctx = apr_palloc(c->pool, sizeof(ssl_filter_ctx_t));
 
+    filter_ctx->config          = myConnConfig(c);
+
     filter_ctx->nobuffer        = 0;
     filter_ctx->pOutputFilter   = ap_add_output_filter(ssl_io_filter,
                                                    filter_ctx, NULL, c);

Modified: httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_kernel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_kernel.c?rev=943879&r1=943878&r2=943879&view=diff
==============================================================================
--- httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/branches/2.0.x/modules/ssl/ssl_engine_kernel.c Thu May 13 13:27:03 2010
@@ -611,6 +611,10 @@ int ssl_hook_Access(request_rec *r)
                                        (unsigned char *)&id,
                                        sizeof(id));
 
+            /* Toggle the renegotiation state to allow the new
+             * handshake to proceed. */
+            sslconn->reneg_state = RENEG_ALLOW;
+            
             SSL_renegotiate(ssl);
             SSL_do_handshake(ssl);
 
@@ -628,6 +632,8 @@ int ssl_hook_Access(request_rec *r)
             SSL_set_state(ssl, SSL_ST_ACCEPT);
             SSL_do_handshake(ssl);
 
+            sslconn->reneg_state = RENEG_REJECT;
+
             if (SSL_get_state(ssl) != SSL_ST_OK) {
                 ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                              "Re-negotiation handshake failed: "
@@ -1700,76 +1706,55 @@ void ssl_callback_DelSessionCacheEntry(S
     return;
 }
 
-/*
- * This callback function is executed while OpenSSL processes the
- * SSL handshake and does SSL record layer stuff. We use it to
- * trace OpenSSL's processing in out SSL logfile.
- */
-void ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc)
+/* Dump debugginfo trace to the log file. */
+static void log_tracing_state(MODSSL_INFO_CB_ARG_TYPE ssl, conn_rec *c,
+                              server_rec *s, int where, int rc)
 {
-    conn_rec *c;
-    server_rec *s;
-    SSLSrvConfigRec *sc;
-
     /*
-     * find corresponding server
+     * create the various trace messages
      */
-    if (!(c = (conn_rec *)SSL_get_app_data((SSL *)ssl))) {
-        return;
+    if (where & SSL_CB_HANDSHAKE_START) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                     "%s: Handshake: start", SSL_LIBRARY_NAME);
     }
-
-    s = c->base_server;
-    if (!(sc = mySrvConfig(s))) {
-        return;
+    else if (where & SSL_CB_HANDSHAKE_DONE) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                     "%s: Handshake: done", SSL_LIBRARY_NAME);
     }
-
-    /*
-     * create the various trace messages
-     */
-    if (s->loglevel >= APLOG_DEBUG) {
-        if (where & SSL_CB_HANDSHAKE_START) {
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "%s: Handshake: start", SSL_LIBRARY_NAME);
-        }
-        else if (where & SSL_CB_HANDSHAKE_DONE) {
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "%s: Handshake: done", SSL_LIBRARY_NAME);
-        }
-        else if (where & SSL_CB_LOOP) {
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "%s: Loop: %s",
-                         SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
-        }
-        else if (where & SSL_CB_READ) {
+    else if (where & SSL_CB_LOOP) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                     "%s: Loop: %s",
+                     SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+    }
+    else if (where & SSL_CB_READ) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                     "%s: Read: %s",
+                     SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+    }
+    else if (where & SSL_CB_WRITE) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                     "%s: Write: %s",
+                     SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+    }
+    else if (where & SSL_CB_ALERT) {
+        char *str = (where & SSL_CB_READ) ? "read" : "write";
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+                     "%s: Alert: %s:%s:%s",
+                     SSL_LIBRARY_NAME, str,
+                     SSL_alert_type_string_long(rc),
+                     SSL_alert_desc_string_long(rc));
+    }
+    else if (where & SSL_CB_EXIT) {
+        if (rc == 0) {
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "%s: Read: %s",
+                         "%s: Exit: failed in %s",
                          SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
         }
-        else if (where & SSL_CB_WRITE) {
+        else if (rc < 0) {
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "%s: Write: %s",
+                         "%s: Exit: error in %s",
                          SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
         }
-        else if (where & SSL_CB_ALERT) {
-            char *str = (where & SSL_CB_READ) ? "read" : "write";
-            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "%s: Alert: %s:%s:%s",
-                         SSL_LIBRARY_NAME, str,
-                         SSL_alert_type_string_long(rc),
-                         SSL_alert_desc_string_long(rc));
-        }
-        else if (where & SSL_CB_EXIT) {
-            if (rc == 0) {
-                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                             "%s: Exit: failed in %s",
-                             SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
-            }
-            else if (rc < 0) {
-                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                             "%s: Exit: error in %s",
-                             SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
-            }
-        }
     }
 
     /*
@@ -1789,3 +1774,48 @@ void ssl_callback_LogTracingState(MODSSL
     }
 }
 
+/*
+ * This callback function is executed while OpenSSL processes the SSL
+ * handshake and does SSL record layer stuff.  It's used to trap
+ * client-initiated renegotiations, and for dumping everything to the
+ * log.
+ */
+void ssl_callback_Info(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc)
+{
+    conn_rec *c;
+    server_rec *s;
+    SSLConnRec *scr;
+
+    /* Retrieve the conn_rec and the associated SSLConnRec. */
+    if ((c = (conn_rec *)SSL_get_app_data((SSL *)ssl)) == NULL) {
+        return;
+    }
+
+    if ((scr = myConnConfig(c)) == NULL) {
+        return;
+    }
+
+    /* If the reneg state is to reject renegotiations, check the SSL
+     * state machine and move to ABORT if a Client Hello is being
+     * read. */
+    if ((where & SSL_CB_ACCEPT_LOOP) && scr->reneg_state == RENEG_REJECT)
{
+        int state = SSL_get_state((SSL *)ssl);
+        
+        if (state == SSL3_ST_SR_CLNT_HELLO_A 
+            || state == SSL23_ST_SR_CLNT_HELLO_A) {
+            scr->reneg_state = RENEG_ABORT;
+            ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c,
+                          "rejecting client initiated renegotiation");
+        }
+    }
+    /* If the first handshake is complete, change state to reject any
+     * subsequent client-initated renegotiation. */
+    else if ((where & SSL_CB_HANDSHAKE_DONE) && scr->reneg_state == RENEG_INIT)
{
+        scr->reneg_state = RENEG_REJECT;
+    }
+
+    s = c->base_server;
+    if (s && s->loglevel >= APLOG_DEBUG) {
+        log_tracing_state(ssl, c, s, where, rc);
+    }
+}



Mime
View raw message