types/tkatype: change MarshaledSignature from []byte to string

The MarshaledSignature's underlying type of []byte didn't play nicely
with cmd/viewer, which mapped the []byte to a mem.RO handle which was
then clumsy to work with.

Instead, change it to a string. (But keep its JSON encoding for
compatibility)

Updates #1909

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

control/controlclient/direct.go

@@ -443,10 +443,10 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 
 	machinePrivKey, err := c.getMachinePrivKey()
 	if err != nil {
-		return false, "", nil, fmt.Errorf("getMachinePrivKey: %w", err)
+		return false, "", "", fmt.Errorf("getMachinePrivKey: %w", err)
 	}
 	if machinePrivKey.IsZero() {
-		return false, "", nil, errors.New("getMachinePrivKey returned zero key")
+		return false, "", "", errors.New("getMachinePrivKey returned zero key")
 	}
 
 	regen := opt.Regen
@@ -468,7 +468,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 	if serverKey.IsZero() {
 		keys, err := loadServerPubKeys(ctx, c.httpc, c.serverURL)
 		if err != nil {
-			return regen, opt.URL, nil, err
+			return regen, opt.URL, "", err
 		}
 		c.logf("control server key from %s: ts2021=%s, legacy=%v", c.serverURL, keys.PublicKey.ShortString(), keys.LegacyPublicKey.ShortString())
 
@@ -511,13 +511,13 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 
 	if tryingNewKey.IsZero() {
 		if opt.Logout {
-			return false, "", nil, errors.New("no nodekey to log out")
+			return false, "", "", errors.New("no nodekey to log out")
 		}
 		log.Fatalf("tryingNewKey is empty, give up")
 	}
 
 	var nodeKeySignature tkatype.MarshaledSignature
-	if !oldNodeKey.IsZero() && opt.OldNodeKeySignature != nil {
+	if !oldNodeKey.IsZero() && opt.OldNodeKeySignature != "" {
 		if nodeKeySignature, err = resignNKS(persist.NetworkLockKey, tryingNewKey.Public(), opt.OldNodeKeySignature); err != nil {
 			c.logf("Failed re-signing node-key signature: %v", err)
 		}
@@ -527,7 +527,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 		// generate a tailnet-lock signature.
 		nk, err := tryingNewKey.Public().MarshalBinary()
 		if err != nil {
-			return false, "", nil, fmt.Errorf("marshalling node-key: %w", err)
+			return false, "", "", fmt.Errorf("marshalling node-key: %w", err)
 		}
 		sig := &tka.NodeKeySignature{
 			SigKind: tka.SigRotation,
@@ -541,7 +541,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 
 	if backendLogID == "" {
 		err = errors.New("hostinfo: BackendLogID missing")
-		return regen, opt.URL, nil, err
+		return regen, opt.URL, "", err
 	}
 	now := c.clock.Now().Round(time.Second)
 	request := tailcfg.RegisterRequest{
@@ -596,33 +596,33 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 		request.Version = tailcfg.CurrentCapabilityVersion
 		httpc, err = c.getNoiseClient()
 		if err != nil {
-			return regen, opt.URL, nil, fmt.Errorf("getNoiseClient: %w", err)
+			return regen, opt.URL, "", fmt.Errorf("getNoiseClient: %w", err)
 		}
 		url = fmt.Sprintf("%s/machine/register", c.serverURL)
 		url = strings.Replace(url, "http:", "https:", 1)
 	}
 	bodyData, err := encode(request, serverKey, serverNoiseKey, machinePrivKey)
 	if err != nil {
-		return regen, opt.URL, nil, err
+		return regen, opt.URL, "", err
 	}
 	req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(bodyData))
 	if err != nil {
-		return regen, opt.URL, nil, err
+		return regen, opt.URL, "", err
 	}
 	res, err := httpc.Do(req)
 	if err != nil {
-		return regen, opt.URL, nil, fmt.Errorf("register request: %w", err)
+		return regen, opt.URL, "", fmt.Errorf("register request: %w", err)
 	}
 	if res.StatusCode != 200 {
 		msg, _ := io.ReadAll(res.Body)
 		res.Body.Close()
-		return regen, opt.URL, nil, fmt.Errorf("register request: http %d: %.200s",
+		return regen, opt.URL, "", fmt.Errorf("register request: http %d: %.200s",
 			res.StatusCode, strings.TrimSpace(string(msg)))
 	}
 	resp := tailcfg.RegisterResponse{}
 	if err := decode(res, &resp, serverKey, serverNoiseKey, 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, nil, fmt.Errorf("register request: %v", err)
+		return regen, opt.URL, "", fmt.Errorf("register request: %v", err)
 	}
 	if debugRegister() {
 		j, _ := json.MarshalIndent(resp, "", "\t")
@@ -634,7 +634,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 		resp.NodeKeyExpired, resp.MachineAuthorized, resp.AuthURL != "")
 
 	if resp.Error != "" {
-		return false, "", nil, UserVisibleError(resp.Error)
+		return false, "", "", UserVisibleError(resp.Error)
 	}
 	if len(resp.NodeKeySignature) > 0 {
 		return true, "", resp.NodeKeySignature, nil
@@ -642,11 +642,11 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 
 	if resp.NodeKeyExpired {
 		if regen {
-			return true, "", nil, fmt.Errorf("weird: regen=true but server says NodeKeyExpired: %v", request.NodeKey)
+			return true, "", "", fmt.Errorf("weird: regen=true but server says NodeKeyExpired: %v", request.NodeKey)
 		}
 		c.logf("server reports new node key %v has expired",
 			request.NodeKey.ShortString())
-		return true, "", nil, nil
+		return true, "", "", nil
 	}
 	if resp.Login.Provider != "" {
 		persist.Provider = resp.Login.Provider
@@ -682,12 +682,12 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 	c.mu.Unlock()
 
 	if err != nil {
-		return regen, "", nil, err
+		return regen, "", "", err
 	}
 	if ctx.Err() != nil {
-		return regen, "", nil, ctx.Err()
+		return regen, "", "", ctx.Err()
 	}
-	return false, resp.AuthURL, nil, nil
+	return false, resp.AuthURL, "", nil
 }
 
 // resignNKS re-signs a node-key signature for a new node-key.
@@ -703,12 +703,12 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 func resignNKS(priv key.NLPrivate, nodeKey key.NodePublic, oldNKS tkatype.MarshaledSignature) (tkatype.MarshaledSignature, error) {
 	var oldSig tka.NodeKeySignature
 	if err := oldSig.Unserialize(oldNKS); err != nil {
-		return nil, fmt.Errorf("decoding NKS: %w", err)
+		return "", fmt.Errorf("decoding NKS: %w", err)
 	}
 
 	nk, err := nodeKey.MarshalBinary()
 	if err != nil {
-		return nil, fmt.Errorf("marshalling node-key: %w", err)
+		return "", fmt.Errorf("marshalling node-key: %w", err)
 	}
 
 	if bytes.Equal(nk, oldSig.Pubkey) {
@@ -723,7 +723,7 @@ func resignNKS(priv key.NLPrivate, nodeKey key.NodePublic, oldNKS tkatype.Marsha
 		Nested:  &oldSig,
 	}
 	if newSig.Signature, err = priv.SignNKS(newSig.SigHash()); err != nil {
-		return nil, fmt.Errorf("signing NKS: %w", err)
+		return "", fmt.Errorf("signing NKS: %w", err)
 	}
 
 	return newSig.Serialize(), nil
@@ -1819,7 +1819,7 @@ func decodeWrappedAuthkey(key string, logf logger.Logf) (authKey string, isWrapp
 	}
 
 	sig = new(tka.NodeKeySignature)
-	if err := sig.Unserialize([]byte(rawSig)); err != nil {
+	if err := sig.Unserialize(tkatype.MarshaledSignature(rawSig)); err != nil {
 		logf("decoding wrapped auth-key: signature: %v", err)
 		return key, false, nil, nil
 	}

control/controlclient/map.go

@@ -330,7 +330,7 @@ func undeltaPeers(mapRes *tailcfg.MapResponse, prev []*tailcfg.Node) {
 				if v := ec.Capabilities; v != nil {
 					n.Capabilities = *v
 				}
-				if v := ec.KeySignature; v != nil {
+				if v := ec.KeySignature; v != "" {
 					n.KeySignature = v
 				}
 			}

control/controlclient/map_test.go

@@ -212,12 +212,12 @@ func TestUndeltaPeers(t *testing.T) {
 			mapRes: &tailcfg.MapResponse{
 				PeersChangedPatch: []*tailcfg.PeerChange{{
 					NodeID:       1,
-					KeySignature: []byte{3, 4},
+					KeySignature: "ab",
 				}},
 			}, want: peers(&tailcfg.Node{
 				ID:           1,
 				Name:         "foo",
-				KeySignature: []byte{3, 4},
+				KeySignature: "ab",
 			}),
 		},
 		{

ipn/ipnlocal/network-lock.go

@@ -73,8 +73,8 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
 			// Not subject to tailnet lock.
 			continue
 		}
-		keySig := tkatype.MarshaledSignature(p.KeySignature().StringCopy()) // TODO(bradfitz,maisem): this is unfortunate. Change tkatype.MarshaledSignature to a string for viewer?
-		if len(keySig) == 0 {
+		keySig := p.KeySignature()
+		if keySig == "" {
 			b.logf("Network lock is dropping peer %v(%v) due to missing signature", p.ID(), p.StableID())
 			mak.Set(&toDelete, i, true)
 		} else {
@@ -965,7 +965,7 @@ func (b *LocalBackend) NetworkLockWrapPreauthKey(preauthKey string, tkaKey key.N
 	}
 
 	b.logf("Generated network-lock credential signature using %s", tkaKey.Public().CLIString())
-	return fmt.Sprintf("%s--TL%s-%s", preauthKey, tkaSuffixEncoder.EncodeToString(sig.Serialize()), tkaSuffixEncoder.EncodeToString(priv)), nil
+	return fmt.Sprintf("%s--TL%s-%s", preauthKey, tkaSuffixEncoder.EncodeToString([]byte(sig.Serialize())), tkaSuffixEncoder.EncodeToString(priv)), nil
 }
 
 // NetworkLockVerifySigningDeeplink asks the authority to verify the given deeplink

ipn/ipnlocal/network-lock_test.go

@@ -561,7 +561,7 @@ func TestTKAFilterNetmap(t *testing.T) {
 	nm := &netmap.NetworkMap{
 		Peers: nodeViews([]*tailcfg.Node{
 			{ID: 1, Key: n1.Public(), KeySignature: n1GoodSig.Serialize()},
-			{ID: 2, Key: n2.Public(), KeySignature: nil},                   // missing sig
+			{ID: 2, Key: n2.Public(), KeySignature: ""},                    // missing sig
 			{ID: 3, Key: n3.Public(), KeySignature: n1GoodSig.Serialize()}, // someone elses sig
 			{ID: 4, Key: n4.Public(), KeySignature: n4Sig.Serialize()},     // messed-up signature
 			{ID: 5, Key: n5.Public(), KeySignature: n5GoodSig.Serialize()},
@@ -987,7 +987,7 @@ func TestTKAAffectedSigs(t *testing.T) {
 				if len(sigs) != 1 {
 					t.Fatalf("len(sigs) = %d, want 1", len(sigs))
 				}
-				if !bytes.Equal(s.Serialize(), sigs[0]) {
+				if s.Serialize() != sigs[0] {
 					t.Errorf("unexpected signature: got %v, want %v", sigs[0], s.Serialize())
 				}
 			}

tailcfg/tailcfg.go

@@ -6,7 +6,6 @@ package tailcfg
 //go:generate go run tailscale.com/cmd/viewer --type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile --clonefunc
 
 import (
-	"bytes"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -1786,7 +1785,7 @@ func (n *Node) Equal(n2 *Node) bool {
 		n.UnsignedPeerAPIOnly == n2.UnsignedPeerAPIOnly &&
 		n.Key == n2.Key &&
 		n.KeyExpiry.Equal(n2.KeyExpiry) &&
-		bytes.Equal(n.KeySignature, n2.KeySignature) &&
+		n.KeySignature == n2.KeySignature &&
 		n.Machine == n2.Machine &&
 		n.DiscoKey == n2.DiscoKey &&
 		eqPtr(n.Online, n2.Online) &&

tailcfg/tailcfg_clone.go

@@ -46,7 +46,6 @@ func (src *Node) Clone() *Node {
 	}
 	dst := new(Node)
 	*dst = *src
-	dst.KeySignature = append(src.KeySignature[:0:0], src.KeySignature...)
 	dst.Addresses = append(src.Addresses[:0:0], src.Addresses...)
 	dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...)
 	dst.Endpoints = append(src.Endpoints[:0:0], src.Endpoints...)
@@ -271,7 +270,6 @@ func (src *RegisterResponse) Clone() *RegisterResponse {
 	dst := new(RegisterResponse)
 	*dst = *src
 	dst.User = *src.User.Clone()
-	dst.NodeKeySignature = append(src.NodeKeySignature[:0:0], src.NodeKeySignature...)
 	return dst
 }
 
@@ -320,7 +318,6 @@ func (src *RegisterRequest) Clone() *RegisterRequest {
 	*dst = *src
 	dst.Auth = *src.Auth.Clone()
 	dst.Hostinfo = src.Hostinfo.Clone()
-	dst.NodeKeySignature = append(src.NodeKeySignature[:0:0], src.NodeKeySignature...)
 	if dst.Timestamp != nil {
 		dst.Timestamp = new(time.Time)
 		*dst.Timestamp = *src.Timestamp

tailcfg/tailcfg_view.go

@@ -136,7 +136,7 @@ func (v NodeView) User() UserID                             { return v.ж.User }
 func (v NodeView) Sharer() UserID                           { return v.ж.Sharer }
 func (v NodeView) Key() key.NodePublic                      { return v.ж.Key }
 func (v NodeView) KeyExpiry() time.Time                     { return v.ж.KeyExpiry }
-func (v NodeView) KeySignature() mem.RO                     { return mem.B(v.ж.KeySignature) }
+func (v NodeView) KeySignature() tkatype.MarshaledSignature { return v.ж.KeySignature }
 func (v NodeView) Machine() key.MachinePublic               { return v.ж.Machine }
 func (v NodeView) DiscoKey() key.DiscoPublic                { return v.ж.DiscoKey }
 func (v NodeView) Addresses() views.Slice[netip.Prefix]     { return views.SliceOf(v.ж.Addresses) }
@@ -610,13 +610,15 @@ func (v *RegisterResponseView) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
-func (v RegisterResponseView) User() UserView           { return v.ж.User.View() }
-func (v RegisterResponseView) Login() Login             { return v.ж.Login }
-func (v RegisterResponseView) NodeKeyExpired() bool     { return v.ж.NodeKeyExpired }
-func (v RegisterResponseView) MachineAuthorized() bool  { return v.ж.MachineAuthorized }
-func (v RegisterResponseView) AuthURL() string          { return v.ж.AuthURL }
-func (v RegisterResponseView) NodeKeySignature() mem.RO { return mem.B(v.ж.NodeKeySignature) }
-func (v RegisterResponseView) Error() string            { return v.ж.Error }
+func (v RegisterResponseView) User() UserView          { return v.ж.User.View() }
+func (v RegisterResponseView) Login() Login            { return v.ж.Login }
+func (v RegisterResponseView) NodeKeyExpired() bool    { return v.ж.NodeKeyExpired }
+func (v RegisterResponseView) MachineAuthorized() bool { return v.ж.MachineAuthorized }
+func (v RegisterResponseView) AuthURL() string         { return v.ж.AuthURL }
+func (v RegisterResponseView) NodeKeySignature() tkatype.MarshaledSignature {
+	return v.ж.NodeKeySignature
+}
+func (v RegisterResponseView) Error() string { return v.ж.Error }
 
 // A compilation failure here means this code must be regenerated, with the command at the top of this file.
 var _RegisterResponseViewNeedsRegeneration = RegisterResponse(struct {
@@ -749,8 +751,10 @@ func (v RegisterRequestView) Expiry() time.Time              { return v.ж.Expir
 func (v RegisterRequestView) Followup() string               { return v.ж.Followup }
 func (v RegisterRequestView) Hostinfo() HostinfoView         { return v.ж.Hostinfo.View() }
 func (v RegisterRequestView) Ephemeral() bool                { return v.ж.Ephemeral }
-func (v RegisterRequestView) NodeKeySignature() mem.RO       { return mem.B(v.ж.NodeKeySignature) }
-func (v RegisterRequestView) SignatureType() SignatureType   { return v.ж.SignatureType }
+func (v RegisterRequestView) NodeKeySignature() tkatype.MarshaledSignature {
+	return v.ж.NodeKeySignature
+}
+func (v RegisterRequestView) SignatureType() SignatureType { return v.ж.SignatureType }
 func (v RegisterRequestView) Timestamp() *time.Time {
 	if v.ж.Timestamp == nil {
 		return nil

tka/sig.go

@@ -166,7 +166,7 @@ func (s NodeKeySignature) authorizingKeyID() (tkatype.KeyID, error) {
 func (s NodeKeySignature) SigHash() [blake2s.Size]byte {
 	dupe := s
 	dupe.Signature = nil
-	return blake2s.Sum256(dupe.Serialize())
+	return blake2s.Sum256([]byte(dupe.Serialize()))
 }
 
 // Serialize returns the given NKS in a serialized format.
@@ -186,7 +186,7 @@ func (s *NodeKeySignature) Serialize() tkatype.MarshaledSignature {
 		// Writing to a bytes.Buffer should never fail.
 		panic(err)
 	}
-	return out.Bytes()
+	return tkatype.MarshaledSignature(out.Bytes())
 }
 
 // Unserialize decodes bytes representing a marshaled NKS.
@@ -194,9 +194,9 @@ func (s *NodeKeySignature) Serialize() tkatype.MarshaledSignature {
 // We would implement encoding.BinaryUnmarshaler, except that would
 // unfortunately get called by the cbor unmarshaller resulting in infinite
 // recursion.
-func (s *NodeKeySignature) Unserialize(data []byte) error {
+func (s *NodeKeySignature) Unserialize(data tkatype.MarshaledSignature) error {
 	dec, _ := cborDecOpts.DecMode()
-	return dec.Unmarshal(data, s)
+	return dec.Unmarshal([]byte(data), s)
 }
 
 // verifySignature checks that the NodeKeySignature is authentic & certified

types/tkatype/tkatype.go

@@ -7,6 +7,8 @@
 // because this package encodes wire types that should be lightweight to use.
 package tkatype
 
+import "encoding/json"
+
 // KeyID references a verification key stored in the key authority. A keyID
 // uniquely identifies a key. KeyIDs are all 32 bytes.
 //
@@ -19,7 +21,26 @@ package tkatype
 type KeyID []byte
 
 // MarshaledSignature represents a marshaled tka.NodeKeySignature.
-type MarshaledSignature []byte
+//
+// While its underlying type is a string, it's just the raw signature bytes, not
+// hex or base64, etc.
+//
+// Think of it as []byte, which it used to be. It's a string only to make it
+// easier to use with cmd/viewer.
+type MarshaledSignature string
+
+func (a MarshaledSignature) MarshalJSON() ([]byte, error) {
+	return json.Marshal([]byte(a))
+}
+
+func (a *MarshaledSignature) UnmarshalJSON(b []byte) error {
+	var bs []byte
+	if err := json.Unmarshal(b, &bs); err != nil {
+		return err
+	}
+	*a = MarshaledSignature(bs)
+	return nil
+}
 
 // MarshaledAUM represents a marshaled tka.AUM.
 type MarshaledAUM []byte