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