tka: implement machinery for node-key denylist

Signed-off-by: Tom DNetto <tom@tailscale.com>

tka/aum.go

@@ -13,6 +13,7 @@
 
 	"github.com/fxamacker/cbor/v2"
 	"golang.org/x/crypto/blake2s"
+	"tailscale.com/types/key"
 	"tailscale.com/types/tkatype"
 )
 
@@ -55,7 +56,7 @@ func (h AUMHash) MarshalText() ([]byte, error) {
 	//
 	// Only the Key optional field may be set.
 	AUMAddKey
-	// A RemoveKey AUM describes hte removal of a key trusted by TKA.
+	// A RemoveKey AUM describes the removal of a key trusted by TKA.
 	//
 	// Only the KeyID optional field may be set.
 	AUMRemoveKey
@@ -70,6 +71,16 @@ func (h AUMHash) MarshalText() ([]byte, error) {
 	//
 	// Only the State optional field may be set.
 	AUMCheckpoint
+	// A AddDenylistNodeKey AUM describes the addition of a node key to
+	// the denylist.
+	//
+	// Only the NodeKey optional field may be set.
+	AUMAddDenylistNodeKey
+	// A RemoveDenylistNodeKey AUM describes the removal of a node key from
+	// the denylist.
+	//
+	// Only the NodeKey optional field may be set.
+	AUMRemoveDenylistNodeKey
 )
 
 func (k AUMKind) String() string {
@@ -86,6 +97,10 @@ func (k AUMKind) String() string {
 		return "checkpoint"
 	case AUMUpdateKey:
 		return "update-key"
+	case AUMAddDenylistNodeKey:
+		return "add-nodekey-denylist"
+	case AUMRemoveDenylistNodeKey:
+		return "remove-nodekey-denylist"
 	default:
 		return fmt.Sprintf("AUM?<%d>", int(k))
 	}
@@ -129,6 +144,13 @@ type AUM struct {
 	Votes *uint             `cbor:"6,keyasint,omitempty"`
 	Meta  map[string]string `cbor:"7,keyasint,omitempty"`
 
+	// NodeKey describes the node-key being added or removed from the denylist.
+	// This field is used for AUMAddDenylistNodeKey & AUMRemoveDenylistNodeKey
+	// AUMs.
+	//
+	// NodeKey is the MarshalBinary representation of a key.NodePublic.
+	NodeKey []byte `cbor:"8,keyasint,omitempty"`
+
 	// Signatures lists the signatures over this AUM.
 	// CBOR key 23 is the last key which can be encoded as a single byte.
 	Signatures []tkatype.Signature `cbor:"23,keyasint,omitempty"`
@@ -161,14 +183,14 @@ func (a *AUM) StaticValidate() error {
 		if a.Key == nil {
 			return errors.New("AddKey AUMs must contain a key")
 		}
-		if a.KeyID != nil || a.State != nil || a.Votes != nil || a.Meta != nil {
+		if a.KeyID != nil || a.State != nil || a.Votes != nil || a.Meta != nil || a.NodeKey != nil {
 			return errors.New("AddKey AUMs may only specify a Key")
 		}
 	case AUMRemoveKey:
 		if len(a.KeyID) == 0 {
 			return errors.New("RemoveKey AUMs must specify a key ID")
 		}
-		if a.Key != nil || a.State != nil || a.Votes != nil || a.Meta != nil {
+		if a.Key != nil || a.State != nil || a.Votes != nil || a.Meta != nil || a.NodeKey != nil {
 			return errors.New("RemoveKey AUMs may only specify a KeyID")
 		}
 	case AUMUpdateKey:
@@ -178,16 +200,24 @@ func (a *AUM) StaticValidate() error {
 		if a.Meta == nil && a.Votes == nil {
 			return errors.New("UpdateKey AUMs must contain an update to votes or key metadata")
 		}
-		if a.Key != nil || a.State != nil {
+		if a.Key != nil || a.State != nil || a.NodeKey != nil {
 			return errors.New("UpdateKey AUMs may only specify KeyID, Votes, and Meta")
 		}
 	case AUMCheckpoint:
 		if a.State == nil {
 			return errors.New("Checkpoint AUMs must specify the state")
 		}
-		if a.KeyID != nil || a.Key != nil || a.Votes != nil || a.Meta != nil {
+		if a.KeyID != nil || a.Key != nil || a.Votes != nil || a.Meta != nil || a.NodeKey != nil {
 			return errors.New("Checkpoint AUMs may only specify State")
 		}
+	case AUMAddDenylistNodeKey, AUMRemoveDenylistNodeKey:
+		if a.Key != nil || a.State != nil || a.Votes != nil || a.Meta != nil || a.KeyID != nil {
+			return errors.New("AUMs manipulating the denylist may only specify a NodeKey")
+		}
+		var nodeKey key.NodePublic
+		if err := nodeKey.UnmarshalBinary(a.NodeKey); err != nil {
+			return fmt.Errorf("NodeKey invalid: %v", err)
+		}
 
 	case AUMNoOp:
 	default:

tka/state.go

@@ -36,6 +36,13 @@ type State struct {
 
 	// Keys are the public keys currently trusted by the TKA.
 	Keys []Key `cbor:"3,keyasint"`
+
+	// BannedNodekeys is a denylist of node-keys. Any signatures over
+	// these keys are not considered authorized.
+	//
+	// Each entry is encoded as the MarshalBinary representation of the
+	// underlying key.NodePublic.
+	BannedNodekeys [][]byte `cbor:"4,keyasint,omitempty"`
 }
 
 // GetKey returns the trusted key with the specified KeyID.
@@ -77,6 +84,13 @@ func (s State) Clone() State {
 		}
 	}
 
+	if s.BannedNodekeys != nil {
+		out.BannedNodekeys = make([][]byte, len(s.BannedNodekeys))
+		for i := range s.BannedNodekeys {
+			out.BannedNodekeys[i] = make([]byte, len(s.BannedNodekeys[i]))
+			copy(out.BannedNodekeys[i], s.BannedNodekeys[i])
+		}
+	}
 	return out
 }
 
@@ -193,6 +207,32 @@ func (s State) applyVerifiedAUM(update AUM) (State, error) {
 		out.Keys = append(out.Keys[:idx], out.Keys[idx+1:]...)
 		return out, nil
 
+	case AUMAddDenylistNodeKey:
+		for i := range s.BannedNodekeys {
+			if bytes.Equal(update.NodeKey, s.BannedNodekeys[i]) {
+				return State{}, errors.New("entry already exists")
+			}
+		}
+		out := s.cloneForUpdate(&update)
+		// Validity of node-key already checked by StaticValidate in aumVerify().
+		out.BannedNodekeys = append(out.BannedNodekeys, update.NodeKey)
+		return out, nil
+
+	case AUMRemoveDenylistNodeKey:
+		idx := -1
+		for i := range s.BannedNodekeys {
+			if bytes.Equal(update.NodeKey, s.BannedNodekeys[i]) {
+				idx = i
+				break
+			}
+		}
+		if idx < 0 {
+			return State{}, errors.New("no such entry")
+		}
+		out := s.cloneForUpdate(&update)
+		out.BannedNodekeys = append(out.BannedNodekeys[:idx], out.BannedNodekeys[idx+1:]...)
+		return out, nil
+
 	default:
 		// TODO(tom): Instead of erroring, update lastHash and
 		// continue (to preserve future compatibility).
@@ -205,6 +245,7 @@ func (s State) applyVerifiedAUM(update AUM) (State, error) {
 const (
 	maxDisablementSecrets = 32
 	maxKeys               = 512
+	maxDenylistEntries    = 64
 )
 
 // staticValidateCheckpoint validates that the state is well-formed for
@@ -252,5 +293,19 @@ func (s *State) staticValidateCheckpoint() error {
 			}
 		}
 	}
+
+	if numDenys := len(s.BannedNodekeys); numDenys > maxDenylistEntries {
+		return fmt.Errorf("too many node-key denylist entries (%d, max %d)", numDenys, maxDenylistEntries)
+	}
+	for i, nk := range s.BannedNodekeys {
+		for j, nk2 := range s.BannedNodekeys {
+			if i == j {
+				continue
+			}
+			if bytes.Equal(nk, nk2) {
+				return fmt.Errorf("node-key entry %d duplicates entry %d", i, j)
+			}
+		}
+	}
 	return nil
 }

tka/state_test.go

@@ -53,6 +53,15 @@ func TestCloneState(t *testing.T) {
 				},
 			},
 		},
+		{
+			"BannedNodekeys",
+			State{
+				BannedNodekeys: [][]byte{
+					{1, 2, 3, 4},
+					{5, 6, 7, 8},
+				},
+			},
+		},
 	}
 
 	for _, tc := range tcs {
@@ -153,6 +162,26 @@ func TestApplyUpdatesChain(t *testing.T) {
 				LastAUMHash: hashFromHex("57343671da5eea3cfb502954e976e8028bffd3540b50a043b2a65a8d8d8217d0"),
 			},
 		},
+		{
+			"AddDenyNodekey",
+			[]AUM{{MessageKind: AUMAddDenylistNodeKey, NodeKey: []byte{1, 2, 3, 4}}},
+			State{},
+			State{
+				BannedNodekeys: [][]byte{{1, 2, 3, 4}},
+				LastAUMHash:    hashFromHex("e6d5db2b73e6ad69ae6b852f9a1c163c17bc98d3acf257ef9616e3a36c7368f2"),
+			},
+		},
+		{
+			"RemoveDenyNodekey",
+			[]AUM{{MessageKind: AUMRemoveDenylistNodeKey, NodeKey: []byte{1, 2, 3, 4}, PrevAUMHash: fromHex("e6d5db2b73e6ad69ae6b852f9a1c163c17bc98d3acf257ef9616e3a36c7368f2")}},
+			State{
+				BannedNodekeys: [][]byte{{1, 2, 3, 4}},
+				LastAUMHash:    hashFromHex("e6d5db2b73e6ad69ae6b852f9a1c163c17bc98d3acf257ef9616e3a36c7368f2"),
+			},
+			State{
+				LastAUMHash: hashFromHex("beea03557a839a51de8822ffbe0819035ec85a1766c301ac2cd992e2b79b5068"),
+			},
+		},
 	}
 
 	for _, tc := range tcs {
@@ -166,6 +195,7 @@ func TestApplyUpdatesChain(t *testing.T) {
 					t.Fatalf("Apply message[%d] failed: %v", i, err)
 				}
 				// t.Logf("update[%d] end-state = %+v", i, state)
+				// t.Logf("state.LastAUMHash = %x", state.LastAUMHash[:])
 
 				updateHash := tc.Updates[i].Hash()
 				if got, want := *state.LastAUMHash, updateHash[:]; !bytes.Equal(got[:], want) {
@@ -223,6 +253,18 @@ func TestApplyUpdateErrors(t *testing.T) {
 			},
 			errors.New("parent AUMHash mismatch"),
 		},
+		{
+			"AddDenyNodekey exists",
+			[]AUM{{MessageKind: AUMAddDenylistNodeKey, NodeKey: []byte{1, 2, 3, 4}}},
+			State{BannedNodekeys: [][]byte{{1, 2, 3, 4}}},
+			errors.New("entry already exists"),
+		},
+		{
+			"RemoveDenyNodekey notfound",
+			[]AUM{{MessageKind: AUMRemoveDenylistNodeKey, NodeKey: []byte{1, 2, 3, 4}}},
+			State{},
+			errors.New("no such entry"),
+		},
 	}
 
 	for _, tc := range tcs {

tka/tka.go

@@ -677,6 +677,18 @@ func (a *Authority) NodeKeyAuthorized(nodeKey key.NodePublic, nodeKeySignature t
 		return errors.New("credential signatures cannot authorize nodes on their own")
 	}
 
+	if len(a.state.BannedNodekeys) > 0 {
+		nodeBytes, err := nodeKey.MarshalBinary()
+		if err != nil {
+			return fmt.Errorf("marshalling pubkey: %v", err)
+		}
+		for _, bannedNode := range a.state.BannedNodekeys {
+			if bytes.Equal(bannedNode, nodeBytes) {
+				return errors.New("pubkey is on the denylist")
+			}
+		}
+	}
+
 	key, err := a.state.GetKey(decoded.KeyID)
 	if err != nil {
 		return fmt.Errorf("key: %v", err)