SSL + POST + renegotiation

[Nathan Friess <na...@shaw.ca>]

Hello, I'm hoping that some of the more experienced persons can take a look
at my possible fix to the SSL + POST + renegotiation problem (httpd 2.0.35).
I just started to look into the apache sources about a week ago, and I
developed these patches to fix this annoying problem (patches attached at
the bottom).  I have a couple questions below, plus any constructive
criticism (especially from anyone who would test this more thoroughly) would
be nice.

In case you don't know what the problem is, here is a brief explanation
(from what I understand).  When using SSL (particularly with client
certificates), a per-directory SSL configuration, and anything (like cgi
scripts) that makes the client send an HTTP POST, the server runs into a
problem.  What happens, is the client sends the post, but does not present
the client's certificate.  The server parses the header, and then needs to
verify the client, due to an SSLRequire directive for this
directory/location.  The server gets mod_ssl to verify the client, but
mod_ssl doesn't have the client certificate, so mod_ssl needs to renegotiate
at the SSL layer to get it.  The problem becomes that the body of the POST
is sitting in the input buffers, and a renegotiation would cause the buffer
to be lost.  Currently, mod_ssl 'solves' this by always returning "method
not allowed".

My idea to fix this is based on the explanation in ssl_engine_kernel.c, just
before the problem occurs.  I 'eat up' the input buffer in the same way that
mod_cgi does, and then after the renegotiation I 'push' the buffer back into
http_protocol.c.  Also, if there is a 'pushed' buffer, then different code
is used to read my buffer in http_protocol.c.  I know that my fix is a bit
of a kludge, but it is simple, and seems to work. So far, I have only tested
it with cygwin, when everything is compiled into the executable.  Hopefully,
I will be able to patch and test my server (running linux).

A few questions that I have:
1) can I just copy the 'buckets' or 'brigade' that ap_get_client_block()
uses?  Where would I find info on how these buckets work?
2) where can I get info on coding standards, or other direction for
developers of httpd? (like I put my name near all the places that I changed
to make it apparent, is that standard?)
3) if this solution proves useful, how would I get it into mainstream
sources?

Thank you for your time.

Nathan Friess

PS: Please forgive me if this is the wrong mailing list or the message is
too long; I've never used this mailing list before.


=== modules/ssl/ssl_engine_kernel.c.patch ===
--- httpd-2.0.35-orig/modules/ssl/ssl_engine_kernel.c Thu Apr 4 19:31:04
2002
+++ httpd-2.0.35/modules/ssl/ssl_engine_kernel.c Wed May 8 09:21:35 2002
@@ -307,6 +307,7 @@ int ssl_hook_Handler(request_rec *r)
return HTTP_BAD_REQUEST;
}
+
/*
* Access Handler
*/
@@ -328,6 +329,12 @@ int ssl_hook_Access(request_rec *r)
STACK_OF(SSL_CIPHER) *cipher_list_old = NULL, *cipher_list = NULL;
SSL_CIPHER *cipher = NULL;
int depth, verify_old, verify, n;
+
+ /* SSL/POST Fix: Added by Nathan Friess, 3/5/02 */
+ char *pushBuff = 0;
+ long pushLen = 0;
+ int len_read;
+ char argsbuffer[HUGE_STRING_LEN+1];
if (ssl) {
ctx = SSL_get_SSL_CTX(ssl);
@@ -691,12 +698,35 @@ int ssl_hook_Access(request_rec *r)
*
* !! BUT ALL THIS IS STILL NOT RE-IMPLEMENTED FOR APACHE 2.0 !!
*/
+
+ /*
+ SSL/POST Fix: by Nathan Friess, 3/5/02.
+ Grab all data, (will be pushed back to buffers after).
+ */
+
if (renegotiate && (r->method_number == M_POST)) {
- ssl_log(r->server, SSL_LOG_ERROR,
+ ssl_log(r->server, SSL_LOG_INFO,
"SSL Re-negotiation in conjunction "
- "with POST method not supported!");
+ "with POST method not usually supported!");
+
+ ssl_log(r->server, SSL_LOG_INFO,
+ "But, we'll use the SSL/POST fix by Nathan Friess");
+ if (ap_setup_client_block(r, REQUEST_CHUNKED_ERROR))
+ return HTTP_METHOD_NOT_ALLOWED;
+
+ while ((len_read =
+ ap_get_client_block(r, argsbuffer, HUGE_STRING_LEN)) > 0) {
+
+ pushBuff = realloc(pushBuff, pushLen+len_read+1);
+ memcpy(pushBuff+pushLen, argsbuffer, len_read);
+ pushLen += len_read;
+ memset(pushBuff+pushLen, 0, 1);
+ }
+
+/* Not needed any more.
return HTTP_METHOD_NOT_ALLOWED;
+*/
}
/*
@@ -878,6 +908,15 @@ int ssl_hook_Access(request_rec *r)
return HTTP_FORBIDDEN;
}
+ }
+
+ /* SSL/POST: Fix by Nathan Friess, 3/5/02.
+ Push my data back into the client's buff.
+ */
+ if (pushBuff && pushLen)
+ {
+ ap_push_client_body(r, pushBuff, pushLen);
+ free(pushBuff);
}
/*
===end file===

=== modules/http/http_protocol.c.patch ===
--- httpd-2.0.35-orig/modules/http/http_protocol.c Mon Apr 1 15:26:09 2002
+++ httpd-2.0.35/modules/http/http_protocol.c Wed May 8 10:05:11 2002
@@ -1570,10 +1570,22 @@ AP_DECLARE(int) ap_setup_client_block(re
return OK;
}
+/* SSL/POST Fix: by Nathan Friess, 3/5/02.
+Used by mod_ssl only.
+*/
+char *pushStr=0;
+long pushLen=0;
+
AP_DECLARE(int) ap_should_client_block(request_rec *r)
{
/* First check if we have already read the request body */
+ /* SSL/POST Fix: by Nathan Friess, 3/5/02.
+ Return true if there is data to push.
+ */
+ if (pushStr && pushLen)
+ return 1;
+
if (r->read_length || (!r->read_chunked && (r->remaining <= 0))) {
return 0;
}
@@ -1623,6 +1635,17 @@ static long get_chunk_size(char *b)
return chunksize;
}
+/* SSL/POST Fix: by Nathan Friess, 3/5/02.
+ Used by mod_ssl only.
+*/
+AP_DECLARE(long) ap_push_client_body(request_rec *r, const char *str, long
len)
+{
+ pushStr = apr_palloc(r->pool, len);
+ memcpy(pushStr, str, len);
+ pushLen = len;
+ return 0;
+}
+
/* get_client_block is called in a loop to get the request message body.
* This is quite simple if the client includes a content-length
* (the normal case), but gets messy if the body is chunked. Note that
@@ -1643,6 +1666,13 @@ AP_DECLARE(long) ap_get_client_block(req
apr_status_t rv;
apr_bucket *b;
const char *tempbuf;
+
+ /* SSL/POST Fix: Nathan Friess, 3/5/02.
+ Do my special code if there is some data (else part)
+ */
+ if(!pushStr || !pushLen)
+ {
+
core_request_config *req_cfg =
(core_request_config *)ap_get_module_config(r->request_config,
&core_module);
@@ -1703,6 +1733,25 @@ AP_DECLARE(long) ap_get_client_block(req
b = APR_BUCKET_NEXT(b);
apr_bucket_delete(old);
}
+ }
+ else
+ {
+
+ if (pushLen > bufsiz)
+ {
+ total = bufsiz;
+ memcpy(buffer, pushStr, total);
+ memmove(pushStr, pushStr+bufsiz, pushLen-bufsiz+1);
+ pushLen -= bufsiz;
+ }
+ else
+ {
+ total = pushLen;
+ memcpy(buffer, pushStr, total);
+ pushStr = 0;
+ pushLen = 0;
+ }
+ }
return total;
}
===end file===

Re: SSL + POST + renegotiation

[Cliff Woolley <jw...@virginia.edu>]

On Wed, 8 May 2002, Nathan Friess wrote:

> A few questions that I have:
> 1) can I just copy the 'buckets' or 'brigade' that ap_get_client_block()
> uses?  Where would I find info on how these buckets work?

http://apr.apache.org/docs/apr-util/ under Bucket Brigades, which is a
formatted version of the documentation in apr_buckets.h in
httpd-2.0/srclib/apr-util/include/.  In general I think this should be
accomplishable with buckets rather than passing buffers around, though
admittedly I haven't looked closely at it yet.

> 2) where can I get info on coding standards, or other direction for
> developers of httpd? (like I put my name near all the places that I changed
> to make it apparent, is that standard?)

We put that sort of commentary in the commit log rather than in the code.
We try to avoid both static buffers and malloc/realloc/free wherever
possible, using pools instead.  We have relatively well-defined style
guidelines.  See http://httpd.apache.org/dev/styleguide.html and
http://httpd.apache.org/dev/patches.html .

> 3) if this solution proves useful, how would I get it into mainstream
> sources?

After review, if one of the committers approves it, that person will
commit it to the tree.

Thanks,
Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA