Run the Noise handlers under a new struct so we can access the noiseC…

…onn from the handlers

In TS2021 the MachineKey can be obtained from noiseConn.Peer() - contrary to what I thought before,
where I assumed MachineKey was dropped in TS2021.

By having a ts2021App and hanging from there the TS2021 handlers, we can fetch again the MachineKey.

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Reworked routing and added support for subnet router failover [#1024](https://github.com/juanfont/headscale/pull/1024)
 - Added an OIDC AllowGroups Configuration options and authorization check [#1041](https://github.com/juanfont/headscale/pull/1041)
 - Set `db_ssl` to false by default [#1052](https://github.com/juanfont/headscale/pull/1052)
+- Fix duplicate nodes due to incorrect implementation of the protocol [#1058](https://github.com/juanfont/headscale/pull/1058)
 - Report if a machine is online in CLI more accurately [#1062](https://github.com/juanfont/headscale/pull/1062)
 
 ## 0.17.1 (2022-12-05)

app.go

@@ -81,8 +81,6 @@ type Headscale struct {
 	privateKey      *key.MachinePrivate
 	noisePrivateKey *key.MachinePrivate
 
-	noiseMux *mux.Router
-
 	DERPMap    *tailcfg.DERPMap
 	DERPServer *DERPServer
 
@@ -472,16 +470,6 @@ func (h *Headscale) createRouter(grpcMux *runtime.ServeMux) *mux.Router {
 	return router
 }
 
-func (h *Headscale) createNoiseMux() *mux.Router {
-	router := mux.NewRouter()
-
-	router.HandleFunc("/machine/register", h.NoiseRegistrationHandler).
-		Methods(http.MethodPost)
-	router.HandleFunc("/machine/map", h.NoisePollNetMapHandler)
-
-	return router
-}
-
 // Serve launches a GIN server with the Headscale API.
 func (h *Headscale) Serve() error {
 	var err error
@@ -641,12 +629,6 @@ func (h *Headscale) Serve() error {
 	// over our main Addr. It also serves the legacy Tailcale API
 	router := h.createRouter(grpcGatewayMux)
 
-	// This router is served only over the Noise connection, and exposes only the new API.
-	//
-	// The HTTP2 server that exposes this router is created for
-	// a single hijacked connection from /ts2021, using netutil.NewOneConnListener
-	h.noiseMux = h.createNoiseMux()
-
 	httpServer := &http.Server{
 		Addr:        h.cfg.Addr,
 		Handler:     router,

cmd/headscale/cli/nodes.go

@@ -469,6 +469,7 @@ func nodesToPtables(
 		"ID",
 		"Hostname",
 		"Name",
+		"MachineKey",
 		"NodeKey",
 		"Namespace",
 		"IP addresses",
@@ -504,8 +505,16 @@ func nodesToPtables(
 			expiry = machine.Expiry.AsTime()
 		}
 
+		var machineKey key.MachinePublic
+		err := machineKey.UnmarshalText(
+			[]byte(headscale.MachinePublicKeyEnsurePrefix(machine.MachineKey)),
+		)
+		if err != nil {
+			machineKey = key.MachinePublic{}
+		}
+
 		var nodeKey key.NodePublic
-		err := nodeKey.UnmarshalText(
+		err = nodeKey.UnmarshalText(
 			[]byte(headscale.NodePublicKeyEnsurePrefix(machine.NodeKey)),
 		)
 		if err != nil {
@@ -568,6 +577,7 @@ func nodesToPtables(
 			strconv.FormatUint(machine.Id, headscale.Base10),
 			machine.Name,
 			machine.GetGivenName(),
+			machineKey.ShortString(),
 			nodeKey.ShortString(),
 			namespace,
 			strings.Join([]string{IPV4Address, IPV6Address}, ", "),

machine.go

@@ -418,13 +418,15 @@ func (h *Headscale) GetMachineByNodeKey(
 	return &machine, nil
 }
 
-// GetMachineByAnyNodeKey finds a Machine by its current NodeKey or the old one, and returns the Machine struct.
-func (h *Headscale) GetMachineByAnyNodeKey(
-	nodeKey key.NodePublic, oldNodeKey key.NodePublic,
+// GetMachineByAnyNodeKey finds a Machine by its MachineKey, its current NodeKey or the old one, and returns the Machine struct.
+func (h *Headscale) GetMachineByAnyKey(
+	machineKey key.MachinePublic, nodeKey key.NodePublic, oldNodeKey key.NodePublic,
 ) (*Machine, error) {
 	machine := Machine{}
-	if result := h.db.Preload("Namespace").First(&machine, "node_key = ? OR node_key = ?",
-		NodePublicKeyStripPrefix(nodeKey), NodePublicKeyStripPrefix(oldNodeKey)); result.Error != nil {
+	if result := h.db.Preload("Namespace").First(&machine, "machine_key = ? OR node_key = ? OR node_key = ?",
+		MachinePublicKeyStripPrefix(machineKey),
+		NodePublicKeyStripPrefix(nodeKey),
+		NodePublicKeyStripPrefix(oldNodeKey)); result.Error != nil {
 		return nil, result.Error
 	}
 
@@ -850,6 +852,12 @@ func (h *Headscale) RegisterMachineFromAuthCallback(
 		return nil, err
 	}
 
+	log.Debug().
+		Str("nodeKey", nodeKey.ShortString()).
+		Str("namespaceName", namespaceName).
+		Str("registrationMethod", registrationMethod).
+		Msg("Registering machine from API/CLI or auth callback")
+
 	if machineInterface, ok := h.registrationCache.Get(NodePublicKeyStripPrefix(nodeKey)); ok {
 		if registrationMachine, ok := machineInterface.(Machine); ok {
 			namespace, err := h.GetNamespace(namespaceName)
@@ -889,15 +897,31 @@ func (h *Headscale) RegisterMachineFromAuthCallback(
 // RegisterMachine is executed from the CLI to register a new Machine using its MachineKey.
 func (h *Headscale) RegisterMachine(machine Machine,
 ) (*Machine, error) {
-	log.Trace().
-		Caller().
+	log.Debug().
+		Str("machine", machine.Hostname).
 		Str("machine_key", machine.MachineKey).
+		Str("node_key", machine.NodeKey).
+		Str("namespace", machine.Namespace.Name).
 		Msg("Registering machine")
 
-	log.Trace().
-		Caller().
-		Str("machine", machine.Hostname).
-		Msg("Attempting to register machine")
+	// If the machine exists and we had already IPs for it, we just save it
+	// so we store the machine.Expire and machine.Nodekey that has been set when
+	// adding it to the registrationCache
+	if len(machine.IPAddresses) > 0 {
+		if err := h.db.Save(&machine).Error; err != nil {
+			return nil, fmt.Errorf("failed register existing machine in the database: %w", err)
+		}
+
+		log.Trace().
+			Caller().
+			Str("machine", machine.Hostname).
+			Str("machine_key", machine.MachineKey).
+			Str("node_key", machine.NodeKey).
+			Str("namespace", machine.Namespace.Name).
+			Msg("Machine authorized again")
+
+		return &machine, nil
+	}
 
 	h.ipAllocationMutex.Lock()
 	defer h.ipAllocationMutex.Unlock()

machine_test.go

@@ -77,10 +77,11 @@ func (s *Suite) TestGetMachineByNodeKey(c *check.C) {
 	c.Assert(err, check.NotNil)
 
 	nodeKey := key.NewNode()
+	machineKey := key.NewMachine()
 
 	machine := Machine{
 		ID:             0,
-		MachineKey:     "foo",
+		MachineKey:     MachinePublicKeyStripPrefix(machineKey.Public()),
 		NodeKey:        NodePublicKeyStripPrefix(nodeKey.Public()),
 		DiscoKey:       "faa",
 		Hostname:       "testmachine",
@@ -107,9 +108,11 @@ func (s *Suite) TestGetMachineByAnyNodeKey(c *check.C) {
 	nodeKey := key.NewNode()
 	oldNodeKey := key.NewNode()
 
+	machineKey := key.NewMachine()
+
 	machine := Machine{
 		ID:             0,
-		MachineKey:     "foo",
+		MachineKey:     MachinePublicKeyStripPrefix(machineKey.Public()),
 		NodeKey:        NodePublicKeyStripPrefix(nodeKey.Public()),
 		DiscoKey:       "faa",
 		Hostname:       "testmachine",
@@ -119,7 +122,7 @@ func (s *Suite) TestGetMachineByAnyNodeKey(c *check.C) {
 	}
 	app.db.Save(&machine)
 
-	_, err = app.GetMachineByAnyNodeKey(nodeKey.Public(), oldNodeKey.Public())
+	_, err = app.GetMachineByAnyKey(machineKey.Public(), nodeKey.Public(), oldNodeKey.Public())
 	c.Assert(err, check.IsNil)
 }
 

noise.go

@@ -3,9 +3,11 @@ package headscale
 import (
 	"net/http"
 
+	"github.com/gorilla/mux"
 	"github.com/rs/zerolog/log"
 	"golang.org/x/net/http2"
 	"golang.org/x/net/http2/h2c"
+	"tailscale.com/control/controlbase"
 	"tailscale.com/control/controlhttp"
 	"tailscale.com/net/netutil"
 )
@@ -15,6 +17,12 @@ const (
 	ts2021UpgradePath = "/ts2021"
 )
 
+type ts2021App struct {
+	headscale *Headscale
+
+	conn *controlbase.Conn
+}
+
 // NoiseUpgradeHandler is to upgrade the connection and hijack the net.Conn
 // in order to use the Noise-based TS2021 protocol. Listens in /ts2021.
 func (h *Headscale) NoiseUpgradeHandler(
@@ -44,10 +52,25 @@ func (h *Headscale) NoiseUpgradeHandler(
 		return
 	}
 
+	ts2021App := ts2021App{
+		headscale: h,
+		conn:      noiseConn,
+	}
+
+	// This router is served only over the Noise connection, and exposes only the new API.
+	//
+	// The HTTP2 server that exposes this router is created for
+	// a single hijacked connection from /ts2021, using netutil.NewOneConnListener
+	router := mux.NewRouter()
+
+	router.HandleFunc("/machine/register", ts2021App.NoiseRegistrationHandler).
+		Methods(http.MethodPost)
+	router.HandleFunc("/machine/map", ts2021App.NoisePollNetMapHandler)
+
 	server := http.Server{
 		ReadTimeout: HTTPReadTimeout,
 	}
-	server.Handler = h2c.NewHandler(h.noiseMux, &http2.Server{})
+	server.Handler = h2c.NewHandler(router, &http2.Server{})
 	err = server.Serve(netutil.NewOneConnListener(noiseConn, nil))
 	if err != nil {
 		log.Info().Err(err).Msg("The HTTP2 server was closed")