You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Mathihalli, Madhusudan" <ma...@hp.com> on 2004/02/26 02:12:33 UTC

[PATCH-Modified-2] SSL not sending close alert message

More feedback incorporated !

-Madhu


Index: server/Makefile.in
===================================================================
RCS file: /home/cvs/httpd-2.0/server/Makefile.in,v
retrieving revision 1.91
diff -u -r1.91 Makefile.in
--- server/Makefile.in  2 Feb 2004 17:04:10 -0000       1.91
+++ server/Makefile.in  26 Feb 2004 01:06:45 -0000
@@ -13,7 +13,8 @@
        connection.c listen.c \
        mpm_common.c util_charset.c util_debug.c util_xml.c \
        util_filter.c exports.c buildmark.c \
-       scoreboard.c error_bucket.c protocol.c core.c request.c provider.c
+       scoreboard.c error_bucket.c protocol.c core.c request.c provider.c \
+       eoc_bucket.c

 TARGETS = delete-exports $(LTLIBRARY_NAME) $(CORE_IMPLIB_FILE) export_vars.h ht
tpd.exp

Index: server/connection.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/connection.c,v
retrieving revision 1.114
diff -u -r1.114 connection.c
--- server/connection.c 9 Feb 2004 20:40:49 -0000       1.114
+++ server/connection.c 26 Feb 2004 01:06:45 -0000
@@ -67,13 +67,18 @@

 AP_CORE_DECLARE(void) ap_flush_conn(conn_rec *c)
 {
-    apr_bucket_brigade *bb;
-    apr_bucket *b;
+    apr_bucket_brigade *bb, *bb_eoc;
+    apr_bucket *b, *b_eoc;

     bb = apr_brigade_create(c->pool, c->bucket_alloc);
     b = apr_bucket_flush_create(c->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(bb, b);
     ap_pass_brigade(c->output_filters, bb);
+
+    bb_eoc = apr_brigade_create(c->pool, c->bucket_alloc);
+    b_eoc = apr_bucket_eoc_create(c->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(bb_eoc, b_eoc);
+    ap_pass_brigade(c->output_filters, bb_eoc);
 }

 /* we now proceed to read from the client until we get EOF, or until
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.261
diff -u -r1.261 core.c
--- server/core.c       19 Feb 2004 11:19:43 -0000      1.261
+++ server/core.c       26 Feb 2004 01:06:45 -0000
@@ -3854,6 +3854,9 @@
             if (APR_BUCKET_IS_EOS(e)) {
                 break;
             }
+            if (AP_BUCKET_IS_EOC(e)) {
+                apr_bucket_delete(e);
+            }
             if (APR_BUCKET_IS_FLUSH(e)) {
                 if (e != APR_BRIGADE_LAST(b)) {
                     more = apr_brigade_split(b, APR_BUCKET_NEXT(e));
Index: include/http_protocol.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
retrieving revision 1.90
diff -u -r1.90 http_protocol.h
--- include/http_protocol.h     9 Feb 2004 20:38:21 -0000       1.90
+++ include/http_protocol.h     26 Feb 2004 01:06:45 -0000
@@ -623,6 +623,32 @@
  */
 AP_DECLARE_HOOK(apr_port_t,default_port,(const request_rec *r))

+AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_eoc;
+
+/**
+ * Determine if a bucket is an End Of Connection (EOC) bucket
+ * @param e The bucket to inspect
+ * @return true or false
+ */
+#define AP_BUCKET_IS_EOC(e)         (e->type == &ap_bucket_type_eoc)
+
+/**
+ * Make the bucket passed in an End Of Connection (EOC) bucket
+ * @param b The bucket to make into an EOC bucket
+ * @return The new bucket, or NULL if allocation failed
+ * @deffunc apr_bucket *ap_bucket_eoc_make(apr_bucket *b)
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_eoc_make(apr_bucket *b);
+
+/**
+ * Create a bucket referring to an End Of Connection (EOC). This indicates
+ * that the connection will be closed.
+ * @param list The freelist from which this bucket should be allocated
+ * @return The new bucket, or NULL if allocation failed
+ * @deffunc apr_bucket *ap_bucket_eoc_create(apr_bucket_alloc_t *list)
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_eoc_create(apr_bucket_alloc_t *list);
+
 typedef struct ap_bucket_error ap_bucket_error;

 /**
Index: modules/ssl/ssl_engine_io.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.118
diff -u -r1.118 ssl_engine_io.c
--- modules/ssl/ssl_engine_io.c 25 Feb 2004 10:54:29 -0000      1.118
+++ modules/ssl/ssl_engine_io.c 26 Feb 2004 01:06:45 -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)
          */
@@ -1395,6 +1397,17 @@
                  */
                 apr_bucket_delete(bucket);
             }
+        }
+        else if (AP_BUCKET_IS_EOC(bucket)) {
+            /* The special "EOC" 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);
+            if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {
+                return status;
+            }
+            break;
         }
         else {
             /* filter output */


------------------------------------------------------
START of eoc_bucket.c
------------------------------------------------------

/* Copyright 2000-2004 The Apache Software Foundation
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#include "apr_buckets.h"

static apr_status_t eoc_bucket_read(apr_bucket *b, const char **str,
                                    apr_size_t *len, apr_read_type_e block)
{
    *str = NULL;
    *len = 0;
    return APR_SUCCESS;
}

APU_DECLARE(apr_bucket *) ap_bucket_eoc_make(apr_bucket *b)
{
    b->length      = 0;
    b->start       = 0;
    b->data        = NULL;
    b->type        = &ap_bucket_type_eoc;

    return b;
}

APU_DECLARE(apr_bucket *) ap_bucket_eoc_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;
    return ap_bucket_eoc_make(b);
}

APU_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eoc = {
    "EOC", 5, APR_BUCKET_METADATA,
    apr_bucket_destroy_noop,
    eoc_bucket_read,
    apr_bucket_setaside_noop,
    apr_bucket_split_notimpl,
    apr_bucket_simple_copy
};


Re: [PATCH-Modified-2] SSL not sending close alert message

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Thursday, February 26, 2004 11:58 AM +0000 Joe Orton <jo...@redhat.com> 
wrote:

> On Wed, Feb 25, 2004 at 05:12:33PM -0800, Mathihalli, Madhusudan wrote:
>> More feedback incorporated !
>
> ap_flush_conn can just use a single brigade with two buckets, no extra
> variables needed there, also needs s/APU_DECLARE/AP_DECLARE in
> eoc_bucket.c, and perhaps the prototypes are more appropriate in
> http_connection.h but it's pretty arbitrary with the error_bucket in
> http_protocol.h already.
>
>> +            filter_ctx->nobuffer = 1;
>> +            status = ssl_filter_io_shutdown(filter_ctx, f->c, 0);
>> +            if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {
>
> should either do something with the return value of io_shutdown or not
> assign it to status.
>
> Nearly there ;)

Ditto.

My only long-term comment is that I'd wonder if the core output filter should 
eventually do the socket shutdown when it gets the EOC rather than delete the 
bucket.  But, that's not related to this patch at all (other than it'd just do 
the delete now).

Thanks!  -- justin

Re: [PATCH-Modified-2] SSL not sending close alert message

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Feb 25, 2004 at 05:12:33PM -0800, Mathihalli, Madhusudan wrote:
> More feedback incorporated !

ap_flush_conn can just use a single brigade with two buckets, no extra
variables needed there, also needs s/APU_DECLARE/AP_DECLARE in
eoc_bucket.c, and perhaps the prototypes are more appropriate in
http_connection.h but it's pretty arbitrary with the error_bucket in
http_protocol.h already.

> +            filter_ctx->nobuffer = 1;
> +            status = ssl_filter_io_shutdown(filter_ctx, f->c, 0);
> +            if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {

should either do something with the return value of io_shutdown or not
assign it to status.

Nearly there ;)

joe