diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 765589bd2..5cfebf4de 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -195,7 +195,8 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa golang.org/x/exp/maps from tailscale.com/util/syspolicy/setting L golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/net/dnscache - golang.org/x/net/idna from golang.org/x/crypto/acme/autocert + golang.org/x/net/http/httpguts from tailscale.com/derp/derphttp + golang.org/x/net/idna from golang.org/x/crypto/acme/autocert+ golang.org/x/net/internal/socks from golang.org/x/net/proxy golang.org/x/net/proxy from tailscale.com/net/netns D golang.org/x/net/route from tailscale.com/net/netmon+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 6eb51318c..4be4bfcd0 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -369,6 +369,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep L golang.org/x/image/math/fixed from github.com/fogleman/gg+ golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/cmd/tailscale/cli+ + golang.org/x/net/http/httpguts from tailscale.com/derp/derphttp golang.org/x/net/http/httpproxy from tailscale.com/net/tshttpproxy golang.org/x/net/icmp from tailscale.com/net/ping golang.org/x/net/idna from golang.org/x/net/http/httpproxy+ diff --git a/cmd/tailscaled/depaware-min.txt b/cmd/tailscaled/depaware-min.txt index 497cf0445..fb12159c3 100644 --- a/cmd/tailscaled/depaware-min.txt +++ b/cmd/tailscaled/depaware-min.txt @@ -220,7 +220,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de golang.org/x/exp/maps from tailscale.com/ipn/store/mem golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/ipn/ipnlocal+ - golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal + golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal+ golang.org/x/net/icmp from tailscale.com/net/ping golang.org/x/net/idna from golang.org/x/net/http/httpguts golang.org/x/net/internal/iana from golang.org/x/net/icmp+ diff --git a/cmd/tailscaled/depaware-minbox.txt b/cmd/tailscaled/depaware-minbox.txt index d8edca0c9..caf037ae0 100644 --- a/cmd/tailscaled/depaware-minbox.txt +++ b/cmd/tailscaled/depaware-minbox.txt @@ -241,7 +241,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de golang.org/x/exp/maps from tailscale.com/ipn/store/mem golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/cmd/tailscale/cli+ - golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal + golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal+ golang.org/x/net/icmp from tailscale.com/net/ping golang.org/x/net/idna from golang.org/x/net/http/httpguts+ golang.org/x/net/internal/iana from golang.org/x/net/icmp+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 7d14816b8..5eccd04cc 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -564,7 +564,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de golang.org/x/exp/maps from tailscale.com/ipn/store/mem+ golang.org/x/net/bpf from github.com/mdlayher/genetlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/appc+ - golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal + golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal+ golang.org/x/net/http/httpproxy from tailscale.com/net/tshttpproxy golang.org/x/net/icmp from tailscale.com/net/ping golang.org/x/net/idna from golang.org/x/net/http/httpguts+ diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index 2c45030a8..da2a4b867 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -352,7 +352,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar golang.org/x/exp/maps from tailscale.com/ipn/store/mem+ golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/appc+ - golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal + golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal+ golang.org/x/net/http/httpproxy from tailscale.com/net/tshttpproxy golang.org/x/net/icmp from tailscale.com/net/ping golang.org/x/net/idna from golang.org/x/net/http/httpguts+ diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index 1f97237d7..dfa0d1b52 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -29,6 +29,7 @@ import ( "time" "go4.org/mem" + "golang.org/x/net/http/httpguts" "tailscale.com/derp" "tailscale.com/derp/derpconst" "tailscale.com/envknob" @@ -845,6 +846,16 @@ func firstStr(a, b string) string { // dialNodeUsingProxy connects to n using a CONNECT to the HTTP(s) proxy in proxyURL. func (c *Client) dialNodeUsingProxy(ctx context.Context, n *tailcfg.DERPNode, proxyURL *url.URL) (_ net.Conn, err error) { + // n.HostName comes from the control-supplied DERP map and is written + // verbatim into the CONNECT request line and Host header below. Reject + // anything that isn't a valid host value so a hostname carrying CR/LF (or + // other control bytes) can't inject extra headers or a second request into + // the proxy connection. ValidHostHeader accepts the empty string, so check + // for that separately. + if n.HostName == "" || !httpguts.ValidHostHeader(n.HostName) { + return nil, fmt.Errorf("derphttp: invalid DERP node hostname %q", n.HostName) + } + pu := proxyURL var proxyConn net.Conn if pu.Scheme == "https" { diff --git a/derp/derphttp/derphttp_proxy_test.go b/derp/derphttp/derphttp_proxy_test.go new file mode 100644 index 000000000..038aebd24 --- /dev/null +++ b/derp/derphttp/derphttp_proxy_test.go @@ -0,0 +1,101 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package derphttp + +import ( + "bufio" + "context" + "io" + "net" + "net/http" + "net/url" + "strings" + "testing" + "time" + + "tailscale.com/net/netmon" + "tailscale.com/tailcfg" + "tailscale.com/types/key" +) + +// TestDialNodeUsingProxyHostname checks which control-supplied DERP hostnames +// dialNodeUsingProxy is willing to write into the proxy CONNECT request. A +// rejected hostname must fail before anything is sent to the proxy; an accepted +// one must show up as the CONNECT target. +func TestDialNodeUsingProxyHostname(t *testing.T) { + tests := []struct { + name string + hostname string + wantRej bool + wantTarget string // CONNECT target seen by the proxy, if !wantRej + }{ + {"crlf_injection", "127.0.0.1\r\nX-Injected: 1", true, ""}, + {"lf_only", "127.0.0.1\nX-Injected: 1", true, ""}, + {"empty", "", true, ""}, + {"dns_name", "derp1.example.com", false, "derp1.example.com:443"}, + {"ipv4", "127.0.0.1", false, "127.0.0.1:443"}, + {"ipv6", "fe80::1", false, "[fe80::1]:443"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Fake HTTP proxy that records the CONNECT target and replies 200. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + gotTarget := make(chan string, 1) + go func() { + conn, err := ln.Accept() + if err != nil { + return + } + defer conn.Close() + req, err := http.ReadRequest(bufio.NewReader(conn)) + if err != nil { + return + } + gotTarget <- req.RequestURI + io.WriteString(conn, "HTTP/1.1 200 OK\r\n\r\n") + }() + + c := NewRegionClient(key.NewNode(), t.Logf, netmon.NewStatic(), + func() *tailcfg.DERPRegion { return nil }) + defer c.Close() + + n := &tailcfg.DERPNode{ + HostName: tt.hostname, + DERPPort: 443, + } + proxyURL := &url.URL{Scheme: "http", Host: ln.Addr().String()} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + conn, err := c.dialNodeUsingProxy(ctx, n, proxyURL) + if tt.wantRej { + if err == nil { + conn.Close() + t.Fatalf("dialNodeUsingProxy accepted hostname %q", tt.hostname) + } + if !strings.Contains(err.Error(), "invalid DERP node hostname") { + t.Fatalf("got error %v, want an invalid-hostname error", err) + } + return + } + if err != nil { + t.Fatalf("dialNodeUsingProxy(%q): %v", tt.hostname, err) + } + defer conn.Close() + select { + case got := <-gotTarget: + if got != tt.wantTarget { + t.Errorf("CONNECT target = %q, want %q", got, tt.wantTarget) + } + case <-ctx.Done(): + t.Fatal("timed out waiting for CONNECT request") + } + }) + } +} diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 03b508063..ec962ab08 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -345,7 +345,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) golang.org/x/exp/maps from tailscale.com/ipn/store/mem+ golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from tailscale.com/appc+ - golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal + golang.org/x/net/http/httpguts from tailscale.com/ipn/ipnlocal+ golang.org/x/net/http/httpproxy from tailscale.com/net/tshttpproxy golang.org/x/net/icmp from tailscale.com/net/ping golang.org/x/net/idna from golang.org/x/net/http/httpguts+