You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2004/02/08 18:27:01 UTC
[PATCH] new finish_connection hook & SSL closure fix
This adds a finish_connection hook as discussed with Madhu, and uses it
in mod_ssl to ensure that the SSL close_notify alert is sent before the
connection is closed. Any comments?
- added a new bucket type in mod_ssl to represent the closure alert: any
simpler approaches?
- needed to turn off the bio_filter_out_write() buffering to prevent the
alert getting buffered, annoying but OpenSSL doesn't flush the bio
itself.
Index: server/connection.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/connection.c,v
retrieving revision 1.113
diff -u -r1.113 connection.c
--- server/connection.c 8 Feb 2004 13:58:22 -0000 1.113
+++ server/connection.c 8 Feb 2004 16:50:46 -0000
@@ -35,12 +35,15 @@
APR_HOOK_LINK(create_connection)
APR_HOOK_LINK(process_connection)
APR_HOOK_LINK(pre_connection)
+ APR_HOOK_LINK(finish_connection)
)
AP_IMPLEMENT_HOOK_RUN_FIRST(conn_rec *,create_connection,
(apr_pool_t *p, server_rec *server, apr_socket_t *csd, long conn_id, void *sbh, apr_bucket_alloc_t *alloc),
(p, server, csd, conn_id, sbh, alloc), NULL)
AP_IMPLEMENT_HOOK_RUN_FIRST(int,process_connection,(conn_rec *c),(c),DECLINED)
AP_IMPLEMENT_HOOK_RUN_ALL(int,pre_connection,(conn_rec *c, void *csd),(c, csd),OK,DECLINED)
+AP_IMPLEMENT_HOOK_RUN_ALL(int,finish_connection,(conn_rec *c),(c),OK,DECLINED)
+
/*
* More machine-dependent networking gooo... on some systems,
* you've got to be *really* sure that all the packets are acknowledged
@@ -166,6 +169,10 @@
if (!c->aborted) {
ap_run_process_connection(c);
+
+ if (!c->aborted) {
+ ap_run_finish_connection(c);
+ }
}
}
Index: include/http_connection.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_connection.h,v
retrieving revision 1.58
diff -u -r1.58 http_connection.h
--- include/http_connection.h 7 Feb 2004 19:27:57 -0000 1.58
+++ include/http_connection.h 8 Feb 2004 16:50:46 -0000
@@ -103,6 +103,15 @@
*/
AP_DECLARE_HOOK(int,process_connection,(conn_rec *c))
+/**
+ * This hook allows connection-level filters to perform any necessary
+ * processing before a connection is closed.
+ * @param c The connection which is about to be closed
+ * @return OK or DECLINED
+ * @deffunc int ap_run_finish_connection(conn_rec *c)
+ */
+AP_DECLARE_HOOK(int,finish_connection,(conn_rec *c))
+
#ifdef __cplusplus
}
#endif
Index: modules/ssl/mod_ssl.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.c,v
retrieving revision 1.94
diff -u -r1.94 mod_ssl.c
--- modules/ssl/mod_ssl.c 8 Feb 2004 12:52:25 -0000 1.94
+++ modules/ssl/mod_ssl.c 8 Feb 2004 16:50:46 -0000
@@ -451,6 +451,17 @@
return ssl_init_ssl_connection(c);
}
+static int ssl_hook_finish_connection(conn_rec *c)
+{
+ apr_bucket_brigade *bb = apr_brigade_create(c->pool, c->bucket_alloc);
+ apr_bucket *e;
+
+ e = ssl_io_close_bucket_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+
+ ap_pass_brigade(c->output_filters, bb);
+ return OK;
+}
static void ssl_hook_Insert_Filter(request_rec *r)
{
@@ -470,6 +481,7 @@
ssl_io_filter_register(p);
ap_hook_pre_connection(ssl_hook_pre_connection,NULL,NULL, APR_HOOK_MIDDLE);
+ ap_hook_finish_connection(ssl_hook_finish_connection, NULL,NULL, APR_HOOK_MIDDLE);
ap_hook_post_config (ssl_init_Module, NULL,NULL, APR_HOOK_MIDDLE);
ap_hook_http_method (ssl_hook_http_method, NULL,NULL, APR_HOOK_MIDDLE);
ap_hook_default_port (ssl_hook_default_port, NULL,NULL, APR_HOOK_MIDDLE);
Index: modules/ssl/mod_ssl.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.h,v
retrieving revision 1.147
diff -u -r1.147 mod_ssl.h
--- modules/ssl/mod_ssl.h 8 Feb 2004 12:52:25 -0000 1.147
+++ modules/ssl/mod_ssl.h 8 Feb 2004 16:50:46 -0000
@@ -60,6 +60,7 @@
#include "apr_shm.h"
#include "apr_global_mutex.h"
#include "apr_optional.h"
+#include "apr_buckets.h"
#define MOD_SSL_VERSION AP_SERVER_BASEREVISION
@@ -630,6 +631,9 @@
/* I/O */
void ssl_io_filter_init(conn_rec *, SSL *);
void ssl_io_filter_register(apr_pool_t *);
+/* Create a bucket used to represent the close_notify alert. */
+apr_bucket *ssl_io_close_bucket_create(apr_bucket_alloc_t *list);
+
long ssl_io_data_cb(BIO *, int, MODSSL_BIO_CB_ARG_TYPE *, int, long, long);
/* PRNG */
Index: modules/ssl/ssl_engine_io.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.116
diff -u -r1.116 ssl_engine_io.c
--- modules/ssl/ssl_engine_io.c 8 Feb 2004 12:52:25 -0000 1.116
+++ modules/ssl/ssl_engine_io.c 8 Feb 2004 16:50:47 -0000
@@ -100,6 +100,7 @@
BIO *pbioWrite;
ap_filter_t *pInputFilter;
ap_filter_t *pOutputFilter;
+ int nobuffer; /* non-zero to prevent buffering */
} ssl_filter_ctx_t;
typedef struct {
@@ -193,7 +194,8 @@
*/
BIO_clear_retry_flags(bio);
- if (!outctx->length && (inl + outctx->blen < sizeof(outctx->buffer))) {
+ if (!outctx->length && (inl + outctx->blen < sizeof(outctx->buffer)) &&
+ !outctx->filter_ctx->nobuffer) {
/* the first two SSL_writes (of 1024 and 261 bytes)
* need to be in the same packet (vec[0].iov_base)
*/
@@ -544,6 +546,42 @@
#endif
};
+/* A special bucket type; sent last on the connection to represent the
+ * close_notify alert. */
+static apr_status_t close_bucket_read(apr_bucket *b, const char **str,
+ apr_size_t *len, apr_read_type_e block)
+{
+ *str = NULL;
+ *len = 0;
+ return APR_SUCCESS;
+}
+
+static const apr_bucket_type_t bucket_type_close;
+
+apr_bucket *ssl_io_close_bucket_create(apr_bucket_alloc_t *list)
+{
+ apr_bucket *b = apr_bucket_alloc(sizeof(*b), list);
+
+ APR_BUCKET_INIT(b);
+ b->free = apr_bucket_free;
+ b->list = list;
+ b->length = b->start = 0;
+ b->data = NULL;
+ b->type = &bucket_type_close;
+
+ return b;
+}
+
+static const apr_bucket_type_t bucket_type_close = {
+ "CLOSE", 5, APR_BUCKET_METADATA,
+ apr_bucket_destroy_noop,
+ close_bucket_read,
+ apr_bucket_setaside_noop,
+ apr_bucket_split_notimpl,
+ apr_bucket_simple_copy
+};
+
+#define BUCKET_IS_CLOSE(e) ((e)->type == &bucket_type_close)
static apr_status_t ssl_io_input_read(bio_filter_in_ctx_t *inctx,
char *buf,
@@ -1394,6 +1432,13 @@
*/
apr_bucket_delete(bucket);
}
+ }
+ else if (BUCKET_IS_CLOSE(bucket)) {
+ /* The special "CLOSE" bucket means a shutdown is needed;
+ * turn off buffering in bio_filter_out_write. */
+ filter_ctx->nobuffer = 1;
+ status = ssl_filter_io_shutdown(filter_ctx, f->c, 0);
+ apr_bucket_delete(bucket);
}
else {
/* filter output */
Re: [PATCH] new finish_connection hook & SSL closure fix
Posted by Joe Orton <jo...@redhat.com>.
On Sun, Feb 08, 2004 at 11:08:37AM -0800, Justin Erenkrantz wrote:
> --On Sunday, February 8, 2004 5:27 PM +0000 Joe Orton <jo...@redhat.com>
> wrote:
>
> >This adds a finish_connection hook as discussed with Madhu, and uses it
> >in mod_ssl to ensure that the SSL close_notify alert is sent before the
> >connection is closed. Any comments?
>
> Would an EOS sent to the output connection-level filters before a close is
> issued be sufficient? Could we change ap_flush_conn to also pass down an
> EOS after the flush? Or, if an EOS can be sent otherwise (I forget if it
> is), could we just create a generic EOC bucket to pass in ap_flush_conn?
The mod_ssl output filter already gets an EOS after every request on the
connection currently. Doing a generic EOC bucket sounds good.
> That should serve the same purpose, I think, without the hook. -- justin
Makes sense also since any other SSL-like connection filter might well
need similar shutdown logic. I'll have a go at this - thanks Justin!
joe
Re: [PATCH] new finish_connection hook & SSL closure fix
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Sunday, February 8, 2004 5:27 PM +0000 Joe Orton <jo...@redhat.com>
wrote:
> This adds a finish_connection hook as discussed with Madhu, and uses it
> in mod_ssl to ensure that the SSL close_notify alert is sent before the
> connection is closed. Any comments?
Would an EOS sent to the output connection-level filters before a close is
issued be sufficient? Could we change ap_flush_conn to also pass down an EOS
after the flush? Or, if an EOS can be sent otherwise (I forget if it is),
could we just create a generic EOC bucket to pass in ap_flush_conn?
That should serve the same purpose, I think, without the hook. -- justin