backport of commit 28e3507680 (#24143)

Co-authored-by: Robert Hanzlík <robi@junyks.cz>

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.

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)
+		t.Run("subtest-"+strconv.Itoa(index), func(st *testing.T) {
-		if !isValid && err == nil {
+			isValid, err := ValidateKeyAuthorization(tc.keyAuthz, tc.token, tc.thumbprint)
-			t.Fatalf("[%d] expected failure to give reason via err (%v / %v)", index, isValid, err)
+			if !isValid && err == nil {
-		}
+				st.Fatalf("[%d] expected failure to give reason via err (%v / %v)", index, isValid, err)
+			}
 
-		expectedValid := !tc.shouldFail
+			expectedValid := !tc.shouldFail
-		if expectedValid != isValid {
+			if expectedValid != isValid {
-			t.Fatalf("[%d] got ret=%v, expected ret=%v (shouldFail=%v)", index, isValid, expectedValid, tc.shouldFail)
+				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)
+			}
+		})
+	}
+}

changelog/22521.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: do not check TLS validity on ACME requests redirected to https
+```