diff --git a/builtin/logical/pki/acme_challenges.go b/builtin/logical/pki/acme_challenges.go index 90891bd18..54441e0bc 100644 --- a/builtin/logical/pki/acme_challenges.go +++ b/builtin/logical/pki/acme_challenges.go @@ -138,6 +138,7 @@ func ValidateHTTP01Challenge(domain string, token string, thumbprint string, con MaxIdleConnsPerHost: 1, MaxConnsPerHost: 1, IdleConnTimeout: 1 * time.Second, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // We'd rather timeout and re-attempt validation later than hang // too many validators waiting for slow hosts. diff --git a/builtin/logical/pki/acme_challenges_test.go b/builtin/logical/pki/acme_challenges_test.go index 11c2cf279..ed5dfa4cb 100644 --- a/builtin/logical/pki/acme_challenges_test.go +++ b/builtin/logical/pki/acme_challenges_test.go @@ -19,6 +19,7 @@ import ( "math/big" "net/http" "net/http/httptest" + "strconv" "strings" "testing" "time" @@ -99,15 +100,17 @@ func TestAcmeValidateKeyAuthorization(t *testing.T) { t.Parallel() for index, tc := range keyAuthorizationTestCases { - isValid, err := ValidateKeyAuthorization(tc.keyAuthz, tc.token, tc.thumbprint) - if !isValid && err == nil { - t.Fatalf("[%d] expected failure to give reason via err (%v / %v)", index, isValid, err) - } + t.Run("subtest-"+strconv.Itoa(index), func(st *testing.T) { + isValid, err := ValidateKeyAuthorization(tc.keyAuthz, tc.token, tc.thumbprint) + if !isValid && err == nil { + st.Fatalf("[%d] expected failure to give reason via err (%v / %v)", index, isValid, err) + } - expectedValid := !tc.shouldFail - if expectedValid != isValid { - t.Fatalf("[%d] got ret=%v, expected ret=%v (shouldFail=%v)", index, isValid, expectedValid, tc.shouldFail) - } + expectedValid := !tc.shouldFail + if expectedValid != isValid { + st.Fatalf("[%d] got ret=%v, expected ret=%v (shouldFail=%v)", index, isValid, expectedValid, tc.shouldFail) + } + }) } } @@ -714,3 +717,43 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) { } } } + +// TestAcmeValidateHttp01TLSRedirect verify that we allow a http-01 challenge to redirect +// to a TLS server and not validate the certificate chain is valid. We don't validate the +// TLS chain as we would have accepted the auth over a non-secured channel anyway had +// the original request not redirected us. +func TestAcmeValidateHttp01TLSRedirect(t *testing.T) { + t.Parallel() + + for index, tc := range keyAuthorizationTestCases { + t.Run("subtest-"+strconv.Itoa(index), func(st *testing.T) { + validFunc := func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/.well-known/") { + w.Write([]byte(tc.keyAuthz)) + return + } + http.Error(w, "status not found", http.StatusNotFound) + } + + tlsTs := httptest.NewTLSServer(http.HandlerFunc(validFunc)) + defer tlsTs.Close() + + // Set up a http server that will redirect to our TLS server + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, tlsTs.URL+r.URL.Path, 301) + })) + defer ts.Close() + + host := ts.URL[len("http://"):] + isValid, err := ValidateHTTP01Challenge(host, tc.token, tc.thumbprint, &acmeConfigEntry{}) + if !isValid && err == nil { + st.Fatalf("[tc=%d] expected failure to give reason via err (%v / %v)", index, isValid, err) + } + + expectedValid := !tc.shouldFail + if expectedValid != isValid { + st.Fatalf("[tc=%d] got ret=%v (err=%v), expected ret=%v (shouldFail=%v)", index, isValid, err, expectedValid, tc.shouldFail) + } + }) + } +} diff --git a/changelog/22521.txt b/changelog/22521.txt new file mode 100644 index 000000000..9310b64c1 --- /dev/null +++ b/changelog/22521.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: do not check TLS validity on ACME requests redirected to https +```