auth: check machine key on the followup registration path

waitForFollowup returns nodeToRegisterResponse for a completed
registration without checking that the Noise session polling for the
result was started with the machine key that opened the registration.
That response carries the registering user's User and Login, so the auth
ID in the followup URL is the only thing protecting it.

handleRegister and handleLogout both call machineKeyMismatch before
handing back a node, so this is the one path of the three that does not.
The key is already available: HandleNodeFromAuthPath resolves the node
from the MachineKey cached in RegistrationData, so on the normal path the
node and the session agree and the check is a no-op.

The auth ID is 96 bits of randomness and is not guessable, so this is not
reachable by brute force. It is logged at info level when a registration
is created, which makes log access the realistic way to obtain one.

The existing followup_registration_success case built its node with
CreateNodeForTest, which picks a random machine key that no real
registration would produce. Set the registering machine key so the
fixture matches the production path.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
(cherry picked from commit 0ce3356b89)

hscontrol/auth.go

@@ -23,6 +23,18 @@ type AuthProvider interface {
 	AuthURL(authID types.AuthID) string
 }
 
+// machineKeyMismatch fails closed when a node looked up by NodeKey was started
+// in a Noise session with a different machine key. Without this anyone holding a
+// target's NodeKey could open a session with a throwaway machine key and act on
+// the owner's node. Returns a 401 [HTTPError] on mismatch, nil otherwise.
+func machineKeyMismatch(node types.NodeView, machineKey key.MachinePublic) error {
+	if node.MachineKey() != machineKey {
+		return NewHTTPError(http.StatusUnauthorized, "node exists with a different machine key", nil)
+	}
+
+	return nil
+}
+
 func (h *Headscale) handleRegister(
 	ctx context.Context,
 	req tailcfg.RegisterRequest,
@@ -301,6 +313,18 @@ func (h *Headscale) waitForFollowup(
 					return h.reqToNewRegisterResponse(req, machineKey)
 				}
 
+				// The followup poll is only authenticated by the auth ID in the
+				// URL, so fail closed unless the Noise session asking for the
+				// result was started with the same machine key that opened the
+				// registration. [State.HandleNodeFromAuthPath] resolves the node
+				// from the cached [types.RegistrationData.MachineKey], so the two
+				// match on the normal path. [Headscale.handleRegister] and
+				// [Headscale.handleLogout] apply the same check.
+				err := machineKeyMismatch(verdict.Node, machineKey)
+				if err != nil {
+					return nil, err
+				}
+
 				return nodeToRegisterResponse(verdict.Node), nil
 			}
 		}

hscontrol/auth_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 	"testing"
@@ -692,6 +693,11 @@ func TestAuthenticationFlows(t *testing.T) {
 					user := app.state.CreateUserForTest("followup-user")
 
 					node := app.state.CreateNodeForTest(user, "followup-success-node")
+					// [State.HandleNodeFromAuthPath] resolves the node from the
+					// machine key cached when the registration was opened, so on
+					// the real path the node carries the polling session's
+					// machine key. CreateNodeForTest picks a random one.
+					node.MachineKey = machineKey1.Public()
 					nodeToRegister.FinishAuth(types.AuthVerdict{Node: node.View()})
 				}()
 
@@ -4101,3 +4107,79 @@ func TestHandleNodeFromAuthPath_OldUserNil_NoPanic(t *testing.T) {
 	assert.NotEqual(t, types.NodeID(99002), node.ID(), "new node, not orphan")
 	assert.Equal(t, userB.ID, node.UserID().Get(), "new node belongs to userB")
 }
+
+// TestWaitForFollowupMachineKeyMismatch covers the followup poll in
+// [Headscale.waitForFollowup]. That poll is authenticated only by the auth ID
+// embedded in the followup URL, so without a machine-key check anyone who
+// learns an ID gets the registering user's User/Login back in the
+// [tailcfg.RegisterResponse].
+//
+// [Headscale.handleRegister] and [Headscale.handleLogout] already fail closed
+// here; see the "existing_node_machine_key_mismatch" case in
+// [TestAuthenticationFlows] for the equivalent assertion on that path.
+//
+// The nodes are given the registering session's machine key because that is
+// what production produces: [State.HandleNodeFromAuthPath] resolves the node
+// from the machine key cached in [types.RegistrationData] when the
+// registration was opened.
+func TestWaitForFollowupMachineKeyMismatch(t *testing.T) {
+	app := createTestApp(t)
+
+	victimMachineKey := key.NewMachine()
+	attackerMachineKey := key.NewMachine()
+
+	// Park a completed registration in the auth cache, as a node that is
+	// already polling for its verdict would see it.
+	newPendingFollowup := func(hostname string) string {
+		authID := types.MustAuthID()
+		regEntry := types.NewRegisterAuthRequest(&types.RegistrationData{
+			MachineKey: victimMachineKey.Public(),
+			NodeKey:    key.NewNode().Public(),
+			Hostname:   hostname,
+		})
+		app.state.SetAuthCacheEntry(authID, regEntry)
+
+		user := app.state.CreateUserForTest(hostname + "-user")
+		node := app.state.CreateNodeForTest(user, hostname)
+		node.MachineKey = victimMachineKey.Public()
+		// CreateNodeForTest only sets UserID, but nodeToRegisterResponse reads
+		// the owner, and the owner's identity is exactly what must not leak.
+		node.User = user
+		regEntry.FinishAuth(types.AuthVerdict{Node: node.View()})
+
+		return fmt.Sprintf("http://localhost:8080/register/%s", authID)
+	}
+
+	followup := func(url string, machineKey key.MachinePublic) (*tailcfg.RegisterResponse, error) {
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+
+		return app.handleRegister(ctx, tailcfg.RegisterRequest{
+			Followup: url,
+			NodeKey:  key.NewNode().Public(),
+		}, machineKey)
+	}
+
+	t.Run("mismatched machine key is rejected", func(t *testing.T) {
+		resp, err := followup(newPendingFollowup("followup-mismatch"), attackerMachineKey.Public())
+
+		require.Error(t, err, "followup with a foreign machine key must not succeed")
+		assert.Nil(t, resp, "no registration details should be returned")
+
+		var httpErr HTTPError
+		require.ErrorAs(t, err, &httpErr)
+		assert.Equal(t, http.StatusUnauthorized, httpErr.Code)
+	})
+
+	// Positive control. Without it a regression that stops the poll from
+	// finding the cache entry at all would still pass the case above, because
+	// waitForFollowup falls back to handing out a fresh AuthURL.
+	t.Run("matching machine key still completes", func(t *testing.T) {
+		resp, err := followup(newPendingFollowup("followup-match"), victimMachineKey.Public())
+
+		require.NoError(t, err)
+		require.NotNil(t, resp)
+		assert.True(t, resp.MachineAuthorized)
+		assert.NotEmpty(t, resp.User.DisplayName, "the owner's identity is returned on the legitimate path")
+	})
+}