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: Transport.TLSConfig.ServerName is ignored #14501

Closed
jeanlucmongrain opened this issue Feb 25, 2016 · 5 comments
Closed

x/net/http2: Transport.TLSConfig.ServerName is ignored #14501

jeanlucmongrain opened this issue Feb 25, 2016 · 5 comments
Labels
FrozenDueToAge

Comments

@jeanlucmongrain
Copy link

http2.Transport behave differently than http.Transport and it break valid use case.

Before, I could use self signed certificate and use the ServerName TLS configuration to force to validate on a specific name.

But since 1.6/http2 it don't works anymore, here is a complete example.

package main

import (
    "crypto/tls"
    "crypto/x509"
    "flag"
    "fmt"
    "golang.org/x/net/http2"
    "log"
    "net"
    "net/http"
    "time"
)

const selfSignedCert = `-----BEGIN CERTIFICATE-----
MIIDIDCCAgigAwIBAgIQCCOHWNBePtS7Wqjmoe5ciDANBgkqhkiG9w0BAQsFADAX
MRUwEwYDVQQKEwxTZWxmIFNpZ25lcnMwHhcNMTYwMjI1MDc1MzQ4WhcNMjEwMjIz
MDc1MzQ4WjAXMRUwEwYDVQQKEwxTZWxmIFNpZ25lcnMwggEiMA0GCSqGSIb3DQEB
AQUAA4IBDwAwggEKAoIBAQDP0iJO3XRZZ+4kffmzOAOly/C15x9p2p1qGxxw8MKk
xxV8g1U8VAXNurP4sbPEdFGUz92UVxHztnYhWODZdXTMR2Ofe4zoPvEqcmKZNq6d
GvSC9PKOys1AqLHRJTKUpRxnZicuwhW/6Pbba/YBZAHTLri9ryM0j3jdv1L72t0V
4fnIf7zRDJ9aoGvh2YfyI+SD5DaVKtGP0WZmKP6sE8faHwHSLgvWMXIeK5dZutEf
jGiJrxo/fBwLCO0hCDxUnjFO24BufWUdH/S+UDHDvhvR7W7exeT/he5c2TqJQKlB
7XZHhVafToImAhAr9vWJ6ySITPSNzZ+Hi3T81wpZIyHDAgMBAAGjaDBmMA4GA1Ud
DwEB/wQEAwICpDATBgNVHSUEDDAKBggrBgEFBQcDATAPBgNVHRMBAf8EBTADAQH/
MC4GA1UdEQQnMCWCCWxvY2FsaG9zdIIAhwR/AAABhxAAAAAAAAAAAAAAAAAAAAAB
MA0GCSqGSIb3DQEBCwUAA4IBAQByK/cS9SekIwiELRzEBxpSnFfM/RB5LTgUeNuV
jV7IyrIbN0Xp/vBaatyOcbTQSN0bSvEx7RtIgdQnRP+UDRR7xpCodL/8PL0VrgmN
M5ME+xNbG7kT5ht/VKUw9MysHhyKjL47uxR59j7bhV8kGp2TXbV6uUn9FfGFhxIg
UXu+BloY+rXsyu22opKr9GvB9tXRx2kkNtWv6M3UP0hcgx1Q9MmgrK6l0bAj+YH0
Gk+ZZq4jJdcSdMhl6sQqvF7EoTfrb0meYrcFbRvzyrgSOmTsguLvJgPd2gwwiCi6
nqFowy94PywK2klmfIA7Wml4G9egR+mO0m37BqXETVMM8Atu
-----END CERTIFICATE-----`

const selfSignedKey = `-----BEGIN RSA PRIVATE KEY-----
MIIEpQIBAAKCAQEAz9IiTt10WWfuJH35szgDpcvwtecfadqdahsccPDCpMcVfINV
PFQFzbqz+LGzxHRRlM/dlFcR87Z2IVjg2XV0zEdjn3uM6D7xKnJimTaunRr0gvTy
jsrNQKix0SUylKUcZ2YnLsIVv+j222v2AWQB0y64va8jNI943b9S+9rdFeH5yH+8
0QyfWqBr4dmH8iPkg+Q2lSrRj9FmZij+rBPH2h8B0i4L1jFyHiuXWbrRH4xoia8a
P3wcCwjtIQg8VJ4xTtuAbn1lHR/0vlAxw74b0e1u3sXk/4XuXNk6iUCpQe12R4VW
n06CJgIQK/b1ieskiEz0jc2fh4t0/NcKWSMhwwIDAQABAoIBADFQ7FNKuhF9WEXQ
nzCoWjU98CE6d3nnJvPG+zjR6V6w36hsgg2O4tGvPIYHpWE5OSLXMP3Cq7/pzJ6d
OL5h4RWY20s9RnLWfORVwJAbKdSeUOfCuMyp04tEfO3kpdwgUl03IJU3+XFRF0N6
myY5VTWIIM7igLg4U7ZLcKXTiiNmzslgY1SlLac2jY9pt1zj9KdXZBo0iFYRCWnX
+rjKJ5W6gUoAf5mz+79nUfxLg1iq1TR5ZBg1AC8R/hiA1iFgxeSmt1dN8TEFpWJi
gbwtZXN9swVrvpggwQ0pMImkpNd1MjlNo/AF14+Tr+iFbkUAaV+fm6VS64EBtpkm
Jxnf1gECgYEA1JKEVYpIc/mfLRlp6ud8t+wGGAqKQ5TAiXQ79+/WniLJwp4TTHNi
7liCB9wCPugiF/kFi8bSJiq44+m8xvyuFF77mrl0JlJo1oM6tk9f97nbOp1dfJjJ
siMYMc94mk84W1YaGou5uyAuCOQVQN72aH3w9rVAJWFIeMcgVzuW9t8CgYEA+kcd
btTZM5pZ7IMMeR0qkpi5DvnkdqEZ04w9izM0M7nurb8jBHxLhyPmYI0JskeC1/Nz
bHhANYu7zYVFbFox1kNrjT71Fsam8mDhRCrmisukAaLgcMP7B9kwviTjvNbjTL7a
bJNrkPEZkVn3dDuR/u/fwbcgsJp0o0YYq72QpZ0CgYEAyWh1iFL4aSJXxjRxHcft
bJpt/7Nk47l6YAJIm5ecQsL1Zbe+0030q54ive2gFxh8zTf+IDzmepE6E4AwhF1/
Vv/T5vrWaUTl3nICXCCC7kYjyLq3jEl7uZP05aUNQS/UVv4Sx5oiBYHs/DGXTZqz
37eMa0S6nXA6aW1KYDyuwgMCgYEAvfF+6wBrH3jDSnZi3wA9sShrOmx48XdurFjA
1IY11hQEiqSHJP5YK+YZD6m6dC1FpjNDpEzXHkxAacf9WjBRpnVgiMABLhnx1f2k
uSF9+lR20i8U+0AhomE5VvWUSVslArfN+Z0gg84XQ3LuFK6pLLxMKy6ahq2SW9/1
FVI3FxkCgYEAs74VdqT9vIlKar5BEW0e1pjjmYnhxcAb5lHKF+g9YS/Ec6TPYI1K
ph0DBf/U8ETJRHOUUOA9SA5fDAtISSk+Q2eTnorkfz6g0MVDRuz9Ozr5eIT9jEfE
XkeGUwcxmZ/Nvjh5/KdaCvjckmjIDmWCbNPTO8VXkE1q8TyCUzivlkA=
-----END RSA PRIVATE KEY-----`

func main() {
    var err error
    serverName := "127.0.0.1" // self signed cert server name
    port := 5555

    // flag for IP address
    ipAddress := flag.String("ip", "", "IP address of any non loopback interface")
    flag.Parse()
    if len(*ipAddress) == 0 {
        log.Fatal("Missing -ip")
    }
    if *ipAddress == serverName {
        log.Fatalf("IP can't be %s", serverName)
    }
    if net.ParseIP(*ipAddress) == nil {
        log.Fatalf("IP %s is invalid", *ipAddress)
    }

    // start server
    handler := http.NewServeMux()
    handler.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
        fmt.Fprint(w, "hello")
    })
    server := &http.Server{
        Addr: fmt.Sprintf("0.0.0.0:%d", port),
        TLSConfig: &tls.Config{
            Certificates: make([]tls.Certificate, 1),
            ServerName:   serverName,
        },
        Handler: handler,
    }
    server.TLSConfig.Certificates[0], err = tls.X509KeyPair([]byte(selfSignedCert), []byte(selfSignedKey))
    if err != nil {
        log.Panic(err)
    }
    go server.ListenAndServeTLS("", "")
    time.Sleep(time.Second)

    // prepare clients
    pool := x509.NewCertPool()
    if !pool.AppendCertsFromPEM([]byte(selfSignedCert)) {
        log.Panic("can't append self signed tls certificate")
    }
    tlsClientConfig := &tls.Config{
        ServerName: serverName,
        RootCAs:    pool,
    }

    url := fmt.Sprintf("https://%s:%d/", *ipAddress, port)

    // http 1 client
    httpClient := &http.Client{
        Transport: &http.Transport{TLSClientConfig: tlsClientConfig},
    }
    resp, err := httpClient.Get(url)
    if err != nil {
        log.Println("HTTP/1: " + err.Error())
    } else {
        log.Println("HTTP/1: " + resp.Status)
    }

    // http2 client
    http2Client := &http.Client{
        Transport: &http2.Transport{TLSClientConfig: tlsClientConfig},
    }
    resp, err = http2Client.Get(url)
    if err != nil {
        log.Println("HTTP/2: " + err.Error())
    } else {
        log.Println("HTTP/2: " + resp.Status)
    }
}

Running output:

$ ifconfig en0
en0: flags=8863<UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST> mtu 1500
    inet 192.168.8.104 netmask 0xffffff00 broadcast 192.168.8.255
    media: autoselect
    status: active
$ ./test -ip 192.168.8.104
2016/02/25 16:00:16 HTTP/1: 200 OK
2016/02/25 16:00:16 HTTP/2: Get https://192.168.8.104:5555/: x509: certificate is valid for 127.0.0.1, ::1, not 192.168.8.104
@mikioh mikioh changed the title x/net/http2: TLS Configuration ServerName is Ignored crypto/tls: IP literals for SNI is Ignored from Go 1.6 Feb 25, 2016
@mikioh
Copy link
Contributor

mikioh commented Feb 25, 2016

Yup, I think this is because of 9f08b6c.

@mikioh mikioh changed the title crypto/tls: IP literals for SNI is Ignored from Go 1.6 crypto/tls: IP literal for SNI is Ignored from Go 1.6 Feb 25, 2016
@mikioh mikioh changed the title crypto/tls: IP literal for SNI is Ignored from Go 1.6 crypto/tls: IP literal for SNI is ignored from Go 1.6 Feb 25, 2016
@bradfitz bradfitz changed the title crypto/tls: IP literal for SNI is ignored from Go 1.6 x/net/http2: Transport.TLSConfig.ServerName is ignored Feb 25, 2016
@bradfitz
Copy link
Contributor

As far as I can tell, this has nothing to do with crypto/tls or Go 1.6. The Go 1.6 http2 path would still verify hostnames in the same way. You're not even hitting the Go 1.6 http2 path because your Transport sets the TLSConfig explicitly. Your repro with x/net/http2.Transport is valid, so I've retitled this bug.

This appears to be the difference, in x/net/http2/transport.go:

func (t *Transport) newTLSConfig(host string) *tls.Config {
        cfg := new(tls.Config)
        if t.TLSClientConfig != nil {
                *cfg = *t.TLSClientConfig
        }
        cfg.NextProtos = []string{NextProtoTLS} // TODO: don't override if already in list                                                                                             
        cfg.ServerName = host
        return cfg
}

The cfg.ServerName is being unconditionally set to host, even ift.TLSClientConfig` was non-nil.

But in the standard library's transport.go:

                cfg := cloneTLSClientConfig(t.TLSClientConfig)
                if cfg.ServerName == "" {
                        cfg.ServerName = cm.tlsHost()
                }

We can make that same change in x/net/http2.

@bradfitz bradfitz self-assigned this Feb 25, 2016
@bradfitz
Copy link
Contributor

Sent https://golang.org/cl/19918

@gopherbot
Copy link
Contributor

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

@jeanlucmongrain
Copy link
Author 

./test -ip 192.168.8.104
2016/02/26 12:33:29 HTTP/1: 200 OK
2016/02/26 12:33:29 HTTP/2: 200 OK

Confirm it's fixed!

@golang golang locked and limited conversation to collaborators Feb 28, 2017
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@bradfitz
http2: don't override user's Transport.TLSConfig.ServerName
c7a3543 
Fixes golang/go#14501

Change-Id: Ibaa7fb1fff404c62c35bb7c63f4a442e4fc0610d
Reviewed-on: https://go-review.googlesource.com/19918
Reviewed-by: Andrew Gerrand <adg@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 @mikioh @jeanlucmongrain @gopherbot