From 910db02652e8825b9829b57d5d89b39699896d92 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 1 Nov 2022 20:37:13 -0700 Subject: [PATCH] client/tailscale, tsnet, ipn/ipnlocal: prove nodekey ownership over noise Fixes #5972 Change-Id: Ic33a93d3613ac5dbf172d6a8a459ca06a7f9e547 Signed-off-by: Brad Fitzpatrick --- client/tailscale/tailscale.go | 26 -------------- control/controlclient/auto.go | 10 ++++++ control/controlclient/direct.go | 27 +++++++++++++++ control/controlclient/noise.go | 29 ++++++++++++---- ipn/ipnlocal/local.go | 60 ++++++++++++++++++++++++++++----- tsnet/tsnet.go | 7 ++-- types/key/chal.go | 3 ++ 7 files changed, 117 insertions(+), 45 deletions(-) diff --git a/client/tailscale/tailscale.go b/client/tailscale/tailscale.go index 0af754fe8..626532e72 100644 --- a/client/tailscale/tailscale.go +++ b/client/tailscale/tailscale.go @@ -18,9 +18,6 @@ import ( "fmt" "io" "net/http" - "net/url" - - "tailscale.com/types/key" ) // I_Acknowledge_This_API_Is_Unstable must be set true to use this package @@ -93,29 +90,6 @@ func (c *Client) setAuth(r *http.Request) { } } -// nodeKeyAuth is an AuthMethod for NewClient that authenticates requests -// using a node key over the Noise protocol. -type nodeKeyAuth key.NodePublic - -func (k nodeKeyAuth) modifyRequest(req *http.Request) { - // QueryEscape the node key since it has a colon in it. - nk := url.QueryEscape(key.NodePublic(k).String()) - req.SetBasicAuth(nk, "") -} - -// NewNoiseClient is a convenience method for instantiating a new Client -// that uses the Noise protocol for authentication. -// -// tailnet is the globally unique identifier for a Tailscale network, such -// as "example.com" or "user@gmail.com". -func NewNoiseClient(tailnet string, noiseRoundTripper http.RoundTripper, nk key.NodePublic) *Client { - return &Client{ - tailnet: tailnet, - auth: nodeKeyAuth(nk), - HTTPClient: &http.Client{Transport: noiseRoundTripper}, - } -} - // NewClient is a convenience method for instantiating a new Client. // // tailnet is the globally unique identifier for a Tailscale network, such diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 0c3c7210c..715fe8a10 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -730,3 +730,13 @@ func (c *Auto) SetDNS(ctx context.Context, req *tailcfg.SetDNSRequest) error { func (c *Auto) DoNoiseRequest(req *http.Request) (*http.Response, error) { return c.direct.DoNoiseRequest(req) } + +// GetSingleUseNoiseRoundTripper returns a RoundTripper that can be only be used +// once (and must be used once) to make a single HTTP request over the noise +// channel to the coordination server. +// +// In addition to the RoundTripper, it returns the HTTP/2 channel's early noise +// payload, if any. +func (c *Auto) GetSingleUseNoiseRoundTripper(ctx context.Context) (http.RoundTripper, *tailcfg.EarlyNoise, error) { + return c.direct.GetSingleUseNoiseRoundTripper(ctx) +} diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 7c9c48271..db3fe9d63 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -1607,6 +1607,33 @@ func (c *Direct) DoNoiseRequest(req *http.Request) (*http.Response, error) { return nc.Do(req) } +// GetSingleUseNoiseRoundTripper returns a RoundTripper that can be only be used +// once (and must be used once) to make a single HTTP request over the noise +// channel to the coordination server. +// +// In addition to the RoundTripper, it returns the HTTP/2 channel's early noise +// payload, if any. +func (c *Direct) GetSingleUseNoiseRoundTripper(ctx context.Context) (http.RoundTripper, *tailcfg.EarlyNoise, error) { + nc, err := c.getNoiseClient() + if err != nil { + return nil, nil, err + } + for tries := 0; tries < 3; tries++ { + conn, err := nc.getConn(ctx) + if err != nil { + return nil, nil, err + } + earlyPayloadMaybeNil, err := conn.getEarlyPayload(ctx) + if err != nil { + return nil, nil, err + } + if conn.h2cc.ReserveNewRequest() { + return conn, earlyPayloadMaybeNil, nil + } + } + return nil, nil, errors.New("[unexpected] failed to reserve a request on a connection") +} + // doPingerPing sends a Ping to pr.IP using pinger, and sends an http request back to // pr.URL with ping response data. func doPingerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger, pingType tailcfg.PingType) { diff --git a/control/controlclient/noise.go b/control/controlclient/noise.go index fdf61562d..d7ecfcafa 100644 --- a/control/controlclient/noise.go +++ b/control/controlclient/noise.go @@ -42,12 +42,24 @@ type noiseConn struct { reader io.Reader // (effectively Conn.Reader after header) earlyPayloadReady chan struct{} // closed after earlyPayload is set (including set to nil) earlyPayload *tailcfg.EarlyNoise + earlyPayloadErr error } func (c *noiseConn) RoundTrip(r *http.Request) (*http.Response, error) { return c.h2cc.RoundTrip(r) } +// getEarlyPayload waits for the early noise payload to arrive. +// It may return (nil, nil) if the server begins HTTP/2 without one. +func (c *noiseConn) getEarlyPayload(ctx context.Context) (*tailcfg.EarlyNoise, error) { + select { + case <-c.earlyPayloadReady: + return c.earlyPayload, c.earlyPayloadErr + case <-ctx.Done(): + return nil, ctx.Err() + } +} + // The first 9 bytes from the server to client over Noise are either an HTTP/2 // settings frame (a normal HTTP/2 setup) or, as we added later, an "early payload" // header that's also 9 bytes long: 5 bytes (earlyPayloadMagic) followed by 4 bytes @@ -80,33 +92,38 @@ func (c *noiseConn) Read(p []byte) (n int, err error) { // c.earlyPayload, closing c.earlyPayloadReady, and initializing c.reader for // future reads. func (c *noiseConn) readHeader() { + defer close(c.earlyPayloadReady) + + setErr := func(err error) { + c.reader = returnErrReader{err} + c.earlyPayloadErr = err + } + var hdr [hdrLen]byte if _, err := io.ReadFull(c.Conn, hdr[:]); err != nil { - c.reader = returnErrReader{err} + setErr(err) return } if string(hdr[:len(earlyPayloadMagic)]) != earlyPayloadMagic { // No early payload. We have to return the 9 bytes read we already // consumed. - close(c.earlyPayloadReady) c.reader = io.MultiReader(bytes.NewReader(hdr[:]), c.Conn) return } epLen := binary.BigEndian.Uint32(hdr[len(earlyPayloadMagic):]) if epLen > 10<<20 { - c.reader = returnErrReader{errors.New("invalid early payload length")} + setErr(errors.New("invalid early payload length")) return } payBuf := make([]byte, epLen) if _, err := io.ReadFull(c.Conn, payBuf); err != nil { - c.reader = returnErrReader{err} + setErr(err) return } if err := json.Unmarshal(payBuf, &c.earlyPayload); err != nil { - c.reader = returnErrReader{err} + setErr(err) return } - close(c.earlyPayloadReady) c.reader = c.Conn } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e18c496cb..3a32d79a0 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -6,12 +6,14 @@ package ipnlocal import ( "context" + "encoding/base64" "errors" "fmt" "io" "net" "net/http" "net/netip" + "net/url" "os" "os/user" "path/filepath" @@ -3765,18 +3767,60 @@ func (b *LocalBackend) magicConn() (*magicsock.Conn, error) { return mc, nil } -type noiseRoundTripper struct { - *LocalBackend +type keyProvingNoiseRoundTripper struct { + b *LocalBackend } -func (n noiseRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - return n.LocalBackend.DoNoiseRequest(req) +func (n keyProvingNoiseRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + b := n.b + + var priv key.NodePrivate + + b.mu.Lock() + cc := b.ccAuto + if nm := b.netMap; nm != nil { + priv = nm.PrivateKey + } + b.mu.Unlock() + if cc == nil { + return nil, errors.New("no client") + } + if priv.IsZero() { + return nil, errors.New("no netmap or private key") + } + rt, ep, err := cc.GetSingleUseNoiseRoundTripper(req.Context()) + if err != nil { + return nil, err + } + if ep == nil || ep.NodeKeyChallenge.IsZero() { + go rt.RoundTrip(new(http.Request)) // return our reservation with a bogus request + return nil, errors.New("this coordination server does not support API calls over the Noise channel") + } + + // QueryEscape the node key since it has a colon in it. + nk := url.QueryEscape(priv.Public().String()) + req.SetBasicAuth(nk, "") + + // genNodeProofHeaderValue returns the Tailscale-Node-Proof header's value to prove + // to chalPub that we control claimedPrivate. + genNodeProofHeaderValue := func(claimedPrivate key.NodePrivate, chalPub key.ChallengePublic) string { + // TODO(bradfitz): cache this somewhere? + box := claimedPrivate.SealToChallenge(chalPub, []byte(chalPub.String())) + return claimedPrivate.Public().String() + " " + base64.StdEncoding.EncodeToString(box) + } + + // And prove we have the private key corresponding to the public key sent + // tin the basic auth username. + req.Header.Set("Tailscale-Node-Proof", genNodeProofHeaderValue(priv, ep.NodeKeyChallenge)) + + return rt.RoundTrip(req) } -// NoiseRoundTripper returns an http.RoundTripper that uses the LocalBackend's -// DoNoiseRequest method. -func (b *LocalBackend) NoiseRoundTripper() http.RoundTripper { - return noiseRoundTripper{b} +// KeyProvingNoiseRoundTripper returns an http.RoundTripper that uses the LocalBackend's +// DoNoiseRequest method and mutates the request to add an authorization header +// to prove the client's nodekey. +func (b *LocalBackend) KeyProvingNoiseRoundTripper() http.RoundTripper { + return keyProvingNoiseRoundTripper{b} } // DoNoiseRequest sends a request to URL over the control plane diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 373b088ce..96e8f9a75 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -493,11 +493,8 @@ func (s *Server) APIClient() (*tailscale.Client, error) { return nil, err } - nm := s.lb.NetMap() - if nm == nil { - return nil, errors.New("no netmap, not logged in?") - } - c := tailscale.NewNoiseClient(nm.Domain, s.lb.NoiseRoundTripper(), nm.NodeKey) + c := tailscale.NewClient("-", nil) + c.HTTPClient = &http.Client{Transport: s.lb.KeyProvingNoiseRoundTripper()} return c, nil } diff --git a/types/key/chal.go b/types/key/chal.go index 7c5405224..b258210ff 100644 --- a/types/key/chal.go +++ b/types/key/chal.go @@ -82,3 +82,6 @@ func (k ChallengePublic) MarshalText() ([]byte, error) { func (k *ChallengePublic) UnmarshalText(b []byte) error { return parseHex(k.k[:], mem.B(b), mem.S(chalPublicHexPrefix)) } + +// IsZero reports whether k is the zero value. +func (k ChallengePublic) IsZero() bool { return k == ChallengePublic{} }