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

Reply via email to