httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jor...@apache.org
Subject svn commit: r592446 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_io.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h
Date Tue, 06 Nov 2007 15:02:33 GMT
Author: jorton
Date: Tue Nov  6 07:02:32 2007
New Revision: 592446

URL: http://svn.apache.org/viewvc?rev=592446&view=rev
Log:
mod_ssl: Fix forever-broken TLS upgrade support; perform the upgrade
in the post_read_request hook rather than in a filter, and fix the
filter insertion issue:

* modules/ssl/ssl_engine_kernel.c (upgrade_connection): New function,
mostly moved from ssl_io_filter_Upgrade.
(ssl_hook_ReadReq): Call upgrade_connection to upgrade to TLS if
required.

* modules/ssl/ssl_engine_io.c (ssl_io_filter_Upgrade): Remove
function.
(ssl_io_input_add_filter, ssl_io_filter_init): Take a request_rec
pointer and pass to ap_add_*_filter to ensure the filter chain
is modified correctly; remove it from the filter afterwards.
(ssl_io_filter_register): Drop UPGRADE_FILTER registration.

* modules/ssl/mod_ssl.c (ssl_init_ssl_connection): Take a request_rec
pointer, pass to ssl_io_filter_init.
(ssl_hook_pre_connection): Pass NULL request_rec pointer to above.
(ssl_hook_Insert_Filter): Remove function.
(ssl_register_hooks): Drop insert_filter hook.

* modules/ssl/ssl_private.h: Update prototypes.

PR: 41231

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/ssl/mod_ssl.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
    httpd/httpd/trunk/modules/ssl/ssl_private.h

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=592446&r1=592445&r2=592446&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue Nov  6 07:02:32 2007
@@ -2,6 +2,8 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) mod_ssl: Fix TLS upgrade (RFC 2817) support.  PR 41231.  [Joe Orton]
+
   *) scoreboard: Correctly declare ap_time_process_request.
      PR 43789 [Tom Donovan <Tom.Donovan acm.org>]
 

Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=592446&r1=592445&r2=592446&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
+++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Tue Nov  6 07:02:32 2007
@@ -318,7 +318,7 @@
     return 1;
 }
 
-int ssl_init_ssl_connection(conn_rec *c)
+int ssl_init_ssl_connection(conn_rec *c, request_rec *r)
 {
     SSLSrvConfigRec *sc = mySrvConfig(c->base_server);
     SSL *ssl;
@@ -381,7 +381,7 @@
 
     SSL_set_verify_result(ssl, X509_V_OK);
 
-    ssl_io_filter_init(c, ssl);
+    ssl_io_filter_init(c, r, ssl);
 
     return APR_SUCCESS;
 }
@@ -442,19 +442,10 @@
                   "Connection to child %ld established "
                   "(server %s)", c->id, sc->vhost_id);
 
-    return ssl_init_ssl_connection(c);
+    return ssl_init_ssl_connection(c, NULL);
 }
 
 
-static void ssl_hook_Insert_Filter(request_rec *r)
-{
-    SSLSrvConfigRec *sc = mySrvConfig(r->server);
-
-    if (sc->enabled == SSL_ENABLED_OPTIONAL) {
-        ap_add_output_filter("UPGRADE_FILTER", NULL, r, r->connection);
-    }
-}
-
 /*
  *  the module registration phase
  */
@@ -479,8 +470,6 @@
     ap_hook_access_checker(ssl_hook_Access,        NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_auth_checker  (ssl_hook_Auth,          NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_post_read_request(ssl_hook_ReadReq, pre_prr,NULL, APR_HOOK_MIDDLE);
-    ap_hook_insert_filter (ssl_hook_Insert_Filter, NULL,NULL, APR_HOOK_MIDDLE);
-/*    ap_hook_handler       (ssl_hook_Upgrade,       NULL,NULL, APR_HOOK_MIDDLE); */
 
     ssl_var_register(p);
 

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=592446&r1=592445&r2=592446&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Tue Nov  6 07:02:32 2007
@@ -1170,85 +1170,6 @@
     return APR_SUCCESS;
 }
 
-#define SWITCH_STATUS_LINE "HTTP/1.1 101 Switching Protocols"
-#define UPGRADE_HEADER "Upgrade: TLS/1.0, HTTP/1.1"
-#define CONNECTION_HEADER "Connection: Upgrade"
-
-static apr_status_t ssl_io_filter_Upgrade(ap_filter_t *f,
-                                          apr_bucket_brigade *bb)
-{
-    const char *upgrade;
-    apr_bucket_brigade *upgradebb;
-    request_rec *r = f->r;
-    SSLConnRec *sslconn;
-    apr_status_t rv;
-    apr_bucket *b;
-    SSL *ssl;
-
-    /* Just remove the filter, if it doesn't work the first time, it won't
-     * work at all for this request.
-     */
-    ap_remove_output_filter(f);
-
-    /* No need to ensure that this is a server with optional SSL, the filter
-     * is only inserted if that is true.
-     */
-
-    upgrade = apr_table_get(r->headers_in, "Upgrade");
-    if (upgrade == NULL
-        || strcmp(ap_getword(r->pool, &upgrade, ','), "TLS/1.0")) {
-        /* "Upgrade: TLS/1.0, ..." header not found, don't do Upgrade */
-        return ap_pass_brigade(f->next, bb);
-    }
-
-    apr_table_unset(r->headers_out, "Upgrade");
-
-    /* Send the interim 101 response. */
-    upgradebb = apr_brigade_create(r->pool, f->c->bucket_alloc);
-
-    ap_fputstrs(f->next, upgradebb, SWITCH_STATUS_LINE, CRLF,
-                UPGRADE_HEADER, CRLF, CONNECTION_HEADER, CRLF, CRLF, NULL);
-
-    b = apr_bucket_flush_create(f->c->bucket_alloc);
-    APR_BRIGADE_INSERT_TAIL(upgradebb, b);
-
-    rv = ap_pass_brigade(f->next, upgradebb);
-    if (rv) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
-                      "could not send interim 101 Upgrade response");
-        return AP_FILTER_ERROR;
-    }
-
-    ssl_init_ssl_connection(f->c);
-
-    ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
-                  "Awaiting re-negotiation handshake");
-
-    sslconn = myConnConfig(f->c);
-    ssl = sslconn->ssl;
-
-    /* XXX: Should replace SSL_set_state with SSL_renegotiate(ssl);
-     * However, this causes failures in perl-framework currently,
-     * perhaps pre-test if we have already negotiated?
-     */
-    SSL_set_accept_state(ssl);
-    SSL_do_handshake(ssl);
-
-    if (SSL_get_state(ssl) != SSL_ST_OK) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                      "TLS Upgrade handshake failed: "
-                      "Not accepted by client!?");
-
-        return AP_FILTER_ERROR;
-    }
-
-    /* Now that we have initialized the ssl connection which added the ssl_io_filter,
-       pass the brigade off to the connection based output filters so that the
-       request can complete encrypted */
-    return ap_pass_brigade(f->c->output_filters, bb);
-
-}
-
 static apr_status_t ssl_io_filter_input(ap_filter_t *f,
                                         apr_bucket_brigade *bb,
                                         ap_input_mode_t mode,
@@ -1651,14 +1572,20 @@
     return APR_SUCCESS;
 }
 
+/* The request_rec pointer is passed in here only to ensure that the
+ * filter chain is modified correctly when doing a TLS upgrade.  It
+ * must *not* be used otherwise. */
 static void ssl_io_input_add_filter(ssl_filter_ctx_t *filter_ctx, conn_rec *c,
-                                    SSL *ssl)
+                                    request_rec *r, SSL *ssl)
 {
     bio_filter_in_ctx_t *inctx;
 
     inctx = apr_palloc(c->pool, sizeof(*inctx));
 
-    filter_ctx->pInputFilter = ap_add_input_filter(ssl_io_filter, inctx, NULL, c);
+    filter_ctx->pInputFilter = ap_add_input_filter(ssl_io_filter, inctx, r, c);
+    /* Immediately forget the request_rec pointer stored in the
+     * filter; it will go out of scope. */
+    filter_ctx->pInputFilter->r = NULL;
 
     filter_ctx->pbioRead = BIO_new(&bio_filter_in_method);
     filter_ctx->pbioRead->ptr = (void *)inctx;
@@ -1675,7 +1602,10 @@
     inctx->filter_ctx = filter_ctx;
 }
 
-void ssl_io_filter_init(conn_rec *c, SSL *ssl)
+/* The request_rec pointer is passed in here only to ensure that the
+ * filter chain is modified correctly when doing a TLS upgrade.  It
+ * must *not* be used otherwise. */
+void ssl_io_filter_init(conn_rec *c, request_rec *r, SSL *ssl)
 {
     ssl_filter_ctx_t *filter_ctx;
     server_rec *s = c->base_server;
@@ -1685,7 +1615,10 @@
 
     filter_ctx->nobuffer        = 0;
     filter_ctx->pOutputFilter   = ap_add_output_filter(ssl_io_filter,
-                                                   filter_ctx, NULL, c);
+                                                       filter_ctx, r, c);
+    /* Immediately forget the request_rec pointer stored in the
+     * filter; it will go out of scope. */
+    filter_ctx->pOutputFilter->r = NULL;
 
     filter_ctx->pbioWrite       = BIO_new(&bio_filter_out_method);
     filter_ctx->pbioWrite->ptr  = (void *)bio_filter_out_ctx_new(filter_ctx, c);
@@ -1693,7 +1626,7 @@
     /* We insert a clogging input filter. Let the core know. */
     c->clogging_input_filters = 1;
 
-    ssl_io_input_add_filter(filter_ctx, c, ssl);
+    ssl_io_input_add_filter(filter_ctx, c, r, ssl);
 
     SSL_set_bio(ssl, filter_ctx->pbioRead, filter_ctx->pbioWrite);
     filter_ctx->pssl            = ssl;
@@ -1712,11 +1645,6 @@
 
 void ssl_io_filter_register(apr_pool_t *p)
 {
-    /* This filter MUST be after the HTTP_HEADER filter, but it also must be
-     * a resource-level filter so it has the request_rec.
-     */
-    ap_register_output_filter ("UPGRADE_FILTER", ssl_io_filter_Upgrade, NULL, AP_FTYPE_PROTOCOL
+ 5);
-
     ap_register_input_filter  (ssl_io_filter, ssl_io_filter_input,  NULL, AP_FTYPE_CONNECTION
+ 5);
     ap_register_output_filter (ssl_io_filter, ssl_io_filter_output, NULL, AP_FTYPE_CONNECTION
+ 5);
 

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=592446&r1=592445&r2=592446&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Tue Nov  6 07:02:32 2007
@@ -32,14 +32,83 @@
 
 static void ssl_configure_env(request_rec *r, SSLConnRec *sslconn);
 
+#define SWITCH_STATUS_LINE "HTTP/1.1 101 Switching Protocols"
+#define UPGRADE_HEADER "Upgrade: TLS/1.0, HTTP/1.1"
+#define CONNECTION_HEADER "Connection: Upgrade"
+
+/* Perform an upgrade-to-TLS for the given request, per RFC 2817. */
+static apr_status_t upgrade_connection(request_rec *r)
+{
+    struct conn_rec *conn = r->connection;
+    apr_bucket_brigade *bb;
+    SSLConnRec *sslconn;
+    apr_status_t rv;
+    SSL *ssl;
+
+    ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, 
+                  "upgrading connection to TLS");
+
+    bb = apr_brigade_create(r->pool, conn->bucket_alloc);
+
+    rv = ap_fputstrs(conn->output_filters, bb, SWITCH_STATUS_LINE, CRLF,
+                     UPGRADE_HEADER, CRLF, CONNECTION_HEADER, CRLF, CRLF, NULL);
+    if (rv == APR_SUCCESS) {
+        APR_BRIGADE_INSERT_TAIL(bb,
+                                apr_bucket_flush_create(conn->bucket_alloc));
+        rv = ap_pass_brigade(conn->output_filters, bb);
+    }
+
+    if (rv) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "failed to send 101 interim response for connection "
+                      "upgrade");
+        return rv;
+    }
+
+    ssl_init_ssl_connection(conn, r);
+    
+    sslconn = myConnConfig(conn);
+    ssl = sslconn->ssl;
+    
+    /* XXX: Should replace SSL_set_state with SSL_renegotiate(ssl);
+     * However, this causes failures in perl-framework currently,
+     * perhaps pre-test if we have already negotiated?
+     */
+    SSL_set_accept_state(ssl);
+    SSL_do_handshake(ssl);
+
+    if (SSL_get_state(ssl) != SSL_ST_OK) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "TLS upgrade handshake failed: not accepted by client!?");
+        
+        return APR_ECONNABORTED;
+    }
+
+    return APR_SUCCESS;
+}
+
 /*
  *  Post Read Request Handler
  */
 int ssl_hook_ReadReq(request_rec *r)
 {
-    SSLConnRec *sslconn = myConnConfig(r->connection);
+    SSLSrvConfigRec *sc = mySrvConfig(r->server);
+    SSLConnRec *sslconn;
+    const char *upgrade;
     SSL *ssl;
+    
+    /* Perform TLS upgrade here if "SSLEngine optional" is configured,
+     * SSL is not already set up for this connection, and the client
+     * has sent a suitable Upgrade header. */
+    if (sc->enabled == SSL_ENABLED_OPTIONAL && !myConnConfig(r->connection)
+        && (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL
+        && strcmp(ap_getword(r->pool, &upgrade, ','), "TLS/1.0") == 0) {
+        if (upgrade_connection(r)) {
+            return HTTP_INTERNAL_SERVER_ERROR;
+        }
+    }
 
+    sslconn = myConnConfig(r->connection);
     if (!sslconn) {
         return DECLINED;
     }

Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=592446&r1=592445&r2=592446&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_private.h Tue Nov  6 07:02:32 2007
@@ -620,7 +620,7 @@
 int ssl_engine_disable(conn_rec *c);
 
 /**  I/O  */
-void         ssl_io_filter_init(conn_rec *, SSL *);
+void         ssl_io_filter_init(conn_rec *, request_rec *r, SSL *);
 void         ssl_io_filter_register(apr_pool_t *);
 long         ssl_io_data_cb(BIO *, int, MODSSL_BIO_CB_ARG_TYPE *, int, long, long);
 
@@ -642,7 +642,7 @@
 ssl_algo_t   ssl_util_algotypeof(X509 *, EVP_PKEY *); 
 char        *ssl_util_algotypestr(ssl_algo_t);
 void         ssl_util_thread_setup(apr_pool_t *);
-int          ssl_init_ssl_connection(conn_rec *c);
+int          ssl_init_ssl_connection(conn_rec *c, request_rec *r);
 
 /**  Pass Phrase Support  */
 void         ssl_pphrase_Handle(server_rec *, apr_pool_t *);



Mime
View raw message