From 8724aa254f98cef7cf56d586fe172a8abe7c465e Mon Sep 17 00:00:00 2001
From: Tom DNetto <tom@tailscale.com>
Date: Tue, 13 Dec 2022 09:33:13 -0800
Subject: [PATCH] cmd/tailscale,tka: implement compat for TKA messages, minor
 UX tweaks

Signed-off-by: Tom DNetto <tom@tailscale.com>
---
 cmd/tailscale/cli/network-lock.go | 15 ++++++++++++++-
 tka/aum.go                        | 11 ++++++++---
 tka/state.go                      | 15 +++++++++------
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/cmd/tailscale/cli/network-lock.go b/cmd/tailscale/cli/network-lock.go
index c65d382ba..9c4e5aad9 100644
--- a/cmd/tailscale/cli/network-lock.go
+++ b/cmd/tailscale/cli/network-lock.go
@@ -5,6 +5,7 @@
 package cli
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"encoding/hex"
@@ -99,6 +100,18 @@ func runNetworkLockInit(ctx context.Context, args []string) error {
 		return err
 	}
 
+	// Common mistake: Not specifying the current node's key as one of the trusted keys.
+	foundSelfKey := false
+	for _, k := range keys {
+		if bytes.Equal(k.ID(), st.PublicKey.KeyID()) {
+			foundSelfKey = true
+			break
+		}
+	}
+	if !foundSelfKey {
+		return errors.New("the tailnet lock key of the current node must be one of the trusted keys during initialization")
+	}
+
 	fmt.Println("You are initializing tailnet lock with the following trusted signing keys:")
 	for _, k := range keys {
 		fmt.Printf(" - tlpub:%x (%s key)\n", k.Public, k.Kind.String())
@@ -196,7 +209,7 @@ func runNetworkLockStatus(ctx context.Context, args []string) error {
 			line.WriteString(fmt.Sprint(k.Votes))
 			line.WriteString("\t")
 			if k.Key == st.PublicKey {
-				line.WriteString("(us)")
+				line.WriteString("(self)")
 			}
 			fmt.Println(line.String())
 		}
diff --git a/tka/aum.go b/tka/aum.go
index 6eb221369..8b59e2f05 100644
--- a/tka/aum.go
+++ b/tka/aum.go
@@ -150,7 +150,7 @@ func (a *AUM) StaticValidate() error {
 		return errors.New("absent parent must be represented by a nil slice")
 	}
 	for i, sig := range a.Signatures {
-		if len(sig.KeyID) == 0 || len(sig.Signature) != ed25519.SignatureSize {
+		if len(sig.KeyID) != 32 || len(sig.Signature) != ed25519.SignatureSize {
 			return fmt.Errorf("signature %d has missing keyID or malformed signature", i)
 		}
 	}
@@ -196,8 +196,13 @@ func (a *AUM) StaticValidate() error {
 
 	case AUMNoOp:
 	default:
-		// TODO(tom): Ignore unknown AUMs for GA.
-		return fmt.Errorf("unknown AUM kind: %v", a.MessageKind)
+		// An AUM with an unknown message kind was received! That means
+		// that a future version of tailscaled added some feature we don't
+		// understand.
+		//
+		// The future-compatibility contract for AUM message types is that
+		// they must only add new features, not change the semantics of existing
+		// mechanisms or features. As such, old clients can safely ignore them.
 	}
 
 	return nil
diff --git a/tka/state.go b/tka/state.go
index fa9dd0d05..3a503ac39 100644
--- a/tka/state.go
+++ b/tka/state.go
@@ -29,9 +29,6 @@ type State struct {
 
 	// DisablementSecrets are KDF-derived values which can be used
 	// to turn off the TKA in the event of a consensus-breaking bug.
-	//
-	// TODO(tom): This is an alpha feature, remove this mechanism once
-	//            we have confidence in our implementation.
 	DisablementSecrets [][]byte `cbor:"2,keyasint"`
 
 	// Keys are the public keys currently trusted by the TKA.
@@ -217,9 +214,15 @@ func (s State) applyVerifiedAUM(update AUM) (State, error) {
 		return out, nil
 
 	default:
-		// TODO(tom): Instead of erroring, update lastHash and
-		// continue (to preserve future compatibility).
-		return State{}, fmt.Errorf("unhandled message: %v", update.MessageKind)
+		// An AUM with an unknown message kind was received! That means
+		// that a future version of tailscaled added some feature we don't
+		// understand.
+		//
+		// The future-compatibility contract for AUM message types is that
+		// they must only add new features, not change the semantics of existing
+		// mechanisms or features. As such, old clients can safely ignore them.
+		out := s.cloneForUpdate(&update)
+		return out, nil
 	}
 }