You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2017/02/03 17:06:28 UTC
ssl_io_filter_output vs EOC
Besides metadata buckets, should the EOC semantic really apply to the
ssl output filter?
It's not really an issue by now because we never send "data" buckets
after an EOC, but should this happen do we really want to send them
out in clear, after the TLS close notify?
I'd rather be safe here and return an error up the filters' stack.
Actually this is what is done already if the data come in the same
brigade as the EOC (ssl_filter_write() in the same loop will fail when
the TLS connection is shutdown), but for subsequent brigade(s) we'd
pass through (per EOC semantic).
This at least requires a consistency "fix" (for the theory, I can't
think of any pratical/reasonable use of data buckets after EOC, e.g.
an Upgrade from TLS to clear looks really hazardous...).
Is it unacceptable to add an exception to the EOC semantic (for data)
here and fail (as sanity check)?
Re: ssl_io_filter_output vs EOC
Posted by Ruediger Pluem <rp...@apache.org>.
On 02/06/2017 01:31 PM, Yann Ylavic wrote:
> On Mon, Feb 6, 2017 at 12:53 PM, Pl�m, R�diger, Vodafone Group
> <ru...@vodafone.com> wrote:
>>
>> IMHO we currently fail after we processed an EOC (no matter if in the
>> same brigade or in a follow up brigade) and we should continue doing
>> so.
>
> We fail in the same brigade thanks to ssl_filter_write() when pssl is
> NULL, but I think that if we wanted to also fail for subsequent
> brigades, we need something like:
>
> Index: modules/ssl/ssl_engine_io.c
> ===================================================================
> --- modules/ssl/ssl_engine_io.c (revision 1781582)
> +++ modules/ssl/ssl_engine_io.c (working copy)
> @@ -1777,14 +1775,15 @@ static apr_status_t ssl_io_filter_output(ap_filter
> return APR_ECONNABORTED;
> }
>
> - /* Reinstate any buffered content */
> - ap_filter_reinstate_brigade(f, bb, &flush_upto);
> -
> if (!filter_ctx->pssl) {
> /* ssl_filter_io_shutdown was called */
> - return ap_pass_brigade(f->next, bb);
> + apr_brigade_cleanup(bb);
> + return APR_EGENERAL;
> }
>
> + /* Reinstate any buffered content */
> + ap_filter_reinstate_brigade(f, bb, &flush_upto);
> +
> inctx = (bio_filter_in_ctx_t *)BIO_get_data(filter_ctx->pbioRead);
> outctx = (bio_filter_out_ctx_t *)BIO_get_data(filter_ctx->pbioWrite);
>
Yes you are correct. I missed that. So some sort of inconsistent behavior here currently.
I still think that we should not pass on any data here. Only metadata seems to be acceptable.
But this would require more logic.
Regards
R�diger
Re: ssl_io_filter_output vs EOC
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Feb 6, 2017 at 12:53 PM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
> IMHO we currently fail after we processed an EOC (no matter if in the
> same brigade or in a follow up brigade) and we should continue doing
> so.
We fail in the same brigade thanks to ssl_filter_write() when pssl is
NULL, but I think that if we wanted to also fail for subsequent
brigades, we need something like:
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1781582)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -1777,14 +1775,15 @@ static apr_status_t ssl_io_filter_output(ap_filter
return APR_ECONNABORTED;
}
- /* Reinstate any buffered content */
- ap_filter_reinstate_brigade(f, bb, &flush_upto);
-
if (!filter_ctx->pssl) {
/* ssl_filter_io_shutdown was called */
- return ap_pass_brigade(f->next, bb);
+ apr_brigade_cleanup(bb);
+ return APR_EGENERAL;
}
+ /* Reinstate any buffered content */
+ ap_filter_reinstate_brigade(f, bb, &flush_upto);
+
inctx = (bio_filter_in_ctx_t *)BIO_get_data(filter_ctx->pbioRead);
outctx = (bio_filter_out_ctx_t *)BIO_get_data(filter_ctx->pbioWrite);
_
Regards,
Yann.
AW: ssl_io_filter_output vs EOC
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Gesendet: Freitag, 3. Februar 2017 18:06
> An: httpd-dev <de...@httpd.apache.org>
> Betreff: ssl_io_filter_output vs EOC
>
> Besides metadata buckets, should the EOC semantic really apply to the
> ssl output filter?
>
> It's not really an issue by now because we never send "data" buckets
> after an EOC, but should this happen do we really want to send them
> out in clear, after the TLS close notify?
>
> I'd rather be safe here and return an error up the filters' stack.
> Actually this is what is done already if the data come in the same
> brigade as the EOC (ssl_filter_write() in the same loop will fail when
> the TLS connection is shutdown), but for subsequent brigade(s) we'd
> pass through (per EOC semantic).
>
> This at least requires a consistency "fix" (for the theory, I can't
> think of any pratical/reasonable use of data buckets after EOC, e.g.
> an Upgrade from TLS to clear looks really hazardous...).
>
> Is it unacceptable to add an exception to the EOC semantic (for data)
> here and fail (as sanity check)?
IMHO we currently fail after we processed an EOC (no matter if in the same brigade or in a follow up brigade)
and we should continue doing so.
Regards
Rüdiger