Any reason to differ from trunk in 2.4? 

The people using spdy already in a 2.4 will most likely have the NPN patch 
deployed, so they'll have it easy with the trunk changes. The only one using 
the alpn patch, I know of, is myself in mod_h2. And that has already been 
adapted.

So, I myself see no reason to not bring the trunk change into 2.4.

> Am 01.04.2015 um 22:33 schrieb Jim Jagielski <j...@jagunet.com>:
> 
> Yeah, I agree. Right now, trunk pretty much uses
> 
>       #ifdef HAVE_TLS_ALPN
>       blah blah
>       #endif
>       #ifdef HAVE_TLS_NPN
>       blah2 blah2
>       #endif
> 
> Instead of
> 
>       #if defined(HAVE_TLS_NPN) || defined(HAVE_TLS_ALPN)
> 
> so that "ripping out" NPN would be easier. The question is
> which to use for 2.4...
> 
>> On Apr 1, 2015, at 1:59 PM, Stefan Eissing <stefan.eiss...@greenbytes.de> 
>> wrote:
>> 
>> Well, I took the trunk version, diffed to 2.4.12 and made a patch for my 
>> sandbox build (removed the non alpn/npn parts). That works for mod_h2 after 
>> adding callbacks for the npn stuff. 
>> 
>> I have no real pref to keep npn and alpn separate or not. my thought when 
>> merging these was that npn will go away rather soon as alpn is supposed to 
>> replace it and is afaik the cryptographically more secure way (i think npn 
>> is prone to mitm downgrade attacks). 
>> 
>> cheers,
>> Stefan
>> 
>> 
>> 
>>> Am 01.04.2015 um 19:28 schrieb Jim Jagielski <j...@jagunet.com>:
>>> 
>>> Yeah, there is some "overlap" which I'm trying to grok,
>>> since trunk had NPN but not ALPN, so I tried to have the
>>> ALPN stuff self-contained. But not sure if that's the best
>>> way since, for example, alpn_proposefns is adjusted
>>> in ssl_callback_AdvertiseNextProtos(), but that is a
>>> NPN "only" function in trunk, so it uses npn_proposefns.
>>> 
>>> I'm thinking that in trunk we shouldn't think of
>>> NPN and ALPN as "distinct".
>>> 
>>>> On Apr 1, 2015, at 12:47 PM, Rainer Jung <rainer.j...@kippdata.de> wrote:
>>>> 
>>>> Hi Stefan,
>>>> 
>>>>> Am 01.04.2015 um 18:22 schrieb Stefan Eissing:
>>>>> Jim,
>>>>> 
>>>>> today I converted your commit to a path on 2.4.12 and tested it with 
>>>>> mod_h2. All fine!
>>>>> 
>>>>> Then I got a trouble report that alpn negotiation always selected 
>>>>> "http/1.1" unless SSLAlpnPreference configured something else. This is 
>>>>> due to the deterministic ordering and "http/1.1." > "h2". So, I made a 
>>>>> slight modification, attached below.
>>>> 
>>>> Maybe related but concerning NPN: There was a difference between the NPN 
>>>> parts of your original Bugzilla attachment and what was already in mod_ssl 
>>>> trunk and therefore was not applied. In your attachment, there was some 
>>>> code for sorting in ssl_callback_AdvertiseNextProtos() which IMHO does not 
>>>> exist in trunk. Is that part necessary?
>>>> 
>>>> A second difference: your original addition to ssl_engine_io.c had the NPN 
>>>> and the ALPN parts merged in the same code block. In trunk those are now 
>>>> two separate pieces coming after each other.
>>>> 
>>>>> --- modules/ssl/ssl_engine_kernel.c    2015-04-01 15:23:48.000000000 +0200
>>>>> +++ 
>>>>> ../../mod-h2/sandbox/httpd/gen/httpd-2.4.12/modules/ssl/ssl_engine_kernel.c
>>>>>     2015-04-01 17:53:03.000000000 +0200
>>>>> @@ -2177,7 +2152,7 @@
>>>>> }
>>>>> 
>>>>> /*
>>>>> - * Compare to ALPN protocol proposal. Result is similar to strcmp():
>>>>> + * Compare two ALPN protocol proposal. Result is similar to strcmp():
>>>>> * 0 gives same precedence, >0 means proto1 is prefered.
>>>>> */
>>>>> static int ssl_cmp_alpn_protos(modssl_ctx_t *ctx,
>>>>> @@ -2254,14 +2229,8 @@
>>>>>       i += plen;
>>>>>   }
>>>>> 
>>>>> -    /* Regardless of installed hooks, the http/1.1 protocol is always
>>>>> -     * supported by us. Add it to the proposals if the client also
>>>>> -     * offers it. */
>>>>>   proposed_protos = apr_array_make(c->pool, client_protos->nelts+1,
>>>>>                                    sizeof(char *));
>>>>> -    if (ssl_array_index(client_protos, alpn_http1) >= 0) {
>>>>> -        APR_ARRAY_PUSH(proposed_protos, const char*) = alpn_http1;
>>>>> -    }
>>>>> 
>>>>>   if (sslconn->alpn_proposefns != NULL) {
>>>>>       /* Invoke our alpn_propos_proto hooks, giving other modules a 
>>>>> chance to
>>>>> @@ -2280,9 +2249,16 @@
>>>>>   }
>>>>> 
>>>>>   if (proposed_protos->nelts <= 0) {
>>>>> -        ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02839)
>>>>> -                      "none of the client alpn protocols are supported");
>>>>> -        return SSL_TLSEXT_ERR_ALERT_FATAL;
>>>>> +        /* Regardless of installed hooks, the http/1.1 protocol is always
>>>>> +         * supported by us. Choose it if none other matches. */
>>>>> +        if (ssl_array_index(client_protos, alpn_http1) < 0) {
>>>>> +            ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02839)
>>>>> +                          "none of the client alpn protocols are 
>>>>> supported");
>>>>> +            return SSL_TLSEXT_ERR_ALERT_FATAL;
>>>>> +        }
>>>>> +        *out = (const unsigned char*)alpn_http1;
>>>>> +        *outlen = (unsigned char)strlen(alpn_http1);
>>>>> +        return SSL_TLSEXT_ERR_OK;
>>>>>   }
>>>>> 
>>>>>   /* Now select the most preferred protocol from the proposals. */
>>> 
> 

<green/>bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782



Reply via email to