Merge branch 'main' into feature-random-suffix-on-collision

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Give a warning when running Headscale with reverse proxy improperly configured for WebSockets [#788](https://github.com/juanfont/headscale/pull/788)
 - Fix subnet routers with Primary Routes [#811](https://github.com/juanfont/headscale/pull/811)
 - Added support for JSON logs [#653](https://github.com/juanfont/headscale/issues/653)
+- Sanitise the node key passed to registration url [#823](https://github.com/juanfont/headscale/pull/823)
 - Add support for generating pre-auth keys with tags [#767](https://github.com/juanfont/headscale/pull/767)
 - Add support for evaluating `autoApprovers` ACL entries when a machine is registered [#763](https://github.com/juanfont/headscale/pull/763)
 - Add config flag to allow Headscale to start if OIDC provider is down [#829](https://github.com/juanfont/headscale/pull/829)

Makefile

@@ -27,16 +27,40 @@ test:
 test_integration: test_integration_cli test_integration_derp test_integration_oidc test_integration_general
 
 test_integration_cli:
-	go test -failfast -tags integration_cli,integration -timeout 30m -count=1 ./...
+	docker network rm $$(docker network ls --filter name=headscale --quiet) || true
+	docker network create headscale-test || true
+	docker run -t --rm \
+		--network headscale-test \
+		-v $$PWD:$$PWD -w $$PWD \
+		-v /var/run/docker.sock:/var/run/docker.sock golang:1 \
+		go test -failfast -tags integration_cli,integration -timeout 30m -count=1 ./...
 
 test_integration_derp:
-	go test -failfast -tags integration_derp,integration -timeout 30m -count=1 ./...
+	docker network rm $$(docker network ls --filter name=headscale --quiet) || true
+	docker network create headscale-test || true
+	docker run -t --rm \
+		--network headscale-test \
+		-v $$PWD:$$PWD -w $$PWD \
+		-v /var/run/docker.sock:/var/run/docker.sock golang:1 \
+		go test -failfast -tags integration_derp,integration -timeout 30m -count=1 ./...
 
 test_integration_general:
-	go test -failfast -tags integration_general,integration -timeout 30m -count=1 ./...
+	docker network rm $$(docker network ls --filter name=headscale --quiet) || true
+	docker network create headscale-test || true
+	docker run -t --rm \
+		--network headscale-test \
+		-v $$PWD:$$PWD -w $$PWD \
+		-v /var/run/docker.sock:/var/run/docker.sock golang:1 \
+		go test -failfast -tags integration_general,integration -timeout 30m -count=1 ./...
 
 test_integration_oidc:
-	go test -failfast -tags integration_oidc,integration -timeout 30m -count=1 ./...
+	docker network rm $$(docker network ls --filter name=headscale --quiet) || true
+	docker network create headscale-test || true
+	docker run -t --rm \
+		--network headscale-test \
+		-v $$PWD:$$PWD -w $$PWD \
+		-v /var/run/docker.sock:/var/run/docker.sock golang:1 \
+		go test -failfast -tags integration_oidc,integration -timeout 30m -count=1 ./...
 
 coverprofile_func:
 	go tool cover -func=coverage.out

api.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/rs/zerolog/log"
+	"tailscale.com/types/key"
 )
 
 const (
@@ -93,7 +94,34 @@ func (h *Headscale) RegisterWebAPI(
 ) {
 	vars := mux.Vars(req)
 	nodeKeyStr, ok := vars["nkey"]
-	if !ok || nodeKeyStr == "" {
+
+	if !NodePublicKeyRegex.Match([]byte(nodeKeyStr)) {
+		log.Warn().Str("node_key", nodeKeyStr).Msg("Invalid node key passed to registration url")
+
+		writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
+		writer.WriteHeader(http.StatusUnauthorized)
+		_, err := writer.Write([]byte("Unauthorized"))
+		if err != nil {
+			log.Error().
+				Caller().
+				Err(err).
+				Msg("Failed to write response")
+		}
+
+		return
+	}
+
+	// We need to make sure we dont open for XSS style injections, if the parameter that
+	// is passed as a key is not parsable/validated as a NodePublic key, then fail to render
+	// the template and log an error.
+	var nodeKey key.NodePublic
+	err := nodeKey.UnmarshalText(
+		[]byte(NodePublicKeyEnsurePrefix(nodeKeyStr)),
+	)
+
+	if !ok || nodeKeyStr == "" || err != nil {
+		log.Warn().Err(err).Msg("Failed to parse incoming nodekey")
+
 		writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
 		writer.WriteHeader(http.StatusBadRequest)
 		_, err := writer.Write([]byte("Wrong params"))
@@ -130,7 +158,7 @@ func (h *Headscale) RegisterWebAPI(
 
 	writer.Header().Set("Content-Type", "text/html; charset=utf-8")
 	writer.WriteHeader(http.StatusOK)
-	_, err := writer.Write(content.Bytes())
+	_, err = writer.Write(content.Bytes())
 	if err != nil {
 		log.Error().
 			Caller().

cmd/headscale/cli/mockoidc.go

@@ -46,6 +46,10 @@ func mockOIDC() error {
 	if clientSecret == "" {
 		return errMockOidcClientSecretNotDefined
 	}
+	addrStr := os.Getenv("MOCKOIDC_ADDR")
+	if addrStr == "" {
+		return errMockOidcPortNotDefined
+	}
 	portStr := os.Getenv("MOCKOIDC_PORT")
 	if portStr == "" {
 		return errMockOidcPortNotDefined
@@ -61,7 +65,7 @@ func mockOIDC() error {
 		return err
 	}
 
-	listener, err := net.Listen("tcp", fmt.Sprintf("mockoidc:%d", port))
+	listener, err := net.Listen("tcp", fmt.Sprintf("%s:%d", addrStr, port))
 	if err != nil {
 		return err
 	}

flake.nix

@@ -128,7 +128,13 @@
       };
     in rec {
       # `nix develop`
-      devShell = pkgs.mkShell {buildInputs = devDeps;};
+      devShell = pkgs.mkShell {
+        buildInputs = devDeps;
+
+        shellHook = ''
+          export GOFLAGS=-tags="integration,integration_general,integration_oidc,integration_cli,integration_derp"
+        '';
+      };
 
       # `nix build`
       packages = with pkgs; {

integration_cli_test.go

@@ -13,6 +13,7 @@ import (
 
 	v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
 	"github.com/ory/dockertest/v3"
+	"github.com/ory/dockertest/v3/docker"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 )
@@ -42,11 +43,11 @@ func (s *IntegrationCLITestSuite) SetupTest() {
 		s.FailNow(fmt.Sprintf("Could not connect to docker: %s", err), "")
 	}
 
-	if pnetwork, err := s.pool.CreateNetwork("headscale-test"); err == nil {
-		s.network = *pnetwork
-	} else {
-		s.FailNow(fmt.Sprintf("Could not create network: %s", err), "")
+	network, err := GetFirstOrCreateNetwork(&s.pool, headscaleNetwork)
+	if err != nil {
+		s.FailNow(fmt.Sprintf("Failed to create or get network: %s", err), "")
 	}
+	s.network = network
 
 	headscaleBuildOptions := &dockertest.BuildOptions{
 		Dockerfile: "Dockerfile",
@@ -63,8 +64,12 @@ func (s *IntegrationCLITestSuite) SetupTest() {
 		Mounts: []string{
 			fmt.Sprintf("%s/integration_test/etc:/etc/headscale", currentPath),
 		},
-		Networks: []*dockertest.Network{&s.network},
-		Cmd:      []string{"headscale", "serve"},
+		Cmd:          []string{"headscale", "serve"},
+		Networks:     []*dockertest.Network{&s.network},
+		ExposedPorts: []string{"8080/tcp"},
+		PortBindings: map[docker.Port][]docker.PortBinding{
+			"8080/tcp": {{HostPort: "8080"}},
+		},
 	}
 
 	err = s.pool.RemoveContainerByName(headscaleHostname)
@@ -87,7 +92,9 @@ func (s *IntegrationCLITestSuite) SetupTest() {
 	fmt.Println("Created headscale container for CLI tests")
 
 	fmt.Println("Waiting for headscale to be ready for CLI tests")
-	hostEndpoint := fmt.Sprintf("localhost:%s", s.headscale.GetPort("8080/tcp"))
+	hostEndpoint := fmt.Sprintf("%s:%s",
+		s.headscale.GetIPInNetwork(&s.network),
+		s.headscale.GetPort("8080/tcp"))
 
 	if err := s.pool.Retry(func() error {
 		url := fmt.Sprintf("http://%s/health", hostEndpoint)

integration_common_test.go

@@ -19,7 +19,8 @@ import (
 )
 
 const (
-	headscaleHostname      = "headscale-derp"
+	headscaleNetwork       = "headscale-test"
+	headscaleHostname      = "headscale"
 	DOCKER_EXECUTE_TIMEOUT = 10 * time.Second
 )
 
@@ -32,7 +33,7 @@ var (
 	tailscaleVersions = []string{
 		// "head",
 		// "unstable",
-		"1.30.0",
+		"1.30.2",
 		"1.28.0",
 		"1.26.2",
 		"1.24.2",
@@ -115,13 +116,19 @@ func ExecuteCommand(
 			fmt.Println("stdout: ", stdout.String())
 			fmt.Println("stderr: ", stderr.String())
 
-			return stdout.String(), stderr.String(), fmt.Errorf("command failed with: %s", stderr.String())
+			return stdout.String(), stderr.String(), fmt.Errorf(
+				"command failed with: %s",
+				stderr.String(),
+			)
 		}
 
 		return stdout.String(), stderr.String(), nil
 	case <-time.After(execConfig.timeout):
 
-		return stdout.String(), stderr.String(), fmt.Errorf("command timed out after %s", execConfig.timeout)
+		return stdout.String(), stderr.String(), fmt.Errorf(
+			"command timed out after %s",
+			execConfig.timeout,
+		)
 	}
 }
 
@@ -316,3 +323,22 @@ func GetEnvBool(key string) (bool, error) {
 
 	return v, nil
 }
+
+func GetFirstOrCreateNetwork(pool *dockertest.Pool, name string) (dockertest.Network, error) {
+	networks, err := pool.NetworksByName(name)
+	if err != nil || len(networks) == 0 {
+
+		if _, err := pool.CreateNetwork(name); err == nil {
+			// Create does not give us an updated version of the resource, so we need to
+			// get it again.
+			networks, err := pool.NetworksByName(name)
+			if err != nil {
+				return dockertest.Network{}, err
+			}
+
+			return networks[0], nil
+		}
+	}
+
+	return networks[0], nil
+}

integration_embedded_derp_test.go

@@ -27,18 +27,20 @@ import (
 )
 
 const (
-	namespaceName   = "derpnamespace"
-	totalContainers = 3
+	headscaleDerpHostname = "headscale-derp"
+	namespaceName         = "derpnamespace"
+	totalContainers       = 3
 )
 
 type IntegrationDERPTestSuite struct {
 	suite.Suite
 	stats *suite.SuiteInformation
 
-	pool      dockertest.Pool
-	networks  map[int]dockertest.Network // so we keep the containers isolated
-	headscale dockertest.Resource
-	saveLogs  bool
+	pool              dockertest.Pool
+	network           dockertest.Network
+	containerNetworks map[int]dockertest.Network // so we keep the containers isolated
+	headscale         dockertest.Resource
+	saveLogs          bool
 
 	tailscales    map[string]dockertest.Resource
 	joinWaitGroup sync.WaitGroup
@@ -53,7 +55,7 @@ func TestDERPIntegrationTestSuite(t *testing.T) {
 	s := new(IntegrationDERPTestSuite)
 
 	s.tailscales = make(map[string]dockertest.Resource)
-	s.networks = make(map[int]dockertest.Network)
+	s.containerNetworks = make(map[int]dockertest.Network)
 	s.saveLogs = saveLogs
 
 	suite.Run(t, s)
@@ -78,7 +80,7 @@ func TestDERPIntegrationTestSuite(t *testing.T) {
 			log.Printf("Could not purge resource: %s\n", err)
 		}
 
-		for _, network := range s.networks {
+		for _, network := range s.containerNetworks {
 			if err := network.Close(); err != nil {
 				log.Printf("Could not close network: %s\n", err)
 			}
@@ -93,9 +95,15 @@ func (s *IntegrationDERPTestSuite) SetupSuite() {
 		s.FailNow(fmt.Sprintf("Could not connect to docker: %s", err), "")
 	}
 
+	network, err := GetFirstOrCreateNetwork(&s.pool, headscaleNetwork)
+	if err != nil {
+		s.FailNow(fmt.Sprintf("Failed to create or get network: %s", err), "")
+	}
+	s.network = network
+
 	for i := 0; i < totalContainers; i++ {
 		if pnetwork, err := s.pool.CreateNetwork(fmt.Sprintf("headscale-derp-%d", i)); err == nil {
-			s.networks[i] = *pnetwork
+			s.containerNetworks[i] = *pnetwork
 		} else {
 			s.FailNow(fmt.Sprintf("Could not create network: %s", err), "")
 		}
@@ -112,22 +120,24 @@ func (s *IntegrationDERPTestSuite) SetupSuite() {
 	}
 
 	headscaleOptions := &dockertest.RunOptions{
-		Name: headscaleHostname,
+
+		Name: headscaleDerpHostname,
 		Mounts: []string{
 			fmt.Sprintf(
 				"%s/integration_test/etc_embedded_derp:/etc/headscale",
 				currentPath,
 			),
 		},
 		Cmd:          []string{"headscale", "serve"},
+		Networks:     []*dockertest.Network{&s.network},
 		ExposedPorts: []string{"8443/tcp", "3478/udp"},
 		PortBindings: map[docker.Port][]docker.PortBinding{
 			"8443/tcp": {{HostPort: "8443"}},
 			"3478/udp": {{HostPort: "3478"}},
 		},
 	}
 
-	err = s.pool.RemoveContainerByName(headscaleHostname)
+	err = s.pool.RemoveContainerByName(headscaleDerpHostname)
 	if err != nil {
 		s.FailNow(
 			fmt.Sprintf(
@@ -153,13 +163,15 @@ func (s *IntegrationDERPTestSuite) SetupSuite() {
 		hostname, container := s.tailscaleContainer(
 			fmt.Sprint(i),
 			version,
-			s.networks[i],
+			s.containerNetworks[i],
 		)
 		s.tailscales[hostname] = *container
 	}
 
 	log.Println("Waiting for headscale to be ready for embedded DERP tests")
-	hostEndpoint := fmt.Sprintf("localhost:%s", s.headscale.GetPort("8443/tcp"))
+	hostEndpoint := fmt.Sprintf("%s:%s",
+		s.headscale.GetIPInNetwork(&s.network),
+		s.headscale.GetPort("8443/tcp"))
 
 	if err := s.pool.Retry(func() error {
 		url := fmt.Sprintf("https://%s/health", hostEndpoint)
@@ -320,7 +332,7 @@ func (s *IntegrationDERPTestSuite) TearDownSuite() {
 			log.Printf("Could not purge resource: %s\n", err)
 		}
 
-		for _, network := range s.networks {
+		for _, network := range s.containerNetworks {
 			if err := network.Close(); err != nil {
 				log.Printf("Could not close network: %s\n", err)
 			}
@@ -428,7 +440,9 @@ func (s *IntegrationDERPTestSuite) TestPingAllPeersByHostname() {
 }
 
 func (s *IntegrationDERPTestSuite) TestDERPSTUN() {
-	headscaleSTUNAddr := fmt.Sprintf("localhost:%s", s.headscale.GetPort("3478/udp"))
+	headscaleSTUNAddr := fmt.Sprintf("%s:%s",
+		s.headscale.GetIPInNetwork(&s.network),
+		s.headscale.GetPort("3478/udp"))
 	client := stun.NewClient()
 	client.SetVerbose(true)
 	client.SetVVerbose(true)