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.

(cherry picked from commit 7472e98f6c)
This commit is contained in:
Andrei Korviakov
2026-09-14 17:04:04 +03:00
committed by Kristoffer Dalby
parent aa4c952bca
commit ebe18cebff
2 changed files with 34 additions and 2 deletions
+7 -2
View File
@@ -1,6 +1,7 @@
package hscontrol
import (
"bytes"
"context"
"crypto/tls"
"errors"
@@ -1228,10 +1229,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
+27
View File
@@ -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))
}