httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded
Date Mon, 12 Oct 2015 08:42:34 GMT
The key here is using AP_MODE_INIT first (where 0 make sense) before
AP_MODE_SPECULATIVE.


On Mon, Oct 12, 2015 at 10:36 AM, Stefan Eissing
<stefan.eissing@greenbytes.de> wrote:
> Yeah, I tried a 0 len read. Unfortunately, the core input filter is not happy about that:
>
> core_filters.c, line 231
>         AP_DEBUG_ASSERT(readbytes > 0);
>
>> Am 12.10.2015 um 10:25 schrieb Plüm, Rüdiger, Vodafone Group <ruediger.pluem@vodafone.com>:
>>
>> How about the following patch?
>>
>> This separates the triggering of the SSL Handshake and the possible reading of the
24 bytes:
>>
>> Index: h2_h2.c
>> ===================================================================
>> --- h2_h2.c     (revision 1707437)
>> +++ h2_h2.c     (working copy)
>> @@ -154,7 +154,7 @@
>>
>>         temp = apr_brigade_create(c->pool, c->bucket_alloc);
>>         status = ap_get_brigade(c->input_filters, temp,
>> -                                AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24);
>> +                                AP_MODE_INIT, APR_BLOCK_READ, 0);
>>
>>         if (status == APR_SUCCESS) {
>>             if (h2_ctx_protocol_get(c)
>> @@ -171,23 +171,31 @@
>>                     char *s = NULL;
>>                     apr_size_t slen;
>>
>> -                    apr_brigade_pflatten(temp, &s, &slen, c->pool);
>> -                    if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24))
{
>> -                        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
>> -                                      "h2_h2, direct mode detected");
>> -                        h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
>> +                    status = ap_get_brigade(c->input_filters, temp,
>> +                                            AP_MODE_SPECULATIVE, APR_BLOCK_READ,
24);
>> +                    if (status == APR_SUCCESS) {
>> +                        apr_brigade_pflatten(temp, &s, &slen, c->pool);
>> +                        if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s,
24)) {
>> +                            ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
>> +                                          "h2_h2, direct mode detected");
>> +                            h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
>> +                        }
>> +                        else {
>> +                            ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>> +                                          "h2_h2, not detected in %d bytes: %s",
>> +                                          (int)slen, s);
>> +                        }
>>                     }
>> -                    else {
>> -                        ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>> -                                      "h2_h2, not detected in %d bytes: %s",
>> -                                      (int)slen, s);
>> -                    }
>>                 }
>> +                else {
>> +                    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
>> +                                  "h2_h2, error reading 24 bytes speculative");
>> +                }
>>             }
>>         }
>>         else {
>>             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
>> -                          "h2_h2, error reading 24 bytes speculative");
>> +                          "h2_h2, Failed to init connection");
>>         }
>>         apr_brigade_destroy(temp);
>>     }
>>
>> This would still block in the non ssl case if directmode is not set to off explicitly.
I would propose to change the default behaviour of directmode here to off as directmode seems
to be something very special to me that should be explicitly enabled.
>>
>> Regards
>>
>> Rüdiger
>>
>>> -----Original Message-----
>>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>>> Sent: Sonntag, 11. Oktober 2015 19:54
>>> To: dev@httpd.apache.org
>>> Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
>>> loaded
>>>
>>> Ok, in ssl_engine_io.c, lines 1426+ I see a hint:
>>>
>>>    /* XXX: we could actually move ssl_io_filter_handshake to an
>>>     * ap_hook_process_connection but would still need to call it for
>>>     * AP_MODE_INIT for protocols that may upgrade the connection
>>>     * rather than have SSLEngine On configured.
>>>     */
>>>    if ((status = ssl_io_filter_handshake(inctx->filter_ctx)) !=
>>> APR_SUCCESS) {
>>>        return ssl_io_filter_error(f, bb, status);
>>>    }
>>>
>>> The handshake handling gets triggered on a filter, so it all happens on
>>> READs (or WRITEs), which explains why connection hooks do not see it. This
>>> is quite some code which looks like we do not want to touch for a "quick
>>> fix". Also, there is no quick fix inside mod_http2 as before handshake,
>>> the SNI might also not be there yet, thus the vhost cannot be determined.
>>> etc.
>>>
>>> For the current release, I think we should leave it as it is and advise
>>> that current mod_http2 is incompatible with things like nntp.
>>>
>>> For the next release, I'd like the server to perform the handshake at a
>>> defined time, so other modules can rely on protocols being selected and
>>> correct vhost being known before the first read/write.
>>>
>>> //Stefan
>>>
>

Mime
View raw message