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.

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

net/http/httptest: make it possible to use a Server (TLS or not) to test cookies #31054

Open
dcormier opened this issue Mar 26, 2019 · 8 comments
Labels
FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dcormier
Copy link
Contributor

dcormier commented Mar 26, 2019

httptest.NewTLSServer uses a cert that is not valid for localhost

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

$ go version
go version go1.12.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/danielcormier/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/danielcormier/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.1/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/84/_6l41bt970l9fmsrwc_p1gv00000gn/T/go-build114647702=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I'm trying to test something that involves cookies that set the Domain attribute. As discussed in #12610, *net/http/cookiejar.Jar won't return cookies for an IP (as per the relevant RFCs). (*httptest.Server).URL has the host set to an IP address (defaults to 127.0.0.1 or ::1).

To do the test I needed, I spun up an *httptest.Server using httptest.NewTLSServer(...), replaced the IP in (*httptest.Server).URL with localhost and attempted to send a request to it with (*httptest.Server).Client().

What did you expect to see?

I expected the httptest.NewTLSServer(...) to use a TLS cert that could be valid for localhost, as well as the loopback IP addresses.

I expected to be able to successfully make an HTTPS request to localhost at the correct port that the *httptest.Server was listening on by using (*httptest.Server).Client().

What did you see instead?

x509: certificate is valid for example.com, not localhost

Example

For completeness, I'm including the suite of tests showing the different behaviors with *cookiejar.Jar and the various combinations of *httptest.Server. The problematic test here is TestCookies/tls/localhost/default_cert (line 174). The test at line 185 shows that the original issue with cookies is resolved if I send requests to localhost with a cert valid for that hostname.

package cookies_test

import (
	"crypto/tls"
	"fmt"
	"io"
	"io/ioutil"
	"net"
	"net/http"
	"net/http/cookiejar"
	"net/http/httptest"
	"net/url"
	"testing"
	"time"
)

func TestCookies(t *testing.T) {
	const (
		routeSetCookie    = "/set-cookie"
		routeExpectCookie = "/expect-cookie"

		cookieName = "token"
	)

	handler := func(tb testing.TB) http.Handler {
		setCookie := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			host, _, err := net.SplitHostPort(r.Host)
			if err != nil {
				host = r.Host
			}

			cookie := &http.Cookie{
				Name:     cookieName,
				Value:    "the value",
				Domain:   host,
				Path:     "/",
				HttpOnly: true,
			}

			tb.Logf("Setting cookie: %s", cookie)

			http.SetCookie(w, cookie)
		})

		expectCookie := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			_, err := r.Cookie(cookieName)
			switch err {
			case nil:
				// Success!

			case http.ErrNoCookie:
				msg := "No cookie"
				tb.Log(msg)
				http.Error(w, msg, http.StatusBadRequest)
				return

			default:
				msg := fmt.Sprintf("Failed to get cookie: %v", err)
				tb.Log(msg)
				http.Error(w, msg, http.StatusInternalServerError)
				return
			}

			tb.Log("The cookie was set")
		})

		mux := http.NewServeMux()
		mux.Handle(routeSetCookie, setCookie)
		mux.Handle(routeExpectCookie, expectCookie)

		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			target := r.RequestURI
			tb.Logf("---- START %s", target)
			mux.ServeHTTP(w, r)
			tb.Logf("---- END %s", target)
		})
	}

	testCookies := func(tb testing.TB, svr *httptest.Server) {
		jar, err := cookiejar.New(nil)
		if err != nil {
			tb.Fatal(err)
		}

		httpClient := svr.Client()
		httpClient.Timeout = 1 * time.Second
		httpClient.Jar = jar

		resp, err := httpClient.Get(svr.URL + routeExpectCookie)
		if err != nil {
			tb.Fatal(err)
		}

		defer func() {
			io.Copy(ioutil.Discard, resp.Body)
			resp.Body.Close()
		}()

		if resp.StatusCode != http.StatusBadRequest {
			body, _ := ioutil.ReadAll(resp.Body)
			tb.Fatalf("Should not have cookie: %s\n%s", resp.Status, body)
		}

		resp, err = httpClient.Get(svr.URL + routeSetCookie)
		if err != nil {
			tb.Fatal(err)
		}

		if resp.StatusCode != http.StatusOK {
			body, _ := ioutil.ReadAll(resp.Body)
			tb.Fatalf("%s\n%s", resp.Status, body)
		}

		resp, err = httpClient.Get(svr.URL + routeExpectCookie)
		if err != nil {
			tb.Fatal(err)
		}

		if resp.StatusCode != http.StatusOK {
			body, _ := ioutil.ReadAll(resp.Body)
			tb.Fatalf("%s\n%s", resp.Status, body)
		}
	}

	useLocalhost := func(tb testing.TB, svr *httptest.Server) {
		svrURL, err := url.Parse(svr.URL)
		if err != nil {
			tb.Fatal(err)
		}

		svrURL.Host = net.JoinHostPort("localhost", svrURL.Port())

		svr.URL = svrURL.String()
	}

	t.Run("no tls", func(t *testing.T) {
		t.Run("ip", func(t *testing.T) {
			// This fails because `cookiejar.CookieJar` follows the RFCs and drops the `Domain` of a
			// cookie where its set to an IP, rather than a domain. We'll skip it, but it's here if
			// you want to see for yourself.
			t.SkipNow()

			svr := httptest.NewServer(handler(t))
			defer svr.Close()

			testCookies(t, svr)
		})

		t.Run("localhost", func(t *testing.T) {
			// This works.

			svr := httptest.NewServer(handler(t))
			defer svr.Close()

			useLocalhost(t, svr)
			testCookies(t, svr)
		})
	})

	t.Run("tls", func(t *testing.T) {
		t.Run("ip", func(t *testing.T) {
			// This fails because `cookiejar.CookieJar` follows the RFCs and drops the `Domain` of a
			// cookie where its set to an IP, rather than a domain. We'll skip it, but it's here if
			// you want to see for yourself.
			t.SkipNow()

			svr := httptest.NewTLSServer(handler(t))
			defer svr.Close()

			testCookies(t, svr)
		})

		t.Run("localhost", func(t *testing.T) {
			t.Run("default cert", func(t *testing.T) {
				// This fails because the cert `httptest.NewTLSServer` serves up is valid for
				// 127.0.0.1, ::1, and examlple.com. Not localhost.

				svr := httptest.NewTLSServer(handler(t))
				defer svr.Close()

				useLocalhost(t, svr)
				testCookies(t, svr)
			})

			t.Run("localhost cert", func(t *testing.T) {
				// This works. But, you need to generate your own cert for localhost.

				svr := httptest.NewUnstartedServer(handler(t))

				certPEM := []byte(`-----BEGIN CERTIFICATE-----
MIIDJTCCAg2gAwIBAgIQas3l/GRJkOGvAZP1CLIvRzANBgkqhkiG9w0BAQsFADAb
MRkwFwYDVQQKExBBY21lIENvcnBvcmF0aW9uMCAXDTAwMDEwMTAwMDAwMFoYDzIx
MDAwMTAxMDAwMDAwWjAbMRkwFwYDVQQKExBBY21lIENvcnBvcmF0aW9uMIIBIjAN
BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAn46okXbPHDmuwHMcQHyPtDl1qoKL
WA/U5x1VcLHGOR6vQKjkNUXbW0yU0HYcyBtmr5gugdmlaFvCRlMSaG1pyC5iCCha
HlTyFyaZi0o2zGT34fS8Jj2WUKE/pR9pOqEoWx8UezBHw/NBZjGCjKe4ASzCQqbn
KA6DxQfRBypU+OFAIK3KsRP6Xvwqd2N/a5FybL9ixKYNbAj7b2vAhW7NIWw++m2T
Hif+bTsEhLAGUG3KGW9OGcJiAewyZb4DgZPgE1ourEud9goVbcCTZBYbpV3U0tZa
XxqJIOfhsfCe4fDcqe1Hspq47SLdvP8FP/qKTbFOqoA/NAlrmboxOw+mXwIDAQAB
o2MwYTAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDAYDVR0T
AQH/BAIwADAsBgNVHREEJTAjgglsb2NhbGhvc3SHEAAAAAAAAAAAAAAAAAAAAAGH
BH8AAAEwDQYJKoZIhvcNAQELBQADggEBAG2cVK1TZvHoaiqA40QEjKehKqq4vKLc
At/FrITEgvNTIkvguEvLw5wsUO/3Nt/atjWtFdSJCLWCLzrgiLOLtJubkrDzzbus
/OsI0cf/fMTyCnjt64efSz2RDPPllRbJd3zZBkuOWhPoxx/Sz0VRvQKGFb9mvPoI
PTZ22ugwZdS/3PnMEoVO46iQumGARXQEbiGApeXPObK0E6Fs7pqwomU9Ny2XsyXS
je06pfouDv8UlLzZLY/fVJLHN6aM7odw5iPp2p7ttFdgn1l/LVlZVX9FBwegHXet
5OSC7pDc+kbLg1cJE8/7dF47VBEVKSvr5ldgRuvDtEf4PupKRl4rhik=
-----END CERTIFICATE-----`)

				keyPEM := []byte(`-----BEGIN RSA PRIVATE KEY-----
MIIEpQIBAAKCAQEAn46okXbPHDmuwHMcQHyPtDl1qoKLWA/U5x1VcLHGOR6vQKjk
NUXbW0yU0HYcyBtmr5gugdmlaFvCRlMSaG1pyC5iCChaHlTyFyaZi0o2zGT34fS8
Jj2WUKE/pR9pOqEoWx8UezBHw/NBZjGCjKe4ASzCQqbnKA6DxQfRBypU+OFAIK3K
sRP6Xvwqd2N/a5FybL9ixKYNbAj7b2vAhW7NIWw++m2THif+bTsEhLAGUG3KGW9O
GcJiAewyZb4DgZPgE1ourEud9goVbcCTZBYbpV3U0tZaXxqJIOfhsfCe4fDcqe1H
spq47SLdvP8FP/qKTbFOqoA/NAlrmboxOw+mXwIDAQABAoIBAQCLVxlNF5WdT56W
ALDOfDk/KeLhSmoIOKM0RkDETuwOHAbuj8/j2iLLo6BeQJe4BX3yoRMUYQ77iQ6r
PYbY3ZxAroj8GMlCrepRX2s94kziyNZVZNYfCy/HMFqViE3sXqsQkJ7hSfOSY1Bc
v6YD0cB2fjET5g/+wlY+7imUeVqFkUd5+CuSa4MVheWRiXCFydm+GdUMbHGJauZk
KYSz6oE5vXkDCbcjpyH2Ay7QuHiE00wI2DqsvkZJy8et+XgYL8iNj10JulnDXJg6
MmSf0ZsDfhfJW9AQDzZjXfbSRsskztnehN4UcJH8enLaLbanlYisPpIsj9jpqLwt
EDcsHX+ZAoGBAMC0nO9MSoxu4Q9WP8Lq5qT1QYrRvmSO9mHz+4wmYvConofucqlK
M6HXD/qXIU8pTHZ5WHjnnEyNOvVdsK/P6uYkdqWRXig8/zgoi6DGuujlthJ7BKYW
I7Fvh2z217p3y0IHQvHYjxQk0ag9kOxkdqiYW6WxNcUj2QeXgDkEjcddAoGBANP2
0XI1tEm+IThXHnnT8DYci4xIi9JBvkWgq+atw8ZNicNfN+a0RX5f42fFUCkGE3F6
JgQgSwIAr6zbLKH8RzwU/V5dpO7vuPrgsCRwFsovKAhyCpW0PflJXIKPY6xrbRnc
t2cSOitZzWBdGQJQANGcd+qdGDG/NBcsYdchKfTrAoGAMS/ovsviW2YR3DBPphj/
NivDxwMybchv6yCznFpP9s2TaW7bpYpjE3Qph/T7c5E/Cx5+Dp5Prtp9qhN3/eg8
NPIptqkcN3kaS+NNgIQ5QSkhCCaOUTZldezZzF5VQitBnmDsHX8BRkr/mMneK/iY
sP/ypKBO8TrtMprhB6y546ECgYEAgFXwejYJ8pwrgPE+goTP6/NcipNiFOu5SG7/
pauP3YEU6DW+ovCDIwDrrujIoA4Nt6c9XUIwKAZCV2Zcn7cfakFLJteMBR8f4MYp
3+X95mym0HY78mgvHcBNQr+OmdZxODdq0/01OwokTzQO8FeAJ2mVMXfsLjKWV3GH
y7lIrgECgYEAiZIEx3fBc3TBcaZb5vbWyAQyfC5vgI0N25ZaIwoG5g6CkjKt8nfH
Xfl1da9pWbcAgRLlq+XhqAJQdUjZ0NfKeWSQxT8TQob8ZfiAHXwjTf20qFrarsPl
jVyKqKuj7Vl7evexIhY03RL6S/koyDGJWdUt9myZB6mdFJBBFQIuv8U=
-----END RSA PRIVATE KEY-----`)

				cert, err := tls.X509KeyPair(certPEM, keyPEM)
				if err != nil {
					t.Fatal(err)
				}

				svr.TLS = &tls.Config{
					Certificates: []tls.Certificate{cert},
				}

				svr.StartTLS()
				defer svr.Close()

				useLocalhost(t, svr)
				testCookies(t, svr)
			})
		})
	})
}

I attempted to have a conversation about the hosts included in the cert used by httptest.NewTLSServer() in the golang-nuts group, but it went nowhere.

@ohir
Copy link

ohir commented Apr 2, 2019

This is a key pair that is available to the general public. Ie. its private part is known to all. Making it match on localhost or loopback interface would be a huge security hole for millions of developers who would add it to the trusted certs store then their machines would be then susceptible to a wide class of threats via localhost MITM.

Please follow howto and make a cert for yourself.
Note the minica link down the page.

@dcormier
Copy link
Contributor Author

dcormier commented Apr 2, 2019

Making it match on localhost or loopback interface would be a huge security hole for millions of developers who would add it to the trusted certs store then their machines would be then susceptible to a wide class of threats via localhost MITM.

It already matches IPv4 and IPv6 loopback addresses.

Adding a cert from a well-known key pair to ones trusted certs is unwise for the reasons you mentioned. Safer to generate your own to trust.

@agnivade
Copy link
Contributor

agnivade commented Apr 2, 2019

@bradfitz

@bradfitz
Copy link
Contributor

bradfitz commented Apr 2, 2019

Dup of #30774, it seems? That's not supported. The httptest.NewTLSServer is for testing and you can only hit it with the URL it gives you.

@bradfitz bradfitz changed the title httptest.NewTLSServer uses a cert that is not valid for localhost net/http/httptest: NewTLSServer uses a cert that is not valid for localhost Apr 2, 2019
@dcormier
Copy link
Contributor Author

dcormier commented Apr 2, 2019

Dup of #30774, it seems? That's not supported. The httptest.NewTLSServer is for testing and you can only hit it with the URL it gives you.

Then this is a feature request, I guess. The goal being to make it easier use httptest.NewTLSServer for testing with cookies.

@bradfitz bradfitz changed the title net/http/httptest: NewTLSServer uses a cert that is not valid for localhost net/http/httptest: make it possible to use a Server (TLS or not) to test cookies Apr 2, 2019
@bradfitz
Copy link
Contributor

bradfitz commented Apr 2, 2019

Feature request SGTM. I'm fine if the returned Client would have a special-cased dialer that makes a certain name(s) ("*.test.example") dial towards that IP, and make the TLS work.

Anybody want to work on it?

@dcormier
Copy link
Contributor Author

dcormier commented Apr 2, 2019

Anybody want to work on it?

I'm interested.

@bcmills bcmills added this to the Unplanned milestone May 28, 2019
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2019
@gopherbot
Copy link

Change https://golang.org/cl/182917 mentions this issue: net/http/httptest: make it possible to use a Server (TLS or not) to test cookies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants