x/net/http2: expose which TLS cipher suites are HTTP/2 safe

[noneymous]

Launching a web server with the SSL ciphers list taken from the TLS package does not work. I get the following error, that I can't get around programatically:

http2: TLSConfig.CipherSuites index 5 contains an HTTP/2-approved cipher suite (0x1301), but it comes after unapproved cipher suites. With this configuration, clients that don't support previous, approved cipher suites may be given an unapproved one and reject the connection.

What version of Go are you using (go version)?

1.14

Does this issue reproduce with the latest release?

yes (1.15)

What operating system and processor architecture are you using (go env)?

go env Output

set GOHOSTARCH=amd64
set GOHOSTOS=windows

What did you do?

I tried to launch a web server with a custom TLS config, with ciphers taken from tls.CipherSuites(). I do have one of these 2 dead ends:

• Ciphers returned by tls.CipherSuites() are not in a suitable order with HTTP/2-approved ciphers first (I guess, they are not intended to be).
• http2isBadCipher() in /net/http/h2_bundle.go is not exposed, so I can't sort ciphers accordingly on my own (I don't want to copy that function, because it seems it could get updated over time).

Here is a sample snippet:

        // Prepare list of accepted cipher suites
        var ciphers []uint16
        for _, cipher := range tls.CipherSuites() {
            if cipher.Insecure == false {
                ciphers = append(ciphers, cipher.ID)
            }
        }

        // Create TLS config
        tlsConf :=  &tls.Config{
            MinVersion:         tls.VersionTLS13,
            MaxVersion:         tls.VersionTLS13,
            CipherSuites:       ciphers,
        }

        // Create TLS web server
        server := &http.Server{Addr: listen, Handler: mux, TLSConfig: tlsConf}

        // Start TLS web server
        _ = server.ListenAndServeTLS("cert.crt", "cert.key")

What did you expect to see?

I just couldn't find a way to programatically order the ciphers as required, with the means available.

What did you see instead?

A critical error and program termination.

[FiloSottile]

We might consider adding an HTTP2 bool field to the CipherSuite type, but why are you doing this in the first place? You are effectively setting CipherSuites to the default list, but with worse ordering. Moreover, you are using TLS 1.3, so CipherSuites is ignored because TLS 1.3 suites are not configurable.

[FiloSottile]

That error does need some tweaking though, because the order doesn't matter unless PreferServerCipherSuites is set, and it could also just ignore the suites if MinVersion is VersionTLS13.

[noneymous]

(MinVersion should/could be 1.2, my bad)

Could you please point me to the place where the default list of ciphers is selected/generated (and their order is decided), if CipherSuites is not set by me? I could not find it, so I didn't trust it and tried to set it on my own...

[FiloSottile]

https://github.com/golang/go/blob/e6583dc95375c4e266bffab6f8888e8e557b6355/src/crypto/tls/common.go#L1418

It's not documented because it can change and evolve for security and performance reasons.

[noneymous]

Hm, okay, I see... many places where things are defined/decided and generally little control, other than hardcoding / maintaining a custom list and always using the latest Golang version to compile, etc...

Somehow I'm starting to doubt whether ciphers hardcoded/compiled into the executable are a good idea... If something changes (e.g. a vulnerability comes up) one needs to have the source code and recompile the program...

I guess this issue can be closed, thank you!

[noneymous]

One additional question:

"varDefaultCipherSuites" (i guess you've got a typo in that variable name, but luckily it's private ^^) is the default list in every tls.Config, right? So also third party packages, consuming tls.Config would use those good ciphers (unless explicitly set differently)...?

[FiloSottile]

Hm, okay, I see... many places where things are defined/decided and generally little control, other than hardcoding / maintaining a custom list and always using the latest Golang version to compile, etc...

Most developers want us to make the decisions on specific algorithms and to keep those up to date.

If something changes (e.g. a vulnerability comes up) one needs to have the source code and recompile the program...

If a vulnerability comes up developers are expected to obtained a patched version and recompile, like any other vulnerability.

I guess this issue can be closed, thank you!

I am going to keep it open to expose a HTTP2 bool in CipherSuite. Thank you!

"varDefaultCipherSuites" (i guess you've got a typo in that variable name, but luckily it's private ^^) is the default list in every tls.Config, right? So also third party packages, consuming tls.Config would use those good ciphers (unless explicitly set differently)...?

Correct! What's the typo?

[noneymous]

If a vulnerability comes up developers are expected to obtained a patched version and recompile, like any other vulnerability.

Well, not if cipher suites aren't part of the application, but instead outsourced to OpenSSL. Then OpenSSL (ideally) maintains a good and secure cipher set. And application adminsitrators can update OpenSSL and change the applied ciphers via application configuration... no need to re-compile.

What's the typo?

Why is "var" part of the variable name?

var (
    once                        sync.Once
    varDefaultCipherSuites      []uint16
    varDefaultCipherSuitesTLS13 []uint16
)

[FiloSottile]

Well, not if cipher suites aren't part of the application, but instead outsourced to OpenSSL. Then OpenSSL (ideally) maintains a good and secure cipher set. And application adminsitrators can update OpenSSL and change the applied ciphers via application configuration... no need to re-compile.

I was referring to any vulnerability in Go, which compiles to static binaries.

Why is "var" part of the variable name?

Not pretty, but it's a somewhat common pattern to hide the global var of a sync.Once based function (which is the symbol name without "var").

[Bsmalhi]

@FiloSottile Any new updates to when you would expose a HTTP2 bool in CipherSuite? Appreciate any timelines.

[FiloSottile]

@rolandshoemaker, can you mail a CL adding an HTTP2 bool to the CIpherSuite struct?

[gopherbot]

Change https://golang.org/cl/266297 mentions this issue: crypto/tls: add HTTP2 bool to CipherSuite struct

[odeke-em]

Sorry to sound like am bikeshedding but I just added a review comment to the CL, and asked perhaps for UsableWithHTTP2 instead of just HTTP2, because that field is a read only property, and not a toggle to enable HTTP/2, hence let’s try to make it more descriptive.

[rolandshoemaker]

After talking through this with the security team we're going to take a different path than was initially suggested here. Since this metadata is not a property of cipher suites themselves, but rather a property of the HTTP/2 protocol (by which I mean certain cipher suites don't support HTTP/2, HTTP/2 supports certain cipher suites) we're going to opt to expose this in x/net/http2 instead.

x/net/http2 already has a private function, isBadCipher, which provides this functionality. Current proposal is just to make this function public. Given we've already hit the freeze, and this extends the API surface for a package which is vendored into the standard library, this is likely to get pushed to 1.17.

[odeke-em]

Thank you @rolandshoemaker for relaying back the new direction. Punting accordingly to Go1.17. Happy holidays!

[gopherbot]

Change https://golang.org/cl/307369 mentions this issue: http2: expose method for checking blacklisted ciphers

[ivandov]

Given we've already hit the freeze, and this extends the API surface for a package which is vendored into the standard library, this is likely to get pushed to 1.17.

Did this make it into Go 1.17?

Can an example of usage be provided?

[ianlancetaylor]

This change has not been committed to any version.

CC @rolandshoemaker