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. */
>
