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.

Cheers,

  Stefan

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

Attachment: httpd-trunk2.unified.diff.patch
Description: Binary data

> Am 31.03.2015 um 21:12 schrieb Jim Jagielski <j...@jagunet.com>:
> 
> Hmmm.. missed a patch.
> 
> r1670434
>> On Mar 31, 2015, at 2:28 PM, Jim Jagielski <j...@jagunet.com> wrote:
>> 
>> Hmmm... let me double check.
>> 
>>> On Mar 31, 2015, at 2:22 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>>> 
>>> 
>>> 
>>> On 03/31/2015 08:08 PM, Jim Jagielski wrote:
>>>> They are used by mod_spdy and/or mod_h2..., iirc
>>> 
>>> They use private structures of mod_ssl directly? That does not sound like a 
>>> good idea.
>>> 
>>> Regards
>>> 
>>> Rüdiger
>>> 
>>>> 
>>>>> On Mar 31, 2015, at 1:57 PM, Ruediger Pluem <rpl...@apache.org> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> On 03/31/2015 07:12 PM, j...@apache.org wrote:
>>>>>> Author: jim
>>>>>> Date: Tue Mar 31 17:12:51 2015
>>>>>> New Revision: 1670397
>>>>>> 
>>>>>> URL: http://svn.apache.org/r1670397
>>>>>> Log:
>>>>>> ALPN support, based on mod_spdy/mod_h2 patch set
>>>>>> 
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/modules/ssl/mod_ssl.c
>>>>>> httpd/httpd/trunk/modules/ssl/mod_ssl.h
>>>>>> httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>>>>>> httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>>>>>> httpd/httpd/trunk/modules/ssl/ssl_private.h
>>>>> 
>>>>> 
>>>>> I don't know if I miss the obvious, but where do we use
>>>>> 
>>>>> ssl_alpn_pref
>>>>> alpn_proposefns
>>>>> 
>>>>> ?
>>>>> 
>>>>> I can only see that we set it, but I fail to see where it is used.
>>>>> 
>>>>> Regards
>>>>> 
>>>>> Rüdiger
>>>> 
>>>> 
>> 
> 

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



Reply via email to