HTTP/2 feature incorrectly includes "h2c" in ALPN/NPN

[lpardue]

Hello,

I have identified a HTTP/2 non-conformity issue when I enable HTTP/2 features of an OpenLiteSpeed installation. The usage of the "h2c" protocol identifier when using a TLS connection is described in RFC 7540 Section 3.3. When using ALPN the server MUST NOT select h2c, the rules for NPN are not as explicit but it seems erroneous to offer a cleartext protocol when negotiating a TLS connection. I have tested other HTTP/2 server implementations and do not observe them exhibiting similar behavior.

This behavior has been observed at scale on live deployments across the web. HTTP/2 Dashboard monitors the adoption and performance of HTTP/2 on the web. Observe the H2C series on the Auxiliary Protocols chart. A significant proportion of the anomalous H2C protocol support is related to domains that report "LiteSpeed" in their Server response header.

My local testing has taken place on on OpenLiteSpeed v.1.4.14, with very basic configuration, running on CentOS 7.

Steps to reproduce
1) Enable HTTP/2 and TLS on the server.
2) Test NPN advertisement, openssl s_client -nextprotoneg '' -connect <HOST>:<PORT>
a) Observe "h2c" protocol identifier in the "Protocols advertised by server:" result
3) Test ALPN, openssl s_client -alpn 'h2c' -connect <HOST>:<PORT>
a) Observe "h2c" in the "ALPN protocol:" result

Correct behaviour
Do not advertise or select "h2c" protocol when using TLS based connections.

Lucas

 

[lsfoo]

Hi Lucas,

Thanks for notifying us, it is updated internally and will be updated on the next release.

If you or anyone reading this would like to patch it for your current version, the patch is listed below.

Cheers,
Kevin

diff --git src/sslpp/sslcontext.cpp src/sslpp/sslcontext.cpp
index 32f06e2..dbebd09 100644
--- src/sslpp/sslcontext.cpp
+++ src/sslpp/sslcontext.cpp
@@ -1141,15 +1141,15 @@ static const char *NEXT_PROTO_STRING[8] =
     "\x06spdy/2\x08http/1.1",
     "\x08spdy/3.1\x06spdy/3\x08http/1.1",
     "\x08spdy/3.1\x06spdy/3\x06spdy/2\x08http/1.1",
-    "\x02h2\x03h2c\x05h2-17\x05h2-14\x08http/1.1",
-    "\x02h2\x03h2c\x05h2-17\x05h2-14\x06spdy/2\x08http/1.1",
-    "\x02h2\x03h2c\x05h2-17\x05h2-14\x08spdy/3.1\x06spdy/3\x08http/1.1",
-    "\x02h2\x03h2c\x05h2-17\x05h2-14\x08spdy/3.1\x06spdy/3\x06spdy/2\x08http/1.1",
+    "\x02h2\x08http/1.1",
+    "\x02h2\x06spdy/2\x08http/1.1",
+    "\x02h2\x08spdy/3.1\x06spdy/3\x08http/1.1",
+    "\x02h2\x08spdy/3.1\x06spdy/3\x06spdy/2\x08http/1.1",
};
static unsigned int NEXT_PROTO_STRING_LEN[8] =
{
-    9, 16, 25, 32, 28, 35, 44, 51,
+    9, 16, 25, 32, 12, 19, 28, 35,
};
//static const char NEXT_PROTO_STRING[] = "\x06spdy/2\x08http/1.1\x08http/1.0";

 

[lpardue]

Hi Kevin.

Thank you for looking into this and getting it fixed so rapidly. We look forward to the new release and will be monitoring the HTTP/2 dashboard to observe the roll out of the new version across the domains that are indexed.