Return-Path: Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 41559 invoked by uid 500); 23 Nov 2001 23:54:06 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 41546 invoked from network); 23 Nov 2001 23:54:04 -0000 X-Authentication-Warning: mako.covalent.net: dougm owned process doing -bs Date: Fri, 23 Nov 2001 15:57:58 -0800 (PST) From: Doug MacEachern X-Sender: dougm@localhost To: dev@httpd.apache.org Subject: Re: [patch] major ssl problem In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N here's a new version of the patch.. still solves the major problem of filling the pbioWrite memory buffer with the same size as the file. this could also be solved with something like: while (len) { len -= SSL_write(smaller buff size); churn_output(); } but using the custom BIO avoids the extra copy that currently happens in bbioWrite. and makes it possible to optimize further based on what is best for bucket brigades and the core output filter. Index: modules/ssl/ssl_engine_io.c =================================================================== RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v retrieving revision 1.44 diff -u -r1.44 ssl_engine_io.c --- modules/ssl/ssl_engine_io.c 2001/11/19 22:37:57 1.44 +++ modules/ssl/ssl_engine_io.c 2001/11/23 23:29:24 @@ -70,6 +70,231 @@ /* XXX THIS STUFF NEEDS A MAJOR CLEANUP -RSE XXX */ +typedef struct { + SSLFilterRec *frec; + conn_rec *c; + apr_bucket_brigade *bb; + apr_size_t length; + char buffer[AP_IOBUFSIZE]; + apr_size_t blen; + int num_buckets; +} BIO_bucket_t; + +#define BIO_bucket_ptr(bio) (BIO_bucket_t *)bio->ptr + +static int bucket_write(BIO *bio, const char *buf, int num); +static int bucket_read(BIO *bio, char *buf, int size); +static int bucket_puts(BIO *bio, const char *str); +static int bucket_gets(BIO *bio, char *str, int size); +static long bucket_ctrl(BIO *bio, int cmd, long arg1, void *arg2); +static int bucket_new(BIO *bio); +static int bucket_free(BIO *bio); + +static BIO_METHOD bucket_method = { + BIO_TYPE_MEM, + "APR bucket buffer", + bucket_write, + bucket_read, + bucket_puts, + bucket_gets, + bucket_ctrl, + bucket_new, + bucket_free, + NULL, +}; + +static BIO_METHOD *BIO_s_bucket(void) +{ + return &bucket_method; +} + +static BIO_bucket_t *BIO_bucket_new(SSLFilterRec *frec, conn_rec *c) +{ + BIO_bucket_t *b = apr_palloc(c->pool, sizeof(*b)); + + b->frec = frec; + b->c = c; + b->bb = apr_brigade_create(c->pool); + b->blen = 0; + b->num_buckets = 0; + + return b; +} + +static int BIO_bucket_flush(BIO *bio) +{ + BIO_bucket_t *b = BIO_bucket_ptr(bio); + + if (!(b->blen || b->length)) { + return APR_SUCCESS; + } + + if (b->blen) { + apr_bucket *bucket = + apr_bucket_transient_create(b->buffer, + b->blen); + /* we filled this buffer first so add it to the + * head of the brigade + */ + APR_BRIGADE_INSERT_HEAD(b->bb, bucket); + b->blen = 0; + } + + b->num_buckets = 0; + b->length = 0; + APR_BRIGADE_INSERT_TAIL(b->bb, apr_bucket_flush_create()); + + return ap_pass_brigade(b->frec->pOutputFilter->next, b->bb); +} + +static int bucket_new(BIO *bio) +{ + bio->shutdown = 1; + bio->init = 1; + bio->num = -1; + bio->ptr = NULL; + + return 1; +} + +static int bucket_free(BIO *bio) +{ + if (bio == NULL) { + return 0; + } + + if (bio->shutdown) { + if ((bio->init) && (bio->ptr != NULL)) { + BIO_bucket_t *b = BIO_bucket_ptr(bio); + b->bb = NULL; + bio->ptr = NULL; + } + } + + return 1; +} + +static int bucket_read(BIO *bio, char *out, int outl) +{ + /* this is never called */ + return -1; +} + +/* + * if we use heap buckets we can buffer before flushing to the core + * but this means we have to malloc/free data in each bucket. + * if we use transient buckets, no malloc/free for the data + * but we have to flush right away. + */ +static int bucket_write(BIO *bio, const char *in, int inl) +{ + BIO_bucket_t *b = BIO_bucket_ptr(bio); + + /* when handshaking we'll have a small number of bytes. + * max size SSL will pass us here is about 16k. + * (16413 bytes to be exact) + */ + BIO_clear_retry_flags(bio); + + if (!b->length && (inl < (sizeof(b->buffer) - b->blen))) { + /* the first two SSL_writes (of 1024 and 261 bytes) + * need to be in the same packet (vec[0].iov_base) + */ + /* XXX: could use apr_brigade_write() to make code look cleaner + * but this way we avoid the malloc(APR_BUCKET_BUFF_SIZE) + * and free() of it later + */ + memcpy(&b->buffer[b->blen], in, inl); + if (b->blen == 0) { + /* a bucket will be created in BIO_bucket_flush() */ + b->num_buckets++; + } + b->blen += inl; + } + else { +#ifdef SSL_USE_HEAP_BUCKETS + apr_bucket *bucket = apr_bucket_heap_create(in, inl, 1); +#else + apr_bucket *bucket = apr_bucket_transient_create(in, inl); +#endif + + b->length += inl; + APR_BRIGADE_INSERT_TAIL(b->bb, bucket); + +#ifdef SSL_USE_HEAP_BUCKETS +# define SSL_MAX_BUCKETS 16 /* MAX_IOVEC_TO_WRITE */ + b->num_buckets++; + if (b->num_buckets == SSL_MAX_BUCKETS) { + /* prevent core_output_filter from doing apr_brigade_split */ + BIO_bucket_flush(bio); + } +#else + BIO_bucket_flush(bio); +#endif + } + + return inl; +} + +static long bucket_ctrl(BIO *bio, int cmd, long num, void *ptr) +{ + long ret = 1; + + BIO_bucket_t *b = BIO_bucket_ptr(bio); + + switch (cmd) { + case BIO_CTRL_RESET: + ret = 0; /*XXX*/ + break; + case BIO_CTRL_EOF: + ret = 0; /*XXX*/ + break; + case BIO_CTRL_INFO: + ret = 0; /*XXX*/ + break; + case BIO_CTRL_GET_CLOSE: + ret = (long)bio->shutdown; + break; + case BIO_CTRL_SET_CLOSE: + bio->shutdown = (int)num; + break; + case BIO_CTRL_WPENDING: + ret = 0L; + break; + case BIO_CTRL_PENDING: + ret = (long)b->length; + break; + case BIO_CTRL_FLUSH: + ret = (BIO_bucket_flush(bio) == APR_SUCCESS); + break; + case BIO_CTRL_DUP: + ret = 1; + break; + case BIO_CTRL_PUSH: + case BIO_CTRL_POP: + case BIO_C_SET_BUF_MEM_EOF_RETURN: + case BIO_C_SET_BUF_MEM: + case BIO_C_GET_BUF_MEM_PTR: + default: + ret = 0; + break; + } + + return ret; +} + +static int bucket_gets(BIO *bio, char *buf, int size) +{ + /* this is never called */ + return -1; +} + +static int bucket_puts(BIO *bio, const char *str) +{ + /* this is never called */ + return -1; +} + static const char ssl_io_filter[] = "SSL/TLS Filter"; static int ssl_io_hook_read(SSL *ssl, char *buf, int len) @@ -146,45 +371,14 @@ return rc; } -#define BIO_mem(b) ((BUF_MEM *)b->ptr) - static apr_status_t churn_output(SSLFilterRec *ctx) { - ap_filter_t *f = ctx->pOutputFilter; - apr_pool_t *p = f->c->pool; - if (!ctx->pssl) { /* we've been shutdown */ return APR_EOF; } - - if (BIO_pending(ctx->pbioWrite)) { - BUF_MEM *bm = BIO_mem(ctx->pbioWrite); - apr_bucket_brigade *bb = apr_brigade_create(p); - apr_bucket *bucket; - /* - * use the BIO memory buffer that has already been allocated, - * rather than making another copy of it. - * use bm directly here is *much* faster than calling BIO_read() - * look at crypto/bio/bss_mem.c:mem_read and you'll see why - */ - - bucket = apr_bucket_transient_create((const char *)bm->data, - bm->length); - - bm->length = 0; /* reset */ - - APR_BRIGADE_INSERT_TAIL(bb, bucket); - - /* XXX: it may be possible to not always flush */ - bucket = apr_bucket_flush_create(); - APR_BRIGADE_INSERT_TAIL(bb, bucket); - - return ap_pass_brigade(f->next, bb); - } - - return APR_SUCCESS; + return BIO_bucket_flush(ctx->pbioWrite); } #define bio_is_renegotiating(bio) \ @@ -583,7 +777,8 @@ 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()); + filter->pbioWrite = BIO_new(BIO_s_bucket()); + filter->pbioWrite->ptr = BIO_bucket_new(filter, c); SSL_set_bio(ssl, filter->pbioRead, filter->pbioWrite); filter->pssl = ssl;