Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/net/http2: The server should not select a banned cipher if it selects h2 #12895

Closed
jay opened this issue Oct 9, 2015 · 3 comments
Closed

x/net/http2: The server should not select a banned cipher if it selects h2 #12895

jay opened this issue Oct 9, 2015 · 3 comments
Labels
FrozenDueToAge

Comments

@jay
Copy link

jay commented Oct 9, 2015

An HTTP/2 capable server that enforces the optional TLS 1.2 cipher suite blacklist from RFC 7540 should not select one of those ciphers if the protocol selection is h2. A client may advertise both h2 and blacklisted ciphers because it doesn't know anything about the server.

For example consider a client such as libcurl that uses nghttp2 and OpenSSL 1.0.2. If HTTP2 is enabled it will send a ClientHello that advertises protocols h2, http/1.1 and ciphers such as [... TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA ...]. The go server currently replies with a ServerHello selecting h2 and the aforementioned blacklisted cipher, which it later bans by default.

TLSv1.2 Record Layer: Handshake Protocol: Server Hello
    Content Type: Handshake (22)
    Version: TLS 1.2 (0x0303)
    Length: 58
    Handshake Protocol: Server Hello
        Handshake Type: Server Hello (2)
        Length: 54
        Version: TLS 1.2 (0x0303)
        Random
            GMT Unix Time: Jan 21, 2084 01:52:46.000000000 Eastern Standard Time
            Random Bytes: 9dddda38a42111e70f3b934867a054c7a17df8aa27ef5139...
        Session ID Length: 0
        Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)
        Compression Method: null (0)
        Extensions Length: 14
        Extension: renegotiation_info
            Type: renegotiation_info (0xff01)
            Length: 1
            Renegotiation Info extension
                Renegotiation info extension length: 0
        Extension: Application Layer Protocol Negotiation
            Type: Application Layer Protocol Negotiation (0x0010)
            Length: 5
            ALPN Extension Length: 3
            ALPN Protocol
                ALPN string length: 2
                ALPN Next Protocol: h2

Ref curl/curl#406
@jay jay mentioned this issue Oct 9, 2015
Closed
@bradfitz
Copy link
Contributor

bradfitz commented Oct 9, 2015

/cc @agl, as this might require more cooperation between the http2 and crypto/tls packages.

@agl
Copy link
Contributor

agl commented Oct 10, 2015

The intended configuration for this is that the server sets PreferServerCipherSuites in the tls.Config and moves the HTTP/2 compatible cipher suites to the top of CipherSuites

@gopherbot
Copy link

CL https://golang.org/cl/15860 mentions this issue.

@bradfitz bradfitz self-assigned this Oct 15, 2015
bradfitz added a commit that referenced this issue Oct 20, 2015
@bradfitz
net/http: return error from Serve if http2.ConfigureServer returns an…
09eb588 
… error

In https://golang.org/cl/15860 http2.ConfigureServer was changed to
return an error if explicit CipherSuites are listed and they're not
compliant with the HTTP/2 spec.

This is the net/http side of the change, to look at the return value
from ConfigureServer and propagate it in Server.Serve.

h2_bundle.go will be updated in a future CL. There are too many other
http2 changes pending to be worth updating it now. Instead,
h2_bundle.go is minimally updated by hand in this CL so at least the
net/http change will compile.

Updates #12895

Change-Id: I4df7a097faff2d235742c2d310c333bd3fd5c08e
Reviewed-on: https://go-review.googlesource.com/16065
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Oct 24, 2016
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@bradfitz
http2: make ConfigureServer set PreferServerCipherSuites, return an e…
d7d8b41 
…rrors

Fixes golang/go#12895

Change-Id: I7cf6e63b1bbdf1f4e8974c00bdaed69b74f6db49
Reviewed-on: https://go-review.googlesource.com/15860
Reviewed-by: Adam Langley <agl@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bradfitz @agl @jay @gopherbot