ASF Bugzilla – Attachment 11736 Details for
Bug 12355
POST incompatible w/ renegotiate https: connection
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
proposed fix
ssl_reneg3.diff (text/plain), 15.15 KB, created by
Joe Orton
on 2004-06-03 09:02:32 UTC
(
hide
)
Description:
proposed fix
Filename:
MIME Type:
Creator:
Joe Orton
Created:
2004-06-03 09:02:32 UTC
Size:
15.15 KB
patch
obsolete
>? modules/ssl/clog >? modules/ssl/diff >? modules/ssl/fns >? modules/ssl/ssl_engine_kernel.c.rn1 >Index: modules/ssl/mod_ssl.c >=================================================================== >RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.c,v >retrieving revision 1.97 >diff -u -w -d -r1.97 mod_ssl.c >--- modules/ssl/mod_ssl.c 5 Mar 2004 02:41:39 -0000 1.97 >+++ modules/ssl/mod_ssl.c 2 Jun 2004 11:06:00 -0000 >@@ -469,6 +469,7 @@ > static void ssl_register_hooks(apr_pool_t *p) > { > ssl_io_filter_register(p); >+ ssl_reneg_filter_register(p); > > ap_hook_pre_connection(ssl_hook_pre_connection,NULL,NULL, APR_HOOK_MIDDLE); > ap_hook_post_config (ssl_init_Module, NULL,NULL, APR_HOOK_MIDDLE); >Index: modules/ssl/ssl_engine_kernel.c >=================================================================== >RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_kernel.c,v >retrieving revision 1.106 >diff -u -w -d -r1.106 ssl_engine_kernel.c >--- modules/ssl/ssl_engine_kernel.c 25 May 2004 12:09:01 -0000 1.106 >+++ modules/ssl/ssl_engine_kernel.c 2 Jun 2004 11:06:00 -0000 >@@ -29,6 +29,8 @@ > -- Unknown */ > #include "ssl_private.h" > >+static int reneg_and_check(request_rec *r, int quick); >+ > /* > * Post Read Request Handler > */ >@@ -159,6 +161,99 @@ > return DECLINED; > } > >+static ap_filter_rec_t *reneg_filter_rec; >+ >+/* The renegotiation input filter is inserted into the input filter >+ * stack to perform an SSL renegotatiation after the request body has >+ * been read. It runs before the HTTP input filter and waits for it to return >+ * an EOS; at which point it is safe to perform the SSL handshake. */ >+static apr_status_t reneg_in_filter(ap_filter_t *f, >+ apr_bucket_brigade *bb, >+ ap_input_mode_t mode, >+ apr_read_type_e block, >+ apr_off_t bytes) >+{ >+ apr_bucket *bkt; >+ apr_status_t rv; >+ >+ /* This filter needs to buffer each brigade into memory to ensure >+ * that when an EOS is found, all data really has been read from >+ * the socket. So, ensure that not too much is buffered: */ >+ if (bytes > HUGE_STRING_LEN) { >+ bytes = HUGE_STRING_LEN; >+ } >+ >+ rv = ap_get_brigade(f->next, bb, mode, block, bytes); >+ if (rv != APR_SUCCESS) { >+ return rv; >+ } >+ >+ for (bkt = APR_BRIGADE_FIRST(bb); >+ bkt != APR_BRIGADE_SENTINEL(bb); >+ bkt = APR_BUCKET_NEXT(bkt)) >+ { >+ if (APR_BUCKET_IS_EOS(bkt)) { >+ /* No more work for this filter. */ >+ ap_remove_input_filter(f); >+ >+ /* Now really do the negotiation and access control checks. */ >+ if (reneg_and_check(f->r, 0)) { >+ >+ /* Access control checks failed: send a 403. */ >+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r, >+ "renegotiation failed; sending 403 error"); >+ bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc); >+ bkt = ap_bucket_error_create(HTTP_FORBIDDEN, NULL, >+ f->r->pool, f->c->bucket_alloc); >+ APR_BRIGADE_INSERT_TAIL(bb, bkt); >+ bkt = apr_bucket_eos_create(f->c->bucket_alloc); >+ APR_BRIGADE_INSERT_TAIL(bb, bkt); >+ >+ rv = ap_pass_brigade(f->r->output_filters, bb); >+ if (rv) >+ ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, f->r, >+ "could not send 403 after renegotiation" >+ " failure"); >+ >+ /* Don't give anything back to the caller, just return >+ * an error. */ >+ apr_brigade_cleanup(bb); >+ return APR_EACCES; >+ } >+ >+ /* ignore the rest of the brigade */ >+ break; >+ } >+ >+ /* For any non-metadata buckets, read from the bucket to >+ * ensure that it is morphed into a heap bucket if it's a >+ * morphing bucket type. */ >+ if (!APR_BUCKET_IS_METADATA(bkt)) { >+ const char *buf; >+ apr_size_t len; >+ >+ rv = apr_bucket_read(bkt, &buf, &len, block); >+ if (rv) { >+ ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, f->r, >+ "bucket read failed"); >+ apr_brigade_cleanup(bb); >+ return rv; >+ } >+ } >+ } >+ >+ return APR_SUCCESS; >+} >+ >+void ssl_reneg_filter_register(apr_pool_t *p) >+{ >+ /* only requirement is that this input filter comes before the >+ * ap_http_filter. */ >+ reneg_filter_rec = >+ ap_register_input_filter("SSL_RENEG", reneg_in_filter, NULL, >+ AP_FTYPE_PROTOCOL - 1); >+} >+ > /* > * Access Handler > */ >@@ -169,18 +264,12 @@ > SSLConnRec *sslconn = myConnConfig(r->connection); > SSL *ssl = sslconn ? sslconn->ssl : NULL; > SSL_CTX *ctx = NULL; >- apr_array_header_t *requires; >- ssl_require_t *ssl_requires; > char *cp; >- int ok, i; > BOOL renegotiate = FALSE, renegotiate_quick = FALSE; >- X509 *cert; > X509 *peercert; >- X509_STORE *cert_store = NULL; >- X509_STORE_CTX cert_store_ctx; > STACK_OF(SSL_CIPHER) *cipher_list_old = NULL, *cipher_list = NULL; > SSL_CIPHER *cipher = NULL; >- int depth, verify_old, verify, n; >+ int verify_old, verify, n; > > if (ssl) { > ctx = SSL_get_SSL_CTX(ssl); >@@ -499,79 +588,56 @@ > } > #endif /* HAVE_SSL_SET_CERT_STORE */ > >- /* >- * SSL renegotiations in conjunction with HTTP >- * requests using the POST method are not supported. >- * >- * Background: >- * >- * 1. When the client sends a HTTP/HTTPS request, Apache's core code >- * reads only the request line ("METHOD /path HTTP/x.y") and the >- * attached MIME headers ("Foo: bar") up to the terminating line ("CR >- * LF"). An attached request body (for instance the data of a POST >- * method) is _NOT_ read. Instead it is read by mod_cgi's content >- * handler and directly passed to the CGI script. >- * >- * 2. mod_ssl supports per-directory re-configuration of SSL parameters. >- * This is implemented by performing an SSL renegotiation of the >- * re-configured parameters after the request is read, but before the >- * response is sent. In more detail: the renegotiation happens after the >- * request line and MIME headers were read, but _before_ the attached >- * request body is read. The reason simply is that in the HTTP protocol >- * usually there is no acknowledgment step between the headers and the >- * body (there is the 100-continue feature and the chunking facility >- * only), so Apache has no API hook for this step. >- * >- * 3. the problem now occurs when the client sends a POST request for >- * URL /foo via HTTPS the server and the server has SSL parameters >- * re-configured on a per-URL basis for /foo. Then mod_ssl has to >- * perform an SSL renegotiation after the request was read and before >- * the response is sent. But the problem is the pending POST body data >- * in the receive buffer of SSL (which Apache still has not read - it's >- * pending until mod_cgi sucks it in). When mod_ssl now tries to perform >- * the renegotiation the pending data leads to an I/O error. >- * >- * Solution Idea: >- * >- * There are only two solutions: Either to simply state that POST >- * requests to URLs with SSL re-configurations are not allowed, or to >- * renegotiate really after the _complete_ request (i.e. including >- * the POST body) was read. Obviously the latter would be preferred, >- * but it cannot be done easily inside Apache, because as already >- * mentioned, there is no API step between the body reading and the body >- * processing. And even when we mod_ssl would hook directly into the >- * loop of mod_cgi, we wouldn't solve the problem for other handlers, of >- * course. So the only general solution is to suck in the pending data >- * of the request body from the OpenSSL BIO into the Apache BUFF. Then >- * the renegotiation can be done and after this step Apache can proceed >- * processing the request as before. >- * >- * Solution Implementation: >- * >- * We cannot simply suck in the data via an SSL_read-based loop because of >- * HTTP chunking. Instead we _have_ to use the Apache API for this step which >- * is aware of HTTP chunking. So the trick is to suck in the pending request >- * data via the Apache API (which uses Apache's BUFF code and in the >- * background mod_ssl's I/O glue code) and re-inject it later into the Apache >- * BUFF code again. This way the data flows twice through the Apache BUFF, of >- * course. But this way the solution doesn't depend on any Apache specifics >- * and is fully transparent to Apache modules. >- * >- * !! BUT ALL THIS IS STILL NOT RE-IMPLEMENTED FOR APACHE 2.0 !! >- */ >- if (renegotiate && !renegotiate_quick && (r->method_number == M_POST)) { >- ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, >- "SSL Re-negotiation in conjunction " >- "with POST method not supported!\n" >- "hint: try SSLOptions +OptRenegotiate"); >+ if (!renegotiate) { >+ /* Nothing more to do here. */ >+ return DECLINED; >+ } > >- return HTTP_METHOD_NOT_ALLOWED; >+ /* This function is called after reading the request-header. If >+ * the HTTP request includes a message body, then the client may >+ * already have sent all the SSL records containing that body. >+ * It's not possible to do a renegotiation until all those SSL >+ * records have been read and processed, so the renegotiation has >+ * to be delayed until that point (when an EOS is returned by the >+ * HTTP input filter). */ >+ >+ if (!renegotiate_quick && >+ (apr_table_get(r->headers_in, "Transfer-Encoding") || >+ ((cp = (char *)apr_table_get(r->headers_in, "Content-Length")) != NULL >+ && strcmp(cp, "0")))) { >+ ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, >+ "SSL re-negotiation needed for request with body: " >+ "delaying until after body has been read"); >+ ap_add_input_filter_handle(reneg_filter_rec, NULL, r, r->connection); >+ >+ return DECLINED; > } > >- /* >- * now do the renegotiation if anything was actually reconfigured >- */ >- if (renegotiate) { >+ /* Now actually perform the SSL renegotiation; return DECLINED >+ * here instead of OK, to allow mod_auth and other modules to deny >+ * access. */ >+ return (reneg_and_check(r, renegotiate_quick) >+ ? HTTP_FORBIDDEN : DECLINED); >+} >+ >+/* Do an SSL renegotiation and perform access control checks; do a >+ * "quick" renegotation (which doesn't actually perform an SSL >+ * handshake), if 'quick' is non-zero. Returns non-zero if access >+ * control checks failed. */ >+static int reneg_and_check(request_rec *r, int quick) >+{ >+ SSLDirConfigRec *dc = myDirConfig(r); >+ SSLConnRec *sslconn = myConnConfig(r->connection); >+ SSL *ssl = sslconn ? sslconn->ssl : NULL; >+ apr_array_header_t *requires; >+ ssl_require_t *ssl_requires; >+ X509_STORE *cert_store = NULL; >+ X509_STORE_CTX cert_store_ctx; >+ X509 *cert; >+ SSL_CTX *ctx = SSL_get_SSL_CTX(ssl); >+ int depth, i, ok; >+ char *cp; >+ > /* > * Now we force the SSL renegotation by sending the Hello Request > * message to the client. Here we have to do a workaround: Actually >@@ -586,7 +652,7 @@ > ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, > "Requesting connection re-negotiation"); > >- if (renegotiate_quick) { >+ if (quick) { > STACK_OF(X509) *cert_stack; > > /* perform just a manual re-verification of the peer */ >@@ -612,7 +678,7 @@ > ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, > "Cannot find peer certificate chain"); > >- return HTTP_FORBIDDEN; >+ return 1; > } > > if (!(cert_store || >@@ -621,7 +687,7 @@ > ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, > "Cannot find certificate storage"); > >- return HTTP_FORBIDDEN; >+ return 1; > } > > if (!cert) { >@@ -673,7 +739,7 @@ > "Re-negotiation request failed"); > > r->connection->aborted = 1; >- return HTTP_FORBIDDEN; >+ return 1; > } > > ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, >@@ -692,7 +758,7 @@ > "Not accepted by client!?"); > > r->connection->aborted = 1; >- return HTTP_FORBIDDEN; >+ return 1; > } > } > >@@ -717,23 +783,22 @@ > ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, > "Re-negotiation handshake failed: " > "Client verification failed"); >- >- return HTTP_FORBIDDEN; >+ return 1; > } > > if (do_verify) { >+ X509 *peercert; >+ > if ((peercert = SSL_get_peer_certificate(ssl)) == NULL) { > ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, > "Re-negotiation handshake failed: " > "Client certificate missing"); >- >- return HTTP_FORBIDDEN; >+ return 1; > } > > X509_free(peercert); > } > } >- } > > /* > * Check SSLRequire boolean expressions >@@ -758,7 +823,7 @@ > /* remember forbidden access for strict require option */ > apr_table_setn(r->notes, "ssl-access-forbidden", "1"); > >- return HTTP_FORBIDDEN; >+ return 1; > } > > if (ok != 1) { >@@ -779,18 +844,11 @@ > /* remember forbidden access for strict require option */ > apr_table_setn(r->notes, "ssl-access-forbidden", "1"); > >- return HTTP_FORBIDDEN; >+ return 1; > } > } > >- /* >- * Else access is granted from our point of view (except vendor >- * handlers override). But we have to return DECLINED here instead >- * of OK, because mod_auth and other modules still might want to >- * deny access. >- */ >- >- return DECLINED; >+ return 0; > } > > /* >Index: modules/ssl/ssl_private.h >=================================================================== >RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_private.h,v >retrieving revision 1.4 >diff -u -w -d -r1.4 ssl_private.h >--- modules/ssl/ssl_private.h 27 May 2004 09:20:00 -0000 1.4 >+++ modules/ssl/ssl_private.h 2 Jun 2004 11:06:01 -0000 >@@ -577,6 +577,7 @@ > void ssl_io_filter_init(conn_rec *, SSL *); > void ssl_io_filter_register(apr_pool_t *); > long ssl_io_data_cb(BIO *, int, MODSSL_BIO_CB_ARG_TYPE *, int, long, long); >+void ssl_reneg_filter_register(apr_pool_t *p); > > /* PRNG */ > int ssl_rand_seed(server_rec *, apr_pool_t *, ssl_rsctx_t, char *);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 12355
:
9626
|
11681
|
11736
|
11745
|
16491
|
16495