From dev-return-34501-apmail-httpd-dev-archive=httpd.apache.org@httpd.apache.org Tue Nov 05 21:40:37 2002 Return-Path: Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 36073 invoked by uid 500); 5 Nov 2002 21:40:37 -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 35681 invoked from network); 5 Nov 2002 21:40:31 -0000 Date: Tue, 05 Nov 2002 13:40:31 -0800 From: Justin Erenkrantz Reply-To: Justin Erenkrantz To: dev@httpd.apache.org Subject: Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.c mod_ssl.h ssl_engine_io.c ssl_engine_kernel.c Message-ID: <377960000.1036532431@cite.ics.uci.edu> In-Reply-To: <20021105204701.11963.qmail@icarus.apache.org> References: <20021105204701.11963.qmail@icarus.apache.org> X-Mailer: Mulberry/3.0.0a5 (SunOS/SPARC) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline X-Spam-Status: No, hits=-8.2 required=5.0 tests=IN_REP_TO,QUOTED_EMAIL_TEXT,REFERENCES,SPAM_PHRASE_00_01 version=2.50-cvs X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N --On Tuesday, November 05, 2002 20:47:01 +0000 wrowe@apache.org wrote: > Index: ssl_engine_io.c > =================================================================== > RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v > retrieving revision 1.95 > retrieving revision 1.96 > diff -u -r1.95 -r1.96 > --- ssl_engine_io.c 5 Nov 2002 06:38:41 -0000 1.95 > +++ ssl_engine_io.c 5 Nov 2002 20:47:01 -0000 1.96 > @@ -73,15 +73,15 @@ > * remember what is in this file. So, first, a quick overview. > * > * In this file, you will find: > - * - ssl_io_filter_Input (Apache input filter) > - * - ssl_io_filter_Output (Apache output filter) > + * - ssl_io_filter_input (Apache input filter) > + * - ssl_io_filter_output (Apache output filter) Woo-hoo! > @@ -129,7 +129,15 @@ > */ > > typedef struct { > - SSLFilterRec *filter_ctx; > + SSL *pssl; > + BIO *pbioRead; > + BIO *pbioWrite; > + ap_filter_t *pInputFilter; > + ap_filter_t *pOutputFilter; > +} ssl_filter_ctx_t; > + > +typedef struct { > + ssl_filter_ctx_t *filter_ctx; > conn_rec *c; > apr_bucket_brigade *bb; > apr_size_t length; > @@ -138,7 +146,7 @@ > apr_status_t rc; > } bio_filter_out_ctx_t; > > -static bio_filter_out_ctx_t *bio_filter_out_ctx_new(SSLFilterRec > *filter_ctx, +static bio_filter_out_ctx_t > *bio_filter_out_ctx_new(ssl_filter_ctx_t *filter_ctx, > conn_rec *c) { > bio_filter_out_ctx_t *outctx = apr_palloc(c->pool, > sizeof(*outctx)); @@ -348,7 +356,7 @@ > char_buffer_t cbuf; > apr_pool_t *pool; > char buffer[AP_IOBUFSIZE]; > - SSLFilterRec *filter_ctx; > + ssl_filter_ctx_t *filter_ctx; > } bio_filter_in_ctx_t; > > /* > @@ -887,6 +754,74 @@ > return APR_SUCCESS; > } > > + > +static apr_status_t ssl_filter_write(ap_filter_t *f, > + const char *data, > + apr_size_t len) > +{ > + ssl_filter_ctx_t *filter_ctx = f->ctx; > + bio_filter_out_ctx_t *outctx = > + (bio_filter_out_ctx_t *)(filter_ctx->pbioWrite->ptr); > + int res; > + > + /* write SSL */ > + if (filter_ctx->pssl == NULL) { > + return APR_EGENERAL; > + } > + > + res = SSL_write(filter_ctx->pssl, (unsigned char *)data, len); > + > + if (res < 0) { > + int ssl_err = SSL_get_error(filter_ctx->pssl, res); > + > + if (ssl_err == SSL_ERROR_WANT_WRITE) { > + /* > + * If OpenSSL wants to write more, and we were nonblocking, > + * report as an EAGAIN. Otherwise loop, pushing more > + * data at the network filter. > + * > + * (This is usually the case when the client forces an SSL > + * renegotation which is handled implicitly by OpenSSL.) > + */ > + outctx->rc = APR_EAGAIN; > + } > + else if (ssl_err == SSL_ERROR_SYSCALL) { > + conn_rec *c = > (conn_rec*)SSL_get_app_data(outctx->filter_ctx->pssl); > + ap_log_error(APLOG_MARK, APLOG_ERR, outctx->rc, c->base_server, > + "SSL filter out error writing data"); Can't we get the conn_rec from outctx->filter_ctx->pOutputFilter->c rather than trying to be cute and calling SSL_get_app_data? This pattern seems to be happening a lot in this code and this seems like a better and much faster approach. > + } > + else /* if (ssl_err == SSL_ERROR_SSL) */ { > + /* > + * Log SSL errors > + */ > + conn_rec *c = (conn_rec > *)SSL_get_app_data(filter_ctx->pssl); + > ap_log_error(APLOG_MARK, APLOG_ERR, outctx->rc, c->base_server, + > "SSL library out error writing data"); > + ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, c->base_server); > + > + } > + if (outctx->rc == APR_SUCCESS) { > + outctx->rc = APR_EGENERAL; > + } > + } > + else if ((apr_size_t)res != len) { Just as a note that might want to enable partial write support in OpenSSL. Not sure, but it might perform better? So, we may want to remove the res != len check at some point. -- justin