You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2019/11/21 15:51:32 UTC
svn commit: r1870095 - /httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
Author: jorton
Date: Thu Nov 21 15:51:32 2019
New Revision: 1870095
URL: http://svn.apache.org/viewvc?rev=1870095&view=rev
Log:
Buffer HTTP request bodies for TLSv1.3 PHA in the same way as for
TLSv<1.3 renegotiation.
* modules/ssl/ssl_engine_kernel.c (fill_reneg_buffer): Factor
out...
(ssl_hook_Access_classic): ... from here.
(ssl_hook_Access_modern): Use it here too.
Github: closes #75
Modified:
httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
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=1870095&r1=1870094&r2=1870095&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Thu Nov 21 15:51:32 2019
@@ -114,6 +114,45 @@ static int has_buffered_data(request_rec
return result;
}
+/* If a renegotiation is required for the location, and the request
+ * includes a message body (and the client has not requested a "100
+ * Continue" response), then the client will be streaming the request
+ * body over the wire already. In that case, it is not possible to
+ * stop and perform a new SSL handshake immediately; once the SSL
+ * library moves to the "accept" state, it will reject the SSL packets
+ * which the client is sending for the request body.
+ *
+ * To allow authentication to complete in the hook, the solution used
+ * here is to fill a (bounded) buffer with the request body, and then
+ * to reinject that request body later.
+ *
+ * This function is called to fill the renegotiation buffer for the
+ * location as required, or fail. Returns zero on success or HTTP_
+ * error code on failure.
+ */
+static int fill_reneg_buffer(request_rec *r, SSLDirConfigRec *dc)
+{
+ int rv;
+ apr_size_t rsize;
+
+ /* ### this is HTTP/1.1 specific, special case for protocol? */
+ if (r->expecting_100 || !ap_request_has_body(r)) {
+ return 0;
+ }
+
+ rsize = dc->nRenegBufferSize == UNSET ? DEFAULT_RENEG_BUFFER_SIZE : dc->nRenegBufferSize;
+ if (rsize > 0) {
+ /* Fill the I/O buffer with the request body if possible. */
+ rv = ssl_io_buffer_fill(r, rsize);
+ }
+ else {
+ /* If the reneg buffer size is set to zero, just fail. */
+ rv = HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+
+ return rv;
+}
+
#ifdef HAVE_TLSEXT
static int ap_array_same_str_set(apr_array_header_t *s1, apr_array_header_t *s2)
{
@@ -814,41 +853,14 @@ static int ssl_hook_Access_classic(reque
}
}
- /* If a renegotiation is now required for this location, and the
- * request includes a message body (and the client has not
- * requested a "100 Continue" response), then the client will be
- * streaming the request body over the wire already. In that
- * case, it is not possible to stop and perform a new SSL
- * handshake immediately; once the SSL library moves to the
- * "accept" state, it will reject the SSL packets which the client
- * is sending for the request body.
- *
- * To allow authentication to complete in this auth hook, the
- * solution used here is to fill a (bounded) buffer with the
- * request body, and then to reinject that request body later.
- */
- if (renegotiate && !renegotiate_quick
- && !r->expecting_100
- && ap_request_has_body(r)) {
- int rv;
- apr_size_t rsize;
-
- rsize = dc->nRenegBufferSize == UNSET ? DEFAULT_RENEG_BUFFER_SIZE :
- dc->nRenegBufferSize;
- if (rsize > 0) {
- /* Fill the I/O buffer with the request body if possible. */
- rv = ssl_io_buffer_fill(r, rsize);
- }
- else {
- /* If the reneg buffer size is set to zero, just fail. */
- rv = HTTP_REQUEST_ENTITY_TOO_LARGE;
- }
-
- if (rv) {
+ /* Fill reneg buffer if required. */
+ if (renegotiate && !renegotiate_quick) {
+ rc = fill_reneg_buffer(r, dc);
+ if (rc) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02257)
"could not buffer message body to allow "
"SSL renegotiation to proceed");
- return rv;
+ return rc;
}
}
@@ -1132,6 +1144,17 @@ static int ssl_hook_Access_modern(reques
}
}
+ /* Fill reneg buffer if required. */
+ if (change_vmode) {
+ rc = fill_reneg_buffer(r, dc);
+ if (rc) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
+ "could not buffer message body to allow "
+ "TLS Post-Handshake Authentication to proceed");
+ return rc;
+ }
+ }
+
if (change_vmode) {
char peekbuf[1];
Re: svn commit: r1870095 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Aug 17, 2020 at 02:07:33PM +0200, Ruediger Pluem wrote:
> On 11/21/19 4:51 PM, jorton@apache.org wrote:
> > Author: jorton
> > Date: Thu Nov 21 15:51:32 2019
> > New Revision: 1870095
> >
> > URL: http://svn.apache.org/viewvc?rev=1870095&view=rev
...
> > @@ -1132,6 +1144,17 @@ static int ssl_hook_Access_modern(reques
> > }
> > }
> >
> > + /* Fill reneg buffer if required. */
> > + if (change_vmode) {
>
> Why can't we put this block below the
>
> if (change_vmode)
>
> a few lines below?
Good question, thanks for the review. No good reason, and moving the
call after the r->connection->master test is presumably better. Done in
r1880927.
Regards, Joe
Re: svn commit: r1870095 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 11/21/19 4:51 PM, jorton@apache.org wrote:
> Author: jorton
> Date: Thu Nov 21 15:51:32 2019
> New Revision: 1870095
>
> URL: http://svn.apache.org/viewvc?rev=1870095&view=rev
> Log:
> Buffer HTTP request bodies for TLSv1.3 PHA in the same way as for
> TLSv<1.3 renegotiation.
>
> * modules/ssl/ssl_engine_kernel.c (fill_reneg_buffer): Factor
> out...
> (ssl_hook_Access_classic): ... from here.
> (ssl_hook_Access_modern): Use it here too.
>
> Github: closes #75
>
> Modified:
> httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>
> 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=1870095&r1=1870094&r2=1870095&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Thu Nov 21 15:51:32 2019
> @@ -1132,6 +1144,17 @@ static int ssl_hook_Access_modern(reques
> }
> }
>
> + /* Fill reneg buffer if required. */
> + if (change_vmode) {
Why can't we put this block below the
if (change_vmode)
a few lines below?
> + rc = fill_reneg_buffer(r, dc);
> + if (rc) {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
> + "could not buffer message body to allow "
> + "TLS Post-Handshake Authentication to proceed");
> + return rc;
> + }
> + }
> +
> if (change_vmode) {
> char peekbuf[1];
Regards
Rüdiger