From 980acc38bad2c02b1236833d8b9fbf8f8cbfecc9 Mon Sep 17 00:00:00 2001 From: Dave Anderson Date: Fri, 3 Sep 2021 13:17:46 -0700 Subject: [PATCH] types/key: add a special key with custom serialization for control private keys (#2792) * Revert "Revert "types/key: add MachinePrivate and MachinePublic."" This reverts commit 61c3b98a24317dcfd5cbe3db29e7d6b64b8c27a7. Signed-off-by: David Anderson * types/key: add ControlPrivate, with custom serialization. ControlPrivate is just a MachinePrivate that serializes differently in JSON, to be compatible with how the Tailscale control plane historically serialized its private key. Signed-off-by: David Anderson --- cmd/tailscale/depaware.txt | 2 +- cmd/tailscaled/depaware.txt | 2 +- control/controlclient/direct.go | 102 ++++------- control/controlclient/direct_test.go | 20 +- control/controlclient/map.go | 3 +- control/controlclient/sign.go | 4 +- control/controlclient/sign_supported.go | 4 +- control/controlclient/sign_unsupported.go | 4 +- ipn/ipnlocal/local.go | 28 ++- ipn/ipnlocal/state_test.go | 3 +- tailcfg/tailcfg.go | 13 +- tailcfg/tailcfg_clone.go | 3 +- tailcfg/tailcfg_test.go | 18 +- tstest/integration/testcontrol/testcontrol.go | 56 ++---- types/key/control.go | 64 +++++++ types/key/control_test.go | 39 ++++ types/key/key.go | 43 ++--- types/key/key_test.go | 92 ++++++---- types/key/machine.go | 173 ++++++++++++++++++ types/key/machine_test.go | 92 ++++++++++ types/key/util.go | 111 +++++++++++ types/key/util_test.go | 38 ++++ types/netmap/netmap.go | 3 +- types/persist/persist.go | 10 +- types/persist/persist_clone.go | 3 +- types/persist/persist_test.go | 10 +- 26 files changed, 707 insertions(+), 233 deletions(-) create mode 100644 types/key/control.go create mode 100644 types/key/control_test.go create mode 100644 types/key/machine.go create mode 100644 types/key/machine_test.go create mode 100644 types/key/util.go create mode 100644 types/key/util_test.go diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index c7a2f48d4..365c35a5d 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -77,7 +77,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep golang.org/x/crypto/cryptobyte/asn1 from crypto/ecdsa+ golang.org/x/crypto/curve25519 from crypto/tls+ golang.org/x/crypto/hkdf from crypto/tls - golang.org/x/crypto/nacl/box from tailscale.com/derp + golang.org/x/crypto/nacl/box from tailscale.com/derp+ golang.org/x/crypto/nacl/secretbox from golang.org/x/crypto/nacl/box golang.org/x/crypto/poly1305 from golang.org/x/crypto/chacha20poly1305+ golang.org/x/crypto/salsa20/salsa from golang.org/x/crypto/nacl/box+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 855b36c66..6e3ed913f 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -189,7 +189,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de golang.org/x/crypto/cryptobyte/asn1 from crypto/ecdsa+ golang.org/x/crypto/curve25519 from crypto/tls+ golang.org/x/crypto/hkdf from crypto/tls - golang.org/x/crypto/nacl/box from tailscale.com/control/controlclient+ + golang.org/x/crypto/nacl/box from tailscale.com/derp+ golang.org/x/crypto/nacl/secretbox from golang.org/x/crypto/nacl/box golang.org/x/crypto/poly1305 from golang.org/x/crypto/chacha20poly1305+ golang.org/x/crypto/salsa20/salsa from golang.org/x/crypto/nacl/box+ diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 9eb7b3b65..0a7105cb3 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -7,7 +7,6 @@ package controlclient import ( "bytes" "context" - "crypto/rand" "encoding/binary" "encoding/json" "errors" @@ -28,7 +27,7 @@ import ( "sync/atomic" "time" - "golang.org/x/crypto/nacl/box" + "go4.org/mem" "inet.af/netaddr" "tailscale.com/control/controlknobs" "tailscale.com/health" @@ -42,6 +41,7 @@ import ( "tailscale.com/net/tlsdial" "tailscale.com/net/tshttpproxy" "tailscale.com/tailcfg" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/types/opt" @@ -62,14 +62,14 @@ type Direct struct { logf logger.Logf linkMon *monitor.Mon // or nil discoPubKey tailcfg.DiscoKey - getMachinePrivKey func() (wgkey.Private, error) + getMachinePrivKey func() (key.MachinePrivate, error) debugFlags []string keepSharerAndUserSplit bool skipIPForwardingCheck bool pinger Pinger mu sync.Mutex // mutex guards the following fields - serverKey wgkey.Key + serverKey key.MachinePublic persist persist.Persist authKey string tryingNewKey wgkey.Private @@ -83,12 +83,12 @@ type Direct struct { } type Options struct { - Persist persist.Persist // initial persistent data - GetMachinePrivateKey func() (wgkey.Private, error) // returns the machine key to use - ServerURL string // URL of the tailcontrol server - AuthKey string // optional node auth key for auto registration - TimeNow func() time.Time // time.Now implementation used by Client - Hostinfo *tailcfg.Hostinfo // non-nil passes ownership, nil means to use default using os.Hostname, etc + Persist persist.Persist // initial persistent data + GetMachinePrivateKey func() (key.MachinePrivate, error) // returns the machine key to use + ServerURL string // URL of the tailcontrol server + AuthKey string // optional node auth key for auto registration + TimeNow func() time.Time // time.Now implementation used by Client + Hostinfo *tailcfg.Hostinfo // non-nil passes ownership, nil means to use default using os.Hostname, etc DiscoPublicKey tailcfg.DiscoKey NewDecompressor func() (Decompressor, error) KeepAlive bool @@ -320,7 +320,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new if err != nil { return regen, opt.URL, err } - c.logf("control server key %s from %s", serverKey.HexString(), c.serverURL) + c.logf("control server key %s from %s", serverKey.ShortString(), c.serverURL) c.mu.Lock() c.serverKey = serverKey @@ -398,13 +398,13 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new c.logf("RegisterRequest: %s", j) } - bodyData, err := encode(request, &serverKey, &machinePrivKey) + bodyData, err := encode(request, serverKey, machinePrivKey) if err != nil { return regen, opt.URL, err } body := bytes.NewReader(bodyData) - u := fmt.Sprintf("%s/machine/%s", c.serverURL, machinePrivKey.Public().HexString()) + u := fmt.Sprintf("%s/machine/%s", c.serverURL, machinePrivKey.Public().UntypedHexString()) req, err := http.NewRequest("POST", u, body) if err != nil { return regen, opt.URL, err @@ -422,7 +422,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new res.StatusCode, strings.TrimSpace(string(msg))) } resp := tailcfg.RegisterResponse{} - if err := decode(res, &resp, &serverKey, &machinePrivKey); err != nil { + if err := decode(res, &resp, serverKey, machinePrivKey); err != nil { c.logf("error decoding RegisterResponse with server key %s and machine key %s: %v", serverKey, machinePrivKey.Public(), err) return regen, opt.URL, fmt.Errorf("register request: %v", err) } @@ -636,7 +636,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm request.ReadOnly = true } - bodyData, err := encode(request, &serverKey, &machinePrivKey) + bodyData, err := encode(request, serverKey, machinePrivKey) if err != nil { vlogf("netmap: encode: %v", err) return err @@ -645,9 +645,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm ctx, cancel := context.WithCancel(ctx) defer cancel() - machinePubKey := tailcfg.MachineKey(machinePrivKey.Public()) + machinePubKey := machinePrivKey.Public() t0 := time.Now() - u := fmt.Sprintf("%s/machine/%s/map", serverURL, machinePubKey.HexString()) + u := fmt.Sprintf("%s/machine/%s/map", serverURL, machinePubKey.UntypedHexString()) req, err := http.NewRequestWithContext(ctx, "POST", u, bytes.NewReader(bodyData)) if err != nil { @@ -734,7 +734,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm vlogf("netmap: read body after %v", time.Since(t0).Round(time.Millisecond)) var resp tailcfg.MapResponse - if err := c.decodeMsg(msg, &resp, &machinePrivKey); err != nil { + if err := c.decodeMsg(msg, &resp, machinePrivKey); err != nil { vlogf("netmap: decode error: %v") return err } @@ -830,7 +830,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm return nil } -func decode(res *http.Response, v interface{}, serverKey *wgkey.Key, mkey *wgkey.Private) error { +func decode(res *http.Response, v interface{}, serverKey key.MachinePublic, mkey key.MachinePrivate) error { defer res.Body.Close() msg, err := ioutil.ReadAll(io.LimitReader(res.Body, 1<<20)) if err != nil { @@ -849,14 +849,14 @@ var ( var jsonEscapedZero = []byte(`\u0000`) -func (c *Direct) decodeMsg(msg []byte, v interface{}, machinePrivKey *wgkey.Private) error { +func (c *Direct) decodeMsg(msg []byte, v interface{}, machinePrivKey key.MachinePrivate) error { c.mu.Lock() serverKey := c.serverKey c.mu.Unlock() - decrypted, err := decryptMsg(msg, &serverKey, machinePrivKey) - if err != nil { - return err + decrypted, ok := machinePrivKey.OpenFrom(serverKey, msg) + if !ok { + return errors.New("cannot decrypt response") } var b []byte if c.newDecompressor == nil { @@ -888,10 +888,10 @@ func (c *Direct) decodeMsg(msg []byte, v interface{}, machinePrivKey *wgkey.Priv } -func decodeMsg(msg []byte, v interface{}, serverKey *wgkey.Key, machinePrivKey *wgkey.Private) error { - decrypted, err := decryptMsg(msg, serverKey, machinePrivKey) - if err != nil { - return err +func decodeMsg(msg []byte, v interface{}, serverKey key.MachinePublic, machinePrivKey key.MachinePrivate) error { + decrypted, ok := machinePrivKey.OpenFrom(serverKey, msg) + if !ok { + return errors.New("cannot decrypt response") } if bytes.Contains(decrypted, jsonEscapedZero) { log.Printf("[unexpected] zero byte in controlclient decodeMsg into %T: %q", v, decrypted) @@ -902,23 +902,7 @@ func decodeMsg(msg []byte, v interface{}, serverKey *wgkey.Key, machinePrivKey * return nil } -func decryptMsg(msg []byte, serverKey *wgkey.Key, mkey *wgkey.Private) ([]byte, error) { - var nonce [24]byte - if len(msg) < len(nonce)+1 { - return nil, fmt.Errorf("response missing nonce, len=%d", len(msg)) - } - copy(nonce[:], msg) - msg = msg[len(nonce):] - - pub, pri := (*[32]byte)(serverKey), (*[32]byte)(mkey) - decrypted, ok := box.Open(nil, msg, &nonce, pub, pri) - if !ok { - return nil, fmt.Errorf("cannot decrypt response (len %d + nonce %d = %d)", len(msg), len(nonce), len(msg)+len(nonce)) - } - return decrypted, nil -} - -func encode(v interface{}, serverKey *wgkey.Key, mkey *wgkey.Private) ([]byte, error) { +func encode(v interface{}, serverKey key.MachinePublic, mkey key.MachinePrivate) ([]byte, error) { b, err := json.Marshal(v) if err != nil { return nil, err @@ -928,38 +912,32 @@ func encode(v interface{}, serverKey *wgkey.Key, mkey *wgkey.Private) ([]byte, e log.Printf("MapRequest: %s", b) } } - var nonce [24]byte - if _, err := io.ReadFull(rand.Reader, nonce[:]); err != nil { - panic(err) - } - pub, pri := (*[32]byte)(serverKey), (*[32]byte)(mkey) - msg := box.Seal(nonce[:], b, &nonce, pub, pri) - return msg, nil + return mkey.SealTo(serverKey, b), nil } -func loadServerKey(ctx context.Context, httpc *http.Client, serverURL string) (wgkey.Key, error) { +func loadServerKey(ctx context.Context, httpc *http.Client, serverURL string) (key.MachinePublic, error) { req, err := http.NewRequest("GET", serverURL+"/key", nil) if err != nil { - return wgkey.Key{}, fmt.Errorf("create control key request: %v", err) + return key.MachinePublic{}, fmt.Errorf("create control key request: %v", err) } req = req.WithContext(ctx) res, err := httpc.Do(req) if err != nil { - return wgkey.Key{}, fmt.Errorf("fetch control key: %v", err) + return key.MachinePublic{}, fmt.Errorf("fetch control key: %v", err) } defer res.Body.Close() b, err := ioutil.ReadAll(io.LimitReader(res.Body, 1<<16)) if err != nil { - return wgkey.Key{}, fmt.Errorf("fetch control key response: %v", err) + return key.MachinePublic{}, fmt.Errorf("fetch control key response: %v", err) } if res.StatusCode != 200 { - return wgkey.Key{}, fmt.Errorf("fetch control key: %d: %s", res.StatusCode, string(b)) + return key.MachinePublic{}, fmt.Errorf("fetch control key: %d: %s", res.StatusCode, string(b)) } - key, err := wgkey.ParseHex(string(b)) + k, err := key.ParseMachinePublicUntyped(mem.B(b)) if err != nil { - return wgkey.Key{}, fmt.Errorf("fetch control key: %v", err) + return key.MachinePublic{}, fmt.Errorf("fetch control key: %v", err) } - return key, nil + return k, nil } // Debug contains temporary internal-only debug knobs. @@ -1207,13 +1185,13 @@ func (c *Direct) SetDNS(ctx context.Context, req *tailcfg.SetDNSRequest) error { return errors.New("getMachinePrivKey returned zero key") } - bodyData, err := encode(req, &serverKey, &machinePrivKey) + bodyData, err := encode(req, serverKey, machinePrivKey) if err != nil { return err } body := bytes.NewReader(bodyData) - u := fmt.Sprintf("%s/machine/%s/set-dns", c.serverURL, machinePrivKey.Public().HexString()) + u := fmt.Sprintf("%s/machine/%s/set-dns", c.serverURL, machinePrivKey.Public().UntypedHexString()) hreq, err := http.NewRequestWithContext(ctx, "POST", u, body) if err != nil { return err @@ -1228,7 +1206,7 @@ func (c *Direct) SetDNS(ctx context.Context, req *tailcfg.SetDNSRequest) error { return fmt.Errorf("set-dns response: %v, %.200s", res.Status, strings.TrimSpace(string(msg))) } var setDNSRes struct{} // no fields yet - if err := decode(res, &setDNSRes, &serverKey, &machinePrivKey); err != nil { + if err := decode(res, &setDNSRes, serverKey, machinePrivKey); err != nil { c.logf("error decoding SetDNSResponse with server key %s and machine key %s: %v", serverKey, machinePrivKey.Public(), err) return fmt.Errorf("set-dns-response: %v", err) } diff --git a/control/controlclient/direct_test.go b/control/controlclient/direct_test.go index 49ce695d8..ee0fbf99c 100644 --- a/control/controlclient/direct_test.go +++ b/control/controlclient/direct_test.go @@ -15,7 +15,7 @@ import ( "tailscale.com/hostinfo" "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" - "tailscale.com/types/wgkey" + "tailscale.com/types/key" ) func TestNewDirect(t *testing.T) { @@ -23,15 +23,12 @@ func TestNewDirect(t *testing.T) { ni := tailcfg.NetInfo{LinkType: "wired"} hi.NetInfo = &ni - key, err := wgkey.NewPrivate() - if err != nil { - t.Error(err) - } + k := key.NewMachine() opts := Options{ ServerURL: "https://example.com", Hostinfo: hi, - GetMachinePrivateKey: func() (wgkey.Private, error) { - return key, nil + GetMachinePrivateKey: func() (key.MachinePrivate, error) { + return k, nil }, } c, err := NewDirect(opts) @@ -102,15 +99,12 @@ func TestTsmpPing(t *testing.T) { ni := tailcfg.NetInfo{LinkType: "wired"} hi.NetInfo = &ni - key, err := wgkey.NewPrivate() - if err != nil { - t.Error(err) - } + k := key.NewMachine() opts := Options{ ServerURL: "https://example.com", Hostinfo: hi, - GetMachinePrivateKey: func() (wgkey.Private, error) { - return key, nil + GetMachinePrivateKey: func() (key.MachinePrivate, error) { + return k, nil }, } diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 051c82cac..40d3109e3 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -12,6 +12,7 @@ import ( "inet.af/netaddr" "tailscale.com/tailcfg" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/types/wgkey" @@ -31,7 +32,7 @@ type mapSession struct { privateNodeKey wgkey.Private logf logger.Logf vlogf logger.Logf - machinePubKey tailcfg.MachineKey + machinePubKey key.MachinePublic keepSharerAndUserSplit bool // see Options.KeepSharerAndUserSplit // Fields storing state over the the coards of multiple MapResponses. diff --git a/control/controlclient/sign.go b/control/controlclient/sign.go index 83b35f6f7..846d562dd 100644 --- a/control/controlclient/sign.go +++ b/control/controlclient/sign.go @@ -10,7 +10,7 @@ import ( "fmt" "time" - "tailscale.com/types/wgkey" + "tailscale.com/types/key" ) var ( @@ -20,7 +20,7 @@ var ( // HashRegisterRequest generates the hash required sign or verify a // tailcfg.RegisterRequest with tailcfg.SignatureV1. -func HashRegisterRequest(ts time.Time, serverURL string, deviceCert []byte, serverPubKey, machinePubKey wgkey.Key) []byte { +func HashRegisterRequest(ts time.Time, serverURL string, deviceCert []byte, serverPubKey, machinePubKey key.MachinePublic) []byte { h := crypto.SHA256.New() // hash.Hash.Write never returns an error, so we don't check for one here. diff --git a/control/controlclient/sign_supported.go b/control/controlclient/sign_supported.go index bb6a38f25..7a258ae10 100644 --- a/control/controlclient/sign_supported.go +++ b/control/controlclient/sign_supported.go @@ -21,7 +21,7 @@ import ( "github.com/tailscale/certstore" "tailscale.com/tailcfg" - "tailscale.com/types/wgkey" + "tailscale.com/types/key" "tailscale.com/util/winutil" ) @@ -125,7 +125,7 @@ func findIdentity(subject string, st certstore.Store) (certstore.Identity, []*x5 // using that identity's public key. In addition to the signature, the full // certificate chain is included so that the control server can validate the // certificate from a copy of the root CA's certificate. -func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey wgkey.Key) (err error) { +func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey key.MachinePublic) (err error) { defer func() { if err != nil { err = fmt.Errorf("signRegisterRequest: %w", err) diff --git a/control/controlclient/sign_unsupported.go b/control/controlclient/sign_unsupported.go index af3397ccd..936806557 100644 --- a/control/controlclient/sign_unsupported.go +++ b/control/controlclient/sign_unsupported.go @@ -9,10 +9,10 @@ package controlclient import ( "tailscale.com/tailcfg" - "tailscale.com/types/wgkey" + "tailscale.com/types/key" ) // signRegisterRequest on non-supported platforms always returns errNoCertStore. -func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey wgkey.Key) error { +func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey key.MachinePublic) error { return errNoCertStore } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 35d280667..7c175d37d 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -110,7 +110,7 @@ type LocalBackend struct { userID string // current controlling user ID (for Windows, primarily) prefs *ipn.Prefs inServerMode bool - machinePrivKey wgkey.Private + machinePrivKey key.MachinePrivate state ipn.State capFileSharing bool // whether netMap contains the file sharing capability // hostinfo is mutated in-place while mu is held. @@ -293,7 +293,7 @@ func (b *LocalBackend) Prefs() *ipn.Prefs { defer b.mu.Unlock() p := b.prefs.Clone() if p != nil && p.Persist != nil { - p.Persist.LegacyFrontendPrivateMachineKey = wgkey.Private{} + p.Persist.LegacyFrontendPrivateMachineKey = key.MachinePrivate{} p.Persist.PrivateNodeKey = wgkey.Private{} p.Persist.OldPrivateNodeKey = wgkey.Private{} } @@ -1239,22 +1239,22 @@ func (b *LocalBackend) popBrowserAuthNow() { // For testing lazy machine key generation. var panicOnMachineKeyGeneration, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_PANIC_MACHINE_KEY")) -func (b *LocalBackend) createGetMachinePrivateKeyFunc() func() (wgkey.Private, error) { +func (b *LocalBackend) createGetMachinePrivateKeyFunc() func() (key.MachinePrivate, error) { var cache atomic.Value - return func() (wgkey.Private, error) { + return func() (key.MachinePrivate, error) { if panicOnMachineKeyGeneration { panic("machine key generated") } - if v, ok := cache.Load().(wgkey.Private); ok { + if v, ok := cache.Load().(key.MachinePrivate); ok { return v, nil } b.mu.Lock() defer b.mu.Unlock() - if v, ok := cache.Load().(wgkey.Private); ok { + if v, ok := cache.Load().(key.MachinePrivate); ok { return v, nil } if err := b.initMachineKeyLocked(); err != nil { - return wgkey.Private{}, err + return key.MachinePrivate{}, err } cache.Store(b.machinePrivKey) return b.machinePrivKey, nil @@ -1272,7 +1272,7 @@ func (b *LocalBackend) initMachineKeyLocked() (err error) { return nil } - var legacyMachineKey wgkey.Private + var legacyMachineKey key.MachinePrivate if b.prefs.Persist != nil { legacyMachineKey = b.prefs.Persist.LegacyFrontendPrivateMachineKey } @@ -1285,7 +1285,7 @@ func (b *LocalBackend) initMachineKeyLocked() (err error) { if b.machinePrivKey.IsZero() { return fmt.Errorf("invalid zero key stored in %v key of %v", ipn.MachineKeyStateKey, b.store) } - if !legacyMachineKey.IsZero() && !bytes.Equal(legacyMachineKey[:], b.machinePrivKey[:]) { + if !legacyMachineKey.IsZero() && !legacyMachineKey.Equal(b.machinePrivKey) { b.logf("frontend-provided legacy machine key ignored; used value from server state") } return nil @@ -1306,11 +1306,7 @@ func (b *LocalBackend) initMachineKeyLocked() (err error) { b.machinePrivKey = legacyMachineKey } else { b.logf("generating new machine key") - var err error - b.machinePrivKey, err = wgkey.NewPrivate() - if err != nil { - return fmt.Errorf("initializing new machine key: %w", err) - } + b.machinePrivKey = key.NewMachine() } keyText, _ = b.machinePrivKey.MarshalText() @@ -2604,7 +2600,7 @@ func (b *LocalBackend) OperatorUserID() string { // TestOnlyPublicKeys returns the current machine and node public // keys. Used in tests only to facilitate automated node authorization // in the test harness. -func (b *LocalBackend) TestOnlyPublicKeys() (machineKey tailcfg.MachineKey, nodeKey tailcfg.NodeKey) { +func (b *LocalBackend) TestOnlyPublicKeys() (machineKey key.MachinePublic, nodeKey tailcfg.NodeKey) { b.mu.Lock() prefs := b.prefs machinePrivKey := b.machinePrivKey @@ -2616,7 +2612,7 @@ func (b *LocalBackend) TestOnlyPublicKeys() (machineKey tailcfg.MachineKey, node mk := machinePrivKey.Public() nk := prefs.Persist.PrivateNodeKey.Public() - return tailcfg.MachineKey(mk), tailcfg.NodeKey(nk) + return mk, tailcfg.NodeKey(nk) } func (b *LocalBackend) WaitingFiles() ([]apitype.WaitingFile, error) { diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index c01097b4f..25b63efac 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -17,6 +17,7 @@ import ( "tailscale.com/syncs" "tailscale.com/tailcfg" "tailscale.com/types/empty" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/types/persist" @@ -93,7 +94,7 @@ type mockControl struct { calls []string authBlocked bool persist persist.Persist - machineKey wgkey.Private + machineKey key.MachinePrivate } func newMockControl() *mockControl { diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 0a2c585fe..4e22afd33 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -77,9 +77,6 @@ func (u StableNodeID) IsZero() bool { return u == "" } -// MachineKey is the curve25519 public key for a machine. -type MachineKey [32]byte - // NodeKey is the curve25519 public key for a node. type NodeKey [32]byte @@ -157,7 +154,7 @@ type Node struct { Key NodeKey KeyExpiry time.Time - Machine MachineKey + Machine key.MachinePublic DiscoKey DiscoKey Addresses []netaddr.IPPrefix // IP addresses of this Node directly AllowedIPs []netaddr.IPPrefix // range of IP addresses to route to this node @@ -1078,11 +1075,6 @@ type Debug struct { DisableUPnP opt.Bool `json:",omitempty"` } -func (k MachineKey) String() string { return fmt.Sprintf("mkey:%x", k[:]) } -func (k MachineKey) MarshalText() ([]byte, error) { return keyMarshalText("mkey:", k), nil } -func (k MachineKey) HexString() string { return fmt.Sprintf("%x", k[:]) } -func (k *MachineKey) UnmarshalText(text []byte) error { return keyUnmarshalText(k[:], "mkey:", text) } - func appendKey(base []byte, prefix string, k [32]byte) []byte { ret := append(base, make([]byte, len(prefix)+64)...) buf := ret[len(base):] @@ -1116,9 +1108,6 @@ func (k *NodeKey) UnmarshalText(text []byte) error { return keyUnmarshalText(k[: // IsZero reports whether k is the zero value. func (k NodeKey) IsZero() bool { return k == NodeKey{} } -// IsZero reports whether k is the zero value. -func (k MachineKey) IsZero() bool { return k == MachineKey{} } - func (k DiscoKey) String() string { return fmt.Sprintf("discokey:%x", k[:]) } func (k DiscoKey) MarshalText() ([]byte, error) { return keyMarshalText("discokey:", k), nil } func (k *DiscoKey) UnmarshalText(text []byte) error { return keyUnmarshalText(k[:], "discokey:", text) } diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 16e5f1a64..eb955df15 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -9,6 +9,7 @@ package tailcfg import ( "inet.af/netaddr" "tailscale.com/types/dnstype" + "tailscale.com/types/key" "tailscale.com/types/opt" "tailscale.com/types/structs" "time" @@ -73,7 +74,7 @@ var _NodeNeedsRegeneration = Node(struct { Sharer UserID Key NodeKey KeyExpiry time.Time - Machine MachineKey + Machine key.MachinePublic DiscoKey DiscoKey Addresses []netaddr.IPPrefix AllowedIPs []netaddr.IPPrefix diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index a3c60355d..f5fa60642 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -13,6 +13,7 @@ import ( "time" "inet.af/netaddr" + "tailscale.com/types/key" "tailscale.com/types/wgkey" "tailscale.com/version" ) @@ -213,6 +214,7 @@ func TestNodeEqual(t *testing.T) { return k.Public() } n1 := newPublicKey(t) + m1 := key.NewMachine().Public() now := time.Now() tests := []struct { @@ -290,13 +292,13 @@ func TestNodeEqual(t *testing.T) { true, }, { - &Node{Machine: MachineKey(n1)}, - &Node{Machine: MachineKey(newPublicKey(t))}, + &Node{Machine: m1}, + &Node{Machine: key.NewMachine().Public()}, false, }, { - &Node{Machine: MachineKey(n1)}, - &Node{Machine: MachineKey(n1)}, + &Node{Machine: m1}, + &Node{Machine: m1}, true, }, { @@ -393,14 +395,6 @@ func TestNetInfoFields(t *testing.T) { } } -func TestMachineKeyMarshal(t *testing.T) { - var k1, k2 MachineKey - for i := range k1 { - k1[i] = byte(i) - } - testKey(t, "mkey:", k1, &k2) -} - func TestNodeKeyMarshal(t *testing.T) { var k1, k2 NodeKey for i := range k1 { diff --git a/tstest/integration/testcontrol/testcontrol.go b/tstest/integration/testcontrol/testcontrol.go index 722c3fe66..dd54d2cb1 100644 --- a/tstest/integration/testcontrol/testcontrol.go +++ b/tstest/integration/testcontrol/testcontrol.go @@ -26,14 +26,13 @@ import ( "time" "github.com/klauspost/compress/zstd" - "golang.org/x/crypto/nacl/box" + "go4.org/mem" "inet.af/netaddr" "tailscale.com/net/tsaddr" "tailscale.com/smallzstd" "tailscale.com/tailcfg" "tailscale.com/types/key" "tailscale.com/types/logger" - "tailscale.com/types/wgkey" ) const msgLimit = 1 << 20 // encrypted message length limit @@ -57,8 +56,8 @@ type Server struct { mu sync.Mutex inServeMap int cond *sync.Cond // lazily initialized by condLocked - pubKey wgkey.Key - privKey wgkey.Private + pubKey key.MachinePublic + privKey key.ControlPrivate // not strictly needed vs. MachinePrivate, but handy to test type interactions. nodes map[tailcfg.NodeKey]*tailcfg.Node users map[tailcfg.NodeKey]*tailcfg.User logins map[tailcfg.NodeKey]*tailcfg.Login @@ -199,25 +198,21 @@ func (s *Server) serveUnhandled(w http.ResponseWriter, r *http.Request) { go panic(fmt.Sprintf("testcontrol.Server received unhandled request: %s", got.Bytes())) } -func (s *Server) publicKey() wgkey.Key { +func (s *Server) publicKey() key.MachinePublic { pub, _ := s.keyPair() return pub } -func (s *Server) privateKey() wgkey.Private { +func (s *Server) privateKey() key.ControlPrivate { _, priv := s.keyPair() return priv } -func (s *Server) keyPair() (pub wgkey.Key, priv wgkey.Private) { +func (s *Server) keyPair() (pub key.MachinePublic, priv key.ControlPrivate) { s.mu.Lock() defer s.mu.Unlock() if s.pubKey.IsZero() { - var err error - s.privKey, err = wgkey.NewPrivate() - if err != nil { - go panic(err) // bring down test, even if in http.Handler - } + s.privKey = key.NewControl() s.pubKey = s.privKey.Public() } return s.pubKey, s.privKey @@ -226,7 +221,7 @@ func (s *Server) keyPair() (pub wgkey.Key, priv wgkey.Private) { func (s *Server) serveKey(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/plain") w.WriteHeader(200) - io.WriteString(w, s.publicKey().HexString()) + io.WriteString(w, s.publicKey().UntypedHexString()) } func (s *Server) serveMachine(w http.ResponseWriter, r *http.Request) { @@ -237,12 +232,11 @@ func (s *Server) serveMachine(w http.ResponseWriter, r *http.Request) { mkeyStr = mkeyStr[:i] } - key, err := wgkey.ParseHex(mkeyStr) + mkey, err := key.ParseMachinePublicUntyped(mem.S(mkeyStr)) if err != nil { http.Error(w, "bad machine key hex", 400) return } - mkey := tailcfg.MachineKey(key) if r.Method != "POST" { http.Error(w, "POST required", 400) @@ -281,7 +275,7 @@ func (s *Server) AddFakeNode() { s.nodes = make(map[tailcfg.NodeKey]*tailcfg.Node) } nk := tailcfg.NodeKey(key.NewPrivate().Public()) - mk := tailcfg.MachineKey(key.NewPrivate().Public()) + mk := key.NewMachine().Public() dk := tailcfg.DiscoKey(key.NewPrivate().Public()) id := int64(binary.LittleEndian.Uint64(nk[:])) ip := netaddr.IPv4(nk[0], nk[1], nk[2], nk[3]) @@ -398,7 +392,7 @@ func (s *Server) CompleteAuth(authPathOrURL string) bool { return true } -func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey tailcfg.MachineKey) { +func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key.MachinePublic) { msg, err := ioutil.ReadAll(io.LimitReader(r.Body, msgLimit)) if err != nil { r.Body.Close() @@ -563,7 +557,7 @@ func (s *Server) InServeMap() int { return s.inServeMap } -func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey tailcfg.MachineKey) { +func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey key.MachinePublic) { s.incrInServeMap(1) defer s.incrInServeMap(-1) ctx := r.Context() @@ -741,7 +735,7 @@ func (s *Server) MapResponse(req *tailcfg.MapRequest) (res *tailcfg.MapResponse, return res, nil } -func (s *Server) sendMapMsg(w http.ResponseWriter, mkey tailcfg.MachineKey, compress bool, msg interface{}) error { +func (s *Server) sendMapMsg(w http.ResponseWriter, mkey key.MachinePublic, compress bool, msg interface{}) error { resBytes, err := s.encode(mkey, compress, msg) if err != nil { return err @@ -765,21 +759,12 @@ func (s *Server) sendMapMsg(w http.ResponseWriter, mkey tailcfg.MachineKey, comp return nil } -func (s *Server) decode(mkey tailcfg.MachineKey, msg []byte, v interface{}) error { +func (s *Server) decode(mkey key.MachinePublic, msg []byte, v interface{}) error { if len(msg) == msgLimit { return errors.New("encrypted message too long") } - var nonce [24]byte - if len(msg) < len(nonce)+1 { - return errors.New("missing nonce") - } - copy(nonce[:], msg) - msg = msg[len(nonce):] - - priv := s.privateKey() - pub, pri := (*[32]byte)(&mkey), (*[32]byte)(&priv) - decrypted, ok := box.Open(nil, msg, &nonce, pub, pri) + decrypted, ok := s.privateKey().OpenFrom(mkey, msg) if !ok { return errors.New("can't decrypt request") } @@ -796,7 +781,7 @@ var zstdEncoderPool = &sync.Pool{ }, } -func (s *Server) encode(mkey tailcfg.MachineKey, compress bool, v interface{}) (b []byte, err error) { +func (s *Server) encode(mkey key.MachinePublic, compress bool, v interface{}) (b []byte, err error) { var isBytes bool if b, isBytes = v.([]byte); !isBytes { b, err = json.Marshal(v) @@ -810,14 +795,7 @@ func (s *Server) encode(mkey tailcfg.MachineKey, compress bool, v interface{}) ( encoder.Close() zstdEncoderPool.Put(encoder) } - var nonce [24]byte - if _, err := io.ReadFull(crand.Reader, nonce[:]); err != nil { - panic(err) - } - priv := s.privateKey() - pub, pri := (*[32]byte)(&mkey), (*[32]byte)(&priv) - msgData := box.Seal(nonce[:], b, &nonce, pub, pri) - return msgData, nil + return s.privateKey().SealTo(mkey, b), nil } // filterInvalidIPv6Endpoints removes invalid IPv6 endpoints from eps, diff --git a/types/key/control.go b/types/key/control.go new file mode 100644 index 000000000..cdd18eb4a --- /dev/null +++ b/types/key/control.go @@ -0,0 +1,64 @@ +// Copyright (c) 2021 Tailscale Inc & 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 key + +import "encoding/json" + +// ControlPrivate is a Tailscale control plane private key. +// +// It is functionally equivalent to a MachinePrivate, but serializes +// to JSON as a byte array rather than a typed string, because our +// control plane database stores the key that way. +// +// Deprecated: this type should only be used in Tailscale's control +// plane, where existing database serializations require this +// less-good serialization format to persist. Other control plane +// implementations can use MachinePrivate with no downsides. +type ControlPrivate struct { + mkey MachinePrivate // unexported so we can limit the API surface to only exactly what we need +} + +// NewControl generates and returns a new control plane private key. +func NewControl() ControlPrivate { + return ControlPrivate{NewMachine()} +} + +// IsZero reports whether k is the zero value. +func (k ControlPrivate) IsZero() bool { + return k.mkey.IsZero() +} + +// Public returns the MachinePublic for k. +// Panics if ControlPrivate is zero. +func (k ControlPrivate) Public() MachinePublic { + return k.mkey.Public() +} + +// MarshalJSON implements json.Marshaler. +func (k ControlPrivate) MarshalJSON() ([]byte, error) { + return json.Marshal(k.mkey.k) +} + +// UnmarshalJSON implements json.Unmarshaler. +func (k *ControlPrivate) UnmarshalJSON(bs []byte) error { + return json.Unmarshal(bs, &k.mkey.k) +} + +// SealTo wraps cleartext into a NaCl box (see +// golang.org/x/crypto/nacl) to p, authenticated from k, using a +// random nonce. +// +// The returned ciphertext is a 24-byte nonce concatenated with the +// box value. +func (k ControlPrivate) SealTo(p MachinePublic, cleartext []byte) (ciphertext []byte) { + return k.mkey.SealTo(p, cleartext) +} + +// OpenFrom opens the NaCl box ciphertext, which must be a value +// created by SealTo, and returns the inner cleartext if ciphertext is +// a valid box from p to k. +func (k ControlPrivate) OpenFrom(p MachinePublic, ciphertext []byte) (cleartext []byte, ok bool) { + return k.mkey.OpenFrom(p, ciphertext) +} diff --git a/types/key/control_test.go b/types/key/control_test.go new file mode 100644 index 000000000..771072497 --- /dev/null +++ b/types/key/control_test.go @@ -0,0 +1,39 @@ +// Copyright (c) 2021 Tailscale Inc & 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 key + +import ( + "encoding/json" + "testing" +) + +func TestControlKey(t *testing.T) { + serialized := `{"PrivateKey":[36,132,249,6,73,141,249,49,9,96,49,60,240,217,253,57,3,69,248,64,178,62,121,73,121,88,115,218,130,145,68,254]}` + want := ControlPrivate{ + MachinePrivate{ + k: [32]byte{36, 132, 249, 6, 73, 141, 249, 49, 9, 96, 49, 60, 240, 217, 253, 57, 3, 69, 248, 64, 178, 62, 121, 73, 121, 88, 115, 218, 130, 145, 68, 254}, + }, + } + + var got struct { + PrivateKey ControlPrivate + } + if err := json.Unmarshal([]byte(serialized), &got); err != nil { + t.Fatalf("decoding serialized ControlPrivate: %v", err) + } + + if !got.PrivateKey.mkey.Equal(want.mkey) { + t.Fatalf("Serialized ControlPrivate didn't deserialize as expected, got %v want %v", got.PrivateKey, want) + } + + bs, err := json.Marshal(got) + if err != nil { + t.Fatalf("json reserialization of ControlPrivate failed: %v", err) + } + + if got, want := string(bs), serialized; got != want { + t.Fatalf("ControlPrivate didn't round-trip, got %q want %q", got, want) + } +} diff --git a/types/key/key.go b/types/key/key.go index 890d3fea4..8cba4733a 100644 --- a/types/key/key.go +++ b/types/key/key.go @@ -2,21 +2,27 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Package key defines some types related to curve25519 keys. +// Package key defines some types for the various keys Tailscale uses. package key import ( - crand "crypto/rand" "encoding/base64" "errors" "fmt" - "io" "go4.org/mem" "golang.org/x/crypto/curve25519" ) -// Private represents a curve25519 private key. +// Private represents a curve25519 private key of unspecified purpose. +// +// Deprecated: this key type has been used for several different +// keypairs, which are used in different protocols. This makes it easy +// to accidentally use the wrong key for a particular purpose, because +// the type system doesn't protect you. Please define dedicated key +// types for each purpose (e.g. communication with control, disco, +// wireguard...) instead, even if they are a Curve25519 value under +// the hood. type Private [32]byte // Private reports whether p is the zero value. @@ -25,11 +31,8 @@ func (p Private) IsZero() bool { return p == Private{} } // NewPrivate returns a new private key. func NewPrivate() Private { var p Private - if _, err := io.ReadFull(crand.Reader, p[:]); err != nil { - panic(err) - } - p[0] &= 248 - p[31] = (p[31] & 127) | 64 + rand(p[:]) + clamp25519Private(p[:]) return p } @@ -39,6 +42,14 @@ func NewPrivate() Private { func (k Private) B32() *[32]byte { return (*[32]byte)(&k) } // Public represents a curve25519 public key. +// +// Deprecated: this key type has been used for several different +// keypairs, which are used in different protocols. This makes it easy +// to accidentally use the wrong key for a particular purpose, because +// the type system doesn't protect you. Please define dedicated key +// types for each purpose (e.g. communication with control, disco, +// wireguard...) instead, even if they are a Curve25519 value under +// the hood. type Public [32]byte // Public reports whether p is the zero value. @@ -106,17 +117,3 @@ func NewPublicFromHexMem(m mem.RO) (Public, error) { } return p, nil } - -// fromHexChar converts a hex character into its value and a success flag. -func fromHexChar(c byte) (byte, bool) { - switch { - case '0' <= c && c <= '9': - return c - '0', true - case 'a' <= c && c <= 'f': - return c - 'a' + 10, true - case 'A' <= c && c <= 'F': - return c - 'A' + 10, true - } - - return 0, false -} diff --git a/types/key/key_test.go b/types/key/key_test.go index 7a2155ef0..e8ecf689e 100644 --- a/types/key/key_test.go +++ b/types/key/key_test.go @@ -5,50 +5,70 @@ package key import ( + "bytes" + "encoding" + "reflect" "testing" - - "tailscale.com/types/wgkey" ) -func TestTextUnmarshal(t *testing.T) { - p := Public{1, 2} - text, err := p.MarshalText() - if err != nil { - t.Fatal(err) - } - var p2 Public - if err := p2.UnmarshalText(text); err != nil { - t.Fatal(err) - } - if p != p2 { - t.Fatalf("mismatch; got %x want %x", p2, p) - } +type tmu interface { + encoding.TextMarshaler + encoding.TextUnmarshaler } -func TestClamping(t *testing.T) { - t.Run("NewPrivate", func(t *testing.T) { testClamping(t, NewPrivate) }) +func TestTextMarshal(t *testing.T) { + // Check that keys roundtrip correctly through marshaling, and + // cannot be unmarshaled as other key types. + type keyMaker func() (random, zero tmu) + keys := []keyMaker{ + func() (tmu, tmu) { k := NewMachine(); return &k, &MachinePrivate{} }, + func() (tmu, tmu) { k := NewMachine().Public(); return &k, &MachinePublic{} }, + func() (tmu, tmu) { k := NewPrivate().Public(); return &k, &Public{} }, + } + for i, kf := range keys { + k1, k2 := kf() + // Sanity check: both k's should have the same type, k2 should + // be the zero value. + if t1, t2 := reflect.ValueOf(k1).Elem().Type(), reflect.ValueOf(k2).Elem().Type(); t1 != t2 { + t.Fatalf("got two keys of different types %T and %T", t1, t2) + } + if !reflect.ValueOf(k2).Elem().IsZero() { + t.Fatal("k2 is not the zero value") + } - // Also test the wgkey package, as their behavior should match. - t.Run("wgkey", func(t *testing.T) { - testClamping(t, func() Private { - k, err := wgkey.NewPrivate() - if err != nil { - t.Fatal(err) - } - return Private(k) - }) - }) -} + // All keys should marshal successfully. + t1, err := k1.MarshalText() + if err != nil { + t.Fatalf("MarshalText(%#v): %v", k1, err) + } + + // Marshalling should round-trip. + if err := k2.UnmarshalText(t1); err != nil { + t.Fatalf("UnmarshalText(MarshalText(%#v)): %v", k1, err) + } + if !reflect.DeepEqual(k1, k2) { + t.Fatalf("UnmarshalText(MarshalText(k1)) changed\n old: %#v\n new: %#v", k1, k2) + } -func testClamping(t *testing.T, newKey func() Private) { - for i := 0; i < 100; i++ { - k := newKey() - if k[0]&0b111 != 0 { - t.Fatalf("Bogus clamping in first byte: %#08b", k[0]) - return + // And the text representation should also roundtrip. + t2, err := k2.MarshalText() + if err != nil { + t.Fatalf("MarshalText(%#v): %v", k2, err) } - if k[31]>>6 != 1 { - t.Fatalf("Bogus clamping in last byte: %#08b", k[0]) + if !bytes.Equal(t1, t2) { + t.Fatal("MarshalText(k1) != MarshalText(k2)") + } + + // No other key type should be able to unmarshal the text of a + // different key. + for j, otherkf := range keys { + if i == j { + continue + } + _, otherk := otherkf() + if err := otherk.UnmarshalText(t1); err == nil { + t.Fatalf("key %#v can unmarshal as %#v (marshaled form %q)", k1, otherk, t1) + } } } } diff --git a/types/key/machine.go b/types/key/machine.go new file mode 100644 index 000000000..ca294cde3 --- /dev/null +++ b/types/key/machine.go @@ -0,0 +1,173 @@ +// Copyright (c) 2021 Tailscale Inc & 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 key + +import ( + "crypto/subtle" + "encoding/hex" + + "go4.org/mem" + "golang.org/x/crypto/curve25519" + "golang.org/x/crypto/nacl/box" + "tailscale.com/types/structs" +) + +const ( + // machinePrivateHexPrefix is the prefix used to identify a + // hex-encoded machine private key. + // + // This prefix name is a little unfortunate, in that it comes from + // WireGuard's own key types. Unfortunately we're stuck with it for + // machine keys, because we serialize them to disk with this prefix. + machinePrivateHexPrefix = "privkey:" + + // machinePublicHexPrefix is the prefix used to identify a + // hex-encoded machine public key. + // + // This prefix is used in the control protocol, so cannot be + // changed. + machinePublicHexPrefix = "mkey:" +) + +// MachinePrivate is a machine key, used for communication with the +// Tailscale coordination server. +type MachinePrivate struct { + _ structs.Incomparable // == isn't constant-time + k [32]byte +} + +// NewMachine creates and returns a new machine private key. +func NewMachine() MachinePrivate { + var ret MachinePrivate + rand(ret.k[:]) + clamp25519Private(ret.k[:]) + return ret +} + +// IsZero reports whether k is the zero value. +func (k MachinePrivate) IsZero() bool { + return k.Equal(MachinePrivate{}) +} + +// Equal reports whether k and other are the same key. +func (k MachinePrivate) Equal(other MachinePrivate) bool { + return subtle.ConstantTimeCompare(k.k[:], other.k[:]) == 1 +} + +// Public returns the MachinePublic for k. +// Panics if MachinePrivate is zero. +func (k MachinePrivate) Public() MachinePublic { + if k.IsZero() { + panic("can't take the public key of a zero MachinePrivate") + } + var ret MachinePublic + curve25519.ScalarBaseMult(&ret.k, &k.k) + return ret +} + +// MarshalText implements encoding.TextMarshaler. +func (k MachinePrivate) MarshalText() ([]byte, error) { + return toHex(k.k[:], machinePrivateHexPrefix), nil +} + +// MarshalText implements encoding.TextUnmarshaler. +func (k *MachinePrivate) UnmarshalText(b []byte) error { + return parseHex(k.k[:], mem.B(b), mem.S(machinePrivateHexPrefix)) +} + +// SealTo wraps cleartext into a NaCl box (see +// golang.org/x/crypto/nacl) to p, authenticated from k, using a +// random nonce. +// +// The returned ciphertext is a 24-byte nonce concatenated with the +// box value. +func (k MachinePrivate) SealTo(p MachinePublic, cleartext []byte) (ciphertext []byte) { + if k.IsZero() || p.IsZero() { + panic("can't seal with zero keys") + } + var nonce [24]byte + rand(nonce[:]) + return box.Seal(nonce[:], cleartext, &nonce, &p.k, &k.k) +} + +// OpenFrom opens the NaCl box ciphertext, which must be a value +// created by SealTo, and returns the inner cleartext if ciphertext is +// a valid box from p to k. +func (k MachinePrivate) OpenFrom(p MachinePublic, ciphertext []byte) (cleartext []byte, ok bool) { + if k.IsZero() || p.IsZero() { + panic("can't open with zero keys") + } + if len(ciphertext) < 24 { + return nil, false + } + var nonce [24]byte + copy(nonce[:], ciphertext) + return box.Open(nil, ciphertext[len(nonce):], &nonce, &p.k, &k.k) +} + +// MachinePublic is the public portion of a a MachinePrivate. +type MachinePublic struct { + k [32]byte +} + +// ParseMachinePublicUntyped parses an untyped 64-character hex value +// as a MachinePublic. +// +// Deprecated: this function is risky to use, because it cannot verify +// that the hex string was intended to be a MachinePublic. This can +// lead to accidentally decoding one type of key as another. For new +// uses that don't require backwards compatibility with the untyped +// string format, please use MarshalText/UnmarshalText. +func ParseMachinePublicUntyped(raw mem.RO) (MachinePublic, error) { + var ret MachinePublic + if err := parseHex(ret.k[:], raw, mem.B(nil)); err != nil { + return MachinePublic{}, err + } + return ret, nil +} + +// IsZero reports whether k is the zero value. +func (k MachinePublic) IsZero() bool { + return k == MachinePublic{} +} + +// ShortString returns the Tailscale conventional debug representation +// of a public key: the first five base64 digits of the key, in square +// brackets. +func (k MachinePublic) ShortString() string { + return debug32(k.k) +} + +// UntypedHexString returns k, encoded as an untyped 64-character hex +// string. +// +// Deprecated: this function is risky to use, because it produces +// serialized values that do not identify themselves as a +// MachinePublic, allowing other code to potentially parse it back in +// as the wrong key type. For new uses that don't require backwards +// compatibility with the untyped string format, please use +// MarshalText/UnmarshalText. +func (k MachinePublic) UntypedHexString() string { + return hex.EncodeToString(k.k[:]) +} + +// String returns the output of MarshalText as a string. +func (k MachinePublic) String() string { + bs, err := k.MarshalText() + if err != nil { + panic(err) + } + return string(bs) +} + +// MarshalText implements encoding.TextMarshaler. +func (k MachinePublic) MarshalText() ([]byte, error) { + return toHex(k.k[:], machinePublicHexPrefix), nil +} + +// MarshalText implements encoding.TextUnmarshaler. +func (k *MachinePublic) UnmarshalText(b []byte) error { + return parseHex(k.k[:], mem.B(b), mem.S(machinePublicHexPrefix)) +} diff --git a/types/key/machine_test.go b/types/key/machine_test.go new file mode 100644 index 000000000..88b73fb49 --- /dev/null +++ b/types/key/machine_test.go @@ -0,0 +1,92 @@ +// Copyright (c) 2021 Tailscale Inc & 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 key + +import ( + "bytes" + "encoding/json" + "strings" + "testing" +) + +func TestMachineKey(t *testing.T) { + k := NewMachine() + if k.IsZero() { + t.Fatal("MachinePrivate should not be zero") + } + + p := k.Public() + if p.IsZero() { + t.Fatal("MachinePublic should not be zero") + } + + bs, err := p.MarshalText() + if err != nil { + t.Fatal(err) + } + if full, got := string(bs), ":"+p.UntypedHexString(); !strings.HasSuffix(full, got) { + t.Fatalf("MachinePublic.UntypedHexString is not a suffix of the typed serialization, got %q want suffix of %q", got, full) + } + + z := MachinePublic{} + if !z.IsZero() { + t.Fatal("IsZero(MachinePublic{}) is false") + } + if s := z.ShortString(); s != "" { + t.Fatalf("MachinePublic{}.ShortString() is %q, want \"\"", s) + } +} + +func TestMachineSerialization(t *testing.T) { + serialized := `{ + "Priv": "privkey:40ab1b58e9076c7a4d9d07291f5edf9d1aa017eb949624ba683317f48a640369", + "Pub":"mkey:50d20b455ecf12bc453f83c2cfdb2a24925d06cf2598dcaa54e91af82ce9f765" + }` + + // Carefully check that the expected serialized data decodes and + // reencodes to the expected keys. These types are serialized to + // disk all over the place and need to be stable. + priv := MachinePrivate{ + k: [32]uint8{ + 0x40, 0xab, 0x1b, 0x58, 0xe9, 0x7, 0x6c, 0x7a, 0x4d, 0x9d, 0x7, + 0x29, 0x1f, 0x5e, 0xdf, 0x9d, 0x1a, 0xa0, 0x17, 0xeb, 0x94, + 0x96, 0x24, 0xba, 0x68, 0x33, 0x17, 0xf4, 0x8a, 0x64, 0x3, 0x69, + }, + } + pub := MachinePublic{ + k: [32]uint8{ + 0x50, 0xd2, 0xb, 0x45, 0x5e, 0xcf, 0x12, 0xbc, 0x45, 0x3f, 0x83, + 0xc2, 0xcf, 0xdb, 0x2a, 0x24, 0x92, 0x5d, 0x6, 0xcf, 0x25, 0x98, + 0xdc, 0xaa, 0x54, 0xe9, 0x1a, 0xf8, 0x2c, 0xe9, 0xf7, 0x65, + }, + } + + type keypair struct { + Priv MachinePrivate + Pub MachinePublic + } + + var a keypair + if err := json.Unmarshal([]byte(serialized), &a); err != nil { + t.Fatal(err) + } + if !a.Priv.Equal(priv) { + t.Errorf("wrong deserialization of private key, got %#v want %#v", a.Priv, priv) + } + if a.Pub != pub { + t.Errorf("wrong deserialization of public key, got %#v want %#v", a.Pub, pub) + } + + bs, err := json.MarshalIndent(a, "", " ") + if err != nil { + t.Fatal(err) + } + + var b bytes.Buffer + json.Indent(&b, []byte(serialized), "", " ") + if got, want := string(bs), b.String(); got != want { + t.Error("json serialization doesn't roundtrip") + } +} diff --git a/types/key/util.go b/types/key/util.go new file mode 100644 index 000000000..c5a792fb6 --- /dev/null +++ b/types/key/util.go @@ -0,0 +1,111 @@ +// Copyright (c) 2021 Tailscale Inc & 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 key + +import ( + crand "crypto/rand" + "encoding/base64" + "encoding/hex" + "errors" + "fmt" + "io" + + "go4.org/mem" +) + +// rand fills b with cryptographically strong random bytes. Panics if +// no random bytes are available. +func rand(b []byte) { + if _, err := io.ReadFull(crand.Reader, b[:]); err != nil { + panic(fmt.Sprintf("unable to read random bytes from OS: %v", err)) + } +} + +// clamp25519 clamps b, which must be a 32-byte Curve25519 private +// key, to a safe value. +// +// The clamping effectively constrains the key to a number between +// 2^251 and 2^252-1, which is then multiplied by 8 (the cofactor of +// Curve25519). This produces a value that doesn't have any unsafe +// properties when doing operations like ScalarMult. +// +// See +// https://web.archive.org/web/20210228105330/https://neilmadden.blog/2020/05/28/whats-the-curve25519-clamping-all-about/ +// for a more in-depth explanation of the constraints that led to this +// clamping requirement. +// +// PLEASE NOTE that not all Curve25519 values require clamping. When +// implementing a new key type that uses Curve25519, you must evaluate +// whether that particular key's use requires clamping. Here are some +// existing uses and whether you should clamp private keys at +// creation. +// +// - NaCl box: yes, clamp at creation. +// - WireGuard (userspace uapi or kernel): no, do not clamp. +// - Noise protocols: no, do not clamp. +func clamp25519Private(b []byte) { + b[0] &= 248 + b[31] = (b[31] & 127) | 64 +} + +func toHex(k []byte, prefix string) []byte { + ret := make([]byte, len(prefix)+len(k)*2) + copy(ret, prefix) + hex.Encode(ret[len(prefix):], k) + return ret +} + +// parseHex decodes a key string of the form "" +// into out. The prefix must match, and the decoded base64 must fit +// exactly into out. +// +// Note the errors in this function deliberately do not echo the +// contents of in, because it might be a private key or part of a +// private key. +func parseHex(out []byte, in, prefix mem.RO) error { + if !mem.HasPrefix(in, prefix) { + return fmt.Errorf("key hex string doesn't have expected type prefix %s", prefix.StringCopy()) + } + in = in.SliceFrom(prefix.Len()) + if want := len(out) * 2; in.Len() != want { + return fmt.Errorf("key hex has the wrong size, got %d want %d", in.Len(), want) + } + for i := range out { + a, ok1 := fromHexChar(in.At(i*2 + 0)) + b, ok2 := fromHexChar(in.At(i*2 + 1)) + if !ok1 || !ok2 { + return errors.New("invalid hex character in key") + } + out[i] = (a << 4) | b + } + return nil +} + +// fromHexChar converts a hex character into its value and a success flag. +func fromHexChar(c byte) (byte, bool) { + switch { + case '0' <= c && c <= '9': + return c - '0', true + case 'a' <= c && c <= 'f': + return c - 'a' + 10, true + case 'A' <= c && c <= 'F': + return c - 'A' + 10, true + } + + return 0, false +} + +// debug32 returns the Tailscale conventional debug representation of +// a key: the first five base64 digits of the key, in square brackets. +func debug32(k [32]byte) string { + if k == [32]byte{} { + return "" + } + var b [45]byte // 32 bytes expands to 44 bytes in base64, plus 1 for the leading '[' + base64.StdEncoding.Encode(b[1:], k[:]) + b[0] = '[' + b[6] = ']' + return string(b[:7]) +} diff --git a/types/key/util_test.go b/types/key/util_test.go new file mode 100644 index 000000000..dff52b50c --- /dev/null +++ b/types/key/util_test.go @@ -0,0 +1,38 @@ +// Copyright (c) 2021 Tailscale Inc & 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 key + +import ( + "bytes" + "testing" +) + +func TestRand(t *testing.T) { + var bs [32]byte + rand(bs[:]) + if bs == [32]byte{} { + t.Fatal("rand didn't provide randomness") + } + var bs2 [32]byte + rand(bs2[:]) + if bytes.Equal(bs[:], bs2[:]) { + t.Fatal("rand returned the same data twice") + } +} + +func TestClamp25519Private(t *testing.T) { + for i := 0; i < 100; i++ { + var k [32]byte + rand(k[:]) + clamp25519Private(k[:]) + if k[0]&0b111 != 0 { + t.Fatalf("Bogus clamping in first byte: %#08b", k[0]) + return + } + if k[31]>>6 != 1 { + t.Fatalf("Bogus clamping in last byte: %#08b", k[0]) + } + } +} diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 22800eb58..556ed5d06 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -14,6 +14,7 @@ import ( "inet.af/netaddr" "tailscale.com/tailcfg" + "tailscale.com/types/key" "tailscale.com/types/wgkey" "tailscale.com/wgengine/filter" ) @@ -34,7 +35,7 @@ type NetworkMap struct { Addresses []netaddr.IPPrefix // same as tailcfg.Node.Addresses (IP addresses of this Node directly) LocalPort uint16 // used for debugging MachineStatus tailcfg.MachineStatus - MachineKey tailcfg.MachineKey + MachineKey key.MachinePublic Peers []*tailcfg.Node // sorted by Node.ID DNS tailcfg.DNSConfig Hostinfo tailcfg.Hostinfo diff --git a/types/persist/persist.go b/types/persist/persist.go index 169288280..3bf1bae9b 100644 --- a/types/persist/persist.go +++ b/types/persist/persist.go @@ -8,6 +8,7 @@ package persist import ( "fmt" + "tailscale.com/types/key" "tailscale.com/types/structs" "tailscale.com/types/wgkey" ) @@ -28,7 +29,7 @@ type Persist struct { // needed. This field should be considered read-only from GUI // frontends. The real value should not be written back in // this field, lest the frontend persist it to disk. - LegacyFrontendPrivateMachineKey wgkey.Private `json:"PrivateMachineKey"` + LegacyFrontendPrivateMachineKey key.MachinePrivate `json:"PrivateMachineKey"` PrivateNodeKey wgkey.Private OldPrivateNodeKey wgkey.Private // needed to request key rotation @@ -52,7 +53,10 @@ func (p *Persist) Equals(p2 *Persist) bool { } func (p *Persist) Pretty() string { - var mk, ok, nk wgkey.Key + var ( + mk key.MachinePublic + ok, nk wgkey.Key + ) if !p.LegacyFrontendPrivateMachineKey.IsZero() { mk = p.LegacyFrontendPrivateMachineKey.Public() } @@ -69,5 +73,5 @@ func (p *Persist) Pretty() string { return k.ShortString() } return fmt.Sprintf("Persist{lm=%v, o=%v, n=%v u=%#v}", - ss(mk), ss(ok), ss(nk), p.LoginName) + mk.ShortString(), ss(ok), ss(nk), p.LoginName) } diff --git a/types/persist/persist_clone.go b/types/persist/persist_clone.go index 533e9294d..cb91b76fa 100644 --- a/types/persist/persist_clone.go +++ b/types/persist/persist_clone.go @@ -7,6 +7,7 @@ package persist import ( + "tailscale.com/types/key" "tailscale.com/types/structs" "tailscale.com/types/wgkey" ) @@ -26,7 +27,7 @@ func (src *Persist) Clone() *Persist { // tailscale.com/cmd/cloner -type Persist var _PersistNeedsRegeneration = Persist(struct { _ structs.Incomparable - LegacyFrontendPrivateMachineKey wgkey.Private + LegacyFrontendPrivateMachineKey key.MachinePrivate PrivateNodeKey wgkey.Private OldPrivateNodeKey wgkey.Private Provider string diff --git a/types/persist/persist_test.go b/types/persist/persist_test.go index 04fdb8bc3..ce91460bb 100644 --- a/types/persist/persist_test.go +++ b/types/persist/persist_test.go @@ -8,6 +8,7 @@ import ( "reflect" "testing" + "tailscale.com/types/key" "tailscale.com/types/wgkey" ) @@ -34,6 +35,7 @@ func TestPersistEqual(t *testing.T) { } return k } + m1 := key.NewMachine() k1 := newPrivate() tests := []struct { a, b *Persist @@ -45,13 +47,13 @@ func TestPersistEqual(t *testing.T) { {&Persist{}, &Persist{}, true}, { - &Persist{LegacyFrontendPrivateMachineKey: k1}, - &Persist{LegacyFrontendPrivateMachineKey: newPrivate()}, + &Persist{LegacyFrontendPrivateMachineKey: m1}, + &Persist{LegacyFrontendPrivateMachineKey: key.NewMachine()}, false, }, { - &Persist{LegacyFrontendPrivateMachineKey: k1}, - &Persist{LegacyFrontendPrivateMachineKey: k1}, + &Persist{LegacyFrontendPrivateMachineKey: m1}, + &Persist{LegacyFrontendPrivateMachineKey: m1}, true, },