Use HTTP2-compatible cipher as first preference

[mhils]

This is a follow up of https://sourceforge.net/p/curl/bugs/1487/ - 0d1060f did not solve this issue for me.
When connecting to http2.golang.org, OpenSSL advertises the following cipher suites after sorting ciphers with @STRENGTH:

The go server apparently does not support SHA384. As a consequence, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014) will be picked, which is blacklisted by the HTTP2 spec. This causes the go server to fail the connection.

By listing ECDHE-RSA-AES128-GCM-SHA256 as the first preference, HTTP2 servers will always either pick that (HTTP2 servers must implement that cipher according to the spec) or the server preference, which should not be blacklisted by the HTTP2 spec. Unfortunately, ECDHE-RSA-AES128-GCM-SHA256 is considered to be weaker by OpenSSL (AES128 vs AES256 I suppose), so we need to remove @STRENGTH again.

Test case: curl --http2 https://http2.golang.org/

[jay]

related nghttp2/nghttp2#140

I don't like your fix, why wouldn't we want to sort by strength? Isn't the issue here that the server chokes on the cipher? Even if it's prohibited so what, shouldn't the server just pick another one? Does blacklisted mean obligatory choke? And is there some departure here from HTTPS where the server has the final say? I don't know much about HTTP2.

FWIW in Windows OpenSSL 1.0.2d 9 Jul 2015 I get the same cipher list regardless of whether or not I use @STRENGTH, but it's not always like that. There are lots of different cipher lists and some order differently depending on that. In OpenSSL 1.0.1 that came with Ubuntu 14 I also get no difference in sort order. I built 1.0.2 in Ubuntu and the sort order is different though depending on whether or not strength is specified.

[bagder]

FYI: This is a change in the golang.org server too, I verified my fix against their server at the time I wrote it. It worked then.

[bagder]

So let me try to understand.

We offer a list of ciphers in our preferred order (sorted by strength as we see it). We don't know if the server will speak h1 or h2 so we can't simply use two separate lists. The server picks the one to use in the TLS handshake - the same handshake that also makes the server select to speak h2. A few milliseconds later, it rejects the cipher suite it selected on the basis that it isn't a valid HTTP/2 cipher suite...

Or did I misunderstand anything here?

[bagder]

Section 9.2.2 in RFC 7540 explains exactly this situation: "Note that clients might advertise support of cipher suites that are on the black list in order to allow for connection to servers that do not support HTTP/2. This allows servers to select HTTP/1.1 with a cipher suite that is on the HTTP/2 black list. However, this can result in HTTP/2 being negotiated with a black-listed cipher suite if the application protocol and cipher suite are independently selected."

Tricky.

[mhils]

We offer a list of ciphers in our preferred order (sorted by strength as we see it). We don't know if the server will speak h1 or h2 so we can't simply use two separate lists. The server picks the one to use in the TLS handshake - the same handshake that also makes the server select to speak h2. A few milliseconds later, it rejects the cipher suite it selected on the basis that it isn't a valid HTTP/2 cipher suite...

Yes. The golang HTTP2 server takes the first implemented (but possibly blacklisted) client's cipher preference by default. In my opinion the HTTP2 spec is very weak here:

Note that clients might advertise support of cipher suites that are on the black list in order to allow for connection to servers that do not support HTTP/2. This allows servers to select HTTP/1.1 with a cipher suite that is on the HTTP/2 black list. However, this can result in HTTP/2 being negotiated with a black-listed cipher suite if the application protocol and cipher suite are independently selected.

The workaround is to set ECDHE-RSA-AES128-GCM-SHA256 first, because the spec mandates that...

deployments of HTTP/2 that use TLS 1.2 MUST support TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256

However, that's still not enough. @STRENGTH pushes ECDHE-RSA-AES128-GCM-SHA256 down the list, presumably because it just sorts by AES128 vs AES256. As I see it, the only way to fix this is to remove @STRENGTH. It shouldn't matter too much though, because most servers use SSL_OP_CIPHER_SERVER_PREFERENCE or something along those lines anyway. Just the golang HTTP2 server behaves in a very weird but apparently comformant way.

(cc @bradfitz)

[bagder]

Still, I added "@strength" there for one primary reason - to make it work with the golang server.
Now we need to remove it again for one primary reason - to make it work with the golang server.

I realize we need to be pragmatic and go where things work, just pointing out some facts.

[bagder]

So yes, other people are also telling me that "@strength" doesn't work and that we need to list ciphers explicitly. Let's bite the bullet. Any suggestions?

[mark-kubacki]

Second line, with verification:

$ docker run --read-only=true -ti --rm wmark/curl --http2 -v --head https://http2.golang.org/
…
* Cipher selection: EECDH+HIGH+TLSv1.2:EECDH+HIGH:EDH+HIGH+TLSv1.2:-AES256:HIGH:-3DES:!aNULL:!eNULL:!RC4
…
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256
* ALPN, server accepted to use h2
…

In case you don't want to demote AES256 and risk more of unsuitable pairings such as (P256, AES256):

EECDH+HIGH+TLSv1.2:EECDH+HIGH:EDH+HIGH+TLSv1.2:-SHA1:HIGH:-3DES:!aNULL:!eNULL:!RC4

By the way, there is at least one CL for Golang to include SHA384 as hash for TLS. My patch is not the first one:
https://github.com/wmark/ossdl-overlay/blob/ac4d50add09105b2bdea899c3e61397700ff6b51/dev-lang/go/files/go-1.3-TLS_support_SHA384.patch

[bagder]

But what about the EXPORT ones and LOW that are disabled in the existing set? Shouldn't we then rather do something like:

--- a/lib/vtls/openssl.h
+++ b/lib/vtls/openssl.h
@@ -112,9 +112,10 @@ bool Curl_ossl_cert_status_request(void);
 #define curlssl_sha256sum(a,b,c,d) Curl_ossl_sha256sum(a,b,c,d)
 #endif
 #define curlssl_cert_status_request() Curl_ossl_cert_status_request()

 #define DEFAULT_CIPHER_SELECTION \
-  "ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH"
+  "EECDH+HIGH+TLSv1.2:EECDH+HIGH:EDH+HIGH+TLSv1.2:-SHA1:HIGH:-3DES:" \
+  "!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4"

 #endif /* USE_OPENSSL */
 #endif /* HEADER_CURL_SSLUSE_H */

[mark-kubacki]

HIGH and EXPORTxx are disjoint sets as far as I can check (OpenSSL 1.0.1, 1.0.2; and BoringSSL).

[bagder]

Thanks. I thought about it some more, and since TLS 1.2 wasn't added in openssl until 1.0.1 I think a safer version would be like below, which keeps the previous version for really old versions. Thoughts?

--- a/lib/vtls/openssl.h
+++ b/lib/vtls/openssl.h
@@ -111,10 +111,17 @@ bool Curl_ossl_cert_status_request(void);
 #if (OPENSSL_VERSION_NUMBER >= 0x0090800fL) && !defined(OPENSSL_NO_SHA256)
 #define curlssl_sha256sum(a,b,c,d) Curl_ossl_sha256sum(a,b,c,d)
 #endif
 #define curlssl_cert_status_request() Curl_ossl_cert_status_request()

-#define DEFAULT_CIPHER_SELECTION \
+#if (OPENSSL_VERSION_NUMBER >= 0x10001000L)
+/* OpenSSL 1.0.1 was the first version with TLS 1.2 support */
+#define DEFAULT_CIPHER_SELECTION                                     \
+  "EECDH+HIGH+TLSv1.2:EECDH+HIGH:EDH+HIGH+TLSv1.2:-SHA1:HIGH:-3DES:" \
+  "!aNULL:!eNULL:!RC4"
+#else
+#define DEFAULT_CIPHER_SELECTION                                \
   "ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH"
+#endif

 #endif /* USE_OPENSSL */
 #endif /* HEADER_CURL_SSLUSE_H */

[jay]

I don't understand why it is our problem if the protocol and cipher are independently selected by the server. Shouldn't the responsibility be on them? They're the ones selecting the bad cipher. I filed at golang/go#12895, let's see what happens.

[jay]

Go HTTP/2 server is now accessible via curl HTTP/2 regardless of client cipher order thanks to @bradfitz.

When this was up in the air I wrote a script to generate an HTTP/2 blacklisted cipher check in C. I also experimented with deprioritizing HTTP/2 blacklisted ciphers. Fortunately we do not need to do this now, afaict, but I've released it for reference in case we need to revisit this in the future.

Any objections to closing this?