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 2008/12/12 21:20:40 UTC

svn commit: r726109 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_io.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

Author: jorton
Date: Fri Dec 12 12:20:40 2008
New Revision: 726109

URL: http://svn.apache.org/viewvc?rev=726109&view=rev
Log:
mod_ssl: Make the size of the per-dir-reneg request-body buffer
configurable, by popular demand:

* modules/ssl/ssl_private.h: Define DEFAULT_RENEG_BUFFER_SIZE.
  (SSLDirConfigRec): Add nRenegBufferSize field.

* modules/ssl/ssl_engine_config.c (ssl_cmd_SSLRenegBufferSize): New
  function.
  (ssl_config_perdir_create, ssl_config_perdir_merge): Handle
  nRenegBufferSize.

* modules/ssl/ssl_engine_io.c (ssl_io_buffer_fill): Take max buffer
  size as an argument rather than compile-time constant.

* modules/ssl/ssl_engine_kernel.c (ssl_hook_Access): Pass
  nRenegBufferSize to ssl_io_buffer_fill.

* modules/ssl/mod_ssl.c (ssl_config_cmds): Add SSLRenegBufferSize.

PR: 39243

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/ssl/mod_ssl.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_config.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=726109&r1=726108&r2=726109&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Dec 12 12:20:40 2008
@@ -1,6 +1,10 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.3.1
 [ When backported to 2.2.x, remove entry from this file ]
+  
+  *) mod_ssl: Add SSLRenegBufferSize directive to allow changing the
+     size of the buffer used for the request-body where necessary
+     during a per-dir renegotiation.  PR 39243.  [Joe Orton]
 
   *) mod_proxy_fdpass: New module to pass a client connection over to a separate
      process that is reading from a unix daemon socket.

Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=726109&r1=726108&r2=726109&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
+++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Fri Dec 12 12:20:40 2008
@@ -179,6 +179,10 @@
     SSL_CMD_DIR(Require, AUTHCFG, RAW_ARGS,
                "Require a boolean expression to evaluate to true for granting access"
                "(arbitrary complex boolean expression - see manual)")
+    SSL_CMD_DIR(RenegBufferSize, AUTHCFG, TAKE1,
+                "Configure the amount of memory that will be used for buffering the "
+                "request body if a per-location SSL renegotiation is required due to "
+                "changed access control requirements")
 
     SSL_CMD_SRV(OCSPEnable, FLAG,
                "Enable use of OCSP to verify certificate revocation (`on', `off')")

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=726109&r1=726108&r2=726109&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Dec 12 12:20:40 2008
@@ -295,6 +295,8 @@
     dc->szCACertificateFile    = NULL;
     dc->szUserName             = NULL;
 
+    dc->nRenegBufferSize = DEFAULT_RENEG_BUFFER_SIZE;
+
     return dc;
 }
 
@@ -332,6 +334,8 @@
     cfgMergeString(szCACertificateFile);
     cfgMergeString(szUserName);
 
+    cfgMergeInt(nRenegBufferSize);
+
     return mrg;
 }
 
@@ -1148,6 +1152,19 @@
     return NULL;
 }
 
+const char *ssl_cmd_SSLRenegBufferSize(cmd_parms *cmd, void *dcfg, const char *arg)
+{
+    SSLDirConfigRec *dc = dcfg;
+    
+    dc->nRenegBufferSize = atoi(arg);
+    if (dc->nRenegBufferSize < 0) {
+        return apr_pstrcat(cmd->pool, "Invalid size for SSLRenegBufferSize: ",
+                           arg, NULL);
+    }
+
+    return NULL;
+}
+
 static const char *ssl_cmd_protocol_parse(cmd_parms *parms,
                                           const char *arg,
                                           ssl_proto_t *options)

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=726109&r1=726108&r2=726109&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Fri Dec 12 12:20:40 2008
@@ -1411,16 +1411,11 @@
     return status;
 }
 
-/* 128K maximum buffer size by default. */
-#ifndef SSL_MAX_IO_BUFFER
-#define SSL_MAX_IO_BUFFER (128 * 1024)
-#endif
-
 struct modssl_buffer_ctx {
     apr_bucket_brigade *bb;
 };
 
-int ssl_io_buffer_fill(request_rec *r)
+int ssl_io_buffer_fill(request_rec *r, apr_size_t maxlen)
 {
     conn_rec *c = r->connection;
     struct modssl_buffer_ctx *ctx;
@@ -1437,7 +1432,8 @@
     /* ... and a temporary brigade. */
     tempb = apr_brigade_create(r->pool, c->bucket_alloc);
 
-    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, "filling buffer");
+    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, "filling buffer, max size "
+                  "%" APR_SIZE_T_FMT " bytes", maxlen);
 
     do {
         apr_status_t rv;
@@ -1493,9 +1489,10 @@
                       total, eos);
 
         /* Fail if this exceeds the maximum buffer size. */
-        if (total > SSL_MAX_IO_BUFFER) {
+        if (total > maxlen) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                          "request body exceeds maximum size for SSL buffer");
+                          "request body exceeds maximum size (%" APR_SIZE_T_FMT 
+                          ") for SSL buffer", maxlen);
             return HTTP_REQUEST_ENTITY_TOO_LARGE;
         }
 

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=726109&r1=726108&r2=726109&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Fri Dec 12 12:20:40 2008
@@ -601,8 +601,14 @@
         && !r->expecting_100) {
         int rv;
 
-        /* Fill the I/O buffer with the request body if possible. */
-        rv = ssl_io_buffer_fill(r);
+        if (dc->nRenegBufferSize > 0) {
+            /* Fill the I/O buffer with the request body if possible. */
+            rv = ssl_io_buffer_fill(r, dc->nRenegBufferSize);
+        }
+        else {
+            /* If the reneg buffer size is set to zero, just fail. */
+            rv = HTTP_REQUEST_ENTITY_TOO_LARGE;
+        }
 
         if (rv) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,

Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=726109&r1=726108&r2=726109&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_private.h Fri Dec 12 12:20:40 2008
@@ -139,6 +139,11 @@
 #define SSL_SESSION_CACHE_TIMEOUT  300
 #endif
 
+/* Default setting for per-dir reneg buffer. */
+#ifndef DEFAULT_RENEG_BUFFER_SIZE
+#define DEFAULT_RENEG_BUFFER_SIZE (128 * 1024)
+#endif
+
 /**
  * Define the per-server SSLLogLevel constants which provide
  * finer-than-debug resolution to decide if logs are to be
@@ -488,6 +493,7 @@
     const char   *szCACertificatePath;
     const char   *szCACertificateFile;
     const char   *szUserName;
+    apr_size_t    nRenegBufferSize;
 } SSLDirConfigRec;
 
 /**
@@ -532,6 +538,7 @@
 const char  *ssl_cmd_SSLRequire(cmd_parms *, void *, const char *);
 const char  *ssl_cmd_SSLUserName(cmd_parms *, void *, const char *);
 const char  *ssl_cmd_SSLLogLevelDebugDump(cmd_parms *, void *, const char *);
+const char  *ssl_cmd_SSLRenegBufferSize(cmd_parms *cmd, void *dcfg, const char *arg);
 
 const char  *ssl_cmd_SSLProxyEngine(cmd_parms *cmd, void *dcfg, int flag);
 const char  *ssl_cmd_SSLProxyProtocol(cmd_parms *, void *, const char *);
@@ -603,7 +610,7 @@
 
 /* ssl_io_buffer_fill fills the setaside buffering of the HTTP request
  * to allow an SSL renegotiation to take place. */
-int          ssl_io_buffer_fill(request_rec *r);
+int          ssl_io_buffer_fill(request_rec *r, apr_size_t maxlen);
 
 /**  PRNG  */
 int          ssl_rand_seed(server_rec *, apr_pool_t *, ssl_rsctx_t, char *);



Re: svn commit: r726109 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_io.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 12/12/2008 09:20 PM, jorton@apache.org wrote:
> Author: jorton
> Date: Fri Dec 12 12:20:40 2008
> New Revision: 726109
> 
> URL: http://svn.apache.org/viewvc?rev=726109&view=rev
> Log:
> mod_ssl: Make the size of the per-dir-reneg request-body buffer
> configurable, by popular demand:
> 
> * modules/ssl/ssl_private.h: Define DEFAULT_RENEG_BUFFER_SIZE.
>   (SSLDirConfigRec): Add nRenegBufferSize field.
> 
> * modules/ssl/ssl_engine_config.c (ssl_cmd_SSLRenegBufferSize): New
>   function.
>   (ssl_config_perdir_create, ssl_config_perdir_merge): Handle
>   nRenegBufferSize.
> 
> * modules/ssl/ssl_engine_io.c (ssl_io_buffer_fill): Take max buffer
>   size as an argument rather than compile-time constant.
> 
> * modules/ssl/ssl_engine_kernel.c (ssl_hook_Access): Pass
>   nRenegBufferSize to ssl_io_buffer_fill.
> 
> * modules/ssl/mod_ssl.c (ssl_config_cmds): Add SSLRenegBufferSize.
> 
> PR: 39243
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/ssl/mod_ssl.c
>     httpd/httpd/trunk/modules/ssl/ssl_engine_config.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/modules/ssl/ssl_engine_config.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=726109&r1=726108&r2=726109&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Dec 12 12:20:40 2008
> @@ -295,6 +295,8 @@
>      dc->szCACertificateFile    = NULL;
>      dc->szUserName             = NULL;
>  
> +    dc->nRenegBufferSize = DEFAULT_RENEG_BUFFER_SIZE;
> +
>      return dc;
>  }
>  
> @@ -332,6 +334,8 @@
>      cfgMergeString(szCACertificateFile);
>      cfgMergeString(szUserName);
>  
> +    cfgMergeInt(nRenegBufferSize);
> +

Doesn't this require dc->RenegBufferSize to be initialized with UNSET instead
of DEFAULT_RENEG_BUFFER_SIZE to work correctly?


Regards

RĂ¼diger

Re: svn commit: r726109 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_io.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

Posted by Joe Orton <jo...@redhat.com>.
On Fri, Dec 12, 2008 at 02:41:14PM -0600, William Rowe wrote:
> jorton@apache.org wrote:
> > Author: jorton
> > Date: Fri Dec 12 12:20:40 2008
> > New Revision: 726109
> > 
> > URL: http://svn.apache.org/viewvc?rev=726109&view=rev
> > Log:
> > mod_ssl: Make the size of the per-dir-reneg request-body buffer
> > configurable, by popular demand:
> 
> Nice, now if we just grew apreq refactoring, we would gain request body
> disk-based buffering ;-)

Disk buffering here sounds like it would be a DoS attack waiting to 
happen.

> Question; if they present us the opportunity to present a 100 CONTINUE,
> can we perform the handshake first?  If so, should we just throw in one
> of our sneeky little env flags to allow allow the user to cripple this
> by client?

The buffering was never done if r->expecting_100 is true; I've not 
tested that specific case but it should work already.

Regards, Joe

Re: svn commit: r726109 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_io.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
jorton@apache.org wrote:
> Author: jorton
> Date: Fri Dec 12 12:20:40 2008
> New Revision: 726109
> 
> URL: http://svn.apache.org/viewvc?rev=726109&view=rev
> Log:
> mod_ssl: Make the size of the per-dir-reneg request-body buffer
> configurable, by popular demand:

Nice, now if we just grew apreq refactoring, we would gain request body
disk-based buffering ;-)

Question; if they present us the opportunity to present a 100 CONTINUE,
can we perform the handshake first?  If so, should we just throw in one
of our sneeky little env flags to allow allow the user to cripple this
by client?