On Thu, Jun 4, 2015 at 6:19 PM, Stefan Eissing
<stefan.eiss...@greenbytes.de> wrote:
> I think we need to clarify some things:
>
> 1. ALPN is initiated by the client. When a client does not send ALPN as part
> of client helo, the SSL alpn callbacks are not invoked and the server does
> not send any ALPN information back. This is different from NPN.
Agreed.
>
> 2. SSLAlpnPreference is intended as the final word to make a choice when
> either one ALPN callback proposes many protocols or of several callbacks
> propose protocols.
I may be missing something but this is not how it is implemented.
If the client sends a protocol which is *not* in SSLAlpnPreference,
but only in the ones proposed by the module (mod_h2), this protocol
will still be accepted.
So SSLAlpnPreference does not have the final word, it will make the
choice between the protocols it knows only.
For this to be the case, we would have to exclude httpd's unknown
client_protos from the proposed_protos, i.e.:
Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c (revision 1683271)
+++ modules/ssl/ssl_engine_kernel.c (working copy)
@@ -2242,6 +2234,7 @@ int ssl_callback_alpn_select(SSL *ssl,
client_protos = apr_array_make(c->pool, 0, sizeof(char *));
for (i = 0; i < inlen; /**/) {
+ const char *proto;
unsigned int plen = in[i++];
if (plen + i > inlen) {
// someone tries to trick us?
@@ -2249,8 +2242,10 @@ int ssl_callback_alpn_select(SSL *ssl,
"ALPN protocol identifier too long");
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
- APR_ARRAY_PUSH(client_protos, char *) =
- apr_pstrndup(c->pool, (const char *)in+i, plen);
+ proto = apr_pstrndup(c->pool, (const char *)in+i, plen);
+ if (ssl_array_index(ctx->ssl_alpn_pref, proto) >= 0) {
+ APR_ARRAY_PUSH(client_protos, char *) = proto;
+ }
i += plen;
}
--
Otherwise, all the client_protos will be elligible by mod_h2 (only) as
proposed_protos, and the final one be selected with:
+/*
+ * Compare two ALPN protocol proposal. Result is similar to strcmp():
+ * 0 gives same precedence, >0 means proto1 is preferred.
+ */
+static int ssl_cmp_alpn_protos(modssl_ctx_t *ctx,
+ const char *proto1,
+ const char *proto2)
+{
+ if (ctx && ctx->ssl_alpn_pref) {
+ int index1 = ssl_array_index(ctx->ssl_alpn_pref, proto1);
+ int index2 = ssl_array_index(ctx->ssl_alpn_pref, proto2);
+ if (index2 > index1) {
+ return (index1 >= 0) ? 1 : -1;
+ }
+ else if (index1 > index2) {
+ return (index2 >= 0) ? -1 : 1;
+ }
+ }
+ /* both have the same index (mabye -1 or no pref configured) and we compare
+ * the names so that spdy3 gets precedence over spdy2. That makes
+ * the outcome at least deterministic. */
+ return strcmp((const char *)proto1, (const char *)proto2);
+}
which may bypass SSLAlpnPreference ("-1 or no pref configured").
> So, when mod_spdy and mod_h2 are active *and* the client claims to support
> spdy/3.1 and h2, the SSLAlpnPreference determines what gets chosen and sent
> to the client.
Right, provided both are known.
>
> 3. Independent of the client proposal, as I read the spec, the server is free
> to chose any protocol identifier it desires. This might result in the client
> closing the connection. So, if the client uses ALPN and the server does not
> want/cannot do/is configured not to support any of the clients proposals,
> httpd can always send back „http/1.1“ since this is what it always supports.
Agreed.
>
> In this light, and correct me if I’m wrong, I see no benefit and only
> potential harm by introducing a „SSLALpn on|off“ configuration directive. I
> think the current implementation covers all use cases and if one is missing,
> please point out the scenario.
I also see another issue (already stated in my previous messages): a
client sends an ALPN Hello *without* "http/1.1", mod_h2 is not
configured, the connection is refused.
Unless each ALPN compatible MUST (as per RFC) propose "http/1.1", we
need a way to disable ALPN on httpd.
Regards,
Yann.
