From d5f8f38ac629beb0c0ae4f63582be4de60828041 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 6 Mar 2022 09:32:52 -0800 Subject: [PATCH] tailcfg: rename map request version to "capability version" And add a CapabilityVersion type, primarily for documentation. This makes MapRequest.Version, RegisterRequest.Version, and SetDNSRequest.Version all use the same version, which will avoid confusing in the future if Register or SetDNS ever changed their semantics on Version change. (Currently they're both always 1) This will requre a control server change to allow a SetDNSRequest.Version value other than 1 to be deployed first. Change-Id: I073042a216e0d745f52ee2dbc45cf336b9f84b7c Signed-off-by: Brad Fitzpatrick --- control/controlclient/direct.go | 4 +- ipn/ipnlocal/local.go | 2 +- tailcfg/tailcfg.go | 43 ++++++++++++++----- tstest/integration/testcontrol/testcontrol.go | 7 ++- 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 320f688bb..8becba10b 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -368,7 +368,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new } now := time.Now().Round(time.Second) request := tailcfg.RegisterRequest{ - Version: 1, + Version: 1, // TODO(bradfitz): use tailcfg.CurrentCapabilityVersion when over Noise OldNodeKey: oldNodeKey, NodeKey: tryingNewKey.Public(), Hostinfo: hi, @@ -614,7 +614,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm } request := &tailcfg.MapRequest{ - Version: tailcfg.CurrentMapRequestVersion, + Version: tailcfg.CurrentCapabilityVersion, KeepAlive: c.keepAlive, NodeKey: persist.PrivateNodeKey.Public(), DiscoKey: c.discoPubKey, diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 82f7d717b..2a263f7c0 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2930,7 +2930,7 @@ func (b *LocalBackend) FileTargets() ([]*apitype.FileTarget, error) { // friendly options to get HTTPS certs. func (b *LocalBackend) SetDNS(ctx context.Context, name, value string) error { req := &tailcfg.SetDNSRequest{ - Version: 1, + Version: 1, // TODO(bradfitz,maisem): use tailcfg.CurrentCapabilityVersion when using the Noise transport Type: "TXT", Name: name, Value: value, diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 939a28142..85b6c8651 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -24,7 +24,19 @@ import ( "tailscale.com/util/dnsname" ) -// CurrentMapRequestVersion is the current MapRequest.Version value. +// CapabilityVersion represents the client's capability level. That +// is, it can be thought of as the client's simple version number: a +// single monotonically increasing integer, rather than the relatively +// complex x.y.z-xxxxx semver+hash(es). Whenever the client gains a +// capability or wants to negotiate a change in semantics with the +// server (control plane), bump this number and document what's new. +// +// Previously (prior to 2022-03-06), it was known as the "MapRequest +// version" or "mapVer" or "map cap" and that name and usage persists +// in places. +type CapabilityVersion int + +// CurrentCapabilityVersion is the current capability version of the codebase. // // History of versions: // 3: implicit compression, keep-alives @@ -52,7 +64,7 @@ import ( // 25: 2021-11-01: MapResponse.Debug.Exit // 26: 2022-01-12: (nothing, just bumping for 1.20.0) // 27: 2022-02-18: start of SSHPolicy being respected -const CurrentMapRequestVersion = 27 +const CurrentCapabilityVersion CapabilityVersion = 27 type StableID string @@ -846,8 +858,15 @@ func (st SignatureType) String() string { // using the local machine key, and sent to: // https://login.tailscale.com/machine/ type RegisterRequest struct { - _ structs.Incomparable - Version int // currently 1 + _ structs.Incomparable + + // Version is the client's capabilities when using the Noise + // transport. + // + // When using the original nacl crypto_box transport, the + // value must be 1. + Version CapabilityVersion + NodeKey key.NodePublic OldNodeKey key.NodePublic Auth struct { @@ -961,8 +980,8 @@ type MapRequest struct { // we want to signal to the control server that we're capable of something // different. // - // For current values and history, see CurrentMapRequestVersion above. - Version int + // For current values and history, see the CapabilityVersion type's docs. + Version CapabilityVersion Compress string // "zstd" or "" (no compression) KeepAlive bool // whether server should send keep-alives back to us @@ -1046,7 +1065,7 @@ type FilterRule struct { SrcIPs []string // SrcBits is deprecated; it's the old way to specify a CIDR - // prior to MapRequest.Version 7. Its values correspond to the + // prior to CapabilityVersion 7. Its values correspond to the // SrcIPs above. // // If an entry of SrcBits is present for the same index as a @@ -1491,10 +1510,12 @@ const ( // using the local machine key, and sent to: // https://login.tailscale.com/machine//set-dns type SetDNSRequest struct { - // Version indicates what level of SetDNSRequest functionality - // the client understands. Currently this type only has - // one version; this field should always be 1 for now. - Version int + // Version is the client's capabilities + // (CurrentCapabilityVersion) when using the Noise transport. + // + // When using the original nacl crypto_box transport, the + // value must be 1. + Version CapabilityVersion // NodeKey is the client's current node key. NodeKey key.NodePublic diff --git a/tstest/integration/testcontrol/testcontrol.go b/tstest/integration/testcontrol/testcontrol.go index 71dc3f593..4f481c5d0 100644 --- a/tstest/integration/testcontrol/testcontrol.go +++ b/tstest/integration/testcontrol/testcontrol.go @@ -408,19 +408,18 @@ func (s *Server) CompleteAuth(authPathOrURL string) bool { func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key.MachinePublic) { msg, err := ioutil.ReadAll(io.LimitReader(r.Body, msgLimit)) + r.Body.Close() if err != nil { - r.Body.Close() http.Error(w, fmt.Sprintf("bad map request read: %v", err), 400) return } - r.Body.Close() var req tailcfg.RegisterRequest if err := s.decode(mkey, msg, &req); err != nil { go panic(fmt.Sprintf("serveRegister: decode: %v", err)) } - if req.Version != 1 { - go panic(fmt.Sprintf("serveRegister: unsupported version: %d", req.Version)) + if req.Version == 0 { + panic("serveRegister: zero Version") } if req.NodeKey.IsZero() { go panic("serveRegister: request has zero node key")