From 2d3e52fa923fb42a9e59895301f5351c1ac92cd7 Mon Sep 17 00:00:00 2001
From: hc-github-team-secure-vault-core
 <82990506+hc-github-team-secure-vault-core@users.noreply.github.com>
Date: Wed, 15 Nov 2023 11:33:02 -0500
Subject: [PATCH] backport of commit 28e3507680d78dbf80b3edc78bc16119088d4ba2
 (#24142)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Robert Hanzlík <robi@junyks.cz>
---
 builtin/logical/pki/acme_challenges.go      |  1 +
 builtin/logical/pki/acme_challenges_test.go | 59 ++++++++++++++++++---
 changelog/22521.txt                         |  3 ++
 3 files changed, 55 insertions(+), 8 deletions(-)
 create mode 100644 changelog/22521.txt

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
+```