httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jerenkra...@ebuilt.com>
Subject [PATCH] mod_ssl input filtering...
Date Fri, 05 Oct 2001 09:50:14 GMT
Well, it mostly works.  It isn't perfect and some cases aren't
being handled.  However, I'm not sure how much time I'm going
to have today to work on it.  So, I'll post what I have to the
list and see what you all think.  

I think churn_input is mostly working - most of the problems
are with ssl_io_filter_Input.  On some more thought, I think
I would like it to be quicker to return 0 length buckets back
to the higher levels rather than retrying internally.  That
would simplify some code considerably.

The biggest problem here is that blocking on the socket_read
(what happened previously since it got the socket bucket back) 
would just return the next data set read from the socket.  Now, if 
you rely on the core to give you data, you must specify a length 
and you *only* get that length.  This is what leads to all of 
the complications.  It is due to apr_brigade_partition - I
committed a change into server/core.c that allows it to handle
non-blocking reads correctly.

Ideally, we almost need another mode type that says, "I want you to 
block until the first packet received and then return that to me."
This is along the lines of what Madhu was talking about earlier.
And, I now see the light.  =)

Anyway, I'm tired...  -- justin

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/05 09:34:17
@@ -72,7 +72,7 @@
 
 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;
 
@@ -189,75 +189,64 @@
 
 #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"
 
-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(APR_BRIGADE_EMPTY(pRec->pbbInput)) {
-	ap_get_brigade(pRec->pInputFilter->next,pRec->pbbInput,eMode,readbytes);
-	if(APR_BRIGADE_EMPTY(pRec->pbbInput))
-	    return APR_EOF;
+    /* We have something in the processed brigade.  Use that first. */
+    if (!APR_BRIGADE_EMPTY(ctx->b)) {
+        return APR_SUCCESS;
     }
-
-    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;
-	}
+    /* If we have nothing in the raw brigade, get some more. */
+    if (APR_BRIGADE_EMPTY(ctx->rawb)) {
+        rv = ap_get_brigade(f->next, ctx->rawb, eMode, readbytes);
 
-	/* read filter */
-	ret = apr_bucket_read(pbktIn, &data, &len, (apr_read_type_e)eMode);
+        if (rv != APR_SUCCESS)
+            return rv;
 
-        if (!(eMode == AP_MODE_NONBLOCKING && APR_STATUS_IS_EAGAIN(ret))) {
-            /* allow retry */
-            APR_BUCKET_REMOVE(pbktIn);
+        /* Can't make any progress here. */
+        if (*readbytes == 0)
+        {
+            /* This should be the immortal bucket. */
+            if (!APR_BRIGADE_EMPTY(ctx->rawb)) {
+                APR_BRIGADE_INSERT_TAIL(ctx->b, apr_bucket_eos_create());
+            }
+            return APR_SUCCESS;
         }
+    }
 
-	if (ret == APR_SUCCESS && len == 0 && eMode == AP_MODE_BLOCKING)
-	    ret = APR_EOF;
+    /* Process anything we have that we haven't done so already. */
+    while (!APR_BRIGADE_EMPTY(ctx->rawb)) {
+        const char *data;
+        apr_size_t len;
 
-        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");
+        e = APR_BRIGADE_FIRST(ctx->rawb);
 
-	    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 (APR_BUCKET_IS_EOS(e)) {
+            apr_bucket_delete(e);
+            found_eos = 1;
+            break;
+        }
 
-	n = BIO_write (pRec->pbioRead, data, len);
+        /* 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 +259,84 @@
             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);
 
-        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;
+    }
+
+    /* Flush the output buffers. */
+    churn_output(pRec);
+
+    /* Before we actually read any unencrypted data, go ahead and
+     * let ssl_hook_process_connection have a shot at it. 
+     */
+    rv = ssl_hook_process_connection(pRec);
+
+    /* Flush again. */
+    churn_output(pRec);
+
+    if (rv != 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");
+
+            /* fake the request line */
+            e = apr_bucket_immortal_create(HTTP_ON_HTTPS_PORT,
+                                           sizeof(HTTP_ON_HTTPS_PORT) - 1);
+            APR_BRIGADE_INSERT_TAIL(ctx->b, e);
+            e = apr_bucket_immortal_create(CRLF, sizeof(CRLF) - 1);
+            APR_BRIGADE_INSERT_TAIL(ctx->b, e);
+
+            return APR_SUCCESS;
         }
+        /* It lies.  It really wants a flush and a write. */
+        if (rv == SSL_ERROR_WANT_READ) {
+            apr_off_t tempread = 1024;
+            return churn_input(pRec, AP_MODE_NONBLOCKING, &tempread);
+        }
+        return rv;
+    }
 
-        /* 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;
+    /* 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;
+
+        pbuf = apr_pmemdup(p, buf, n);
+        e = apr_bucket_pool_create(pbuf, n, p);
+        APR_BRIGADE_INSERT_TAIL(ctx->b, e);
 
-	    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;
+        /* Flush the output buffers. */
+        churn_output(pRec);
     }
 
+    if (n < 0 && errno == EINTR) {
+        apr_off_t tempread = 1024;
+        return churn_input(pRec, AP_MODE_NONBLOCKING, &tempread);
+    }
+
+    if (found_eos)
+        APR_BRIGADE_INSERT_TAIL(ctx->b, apr_bucket_eos_create());
+
     return churn_output(pRec);
 }
 
@@ -376,23 +399,99 @@
 
 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);
 
-    /* churn the state machine */
-    ret = churn_input(pRec, eMode, readbytes);
-    if (ret != APR_SUCCESS)
-	return ret;
+        if (ret != APR_SUCCESS)
+	        return ret;
 
-    APR_BRIGADE_CONCAT(pbbOut, pRec->pbbPendingInput);
+        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. */
+    if (APR_BRIGADE_EMPTY(ctx->b)) {
+        tempread = 1024; 
+        rv = churn_input(ctx, AP_MODE_NONBLOCKING, &tempread);
+        if (rv != APR_SUCCESS)
+            return rv;
+        if (APR_BRIGADE_EMPTY(ctx->b))
+        {
+            if (mode == AP_MODE_BLOCKING) {
+                tempread = 1;
+                rv = churn_input(ctx, AP_MODE_BLOCKING, &tempread);
+
+                if (rv != APR_SUCCESS)
+                {
+                    APR_BRIGADE_CONCAT(pbbOut, ctx->b);
+                    return APR_SUCCESS;
+                }
+            }
+            if (APR_BRIGADE_EMPTY(ctx->b))
+            {
+                e = apr_bucket_immortal_create("", 0);
+                APR_BRIGADE_INSERT_TAIL(pbbOut, e);
+                return APR_SUCCESS;
+            }
+        }
+    }
+    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;
+        }
+
+        if ((rv = apr_bucket_read(e, &str, &len, 
+                                  AP_MODE_NONBLOCKING)) != APR_SUCCESS) {
+            return rv;
+        }
+
+        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, AP_MODE_NONBLOCKING, &tempread);
+            if (ret != APR_SUCCESS)
+	            return ret;
+        }
+    }
 
     return APR_SUCCESS;
 }
@@ -423,8 +522,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);
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/05 09:34:17
@@ -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[] = {
 
     /*
@@ -363,47 +361,15 @@
                  * borrowed from openssl_state_machine.c [mod_tls].
                  * TBD.
                  */
-                return 0;
+                return SSL_ERROR_WANT_READ;
             }
             else if (ERR_GET_REASON(ERR_peek_error()) == SSL_R_HTTP_REQUEST) {
                 /*
                  * 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/05 09:34:17
@@ -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 {


Mime
View raw message