You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Justin Erenkrantz <je...@ebuilt.com> on 2001/10/04 22:03:32 UTC
[PATCH] mod_ssl input filtering...
I have to get to class now.
This is within shouting distance of where we need to be.
It compiles. =) Yet another gnarly patch. Oh, well.
I also moved some code from mod_ssl.c that dealt with buckets into
ssl_engine_io.c. That seems logical to do though.
Oh, when we enter churn with data in ctx->b, we need to ensure that
we don't send too much - ctx->b length is more than readbytes - call
partition... That shouldn't be too hard to add...
I'll be back around 7 or 8pm tonight (if not later). I'd appreciate
it if someone else could look at this and see if they can test it or
even make it work (and possibly repost or commit it!).
Later... -- justin
Index: modules/ssl/mod_ssl.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.c,v
retrieving revision 1.30
diff -u -r1.30 mod_ssl.c
--- modules/ssl/mod_ssl.c 2001/08/30 05:33:57 1.30
+++ modules/ssl/mod_ssl.c 2001/10/04 19:54:22
@@ -73,8 +73,6 @@
AP_INIT_##args("SSL"#name, ssl_cmd_SSL##name, NULL, OR_##type, desc),
#define AP_END_CMD { NULL }
-#define HTTP_ON_HTTPS_PORT "GET /mod_ssl:error:HTTP-request HTTP/1.0\r\n"
-
static const command_rec ssl_config_cmds[] = {
/*
@@ -369,41 +367,9 @@
/*
* The case where OpenSSL has recognized a HTTP request:
* This means the client speaks plain HTTP on our HTTPS port.
- * Hmmmm... At least for this error we can be more friendly
- * and try to provide him with a HTML error page. We have only
- * one problem:OpenSSL has already read some bytes from the HTTP
- * request. So we have to skip the request line manually and
- * instead provide a faked one in order to continue the internal
- * Apache processing.
- *
+ * Hmmmm... Punt this out of here after removing our output
+ * filter.
*/
- apr_bucket *e;
- const char *str;
- apr_size_t len;
- /* log the situation */
- ssl_log(c->base_server, SSL_LOG_ERROR|SSL_ADD_SSLERR,
- "SSL handshake failed: HTTP spoken on HTTPS port; "
- "trying to send HTML error page");
-
- /* fake the request line */
- e = apr_bucket_immortal_create(HTTP_ON_HTTPS_PORT,
- strlen(HTTP_ON_HTTPS_PORT));
- APR_BRIGADE_INSERT_HEAD(pRec->pbbPendingInput, e);
-
- APR_BRIGADE_FOREACH(e, pRec->pbbInput) {
- apr_bucket_read(e, &str, &len, APR_NONBLOCK_READ);
- if (len) {
- APR_BUCKET_REMOVE(e);
- APR_BRIGADE_INSERT_TAIL(pRec->pbbPendingInput, e);
- if ((strcmp(str, "\r\n") == 0) ||
- (ap_strstr_c(str, "\r\n\r\n"))) {
- break;
- }
- }
- }
- e = APR_BRIGADE_LAST(pRec->pbbInput);
- APR_BUCKET_REMOVE(e);
-
ap_remove_output_filter(pRec->pOutputFilter);
return HTTP_BAD_REQUEST;
}
Index: modules/ssl/mod_ssl.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.h,v
retrieving revision 1.33
diff -u -r1.33 mod_ssl.h
--- modules/ssl/mod_ssl.h 2001/09/10 04:21:40 1.33
+++ modules/ssl/mod_ssl.h 2001/10/04 19:54:22
@@ -442,8 +442,8 @@
BIO *pbioWrite;
ap_filter_t *pInputFilter;
ap_filter_t *pOutputFilter;
- apr_bucket_brigade *pbbInput; /* encrypted input */
- apr_bucket_brigade *pbbPendingInput; /* decrypted input */
+ apr_bucket_brigade *rawb; /* encrypted input */
+ apr_bucket_brigade *b; /* decrypted input */
} SSLFilterRec;
typedef struct {
Index: modules/ssl/ssl_engine_io.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.37
diff -u -r1.37 ssl_engine_io.c
--- modules/ssl/ssl_engine_io.c 2001/10/04 17:50:39 1.37
+++ modules/ssl/ssl_engine_io.c 2001/10/04 19:54:22
@@ -72,14 +72,10 @@
static const char ssl_io_filter[] = "SSL/TLS Filter";
-static int ssl_io_hook_read(SSL *ssl, unsigned char *buf, int len)
+static int ssl_io_hook_read(SSL *ssl, char *buf, int len)
{
int rc;
- if (ssl == NULL) {
- return -1;
- }
-
rc = SSL_read(ssl, buf, len);
if (rc < 0) {
@@ -189,75 +185,48 @@
#define bio_is_renegotiating(bio) \
(((int)BIO_get_callback_arg(bio)) == SSL_ST_RENEGOTIATE)
+#define HTTP_ON_HTTPS_PORT "GET /mod_ssl:error:HTTP-request HTTP/1.0\r\n\r\n"
-static apr_status_t churn_input(SSLFilterRec *pRec,
- ap_input_mode_t eMode, apr_off_t *readbytes)
+static apr_status_t churn_input(SSLFilterRec *pRec, ap_input_mode_t eMode,
+ apr_off_t *readbytes)
{
- conn_rec *c = pRec->pInputFilter->c;
+ ap_filter_t *f = pRec->pInputFilter;
+ SSLFilterRec *ctx = pRec;
+ conn_rec *c = f->c;
apr_pool_t *p = c->pool;
- apr_bucket *pbktIn;
+ apr_bucket *e;
+ int found_eos = 0, n;
+ char buf[1024];
+ apr_status_t rv;
+
+ /* If we have nothing in the read or raw buckets. */
+ if (APR_BRIGADE_EMPTY(ctx->rawb) && APR_BRIGADE_EMPTY(ctx->b)) {
+ int rv = ap_get_brigade(f->next, ctx->rawb, eMode, readbytes);
- if(APR_BRIGADE_EMPTY(pRec->pbbInput)) {
- ap_get_brigade(pRec->pInputFilter->next,pRec->pbbInput,eMode,readbytes);
- if(APR_BRIGADE_EMPTY(pRec->pbbInput))
- return APR_EOF;
- }
-
- APR_BRIGADE_FOREACH(pbktIn,pRec->pbbInput) {
- const char *data;
- apr_size_t len;
- int n;
- char buf[1024];
- apr_status_t ret;
-
- if (APR_BUCKET_IS_EOS(pbktIn)) {
- break;
- }
-
- /* read filter */
- ret = apr_bucket_read(pbktIn, &data, &len, (apr_read_type_e)eMode);
-
- if (!(eMode == AP_MODE_NONBLOCKING && APR_STATUS_IS_EAGAIN(ret))) {
- /* allow retry */
- APR_BUCKET_REMOVE(pbktIn);
- }
-
- if (ret == APR_SUCCESS && len == 0 && eMode == AP_MODE_BLOCKING)
- ret = APR_EOF;
-
- if (len == 0) {
- /* Lazy frickin browsers just reset instead of shutting down. */
- /* also gotta handle timeout of keepalive connections */
- if (ret == APR_EOF || APR_STATUS_IS_ECONNRESET(ret) ||
- ret == APR_TIMEUP)
- {
- if (APR_BRIGADE_EMPTY(pRec->pbbPendingInput))
- return APR_EOF;
- else
- /* Next time around, the incoming brigade will be empty,
- * so we'll return EOF then
- */
- return APR_SUCCESS;
- }
-
- if (eMode != AP_MODE_NONBLOCKING)
- ap_log_error(APLOG_MARK,APLOG_ERR,ret,NULL,
- "Read failed in ssl input filter");
-
- if ((eMode == AP_MODE_NONBLOCKING) &&
- (APR_STATUS_IS_SUCCESS(ret) || APR_STATUS_IS_EAGAIN(ret)))
- {
- /* In this case, we have data in the output bucket, or we were
- * non-blocking, so returning nothing is fine.
- */
- return APR_SUCCESS;
- }
- else {
- return ret;
- }
- }
+ if (rv != APR_SUCCESS)
+ return rv;
+
+ }
+
+ /* Process anything we have that we haven't done so already. */
+ APR_BRIGADE_FOREACH(e, ctx->rawb) {
+ const char *data;
+ apr_size_t len;
- n = BIO_write (pRec->pbioRead, data, len);
+ if (APR_BUCKET_IS_EOS(e)) {
+ apr_bucket_delete(e);
+ found_eos = 1;
+ break;
+ }
+
+ /* read from the bucket */
+ rv = apr_bucket_read(e, &data, &len, eMode);
+
+ if (rv != APR_SUCCESS)
+ return rv;
+
+ /* Write it to our BIO */
+ n = BIO_write(pRec->pbioRead, data, len);
if ((apr_size_t)n != len) {
/* this should never really happen, since we're just writing
@@ -270,50 +239,61 @@
return APR_ENOMEM;
}
- if (bio_is_renegotiating(pRec->pbioRead)) {
- /* we're doing renegotiation in the access phase */
- if (len >= *readbytes) {
- /* trick ap_http_filter into leaving us alone */
- *readbytes = 0;
- break; /* SSL_renegotiate will take it from here */
- }
- }
+ /* If we reached here, we read the bucket successfully, so toss
+ * it from the raw brigade. */
+ apr_bucket_delete(e);
+ }
+
+ /* Before we actually read any unencrypted data, go ahead and
+ * let ssl_hook_process_connection have a shot at it.
+ */
+ if ((rv = ssl_hook_process_connection(pRec)) != APR_SUCCESS) {
+ /* if process connection says HTTP_BAD_REQUEST, we've seen a
+ * HTTP on HTTPS error.
+ *
+ * The case where OpenSSL has recognized a HTTP request:
+ * This means the client speaks plain HTTP on our HTTPS port.
+ * Hmmmm... At least for this error we can be more friendly
+ * and try to provide him with a HTML error page. We have only
+ * one problem:OpenSSL has already read some bytes from the HTTP
+ * request. So we have to skip the request line manually and
+ * instead provide a faked one in order to continue the internal
+ * Apache processing.
+ *
+ */
+ if (rv == HTTP_BAD_REQUEST) {
+ /* log the situation */
+ ssl_log(c->base_server, SSL_LOG_ERROR|SSL_ADD_SSLERR,
+ "SSL handshake failed: HTTP spoken on HTTPS port; "
+ "trying to send HTML error page");
- if ((ret = ssl_hook_process_connection(pRec)) != APR_SUCCESS) {
- /* if this is the case, ssl connection has been shutdown
- * and pRec->pssl has been freed
- */
- if (ret == HTTP_BAD_REQUEST) {
- return APR_SUCCESS;
- }
- return ret;
+ /* fake the request line */
+ e = apr_bucket_immortal_create(HTTP_ON_HTTPS_PORT,
+ sizeof(HTTP_ON_HTTPS_PORT)-1);
+ APR_BRIGADE_INSERT_HEAD(ctx->b, e);
+
+ return HTTP_BAD_REQUEST;
}
+ /* FIXME: Madhu is right - we should handle WANT_READ ... later... */
+ return rv;
+ }
+
+ /* try to pass along all of the current BIO to ctx->b */
+ /* FIXME: If there's an error and there was EOS, we may not really
+ * reach EOS.
+ */
+ while ((n = ssl_io_hook_read(pRec->pssl, buf, sizeof(buf))) > 0)
+ {
+ char *pbuf;
- /* pass along all of the current BIO */
- while ((n = ssl_io_hook_read(pRec->pssl,
- (unsigned char *)buf, sizeof(buf))) > 0)
- {
- apr_bucket *pbktOut;
- char *pbuf;
-
- pbuf = apr_pmemdup(p, buf, n);
- /* XXX: should we use a heap bucket instead? Or a transient (in
- * which case we need a separate brigade for each bucket)?
- */
- pbktOut = apr_bucket_pool_create(pbuf, n, p);
- APR_BRIGADE_INSERT_TAIL(pRec->pbbPendingInput,pbktOut);
-
- /* Once we've read something, we can move to non-blocking mode (if
- * we weren't already).
- */
- eMode = AP_MODE_NONBLOCKING;
- }
-
- ret=churn_output(pRec);
- if(ret != APR_SUCCESS)
- return ret;
+ pbuf = apr_pmemdup(p, buf, n);
+ e = apr_bucket_pool_create(pbuf, n, p);
+ APR_BRIGADE_INSERT_TAIL(ctx->b, e);
}
+ if (found_eos)
+ APR_BRIGADE_INSERT_TAIL(ctx->b, apr_bucket_eos_create());
+
return churn_output(pRec);
}
@@ -376,23 +356,78 @@
static apr_status_t ssl_io_filter_Input(ap_filter_t *f,
apr_bucket_brigade *pbbOut,
- ap_input_mode_t eMode,
+ ap_input_mode_t mode,
apr_off_t *readbytes)
{
apr_status_t ret;
- SSLFilterRec *pRec = f->ctx;
+ SSLFilterRec *ctx = f->ctx;
+ apr_status_t rv;
+ apr_bucket *e;
+ apr_off_t *tempread;
- /* XXX: we don't currently support peek */
- if (eMode == AP_MODE_PEEK) {
+ /* XXX: we don't currently support peek or readbytes == -1 */
+ if (mode == AP_MODE_PEEK || *readbytes == -1) {
return APR_ENOTIMPL;
}
+
+ /* Return the requested amount or less. */
+ if (*readbytes)
+ {
+ /* churn the state machine */
+ ret = churn_input(ctx, mode, readbytes);
+
+ if (ret != APR_SUCCESS)
+ return ret;
+
+ APR_BRIGADE_CONCAT(pbbOut, ctx->b);
+ return APR_SUCCESS;
+ }
+
+ /* Readbytes == 0 implies we only want a LF line.
+ * 1024 seems like a good number for now. */
+ *tempread = 1024;
+ rv = churn_input(ctx, mode, tempread);
+ if (rv != APR_SUCCESS)
+ return rv;
+
+ while (!APR_BRIGADE_EMPTY(ctx->b)) {
+ const char *pos, *str;
+ apr_size_t len;
+
+ e = APR_BRIGADE_FIRST(ctx->b);
+
+ /* Sure, we'll call this is a line. Whatever. */
+ if (APR_BUCKET_IS_EOS(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(pbbOut, e);
+ break;
+ }
- /* churn the state machine */
- ret = churn_input(pRec, eMode, readbytes);
- if (ret != APR_SUCCESS)
- return ret;
+ if ((rv = apr_bucket_read(e, &str, &len, mode)) != APR_SUCCESS) {
+ return rv;
+ }
- APR_BRIGADE_CONCAT(pbbOut, pRec->pbbPendingInput);
+ pos = memchr(str, APR_ASCII_LF, len);
+ /* We found a match. */
+ if (pos != NULL) {
+ apr_bucket_split(e, pos - str + 1);
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(pbbOut, e);
+ *readbytes += pos - str;
+ return APR_SUCCESS;
+ }
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(pbbOut, e);
+ *readbytes += len;
+
+ /* Hey, we're about to be starved - go fetch more data. */
+ if (APR_BRIGADE_EMPTY(ctx->b)) {
+ *tempread = 1024;
+ ret = churn_input(ctx, mode, readbytes);
+ if (ret != APR_SUCCESS)
+ return ret;
+ }
+ }
return APR_SUCCESS;
}
@@ -423,8 +458,8 @@
filter = apr_pcalloc(c->pool, sizeof(SSLFilterRec));
filter->pInputFilter = ap_add_input_filter(ssl_io_filter, filter, NULL, c);
filter->pOutputFilter = ap_add_output_filter(ssl_io_filter, filter, NULL, c);
- filter->pbbInput = apr_brigade_create(c->pool);
- filter->pbbPendingInput = apr_brigade_create(c->pool);
+ filter->b = apr_brigade_create(c->pool);
+ filter->rawb = apr_brigade_create(c->pool);
filter->pbioRead = BIO_new(BIO_s_mem());
filter->pbioWrite = BIO_new(BIO_s_mem());
SSL_set_bio(ssl, filter->pbioRead, filter->pbioWrite);
Re: [PATCH] mod_ssl input filtering...
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Thu, Oct 04, 2001 at 10:30:08PM -0700, Greg Stein wrote:
> *tempread ?? You might want to assign something to tempread first :-)
Yeah, I caught that one already. =)
> >...
> > + pos = memchr(str, APR_ASCII_LF, len);
>
> How about a new brigade function first?
Okay....
> I don't know much about the rest of how this filter works, but I am
> seriously suspicious of the removal of the renegotiation code.
Madhu mentioned this too. Where is the renegotiation code? I'm
not seeing what you guys are talking about. -- justin
P.S. Vainly trying to set up a build here locally so I don't have
to fight the network...
Re: [PATCH] mod_ssl input filtering...
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Oct 04, 2001 at 01:03:32PM -0700, Justin Erenkrantz wrote:
>...
> --- modules/ssl/ssl_engine_io.c 2001/10/04 17:50:39 1.37
> +++ modules/ssl/ssl_engine_io.c 2001/10/04 19:54:22
>...
> static apr_status_t ssl_io_filter_Input(ap_filter_t *f,
> apr_bucket_brigade *pbbOut,
> - ap_input_mode_t eMode,
> + ap_input_mode_t mode,
> apr_off_t *readbytes)
> {
> apr_status_t ret;
> - SSLFilterRec *pRec = f->ctx;
> + SSLFilterRec *ctx = f->ctx;
> + apr_status_t rv;
> + apr_bucket *e;
> + apr_off_t *tempread;
>
> - /* XXX: we don't currently support peek */
> - if (eMode == AP_MODE_PEEK) {
> + /* XXX: we don't currently support peek or readbytes == -1 */
> + if (mode == AP_MODE_PEEK || *readbytes == -1) {
> return APR_ENOTIMPL;
> }
> +
> + /* Return the requested amount or less. */
> + if (*readbytes)
> + {
> + /* churn the state machine */
> + ret = churn_input(ctx, mode, readbytes);
> +
> + if (ret != APR_SUCCESS)
> + return ret;
> +
> + APR_BRIGADE_CONCAT(pbbOut, ctx->b);
> + return APR_SUCCESS;
> + }
> +
> + /* Readbytes == 0 implies we only want a LF line.
> + * 1024 seems like a good number for now. */
> + *tempread = 1024;
*tempread ?? You might want to assign something to tempread first :-)
>...
> + pos = memchr(str, APR_ASCII_LF, len);
How about a new brigade function first?
I don't know much about the rest of how this filter works, but I am
seriously suspicious of the removal of the renegotiation code.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/