From 07f92e647c01e7faf569a03ec2b5597b55bb1a00 Mon Sep 17 00:00:00 2001 From: LiuHanCheng Date: Sat, 5 Nov 2022 16:07:22 +0800 Subject: [PATCH] fix bug in #912 (#914) --- .golangci.yaml | 1 + cmd/headscale/cli/debug.go | 8 +++---- grpcv1.go | 10 +++++++-- integration_cli_test.go | 44 +++++++++++++++++++------------------- integration_oidc_test.go | 2 +- machine.go | 8 ++++++- oidc.go | 3 +-- protocol_common.go | 6 +++--- 8 files changed, 47 insertions(+), 35 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index d09a2827..5d7c4901 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -26,6 +26,7 @@ linters: - ireturn - execinquery - exhaustruct + - nolintlint # We should strive to enable these: - wrapcheck diff --git a/cmd/headscale/cli/debug.go b/cmd/headscale/cli/debug.go index 8010a9be..0e0e6ff2 100644 --- 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), diff --git a/grpcv1.go b/grpcv1.go index 9fac9aff..25ee7777 100644 --- 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, ) diff --git a/integration_cli_test.go b/integration_cli_test.go index ee6a9e1f..5a5cb0c8 100644 --- 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, diff --git a/integration_oidc_test.go b/integration_oidc_test.go index 8ca46db7..6dd79bed 100644 --- a/integration_oidc_test.go +++ b/integration_oidc_test.go @@ -1,4 +1,4 @@ -//nolint +// nolint package headscale import ( diff --git a/machine.go b/machine.go index 39e5110a..b688be65 100644 --- 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 { diff --git a/oidc.go b/oidc.go index f0af600e..d3423977 100644 --- 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 { diff --git a/protocol_common.go b/protocol_common.go index c6bc2ee5..fa9ccc4e 100644 --- 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)