You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2002/11/01 09:35:20 UTC
cvs commit: httpd-2.0 CHANGES
wrowe 2002/11/01 00:35:19
Modified: modules/ssl ssl_engine_io.c
. CHANGES
Log:
Completely refactor the BIO-side client input handling for the SSL library.
Should eliminate many false spurious interrupt detected errors.
Revision Changes Path
1.83 +189 -137 httpd-2.0/modules/ssl/ssl_engine_io.c
Index: ssl_engine_io.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.82
retrieving revision 1.83
diff -u -r1.82 -r1.83
--- ssl_engine_io.c 17 Oct 2002 13:25:08 -0000 1.82
+++ ssl_engine_io.c 1 Nov 2002 08:35:19 -0000 1.83
@@ -301,7 +301,6 @@
ap_input_mode_t mode;
apr_read_type_e block;
apr_bucket_brigade *bb;
- apr_bucket *bucket;
char_buffer_t cbuf;
} BIO_bucket_in_t;
@@ -352,11 +351,86 @@
*/
#define BIO_bucket_in_ptr(bio) (BIO_bucket_in_t *)bio->ptr
+static apr_status_t brigade_consume(apr_bucket_brigade *bb,
+ apr_read_type_e block,
+ char *c, apr_size_t *len)
+{
+ apr_size_t actual = 0;
+
+ while (!APR_BRIGADE_EMPTY(bb)) {
+ apr_bucket *b = APR_BRIGADE_FIRST(bb);
+ const char *str;
+ apr_status_t status;
+ apr_size_t str_len;
+ apr_size_t consume;
+
+ if (APR_BUCKET_IS_EOS(b)) {
+ *len = 0;
+ return APR_EOF;
+ }
+
+ status = apr_bucket_read(b, &str, &str_len, block);
+
+ if (status != APR_SUCCESS) {
+ if (status == APR_EOF) {
+ /* This stream bucket was consumed */
+ APR_BUCKET_REMOVE(b);
+ apr_bucket_destroy(b);
+ continue;
+ }
+
+ *len = actual;
+ return status;
+ }
+
+ if (str_len > 0) {
+ /* Do not block once some data has been consumed */
+ block = APR_NONBLOCK_READ;
+
+ /* Assure we don't overflow. */
+ consume = (str_len + actual > *len) ? *len - actual : str_len;
+
+ memcpy(c, str, consume);
+
+ c += consume;
+ actual += consume;
+ }
+
+ if (b->start >= 0) {
+ if (consume >= b->length) {
+ /* This physical bucket was consumed */
+ APR_BUCKET_REMOVE(b);
+ apr_bucket_destroy(b);
+ }
+ else {
+ /* Only part of this physical bucket was consumed */
+ b->start += consume;
+ b->length -= consume;
+ }
+ }
+
+ /* This could probably be actual == *len, but be safe from stray
+ * photons. */
+ if (actual >= *len) {
+ break;
+ }
+ }
+
+ *len = actual;
+ return APR_SUCCESS;
+}
+
static int bio_bucket_in_read(BIO *bio, char *in, int inl)
{
BIO_bucket_in_t *inbio = BIO_bucket_in_ptr(bio);
+ apr_read_type_e block = inbio->block;
SSLConnRec *sslconn = myConnConfig(inbio->f->c);
- int len = 0;
+
+ inbio->rc = APR_SUCCESS;
+
+ /* OpenSSL catches this case, so should we. */
+ if (!in)
+ return 0;
/* XXX: flush here only required for SSLv2;
* OpenSSL calls BIO_flush() at the appropriate times for
@@ -366,93 +440,64 @@
BIO_bucket_flush(inbio->wbio);
}
- inbio->rc = APR_SUCCESS;
-
- /* first use data already read from socket if any */
- if ((len = char_buffer_read(&inbio->cbuf, in, inl))) {
- if ((len <= inl) || inbio->mode == AP_MODE_GETLINE) {
- return len;
- }
- inl -= len;
- }
+ BIO_clear_retry_flags(bio);
- while (1) {
- const char *buf;
- apr_size_t buf_len = 0;
+ if (!inbio->bb) {
+ inbio->rc = APR_EOF;
+ return -1;
+ }
- if (inbio->bucket) {
- /* all of the data in this bucket has been read,
- * so we can delete it now.
- */
- apr_bucket_delete(inbio->bucket);
- inbio->bucket = NULL;
- }
+ if (APR_BRIGADE_EMPTY(inbio->bb)) {
- if (APR_BRIGADE_EMPTY(inbio->bb)) {
- /* We will always call with READBYTES even if the user wants
- * GETLINE.
- */
- inbio->rc = ap_get_brigade(inbio->f->next, inbio->bb,
- AP_MODE_READBYTES, inbio->block,
- inl);
+ inbio->rc = ap_get_brigade(inbio->f->next, inbio->bb,
+ AP_MODE_READBYTES, block,
+ inl);
- if ((inbio->rc != APR_SUCCESS) || APR_BRIGADE_EMPTY(inbio->bb))
- {
- break;
- }
+ /* Not a problem, there was simply no data ready yet.
+ */
+ if (APR_STATUS_IS_EAGAIN(inbio->rc) || APR_STATUS_IS_EINTR(inbio->rc)
+ || (inbio->rc == APR_SUCCESS && APR_BRIGADE_EMPTY(inbio->bb))) {
+ BIO_set_retry_read(bio);
+ return 0;
}
- inbio->bucket = APR_BRIGADE_FIRST(inbio->bb);
-
- inbio->rc = apr_bucket_read(inbio->bucket,
- &buf, &buf_len, inbio->block);
-
if (inbio->rc != APR_SUCCESS) {
- apr_bucket_delete(inbio->bucket);
- inbio->bucket = NULL;
- return len;
+ /* Unexpected errors discard the brigade */
+ apr_brigade_cleanup(inbio->bb);
+ inbio->bb = NULL;
+ return -1;
}
+ }
- if (buf_len) {
- /* Protected against len > MAX_INT
- */
- if ((len + (int)buf_len) >= inl || (int)buf_len < 0) {
- /* we have enough to fill the buffer.
- * append if we have already written to the buffer.
- */
- int nibble = inl - len;
- char *value = (char *)buf+nibble;
-
- int length = buf_len - nibble;
- memcpy(in + len, buf, nibble);
+ inbio->rc = brigade_consume(inbio->bb, block, in, &inl);
- char_buffer_write(&inbio->cbuf, value, length);
- len += nibble;
+ if (inbio->rc == APR_SUCCESS) {
+ return inl;
+ }
- break;
- }
- else {
- /* not enough data,
- * save what we have and try to read more.
- */
- memcpy(in + len, buf, buf_len);
- len += buf_len;
- }
- }
+ if (APR_STATUS_IS_EAGAIN(inbio->rc)
+ || APR_STATUS_IS_EINTR(inbio->rc)) {
+ BIO_set_retry_read(bio);
+ return inl;
+ }
+
+ /* Unexpected errors and APR_EOF clean out the brigade.
+ * Subsequent calls will return APR_EOF.
+ */
+ apr_brigade_cleanup(inbio->bb);
+ inbio->bb = NULL;
- if (inbio->mode == AP_MODE_GETLINE) {
- /* only read from the socket once in getline mode.
- * since callers buffer size is likely much larger than
- * the request headers. caller can always come back for more
- * if first read didn't get all the headers.
- */
- break;
- }
+ if ((inbio->rc == APR_EOF) && inl) {
+ /* Provide the results of this read pass,
+ * without resetting the BIO retry_read flag
+ */
+ return inl;
}
- return len;
+ return -1;
}
+
static BIO_METHOD bio_bucket_in_method = {
BIO_TYPE_MEM,
"APR input bucket brigade",
@@ -475,42 +520,6 @@
static const char ssl_io_filter[] = "SSL/TLS Filter";
-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) {
- int ssl_err = SSL_get_error(ssl, rc);
-
- if (ssl_err == SSL_ERROR_WANT_READ) {
- /*
- * Simulate an EINTR in case OpenSSL wants to read more.
- * (This is usually the case when the client forces an SSL
- * renegotation which is handled implicitly by OpenSSL.)
- */
- errno = EINTR;
- rc = 0; /* non fatal error */
- }
- else if (ssl_err == SSL_ERROR_SSL) {
- /*
- * Log SSL errors
- */
- conn_rec *c = (conn_rec *)SSL_get_app_data(ssl);
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, c->base_server,
- "SSL error on reading data");
- ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, c->base_server);
- }
- }
-
- return rc;
-}
-
static int ssl_io_hook_write(SSL *ssl, unsigned char *buf, int len)
{
int rc;
@@ -640,11 +649,6 @@
return status;
}
-/*
- * ctx->cbuf is leftover plaintext from ssl_io_input_getline,
- * use what we have there first if any,
- * then go for more by calling ssl_io_hook_read.
- */
static apr_status_t ssl_io_input_read(ssl_io_input_ctx_t *ctx,
char *buf,
apr_size_t *len)
@@ -668,26 +672,75 @@
}
}
- rc = ssl_io_hook_read(ctx->frec->pssl, buf + bytes, wanted - bytes);
+ while (1) {
- if (rc > 0) {
- *len += rc;
- if (ctx->inbio.mode == AP_MODE_SPECULATIVE) {
- char_buffer_write(&ctx->cbuf, buf, rc);
- }
- }
- else if ((rc == 0) && (errno != EINTR)) {
- /* something other than SSL_ERROR_WANT_READ */
- return APR_EOF;
- }
- else if ((rc == -1) && (ctx->inbio.rc == APR_SUCCESS)) {
- /*
- * bucket read from socket was successful,
- * but there was an error within the ssl runtime.
+ /* SSL_read may not read because we haven't taken enough data
+ * from the stack. This is where we want to consider all of
+ * the blocking and SPECULATIVE semantics
*/
- return APR_EGENERAL;
- }
+ rc = SSL_read(ctx->frec->pssl, buf + bytes, wanted - bytes);
+ if (rc > 0) {
+ *len += rc;
+ if (ctx->inbio.mode == AP_MODE_SPECULATIVE) {
+ char_buffer_write(&ctx->cbuf, buf, rc);
+ }
+ return ctx->inbio.rc;
+ }
+ else if (rc == 0) {
+ /* If EAGAIN, we will loop given a blocking read,
+ * otherwise consider ourselves at EOF.
+ */
+ if (APR_STATUS_IS_EAGAIN(ctx->inbio.rc)
+ || APR_STATUS_IS_EINTR(ctx->inbio.rc)) {
+ if (ctx->inbio.block == APR_NONBLOCK_READ) {
+ break;
+ }
+ }
+ else {
+ ctx->inbio.rc = APR_EOF;
+ break;
+ }
+ }
+ else /* (rc < 0) */ {
+ int ssl_err = SSL_get_error(ctx->frec->pssl, rc);
+
+ if (ssl_err == SSL_ERROR_WANT_READ) {
+ /*
+ * If OpenSSL wants to read more, and we were nonblocking,
+ * report as an EAGAIN. Otherwise loop, pulling more
+ * data from network filter.
+ *
+ * (This is usually the case when the client forces an SSL
+ * renegotation which is handled implicitly by OpenSSL.)
+ */
+ if (ctx->inbio.block == APR_NONBLOCK_READ) {
+ ctx->inbio.rc = APR_EAGAIN;
+ break; /* non fatal error */
+ }
+ }
+ if (ssl_err == SSL_ERROR_SYSCALL) {
+ conn_rec *c = (conn_rec *)SSL_get_app_data(ctx->frec->pssl);
+ ap_log_error(APLOG_MARK, APLOG_ERR, ctx->inbio.rc, c->base_server,
+ "SSL filter error reading data");
+ break;
+ }
+ else if (ssl_err == SSL_ERROR_SSL) {
+ /*
+ * Log SSL errors and any unexpected conditions.
+ */
+ conn_rec *c = (conn_rec *)SSL_get_app_data(ctx->frec->pssl);
+ ap_log_error(APLOG_MARK, APLOG_ERR, ctx->inbio.rc, c->base_server,
+ "SSL library error reading data");
+ ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, c->base_server);
+
+ if (ctx->inbio.rc == APR_SUCCESS) {
+ ctx->inbio.rc = APR_EGENERAL;
+ }
+ break;
+ }
+ }
+ }
return ctx->inbio.rc;
}
@@ -745,13 +798,11 @@
* we use a flag in the conn_rec->conn_vector now. The fake request just
* gets the request back to the Apache core so that a response can be sent.
*
- * We should probably use a 0.9 request, but the BIO bucket code is calling
- * socket_bucket_read one extra time with all 0.9 requests from the client.
- * Until that is resolved, continue to use a 1.0 request, just like we
- * always have.
+ * To avoid calling back for more data from the socket, use an HTTP/0.9
+ * request, and tack on an EOS bucket.
*/
#define HTTP_ON_HTTPS_PORT \
- "GET / HTTP/1.0"
+ "GET /" CRLF
#define HTTP_ON_HTTPS_PORT_BUCKET(alloc) \
apr_bucket_immortal_create(HTTP_ON_HTTPS_PORT, \
@@ -793,6 +844,8 @@
}
APR_BRIGADE_INSERT_TAIL(bb, bucket);
+ bucket = apr_bucket_eos_create(f->c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, bucket);
return APR_SUCCESS;
}
@@ -887,7 +940,6 @@
ctx->inbio.wbio = frec->pbioWrite;
ctx->inbio.f = frec->pInputFilter;
ctx->inbio.bb = apr_brigade_create(c->pool, c->bucket_alloc);
- ctx->inbio.bucket = NULL;
ctx->inbio.cbuf.length = 0;
ctx->cbuf.length = 0;
1.968 +4 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.967
retrieving revision 1.968
diff -u -r1.967 -r1.968
--- CHANGES 31 Oct 2002 11:53:43 -0000 1.967
+++ CHANGES 1 Nov 2002 08:35:19 -0000 1.968
@@ -1,5 +1,9 @@
Changes with Apache 2.0.44
+ *) Gracefully handly retry situations in the SSL input filter,
+ by following the SSL libraries' retry semantics.
+ [William Rowe]
+
*) Terminate CGI scripts when the client connection drops. This
fix only applies to some normal paths in mod_cgi. mod_cgid
is still busted. PR 8388 [Jeff Trawick]