From 7472e98f6c107fd4ed9eb9181e7227e2799e7983 Mon Sep 17 00:00:00 2001 From: Andrei Korviakov <4lifenet@gmail.com> Date: Mon, 14 Sep 2026 17:04:04 +0300 Subject: [PATCH] hscontrol: keep the ACME error body readable in acmeLogger acmeLogger drained and closed the body of every ACME error response before handing the response back to golang.org/x/crypto/acme. The client parses that body to classify errors, so badNonce was no longer recognised: isBadNonce returned false, clearNonces was never called and Client.post treated the 400 as non-retriable. One badNonce reply therefore stops certificate renewal for good. autocert retries every 30-60 minutes, each attempt reuses a nonce stored from the previous failed response, that nonce has already expired, and the loop repeats until the process is restarted or the certificate expires. Restore the body with a fresh reader after logging it. --- CHANGELOG.md | 9 +++++++++ hscontrol/app.go | 9 +++++++-- hscontrol/app_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e6be2f3..83874789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,15 @@ keys remain all-access. - Fix tailsql not shutting down with headscale, leaving the process hanging on graceful shutdown [#3400](https://github.com/juanfont/headscale/pull/3400) - Fix tvOS setup instructions: install the VPN configuration before setting the coordination server URL [#3431](https://github.com/juanfont/headscale/pull/3431) - Map requests that only bump LastSeen, endpoints or DERP region no longer resend the whole node to every peer, and health probes that change nothing no longer write. Adds `headscale_mapper_changes_dropped_total` and `headscale_ha_health_updates_total` [#3417](https://github.com/juanfont/headscale/issues/3417) [#3450](https://github.com/juanfont/headscale/pull/3450) +- Fix ACME renewal stopping permanently after a `badNonce` reply, because the error logging middleware drained the response body the acme client needs to detect it [#3461](https://github.com/juanfont/headscale/pull/3461) + +## 0.29.4 (202x-xx-xx) + +**Minimum supported Tailscale client version: v1.80.0** + +### Changes + +- Lowercase DNS extra record names so mixed-case records resolve [#3366](https://github.com/juanfont/headscale/pull/3366) ## 0.29.3 (2026-07-29) diff --git a/hscontrol/app.go b/hscontrol/app.go index b763e3ac..b4440d85 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -1,6 +1,7 @@ package hscontrol import ( + "bytes" "context" "crypto/tls" "errors" @@ -1095,10 +1096,14 @@ func (l *acmeLogger) RoundTrip(req *http.Request) (*http.Response, error) { } if resp.StatusCode >= http.StatusBadRequest { - defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + log.Error().Int("status_code", resp.StatusCode).Str("url", req.URL.String()).Bytes("body", body).Msg("acme request returned error") + + // The ACME client parses this body to classify errors such as badNonce, + // so give it back a readable copy. + resp.Body = io.NopCloser(bytes.NewReader(body)) } return resp, nil diff --git a/hscontrol/app_test.go b/hscontrol/app_test.go index 75397b5f..60956932 100644 --- a/hscontrol/app_test.go +++ b/hscontrol/app_test.go @@ -2,11 +2,13 @@ package hscontrol import ( "context" + "io" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSecurityHeaders(t *testing.T) { @@ -24,3 +26,28 @@ func TestSecurityHeaders(t *testing.T) { assert.Equal(t, "nosniff", h.Get("X-Content-Type-Options")) assert.Equal(t, "no-referrer", h.Get("Referrer-Policy")) } + +func TestACMELoggerKeepsErrorBodyReadable(t *testing.T) { + const problem = `{"type":"urn:ietf:params:acme:error:badNonce","detail":"JWS has an invalid anti-replay nonce","status":400}` + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/problem+json") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(problem)) + })) + defer server.Close() + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, server.URL, nil) + require.NoError(t, err) + + resp, err := (&acmeLogger{rt: http.DefaultTransport}).RoundTrip(req) + require.NoError(t, err) + + defer resp.Body.Close() + + // acme parses the body to classify errors such as badNonce, so it has to + // survive the logging middleware. + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.JSONEq(t, problem, string(body)) +}