Skip to content

Commit ed1ef0c

Browse files
notrojnotroj
committed
Merge r290965 from trunk:
Implement a (bounded) buffer of request body data to provide a limited but safe fix for the mod_ssl renegotiation-vs-requests-with-bodies bug: * modules/ssl/ssl_private.h (ssl_io_buffer_fill): Add prototype. * modules/ssl/ssl_engine_io.c (ssl_io_buffer_fill, ssl_io_filter_buffer): New functions. * modules/ssl/ssl_engine_kernel.c (ssl_hook_Access): If a renegotiation is needed, and the request has a non-zero content-length, or a t-e header (and 100-continue was not requested), call ssl_io_buffer_fill to set aside the request body data if possible, then proceed with the negotiation. PR: 12355 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@291425 13f79535-47bb-0310-9956-ffa450edef68
1 parent 529fb68 commit ed1ef0c

4 files changed

Lines changed: 220 additions & 66 deletions

File tree

‎CHANGES‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
-*- coding: utf-8 -*-
22
Changes with Apache 2.1.8
33

4+
*) mod_ssl: Support limited buffering of request bodies to allow
5+
per-location renegotiation to proceed. PR 12355. [Joe Orton]
6+
47
*) Fix regression since 2.0.x in AllowOverride Options handling.
58
PR 35330. [kabe <kabe sra-tohoku.co.jp>]
69

‎modules/ssl/ssl_engine_io.c‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,7 @@ static apr_status_t ssl_io_filter_error(ap_filter_t *f,
882882
}
883883

884884
static const char ssl_io_filter[] = "SSL/TLS Filter";
885+
static const char ssl_io_buffer[] = "SSL/TLS Buffer";
885886

886887
/*
887888
* Close the SSL part of the socket connection
@@ -1446,6 +1447,187 @@ static apr_status_t ssl_io_filter_output(ap_filter_t *f,
14461447
return status;
14471448
}
14481449

1450+
/* 128K maximum buffer size by default. */
1451+
#ifndef SSL_MAX_IO_BUFFER
1452+
#define SSL_MAX_IO_BUFFER (128 * 1024)
1453+
#endif
1454+
1455+
struct modssl_buffer_ctx {
1456+
apr_bucket_brigade *bb;
1457+
apr_pool_t *pool;
1458+
};
1459+
1460+
int ssl_io_buffer_fill(request_rec *r)
1461+
{
1462+
conn_rec *c = r->connection;
1463+
struct modssl_buffer_ctx *ctx;
1464+
apr_bucket_brigade *tempb;
1465+
apr_off_t total = 0; /* total length buffered */
1466+
int eos = 0; /* non-zero once EOS is seen */
1467+
1468+
/* Create the context which will be passed to the input filter;
1469+
* containing a setaside pool and a brigade which constrain the
1470+
* lifetime of the buffered data. */
1471+
ctx = apr_palloc(r->pool, sizeof *ctx);
1472+
apr_pool_create(&ctx->pool, r->pool);
1473+
ctx->bb = apr_brigade_create(ctx->pool, c->bucket_alloc);
1474+
1475+
/* ... and a temporary brigade. */
1476+
tempb = apr_brigade_create(r->pool, c->bucket_alloc);
1477+
1478+
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, "filling buffer");
1479+
1480+
do {
1481+
apr_status_t rv;
1482+
apr_bucket *e, *next;
1483+
1484+
/* The request body is read from the protocol-level input
1485+
* filters; the buffering filter will reinject it from that
1486+
* level, allowing content/resource filters to run later, if
1487+
* necessary. */
1488+
1489+
rv = ap_get_brigade(r->proto_input_filters, tempb, AP_MODE_READBYTES,
1490+
APR_BLOCK_READ, 8192);
1491+
if (rv) {
1492+
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
1493+
"could not read request body for SSL buffer");
1494+
return HTTP_INTERNAL_SERVER_ERROR;
1495+
}
1496+
1497+
/* Iterate through the returned brigade: setaside each bucket
1498+
* into the context's pool and move it into the brigade. */
1499+
for (e = APR_BRIGADE_FIRST(tempb);
1500+
e != APR_BRIGADE_SENTINEL(tempb) && !eos; e = next) {
1501+
const char *data;
1502+
apr_size_t len;
1503+
1504+
next = APR_BUCKET_NEXT(e);
1505+
1506+
if (APR_BUCKET_IS_EOS(e)) {
1507+
eos = 1;
1508+
} else if (!APR_BUCKET_IS_METADATA(e)) {
1509+
rv = apr_bucket_read(e, &data, &len, APR_BLOCK_READ);
1510+
if (rv != APR_SUCCESS) {
1511+
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
1512+
"could not read bucket for SSL buffer");
1513+
return HTTP_INTERNAL_SERVER_ERROR;
1514+
}
1515+
total += len;
1516+
}
1517+
1518+
rv = apr_bucket_setaside(e, ctx->pool);
1519+
if (rv != APR_SUCCESS) {
1520+
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
1521+
"could not setaside bucket for SSL buffer");
1522+
return HTTP_INTERNAL_SERVER_ERROR;
1523+
}
1524+
1525+
APR_BUCKET_REMOVE(e);
1526+
APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
1527+
}
1528+
1529+
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
1530+
"total of %" APR_OFF_T_FMT " bytes in buffer, eos=%d",
1531+
total, eos);
1532+
1533+
/* Fail if this exceeds the maximum buffer size. */
1534+
if (total > SSL_MAX_IO_BUFFER) {
1535+
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
1536+
"request body exceeds maximum size for SSL buffer");
1537+
return HTTP_REQUEST_ENTITY_TOO_LARGE;
1538+
}
1539+
1540+
} while (!eos);
1541+
1542+
apr_brigade_destroy(tempb);
1543+
1544+
/* Insert the filter which will supply the buffered data. */
1545+
ap_add_input_filter(ssl_io_buffer, ctx, r, c);
1546+
1547+
return 0;
1548+
}
1549+
1550+
/* This input filter supplies the buffered request body to the caller
1551+
* from the brigade stored in f->ctx. */
1552+
static apr_status_t ssl_io_filter_buffer(ap_filter_t *f,
1553+
apr_bucket_brigade *bb,
1554+
ap_input_mode_t mode,
1555+
apr_read_type_e block,
1556+
apr_off_t bytes)
1557+
{
1558+
struct modssl_buffer_ctx *ctx = f->ctx;
1559+
apr_status_t rv;
1560+
1561+
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c,
1562+
"read from buffered SSL brigade, mode %d, "
1563+
"%" APR_OFF_T_FMT " bytes",
1564+
mode, bytes);
1565+
1566+
if (mode != AP_MODE_READBYTES && mode != AP_MODE_GETLINE) {
1567+
return APR_ENOTIMPL;
1568+
}
1569+
1570+
if (mode == AP_MODE_READBYTES) {
1571+
apr_bucket *e;
1572+
1573+
/* Partition the buffered brigade. */
1574+
rv = apr_brigade_partition(ctx->bb, bytes, &e);
1575+
if (rv && rv != APR_INCOMPLETE) {
1576+
ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, f->c,
1577+
"could not partition buffered SSL brigade");
1578+
ap_remove_input_filter(f);
1579+
return rv;
1580+
}
1581+
1582+
/* If the buffered brigade contains less then the requested
1583+
* length, just pass it all back. */
1584+
if (rv == APR_INCOMPLETE) {
1585+
APR_BRIGADE_CONCAT(bb, ctx->bb);
1586+
} else {
1587+
apr_bucket *d = APR_BRIGADE_FIRST(ctx->bb);
1588+
1589+
e = APR_BUCKET_PREV(e);
1590+
1591+
/* Unsplice the partitioned segment and move it into the
1592+
* passed-in brigade; no convenient way to do this with
1593+
* the APR_BRIGADE_* macros. */
1594+
APR_RING_UNSPLICE(d, e, link);
1595+
APR_RING_SPLICE_HEAD(&bb->list, d, e, apr_bucket, link);
1596+
1597+
APR_BRIGADE_CHECK_CONSISTENCY(bb);
1598+
APR_BRIGADE_CHECK_CONSISTENCY(ctx->bb);
1599+
}
1600+
}
1601+
else {
1602+
/* Split a line into the passed-in brigade. */
1603+
rv = apr_brigade_split_line(bb, ctx->bb, mode, bytes);
1604+
1605+
if (rv) {
1606+
ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, f->c,
1607+
"could not split line from buffered SSL brigade");
1608+
ap_remove_input_filter(f);
1609+
return rv;
1610+
}
1611+
}
1612+
1613+
if (APR_BRIGADE_EMPTY(ctx->bb)) {
1614+
apr_bucket *e = APR_BRIGADE_LAST(bb);
1615+
1616+
/* Ensure that the brigade is terminated by an EOS if the
1617+
* buffered request body has been entirely consumed. */
1618+
if (e == APR_BRIGADE_SENTINEL(bb) || !APR_BUCKET_IS_EOS(e)) {
1619+
e = apr_bucket_eos_create(f->c->bucket_alloc);
1620+
APR_BRIGADE_INSERT_TAIL(bb, e);
1621+
}
1622+
1623+
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c,
1624+
"buffered SSL brigade now exhausted; removing filter");
1625+
ap_remove_input_filter(f);
1626+
}
1627+
1628+
return APR_SUCCESS;
1629+
}
1630+
14491631
static void ssl_io_input_add_filter(ssl_filter_ctx_t *filter_ctx, conn_rec *c,
14501632
SSL *ssl)
14511633
{
@@ -1508,6 +1690,9 @@ void ssl_io_filter_register(apr_pool_t *p)
15081690

15091691
ap_register_input_filter (ssl_io_filter, ssl_io_filter_input, NULL, AP_FTYPE_CONNECTION + 5);
15101692
ap_register_output_filter (ssl_io_filter, ssl_io_filter_output, NULL, AP_FTYPE_CONNECTION + 5);
1693+
1694+
ap_register_input_filter (ssl_io_buffer, ssl_io_filter_buffer, NULL, AP_FTYPE_PROTOCOL - 1);
1695+
15111696
return;
15121697
}
15131698

‎modules/ssl/ssl_engine_kernel.c‎

Lines changed: 28 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -490,73 +490,35 @@ int ssl_hook_Access(request_rec *r)
490490
}
491491
#endif /* HAVE_SSL_SET_CERT_STORE */
492492

493-
/*
494-
* SSL renegotiations in conjunction with HTTP
495-
* requests using the POST method are not supported.
496-
*
497-
* Background:
498-
*
499-
* 1. When the client sends a HTTP/HTTPS request, Apache's core code
500-
* reads only the request line ("METHOD /path HTTP/x.y") and the
501-
* attached MIME headers ("Foo: bar") up to the terminating line ("CR
502-
* LF"). An attached request body (for instance the data of a POST
503-
* method) is _NOT_ read. Instead it is read by mod_cgi's content
504-
* handler and directly passed to the CGI script.
505-
*
506-
* 2. mod_ssl supports per-directory re-configuration of SSL parameters.
507-
* This is implemented by performing an SSL renegotiation of the
508-
* re-configured parameters after the request is read, but before the
509-
* response is sent. In more detail: the renegotiation happens after the
510-
* request line and MIME headers were read, but _before_ the attached
511-
* request body is read. The reason simply is that in the HTTP protocol
512-
* usually there is no acknowledgment step between the headers and the
513-
* body (there is the 100-continue feature and the chunking facility
514-
* only), so Apache has no API hook for this step.
515-
*
516-
* 3. the problem now occurs when the client sends a POST request for
517-
* URL /foo via HTTPS the server and the server has SSL parameters
518-
* re-configured on a per-URL basis for /foo. Then mod_ssl has to
519-
* perform an SSL renegotiation after the request was read and before
520-
* the response is sent. But the problem is the pending POST body data
521-
* in the receive buffer of SSL (which Apache still has not read - it's
522-
* pending until mod_cgi sucks it in). When mod_ssl now tries to perform
523-
* the renegotiation the pending data leads to an I/O error.
524-
*
525-
* Solution Idea:
526-
*
527-
* There are only two solutions: Either to simply state that POST
528-
* requests to URLs with SSL re-configurations are not allowed, or to
529-
* renegotiate really after the _complete_ request (i.e. including
530-
* the POST body) was read. Obviously the latter would be preferred,
531-
* but it cannot be done easily inside Apache, because as already
532-
* mentioned, there is no API step between the body reading and the body
533-
* processing. And even when we mod_ssl would hook directly into the
534-
* loop of mod_cgi, we wouldn't solve the problem for other handlers, of
535-
* course. So the only general solution is to suck in the pending data
536-
* of the request body from the OpenSSL BIO into the Apache BUFF. Then
537-
* the renegotiation can be done and after this step Apache can proceed
538-
* processing the request as before.
539-
*
540-
* Solution Implementation:
541-
*
542-
* We cannot simply suck in the data via an SSL_read-based loop because of
543-
* HTTP chunking. Instead we _have_ to use the Apache API for this step which
544-
* is aware of HTTP chunking. So the trick is to suck in the pending request
545-
* data via the Apache API (which uses Apache's BUFF code and in the
546-
* background mod_ssl's I/O glue code) and re-inject it later into the Apache
547-
* BUFF code again. This way the data flows twice through the Apache BUFF, of
548-
* course. But this way the solution doesn't depend on any Apache specifics
549-
* and is fully transparent to Apache modules.
550-
*
551-
* !! BUT ALL THIS IS STILL NOT RE-IMPLEMENTED FOR APACHE 2.0 !!
493+
/* If a renegotiation is now required for this location, and the
494+
* request includes a message body (and the client has not
495+
* requested a "100 Continue" response), then the client will be
496+
* streaming the request body over the wire already. In that
497+
* case, it is not possible to stop and perform a new SSL
498+
* handshake immediately; once the SSL library moves to the
499+
* "accept" state, it will reject the SSL packets which the client
500+
* is sending for the request body.
501+
*
502+
* To allow authentication to complete in this auth hook, the
503+
* solution used here is to fill a (bounded) buffer with the
504+
* request body, and then to reinject that request body later.
552505
*/
553-
if (renegotiate && !renegotiate_quick && (r->method_number == M_POST)) {
554-
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
555-
"SSL Re-negotiation in conjunction "
556-
"with POST method not supported! "
557-
"hint: try SSLOptions +OptRenegotiate");
558-
559-
return HTTP_METHOD_NOT_ALLOWED;
506+
if (renegotiate && !renegotiate_quick
507+
&& (apr_table_get(r->headers_in, "transfer-encoding")
508+
|| (apr_table_get(r->headers_in, "content-length")
509+
&& strcmp(apr_table_get(r->headers_in, "content-length"), "0")))
510+
&& !r->expecting_100) {
511+
int rv;
512+
513+
/* Fill the I/O buffer with the request body if possible. */
514+
rv = ssl_io_buffer_fill(r);
515+
516+
if (rv) {
517+
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
518+
"could not buffer message body to allow "
519+
"SSL renegotiation to proceed");
520+
return rv;
521+
}
560522
}
561523

562524
/*

‎modules/ssl/ssl_private.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,10 @@ void ssl_io_filter_init(conn_rec *, SSL *);
590590
void ssl_io_filter_register(apr_pool_t *);
591591
long ssl_io_data_cb(BIO *, int, MODSSL_BIO_CB_ARG_TYPE *, int, long, long);
592592

593+
/* ssl_io_buffer_fill fills the setaside buffering of the HTTP request
594+
* to allow an SSL renegotiation to take place. */
595+
int ssl_io_buffer_fill(request_rec *r);
596+
593597
/* PRNG */
594598
int ssl_rand_seed(server_rec *, apr_pool_t *, ssl_rsctx_t, char *);
595599

0 commit comments

Comments
 (0)