From bcff0eaae7d5ff97ac8c49b1901ba6a5ab8dc223 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 23 Feb 2025 08:02:46 -0800 Subject: [PATCH] handle register auth errors (#2435) * handle register auth errors This commit handles register auth errors as the Tailscale clients expect. It returns the error as part of a tailcfg.RegisterResponse and not as a http error. In addition it fixes a nil pointer panic triggered by not handling the errors as part of this chain. Fixes #2434 Signed-off-by: Kristoffer Dalby * changelog Signed-off-by: Kristoffer Dalby --------- Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 7 +++ hscontrol/noise.go | 59 +++++++++++----------- integration/auth_key_test.go | 96 ++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6a18a5f..95c14d99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,13 @@ - View of config, policy, filter, ssh policy per node, connected nodes and DERPmap +## 0.25.1 (2025-02-18) + +### Changes + +- Fix issue where registration errors are sent correctly + [#2435](https://github.com/juanfont/headscale/pull/2435) + ## 0.25.0 (2025-02-11) ### BREAKING diff --git a/hscontrol/noise.go b/hscontrol/noise.go index ebd178a7..1269d032 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -230,6 +230,10 @@ func (ns *noiseServer) NoisePollNetMapHandler( } } +func regErr(err error) *tailcfg.RegisterResponse { + return &tailcfg.RegisterResponse{Error: err.Error()} +} + // NoiseRegistrationHandler handles the actual registration process of a node. func (ns *noiseServer) NoiseRegistrationHandler( writer http.ResponseWriter, @@ -241,52 +245,47 @@ func (ns *noiseServer) NoiseRegistrationHandler( return } - registerRequest, registerResponse, err := func() (*tailcfg.RegisterRequest, []byte, error) { + registerRequest, registerResponse := func() (*tailcfg.RegisterRequest, *tailcfg.RegisterResponse) { + var resp *tailcfg.RegisterResponse body, err := io.ReadAll(req.Body) if err != nil { - return nil, nil, err + return &tailcfg.RegisterRequest{}, regErr(err) } - var registerRequest tailcfg.RegisterRequest - if err := json.Unmarshal(body, ®isterRequest); err != nil { - return nil, nil, err + var regReq tailcfg.RegisterRequest + if err := json.Unmarshal(body, ®Req); err != nil { + return ®Req, regErr(err) } - ns.nodeKey = registerRequest.NodeKey + ns.nodeKey = regReq.NodeKey - resp, err := ns.headscale.handleRegister(req.Context(), registerRequest, ns.conn.Peer()) - // TODO(kradalby): Here we could have two error types, one that is surfaced to the client - // and one that returns 500. + resp, err = ns.headscale.handleRegister(req.Context(), regReq, ns.conn.Peer()) if err != nil { - return nil, nil, err + var httpErr HTTPError + if errors.As(err, &httpErr) { + resp = &tailcfg.RegisterResponse{ + Error: httpErr.Msg, + } + return ®Req, resp + } else { + } + return ®Req, regErr(err) } - respBody, err := json.Marshal(resp) - if err != nil { - return nil, nil, err - } - - return ®isterRequest, respBody, nil + return ®Req, resp }() - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Error handling registration") - http.Error(writer, "Internal server error", http.StatusInternalServerError) - } // Reject unsupported versions if rejectUnsupported(writer, registerRequest.Version, ns.machineKey, registerRequest.NodeKey) { return } + respBody, err := json.Marshal(registerResponse) + if err != nil { + httpError(writer, err) + return + } + writer.Header().Set("Content-Type", "application/json; charset=utf-8") writer.WriteHeader(http.StatusOK) - _, err = writer.Write(registerResponse) - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Failed to write response") - } + writer.Write(respBody) } diff --git a/integration/auth_key_test.go b/integration/auth_key_test.go index d1c2c5d1..a2bda02a 100644 --- a/integration/auth_key_test.go +++ b/integration/auth_key_test.go @@ -228,3 +228,99 @@ func TestAuthKeyLogoutAndReloginNewUser(t *testing.T) { assert.Equal(t, "user1@test.no", status.User[status.Self.UserID].LoginName) } } + +func TestAuthKeyLogoutAndReloginSameUserExpiredKey(t *testing.T) { + IntegrationSkip(t) + t.Parallel() + + for _, https := range []bool{true, false} { + t.Run(fmt.Sprintf("with-https-%t", https), func(t *testing.T) { + scenario, err := NewScenario(dockertestMaxWait()) + assertNoErr(t, err) + defer scenario.ShutdownAssertNoPanics(t) + + spec := map[string]int{ + "user1": len(MustTestVersions), + "user2": len(MustTestVersions), + } + + opts := []hsic.Option{hsic.WithTestName("pingallbyip")} + if https { + opts = append(opts, []hsic.Option{ + hsic.WithTLS(), + }...) + } + + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, opts...) + assertNoErrHeadscaleEnv(t, err) + + allClients, err := scenario.ListTailscaleClients() + assertNoErrListClients(t, err) + + err = scenario.WaitForTailscaleSync() + assertNoErrSync(t, err) + + // assertClientsState(t, allClients) + + clientIPs := make(map[TailscaleClient][]netip.Addr) + for _, client := range allClients { + ips, err := client.IPs() + if err != nil { + t.Fatalf("failed to get IPs for client %s: %s", client.Hostname(), err) + } + clientIPs[client] = ips + } + + headscale, err := scenario.Headscale() + assertNoErrGetHeadscale(t, err) + + listNodes, err := headscale.ListNodes() + assert.Equal(t, len(listNodes), len(allClients)) + nodeCountBeforeLogout := len(listNodes) + t.Logf("node count before logout: %d", nodeCountBeforeLogout) + + for _, client := range allClients { + err := client.Logout() + if err != nil { + t.Fatalf("failed to logout client %s: %s", client.Hostname(), err) + } + } + + err = scenario.WaitForTailscaleLogout() + assertNoErrLogout(t, err) + + t.Logf("all clients logged out") + + // if the server is not running with HTTPS, we have to wait a bit before + // reconnection as the newest Tailscale client has a measure that will only + // reconnect over HTTPS if they saw a noise connection previously. + // https://github.com/tailscale/tailscale/commit/1eaad7d3deb0815e8932e913ca1a862afa34db38 + // https://github.com/juanfont/headscale/issues/2164 + if !https { + time.Sleep(5 * time.Minute) + } + + for userName := range spec { + key, err := scenario.CreatePreAuthKey(userName, true, false) + if err != nil { + t.Fatalf("failed to create pre-auth key for user %s: %s", userName, err) + } + + // Expire the key so it can't be used + _, err = headscale.Execute( + []string{ + "headscale", + "preauthkeys", + "--user", + userName, + "expire", + key.Key, + }) + assertNoErr(t, err) + + err = scenario.RunTailscaleUp(userName, headscale.GetEndpoint(), key.GetKey()) + assert.ErrorContains(t, err, "authkey expired") + } + }) + } +}