Hi,
I've tried to cleanup some of the logic in the filter processing of
the SSL data. The changes include :
- eliminated the use of ssl_log - it used to cause seg faults during cleanup
since the conn_rec will no longer be valid.
- eliminated the "for (;;)" processing loop in ssl_io_filter_Output() -
we'll have to do that in churn_output() if required, so that any remaining
OpenSSL data (if available) is transferred before we call the
CloseConnection.
- Any remaining data in SSL should be cleaned up ideally in the
APR_BUCKET_IS_EOS() processing stage itself, as we close the SSL connection
here.
Pl. let me know if you have any comments / suggestions.
Thanks
-Madhu & Julius
<<patch_filter.txt>>
Index: ssl_engine_io.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.9
diff -u -r1.9 ssl_engine_io.c
--- ssl_engine_io.c 2001/07/24 19:00:12 1.9
+++ ssl_engine_io.c 2001/07/31 00:07:19
@@ -323,62 +323,30 @@
{
SSLFilterRec *pRec=f->ctx;
apr_bucket *pbktIn;
- conn_rec *c = SSL_get_app_data (pRec->pssl);
- if (!c) {
- /* if this happens we have already called ssl_hook_CloseConnection
- * if we dont return here, this routine will segv
- * XXX: this doesnt seem right, ssl_hook_CloseConnection probably
- * is being called to early, but as the README:TODO says:
- * "Cleanup ssl_engine_io.c !!"
- */
- return APR_EOF;
- }
-
APR_BRIGADE_FOREACH(pbktIn,pbbIn) {
const char *data;
apr_size_t len, n;
apr_status_t ret;
if(APR_BUCKET_IS_EOS(pbktIn)) {
- /* XXX: demote to debug */
- ssl_log(c->base_server, SSL_LOG_INFO, "EOS in output");
+ if ((ret = churn_output(pRec)) != APR_SUCCESS)
+ {
+ ap_log_error(
+ APLOG_MARK,APLOG_ERR,ret,NULL, "Error in churn_output");
+ return ret;
+ }
- if (ssl_hook_CloseConnection (pRec) != APR_SUCCESS)
- ssl_log(c->base_server, SSL_LOG_INFO,
+ if ((ret = ssl_hook_CloseConnection (pRec)) != APR_SUCCESS)
+ ap_log_error(APLOG_MARK,APLOG_ERR,ret,NULL,
"Error in ssl_hook_CloseConnection");
-#if 0
- /* XXX: dubious - does this always terminate? Does it return the right
thing? */
- for( ; ; ) {
- ret=churn_output(pRec);
- if(ret != APR_SUCCESS)
- return ret;
- /* XXX - Verify if passing &len is okay for churn - TBD */
- len = 0;
- ret=churn(pRec,APR_NONBLOCK_READ,&len);
- if(ret != APR_SUCCESS) {
- if(ret == APR_EOF)
- return APR_SUCCESS;
- else
- return ret;
- }
- }
-#endif
break;
}
if(APR_BUCKET_IS_FLUSH(pbktIn)) {
- /* assume that churn will flush (or already has) if there's output */
- /* XXX - Verify if passing &len is okay for churn - TBD */
- ssl_log(c->base_server, SSL_LOG_INFO, "FLUSH in output");
- len = 0;
- ret=churn(pRec,APR_NONBLOCK_READ,&len);
- if(ret != APR_SUCCESS)
- return ret;
continue;
}
- ssl_log(c->base_server, SSL_LOG_INFO, "DATA in output");
/* read filter */
apr_bucket_read(pbktIn,&data,&len,APR_BLOCK_READ);
@@ -386,7 +354,6 @@
n = ssl_io_hook_write(pRec->pssl, (unsigned char *)data, len);
assert (n == len);
-
/* churn the state machine */
ret=churn_output(pRec);
if(ret != APR_SUCCESS)
@@ -421,6 +388,12 @@
return APR_SUCCESS;
}
+apr_status_t ssl_io_filter_cleanup (void *data)
+{
+ SSL *ssl = (SSL *)data;
+ return APR_SUCCESS;
+}
+
void ssl_io_filter_init(conn_rec *c, SSL *ssl)
{
SSLFilterRec *filter;
@@ -434,6 +407,10 @@
filter->pbioWrite = BIO_new(BIO_s_mem());
SSL_set_bio(ssl, filter->pbioRead, filter->pbioWrite);
filter->pssl = ssl;
+
+ apr_pool_cleanup_register(c->pool, (void*)ssl,
+ ssl_io_filter_cleanup, apr_pool_cleanup_null);
+
return;
}
Index: ssl_engine_kernel.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/ssl_engine_kernel.c,v
retrieving revision 1.11
diff -u -r1.11 ssl_engine_kernel.c
--- ssl_engine_kernel.c 2001/07/30 22:30:51 1.11
+++ ssl_engine_kernel.c 2001/07/31 00:07:20
@@ -397,7 +397,9 @@
* calls of Apache it would lead to an I/O error in the browser due
* to the fact that the SSL layer was already removed by us.
*/
+#if 0 /* XXX We've flush the OpenSSL buffer and not connection buffer - TBD */
ap_flush_conn(conn);
+#endif
/*
* Now close the SSL layer of the connection. We've to take
