control/controlclient,tempfork/httprec: don't link httptest, test certs for c2n

The c2n handling code was using the Go httptest package's
ResponseRecorder code but that's in a test package which brings in
Go's test certs, etc.

This forks the httptest recorder type into its own package that only
has the recorder and adds a test that we don't re-introduce a
dependency on httptest.

Updates #12614

Change-Id: I3546f49972981e21813ece9064cc2be0b74f4b16
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

5 files changed, 265 insertions, 7 deletions

diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt
index 32af3b25e..fab29ba03 100644
--- a/cmd/k8s-operator/depaware.txt
+++ b/cmd/k8s-operator/depaware.txt
@@ -889,6 +889,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/tailcfg                                        from tailscale.com/client/tailscale+
         tailscale.com/taildrop                                       from tailscale.com/ipn/ipnlocal+
         tailscale.com/tempfork/heap                                  from tailscale.com/wgengine/magicsock
+        tailscale.com/tempfork/httprec                               from tailscale.com/control/controlclient
         tailscale.com/tka                                            from tailscale.com/client/tailscale+
         tailscale.com/tsconst                                        from tailscale.com/net/netmon+
         tailscale.com/tsd                                            from tailscale.com/ipn/ipnlocal+
@@ -1174,12 +1175,10 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         mime/quotedprintable                                         from mime/multipart
         net                                                          from crypto/tls+
         net/http                                                     from expvar+
-        net/http/httptest                                            from tailscale.com/control/controlclient
         net/http/httptrace                                           from github.com/prometheus-community/pro-bing+
         net/http/httputil                                            from github.com/aws/smithy-go/transport/http+
         net/http/internal                                            from net/http+
         net/http/internal/ascii                                      from net/http+
-        net/http/internal/testcert                                   from net/http/httptest
         net/http/pprof                                               from sigs.k8s.io/controller-runtime/pkg/manager+
         net/netip                                                    from github.com/gaissmai/bart+
         net/textproto                                                from github.com/aws/aws-sdk-go-v2/aws/signer/v4+
diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt
index e0ed51ebb..36b6063d5 100644
--- a/cmd/tailscaled/depaware.txt
+++ b/cmd/tailscaled/depaware.txt
@@ -341,6 +341,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/taildrop                                       from tailscale.com/ipn/ipnlocal+
   LD    tailscale.com/tempfork/gliderlabs/ssh                        from tailscale.com/ssh/tailssh
         tailscale.com/tempfork/heap                                  from tailscale.com/wgengine/magicsock
+        tailscale.com/tempfork/httprec                               from tailscale.com/control/controlclient
         tailscale.com/tka                                            from tailscale.com/client/tailscale+
         tailscale.com/tsconst                                        from tailscale.com/net/netmon+
         tailscale.com/tsd                                            from tailscale.com/cmd/tailscaled+
@@ -547,7 +548,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         encoding/xml                                                 from github.com/aws/aws-sdk-go-v2/aws/protocol/xml+
         errors                                                       from archive/tar+
         expvar                                                       from tailscale.com/derp+
-        flag                                                         from net/http/httptest+
+        flag                                                         from tailscale.com/cmd/tailscaled+
         fmt                                                          from archive/tar+
         hash                                                         from compress/zlib+
         hash/adler32                                                 from compress/zlib+
@@ -612,12 +613,10 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         mime/quotedprintable                                         from mime/multipart
         net                                                          from crypto/tls+
         net/http                                                     from expvar+
-        net/http/httptest                                            from tailscale.com/control/controlclient
         net/http/httptrace                                           from github.com/prometheus-community/pro-bing+
         net/http/httputil                                            from github.com/aws/smithy-go/transport/http+
         net/http/internal                                            from net/http+
         net/http/internal/ascii                                      from net/http+
-        net/http/internal/testcert                                   from net/http/httptest
         net/http/pprof                                               from tailscale.com/cmd/tailscaled+
         net/netip                                                    from github.com/tailscale/wireguard-go/conn+
         net/textproto                                                from github.com/aws/aws-sdk-go-v2/aws/signer/v4+
diff --git a/cmd/tailscaled/tailscaled_test.go b/cmd/tailscaled/tailscaled_test.go
index f36120f13..c50c23759 100644
--- a/cmd/tailscaled/tailscaled_test.go
+++ b/cmd/tailscaled/tailscaled_test.go
@@ -22,6 +22,8 @@ func TestDeps(t *testing.T) {
 		BadDeps: map[string]string{
 			"testing":                        "do not use testing package in production code",
 			"gvisor.dev/gvisor/pkg/hostarch": "will crash on non-4K page sizes; see https://github.com/tailscale/tailscale/issues/8658",
+			"net/http/httptest":              "do not use httptest in production code",
+			"net/http/internal/testcert":     "do not use httptest in production code",
 		},
 	}.Check(t)
 
diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go
index c436bc8b1..f327ecc2a 100644
--- a/control/controlclient/direct.go
+++ b/control/controlclient/direct.go
@@ -15,7 +15,6 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"net/http/httptest"
 	"net/netip"
 	"net/url"
 	"os"
@@ -42,6 +41,7 @@ import (
 	"tailscale.com/net/tsdial"
 	"tailscale.com/net/tshttpproxy"
 	"tailscale.com/tailcfg"
+	"tailscale.com/tempfork/httprec"
 	"tailscale.com/tka"
 	"tailscale.com/tstime"
 	"tailscale.com/types/key"
@@ -1384,7 +1384,7 @@ func answerC2NPing(logf logger.Logf, c2nHandler http.Handler, c *http.Client, pr
 	handlerCtx, cancel := context.WithTimeout(context.Background(), handlerTimeout)
 	defer cancel()
 	hreq = hreq.WithContext(handlerCtx)
-	rec := httptest.NewRecorder()
+	rec := httprec.NewRecorder()
 	c2nHandler.ServeHTTP(rec, hreq)
 	cancel()
 
diff --git a/tempfork/httprec/httprec.go b/tempfork/httprec/httprec.go
new file mode 100644
index 000000000..13786aaf6
--- /dev/null
+++ b/tempfork/httprec/httprec.go
@@ -0,0 +1,258 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package httprec is a copy of the Go standard library's httptest.ResponseRecorder
+// type, which we want to use in non-test code without pulling in the rest of
+// the httptest package and its test certs, etc.
+package httprec
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"net/http"
+	"net/textproto"
+	"strconv"
+	"strings"
+
+	"golang.org/x/net/http/httpguts"
+)
+
+// ResponseRecorder is an implementation of [http.ResponseWriter] that
+// records its mutations for later inspection in tests.
+type ResponseRecorder struct {
+	// Code is the HTTP response code set by WriteHeader.
+	//
+	// Note that if a Handler never calls WriteHeader or Write,
+	// this might end up being 0, rather than the implicit
+	// http.StatusOK. To get the implicit value, use the Result
+	// method.
+	Code int
+
+	// HeaderMap contains the headers explicitly set by the Handler.
+	// It is an internal detail.
+	//
+	// Deprecated: HeaderMap exists for historical compatibility
+	// and should not be used. To access the headers returned by a handler,
+	// use the Response.Header map as returned by the Result method.
+	HeaderMap http.Header
+
+	// Body is the buffer to which the Handler's Write calls are sent.
+	// If nil, the Writes are silently discarded.
+	Body *bytes.Buffer
+
+	// Flushed is whether the Handler called Flush.
+	Flushed bool
+
+	result      *http.Response // cache of Result's return value
+	snapHeader  http.Header    // snapshot of HeaderMap at first Write
+	wroteHeader bool
+}
+
+// NewRecorder returns an initialized [ResponseRecorder].
+func NewRecorder() *ResponseRecorder {
+	return &ResponseRecorder{
+		HeaderMap: make(http.Header),
+		Body:      new(bytes.Buffer),
+		Code:      200,
+	}
+}
+
+// DefaultRemoteAddr is the default remote address to return in RemoteAddr if
+// an explicit DefaultRemoteAddr isn't set on [ResponseRecorder].
+const DefaultRemoteAddr = "1.2.3.4"
+
+// Header implements [http.ResponseWriter]. It returns the response
+// headers to mutate within a handler. To test the headers that were
+// written after a handler completes, use the [ResponseRecorder.Result] method and see
+// the returned Response value's Header.
+func (rw *ResponseRecorder) Header() http.Header {
+	m := rw.HeaderMap
+	if m == nil {
+		m = make(http.Header)
+		rw.HeaderMap = m
+	}
+	return m
+}
+
+// writeHeader writes a header if it was not written yet and
+// detects Content-Type if needed.
+//
+// bytes or str are the beginning of the response body.
+// We pass both to avoid unnecessarily generate garbage
+// in rw.WriteString which was created for performance reasons.
+// Non-nil bytes win.
+func (rw *ResponseRecorder) writeHeader(b []byte, str string) {
+	if rw.wroteHeader {
+		return
+	}
+	if len(str) > 512 {
+		str = str[:512]
+	}
+
+	m := rw.Header()
+
+	_, hasType := m["Content-Type"]
+	hasTE := m.Get("Transfer-Encoding") != ""
+	if !hasType && !hasTE {
+		if b == nil {
+			b = []byte(str)
+		}
+		m.Set("Content-Type", http.DetectContentType(b))
+	}
+
+	rw.WriteHeader(200)
+}
+
+// Write implements http.ResponseWriter. The data in buf is written to
+// rw.Body, if not nil.
+func (rw *ResponseRecorder) Write(buf []byte) (int, error) {
+	rw.writeHeader(buf, "")
+	if rw.Body != nil {
+		rw.Body.Write(buf)
+	}
+	return len(buf), nil
+}
+
+// WriteString implements [io.StringWriter]. The data in str is written
+// to rw.Body, if not nil.
+func (rw *ResponseRecorder) WriteString(str string) (int, error) {
+	rw.writeHeader(nil, str)
+	if rw.Body != nil {
+		rw.Body.WriteString(str)
+	}
+	return len(str), nil
+}
+
+func checkWriteHeaderCode(code int) {
+	// Issue 22880: require valid WriteHeader status codes.
+	// For now we only enforce that it's three digits.
+	// In the future we might block things over 599 (600 and above aren't defined
+	// at https://httpwg.org/specs/rfc7231.html#status.codes)
+	// and we might block under 200 (once we have more mature 1xx support).
+	// But for now any three digits.
+	//
+	// We used to send "HTTP/1.1 000 0" on the wire in responses but there's
+	// no equivalent bogus thing we can realistically send in HTTP/2,
+	// so we'll consistently panic instead and help people find their bugs
+	// early. (We can't return an error from WriteHeader even if we wanted to.)
+	if code < 100 || code > 999 {
+		panic(fmt.Sprintf("invalid WriteHeader code %v", code))
+	}
+}
+
+// WriteHeader implements [http.ResponseWriter].
+func (rw *ResponseRecorder) WriteHeader(code int) {
+	if rw.wroteHeader {
+		return
+	}
+
+	checkWriteHeaderCode(code)
+	rw.Code = code
+	rw.wroteHeader = true
+	if rw.HeaderMap == nil {
+		rw.HeaderMap = make(http.Header)
+	}
+	rw.snapHeader = rw.HeaderMap.Clone()
+}
+
+// Flush implements [http.Flusher]. To test whether Flush was
+// called, see rw.Flushed.
+func (rw *ResponseRecorder) Flush() {
+	if !rw.wroteHeader {
+		rw.WriteHeader(200)
+	}
+	rw.Flushed = true
+}
+
+// Result returns the response generated by the handler.
+//
+// The returned Response will have at least its StatusCode,
+// Header, Body, and optionally Trailer populated.
+// More fields may be populated in the future, so callers should
+// not DeepEqual the result in tests.
+//
+// The Response.Header is a snapshot of the headers at the time of the
+// first write call, or at the time of this call, if the handler never
+// did a write.
+//
+// The Response.Body is guaranteed to be non-nil and Body.Read call is
+// guaranteed to not return any error other than [io.EOF].
+//
+// Result must only be called after the handler has finished running.
+func (rw *ResponseRecorder) Result() *http.Response {
+	if rw.result != nil {
+		return rw.result
+	}
+	if rw.snapHeader == nil {
+		rw.snapHeader = rw.HeaderMap.Clone()
+	}
+	res := &http.Response{
+		Proto:      "HTTP/1.1",
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		StatusCode: rw.Code,
+		Header:     rw.snapHeader,
+	}
+	rw.result = res
+	if res.StatusCode == 0 {
+		res.StatusCode = 200
+	}
+	res.Status = fmt.Sprintf("%03d %s", res.StatusCode, http.StatusText(res.StatusCode))
+	if rw.Body != nil {
+		res.Body = io.NopCloser(bytes.NewReader(rw.Body.Bytes()))
+	} else {
+		res.Body = http.NoBody
+	}
+	res.ContentLength = parseContentLength(res.Header.Get("Content-Length"))
+
+	if trailers, ok := rw.snapHeader["Trailer"]; ok {
+		res.Trailer = make(http.Header, len(trailers))
+		for _, k := range trailers {
+			for _, k := range strings.Split(k, ",") {
+				k = http.CanonicalHeaderKey(textproto.TrimString(k))
+				if !httpguts.ValidTrailerHeader(k) {
+					// Ignore since forbidden by RFC 7230, section 4.1.2.
+					continue
+				}
+				vv, ok := rw.HeaderMap[k]
+				if !ok {
+					continue
+				}
+				vv2 := make([]string, len(vv))
+				copy(vv2, vv)
+				res.Trailer[k] = vv2
+			}
+		}
+	}
+	for k, vv := range rw.HeaderMap {
+		if !strings.HasPrefix(k, http.TrailerPrefix) {
+			continue
+		}
+		if res.Trailer == nil {
+			res.Trailer = make(http.Header)
+		}
+		for _, v := range vv {
+			res.Trailer.Add(strings.TrimPrefix(k, http.TrailerPrefix), v)
+		}
+	}
+	return res
+}
+
+// parseContentLength trims whitespace from s and returns -1 if no value
+// is set, or the value if it's >= 0.
+//
+// This a modified version of same function found in net/http/transfer.go. This
+// one just ignores an invalid header.
+func parseContentLength(cl string) int64 {
+	cl = textproto.TrimString(cl)
+	if cl == "" {
+		return -1
+	}
+	n, err := strconv.ParseUint(cl, 10, 63)
+	if err != nil {
+		return -1
+	}
+	return int64(n)
+}