httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Eissing <stefan.eiss...@greenbytes.de>
Subject Re: svn commit: r1670397 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c mod_ssl.h ssl_engine_config.c ssl_engine_io.c ssl_private.h
Date Thu, 02 Apr 2015 07:44:05 GMT
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 <jim@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.eissing@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 <jim@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.jung@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




Mime
View raw message