fix bug in #912 (#914)

2022-11-05 16:07:22 +08:00 · 2022-11-05 16:07:22 +08:00 · 07f92e647c
parent bf87b33292
commit 07f92e647c
8 changed files with 47 additions and 35 deletions
--- a/.golangci.yaml
+++ b/.golangci.yaml
@ -26,6 +26,7 @@ linters:
    - ireturn
    - execinquery
    - exhaustruct
+    - nolintlint

    # We should strive to enable these:
    - wrapcheck
--- a/cmd/headscale/cli/debug.go
+++ b/cmd/headscale/cli/debug.go
@ -3,6 +3,7 @@ package cli
 import (
 	"fmt"

+	"github.com/juanfont/headscale"
 	v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
@ -10,8 +11,7 @@ import (
 )

 const (
-	keyLength             = 64
-	errPreAuthKeyTooShort = Error("key too short, must be 64 hexadecimal characters")
+	errPreAuthKeyMalformed = Error("key is malformed. expected 64 hex characters with `nodekey` prefix")
 )

 // Error is used to compare errors as per https://dave.cheney.net/2016/04/07/constant-errors
@ -87,8 +87,8 @@ var createNodeCmd = &cobra.Command{

 			return
 		}
-		if len(machineKey) != keyLength {
-			err = errPreAuthKeyTooShort
+		if !headscale.NodePublicKeyRegex.Match([]byte(machineKey)) {
+			err = errPreAuthKeyMalformed
 			ErrorOutput(
 				err,
 				fmt.Sprintf("Error: %s", err),
--- a/grpcv1.go
+++ b/grpcv1.go
@ -1,4 +1,4 @@
-//nolint
+// nolint
 package headscale

 import (
@ -12,6 +12,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
 )

 type headscaleV1APIServer struct { // v1.HeadscaleServiceServer
@ -496,9 +497,14 @@ func (api headscaleV1APIServer) DebugCreateMachine(

 		HostInfo: HostInfo(hostinfo),
 	}
+	nodeKey := key.NodePublic{}
+	err = nodeKey.UnmarshalText([]byte(request.GetKey()))
+	if err != nil {
+		log.Panic().Msg("can not add machine for debug. invalid node key")
+	}

 	api.h.registrationCache.Set(
-		request.GetKey(),
+		NodePublicKeyStripPrefix(nodeKey),
 		newMachine,
 		registerCacheExpiration,
 	)
--- a/integration_cli_test.go
+++ b/integration_cli_test.go
@ -1,4 +1,4 @@
-//nolint
+// nolint
 package headscale

 import (
@ -558,8 +558,8 @@ func (s *IntegrationCLITestSuite) TestNodeTagCommand() {
 	assert.Nil(s.T(), err)

 	machineKeys := []string{
-		"9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
-		"6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
+		"nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
+		"nodekey:6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
 	}
 	machines := make([]*v1.Machine, len(machineKeys))
 	assert.Nil(s.T(), err)
@ -691,11 +691,11 @@ func (s *IntegrationCLITestSuite) TestNodeCommand() {

 	// Randomly generated machine keys
 	machineKeys := []string{
-		"9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
-		"6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
-		"f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
-		"8bc13285cee598acf76b1824a6f4490f7f2e3751b201e28aeb3b07fe81d5b4a1",
-		"cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
+		"nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
+		"nodekey:6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
+		"nodekey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
+		"nodekey:8bc13285cee598acf76b1824a6f4490f7f2e3751b201e28aeb3b07fe81d5b4a1",
+		"nodekey:cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
 	}
 	machines := make([]*v1.Machine, len(machineKeys))
 	assert.Nil(s.T(), err)
@ -779,8 +779,8 @@ func (s *IntegrationCLITestSuite) TestNodeCommand() {
 	assert.Equal(s.T(), "machine-5", listAll[4].Name)

 	otherNamespaceMachineKeys := []string{
-		"b5b444774186d4217adcec407563a1223929465ee2c68a4da13af0d0185b4f8e",
-		"dc721977ac7415aafa87f7d4574cbe07c6b171834a6d37375782bdc1fb6b3584",
+		"nodekey:b5b444774186d4217adcec407563a1223929465ee2c68a4da13af0d0185b4f8e",
+		"nodekey:dc721977ac7415aafa87f7d4574cbe07c6b171834a6d37375782bdc1fb6b3584",
 	}
 	otherNamespaceMachines := make([]*v1.Machine, len(otherNamespaceMachineKeys))
 	assert.Nil(s.T(), err)
@ -950,11 +950,11 @@ func (s *IntegrationCLITestSuite) TestNodeExpireCommand() {

 	// Randomly generated machine keys
 	machineKeys := []string{
-		"9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
-		"6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
-		"f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
-		"8bc13285cee598acf76b1824a6f4490f7f2e3751b201e28aeb3b07fe81d5b4a1",
-		"cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
+		"nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
+		"nodekey:6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
+		"nodekey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
+		"nodekey:8bc13285cee598acf76b1824a6f4490f7f2e3751b201e28aeb3b07fe81d5b4a1",
+		"nodekey:cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
 	}
 	machines := make([]*v1.Machine, len(machineKeys))
 	assert.Nil(s.T(), err)
@ -1077,11 +1077,11 @@ func (s *IntegrationCLITestSuite) TestNodeRenameCommand() {

 	// Randomly generated machine keys
 	machineKeys := []string{
-		"cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
-		"8bc13285cee598acf76b1824a6f4490f7f2e3751b201e28aeb3b07fe81d5b4a1",
-		"f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
-		"6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
-		"9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
+		"nodekey:cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
+		"nodekey:8bc13285cee598acf76b1824a6f4490f7f2e3751b201e28aeb3b07fe81d5b4a1",
+		"nodekey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
+		"nodekey:6abd00bb5fdda622db51387088c68e97e71ce58e7056aa54f592b6a8219d524c",
+		"nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
 	}
 	machines := make([]*v1.Machine, len(machineKeys))
 	assert.Nil(s.T(), err)
@ -1248,7 +1248,7 @@ func (s *IntegrationCLITestSuite) TestRouteCommand() {
 	assert.Nil(s.T(), err)

 	// Randomly generated machine keys
-	machineKey := "9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe"
+	machineKey := "nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe"

 	_, _, err = ExecuteCommand(
 		&s.headscale,
@ -1588,7 +1588,7 @@ func (s *IntegrationCLITestSuite) TestNodeMoveCommand() {
 	assert.Nil(s.T(), err)

 	// Randomly generated machine key
-	machineKey := "688411b767663479632d44140f08a9fde87383adc7cdeb518f62ce28a17ef0aa"
+	machineKey := "nodekey:688411b767663479632d44140f08a9fde87383adc7cdeb518f62ce28a17ef0aa"

 	_, _, err = ExecuteCommand(
 		&s.headscale,
--- a/integration_oidc_test.go
+++ b/integration_oidc_test.go
@ -1,4 +1,4 @@
-//nolint
+// nolint
 package headscale

 import (
--- a/machine.go
+++ b/machine.go
@ -839,7 +839,13 @@ func (h *Headscale) RegisterMachineFromAuthCallback(
 	namespaceName string,
 	registrationMethod string,
 ) (*Machine, error) {
-	if machineInterface, ok := h.registrationCache.Get(nodeKeyStr); ok {
+	nodeKey := key.NodePublic{}
+	err := nodeKey.UnmarshalText([]byte(nodeKeyStr))
+	if err != nil {
+		return nil, err
+	}
+
+	if machineInterface, ok := h.registrationCache.Get(NodePublicKeyStripPrefix(nodeKey)); ok {
 		if registrationMachine, ok := machineInterface.(Machine); ok {
 			namespace, err := h.GetNamespace(namespaceName)
 			if err != nil {
--- a/oidc.go
+++ b/oidc.go
@ -604,10 +604,9 @@ func (h *Headscale) registerMachineForOIDCCallback(
 	namespace *Namespace,
 	nodeKey *key.NodePublic,
 ) error {
-	nodeKeyStr := NodePublicKeyStripPrefix(*nodeKey)

 	if _, err := h.RegisterMachineFromAuthCallback(
-		nodeKeyStr,
+		nodeKey.String(),
 		namespace.Name,
 		RegisterMethodOIDC,
 	); err != nil {
--- a/protocol_common.go
+++ b/protocol_common.go
@ -490,12 +490,12 @@ func (h *Headscale) handleNewMachineCommon(
 		resp.AuthURL = fmt.Sprintf(
 			"%s/oidc/register/%s",
 			strings.TrimSuffix(h.cfg.ServerURL, "/"),
-			NodePublicKeyStripPrefix(registerRequest.NodeKey),
+			registerRequest.NodeKey,
 		)
 	} else {
 		resp.AuthURL = fmt.Sprintf("%s/register/%s",
 			strings.TrimSuffix(h.cfg.ServerURL, "/"),
-			NodePublicKeyStripPrefix(registerRequest.NodeKey))
+			registerRequest.NodeKey)
 	}

 	respBody, err := h.marshalResponse(resp, machineKey)
@ -726,7 +726,7 @@ func (h *Headscale) handleMachineExpiredCommon(
 	} else {
 		resp.AuthURL = fmt.Sprintf("%s/register/%s",
 			strings.TrimSuffix(h.cfg.ServerURL, "/"),
-			NodePublicKeyStripPrefix(registerRequest.NodeKey))
+			registerRequest.NodeKey)
 	}

 	respBody, err := h.marshalResponse(resp, machineKey)