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 <[email protected]>: > > 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 <[email protected]> >> 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 <[email protected]>: >>> >>> 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 <[email protected]> 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
