From fa3d41f4525ae825551274410fb1ca5ce3a90d8b Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Mon, 17 Apr 2023 15:23:04 -0400 Subject: [PATCH] Add ACME HTTP-01 Challenge (#20141) * Add HTTP challenge validator This will attempt to safely validate HTTP challenges, following a limited number of redirects and timing out after too much time has passed. Signed-off-by: Alexander Scheel * Add test for ValidateKeyAuthorization Signed-off-by: Alexander Scheel * Add test cases for ValidateHTTP01Challenge Signed-off-by: Alexander Scheel * Add token to HTTP challenge Signed-off-by: Alexander Scheel --------- Signed-off-by: Alexander Scheel --- builtin/logical/pki/acme_challenges.go | 111 ++++++++++++ builtin/logical/pki/acme_challenges_test.go | 184 ++++++++++++++++++++ builtin/logical/pki/acme_state.go | 15 ++ builtin/logical/pki/path_acme_order.go | 30 +++- builtin/logical/pki/path_acme_test.go | 6 +- 5 files changed, 333 insertions(+), 13 deletions(-) create mode 100644 builtin/logical/pki/acme_challenges.go create mode 100644 builtin/logical/pki/acme_challenges_test.go diff --git a/builtin/logical/pki/acme_challenges.go b/builtin/logical/pki/acme_challenges.go new file mode 100644 index 000000000000..2a3e24468217 --- /dev/null +++ b/builtin/logical/pki/acme_challenges.go @@ -0,0 +1,111 @@ +package pki + +import ( + "fmt" + "io" + "net" + "net/http" + "strings" + "time" +) + +// ValidateKeyAuthorization validates that the given keyAuthz from a challenge +// matches our expectation, returning (true, nil) if so, or (false, err) if +// not. +func ValidateKeyAuthorization(keyAuthz string, token string, thumbprint string) (bool, error) { + parts := strings.Split(keyAuthz, ".") + if len(parts) != 2 { + return false, fmt.Errorf("invalid authorization: got %v parts, expected 2", len(parts)) + } + + tokenPart := parts[0] + thumbprintPart := parts[1] + + if token != tokenPart || thumbprint != thumbprintPart { + return false, fmt.Errorf("key authorization was invalid") + } + + return true, nil +} + +// Validates a given ACME http-01 challenge against the specified domain, +// per RFC 8555. +// +// We attempt to be defensive here against timeouts, extra redirects, &c. +func ValidateHTTP01Challenge(domain string, token string, thumbprint string) (bool, error) { + path := "http://" + domain + "/.well-known/acme-challenge/" + token + + transport := &http.Transport{ + // Only a single request is sent to this server as we do not do any + // batching of validation attempts. There is no need to do an HTTP + // KeepAlive as a result. + DisableKeepAlives: true, + MaxIdleConns: 1, + MaxIdleConnsPerHost: 1, + MaxConnsPerHost: 1, + IdleConnTimeout: 1 * time.Second, + + // We'd rather timeout and re-attempt validation later than hang + // too many validators waiting for slow hosts. + DialContext: (&net.Dialer{ + Timeout: 10 * time.Second, + KeepAlive: -1 * time.Second, + }).DialContext, + ResponseHeaderTimeout: 10 * time.Second, + } + + maxRedirects := 10 + urlLength := 2000 + + client := &http.Client{ + Transport: transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via)+1 >= maxRedirects { + return fmt.Errorf("http-01: too many redirects: %v", len(via)+1) + } + + reqUrlLen := len(req.URL.String()) + if reqUrlLen > urlLength { + return fmt.Errorf("http-01: redirect url length too long: %v", reqUrlLen) + } + + return nil + }, + } + + resp, err := client.Get(path) + if err != nil { + return false, fmt.Errorf("http-01: failed to fetch path %v: %w", path, err) + } + + // We provision a buffer which allows for a variable size challenge, some + // whitespace, and a detection gap for too long of a message. + minExpected := len(token) + 1 + len(thumbprint) + maxExpected := 512 + + defer resp.Body.Close() + + // Attempt to read the body, but don't do so infinitely. + body, err := io.ReadAll(io.LimitReader(resp.Body, int64(maxExpected+1))) + if err != nil { + return false, fmt.Errorf("http-01: unexpected error while reading body: %w", err) + } + + if len(body) > maxExpected { + return false, fmt.Errorf("http-01: response too large: received %v > %v bytes", len(body), maxExpected) + } + + if len(body) < minExpected { + return false, fmt.Errorf("http-01: response too small: received %v < %v bytes", len(body), minExpected) + } + + // Per RFC 8555 Section 8.3. HTTP Challenge: + // + // > The server SHOULD ignore whitespace characters at the end of the body. + keyAuthz := string(body) + keyAuthz = strings.TrimSpace(keyAuthz) + + // If we got here, we got no non-EOF error while reading. Try to validate + // the token because we're bounded by a reasonable amount of length. + return ValidateKeyAuthorization(keyAuthz, token, thumbprint) +} diff --git a/builtin/logical/pki/acme_challenges_test.go b/builtin/logical/pki/acme_challenges_test.go new file mode 100644 index 000000000000..c4792a335342 --- /dev/null +++ b/builtin/logical/pki/acme_challenges_test.go @@ -0,0 +1,184 @@ +package pki + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +type keyAuthorizationTestCase struct { + keyAuthz string + token string + thumbprint string + shouldFail bool +} + +var keyAuthorizationTestCases = []keyAuthorizationTestCase{ + { + // Entirely empty + "", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Both empty + ".", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Not equal + "non-.non-", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Empty thumbprint + "non-.", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Empty token + ".non-", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Wrong order + "non-empty-thumbprint.non-empty-token", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Too many pieces + "one.two.three", + "non-empty-token", + "non-empty-thumbprint", + true, + }, + { + // Valid + "non-empty-token.non-empty-thumbprint", + "non-empty-token", + "non-empty-thumbprint", + false, + }, +} + +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) + } + + expectedValid := !tc.shouldFail + if expectedValid != isValid { + t.Fatalf("[%d] got ret=%v, expected ret=%v (shouldFail=%v)", index, isValid, expectedValid, tc.shouldFail) + } + } +} + +func TestAcmeValidateHTTP01Challenge(t *testing.T) { + t.Parallel() + + for index, tc := range keyAuthorizationTestCases { + validFunc := func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(tc.keyAuthz)) + } + withPadding := func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(" " + tc.keyAuthz + " ")) + } + withRedirect := func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/.well-known/") { + http.Redirect(w, r, "/my-http-01-challenge-response", 301) + return + } + + w.Write([]byte(tc.keyAuthz)) + } + withSleep := func(w http.ResponseWriter, r *http.Request) { + // Long enough to ensure any excessively short timeouts are hit, + // not long enough to trigger a failure (hopefully). + time.Sleep(5 * time.Second) + w.Write([]byte(tc.keyAuthz)) + } + + validHandlers := []http.HandlerFunc{ + http.HandlerFunc(validFunc), http.HandlerFunc(withPadding), + http.HandlerFunc(withRedirect), http.HandlerFunc(withSleep), + } + + for handlerIndex, handler := range validHandlers { + func() { + ts := httptest.NewServer(handler) + defer ts.Close() + + host := ts.URL[7:] + isValid, err := ValidateHTTP01Challenge(host, tc.token, tc.thumbprint) + if !isValid && err == nil { + t.Fatalf("[tc=%d/handler=%d] expected failure to give reason via err (%v / %v)", handlerIndex, index, isValid, err) + } + + expectedValid := !tc.shouldFail + if expectedValid != isValid { + t.Fatalf("[tc=%d/handler=%d] got ret=%v (err=%v), expected ret=%v (shouldFail=%v)", index, handlerIndex, isValid, err, expectedValid, tc.shouldFail) + } + }() + } + } + + // Negative test cases for various HTTP-specific scenarios. + redirectLoop := func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/my-http-01-challenge-response", 301) + } + publicRedirect := func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://hashicorp.com/", 301) + } + noData := func(w http.ResponseWriter, r *http.Request) {} + noContent := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + } + notFound := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + } + simulateHang := func(w http.ResponseWriter, r *http.Request) { + time.Sleep(30 * time.Second) + w.Write([]byte("my-token.my-thumbprint")) + } + tooLarge := func(w http.ResponseWriter, r *http.Request) { + for i := 0; i < 512; i++ { + w.Write([]byte("my-token.my-thumbprint")) + } + } + + validHandlers := []http.HandlerFunc{ + http.HandlerFunc(redirectLoop), http.HandlerFunc(publicRedirect), + http.HandlerFunc(noData), http.HandlerFunc(noContent), + http.HandlerFunc(notFound), http.HandlerFunc(simulateHang), + http.HandlerFunc(tooLarge), + } + for handlerIndex, handler := range validHandlers { + func() { + ts := httptest.NewServer(handler) + defer ts.Close() + + host := ts.URL[7:] + isValid, err := ValidateHTTP01Challenge(host, "my-token", "my-thumbprint") + if isValid || err == nil { + t.Fatalf("[handler=%d] expected failure validating challenge (%v / %v)", handlerIndex, isValid, err) + } + }() + } +} diff --git a/builtin/logical/pki/acme_state.go b/builtin/logical/pki/acme_state.go index a3e586ec996e..a59ea1c04ad3 100644 --- a/builtin/logical/pki/acme_state.go +++ b/builtin/logical/pki/acme_state.go @@ -22,6 +22,13 @@ const ( // How long nonces are considered valid. nonceExpiry = 15 * time.Minute + // How many bytes are in a token. Per RFC 8555 Section + // 8.3. HTTP Challenge and Section 11.3 Token Entropy: + // + // > token (required, string): A random value that uniquely identifies + // > the challenge. This value MUST have at least 128 bits of entropy. + tokenBytes = 128 / 8 + // Path Prefixes acmePathPrefix = "acme/" acmeAccountPrefix = acmePathPrefix + "accounts/" @@ -46,6 +53,10 @@ func NewACMEState() *acmeState { } func generateNonce() (string, error) { + return generateRandomBase64(21) +} + +func generateRandomBase64(srcBytes int) (string, error) { data := make([]byte, 21) if _, err := io.ReadFull(rand.Reader, data); err != nil { return "", err @@ -447,3 +458,7 @@ func getAuthorizationPath(accountId string, authId string) string { func getOrderPath(accountId string, orderId string) string { return acmeAccountPrefix + accountId + "/orders/" + orderId } + +func getACMEToken() (string, error) { + return generateRandomBase64(tokenBytes) +} diff --git a/builtin/logical/pki/path_acme_order.go b/builtin/logical/pki/path_acme_order.go index f722ed6ab44f..d44ae5f6dac0 100644 --- a/builtin/logical/pki/path_acme_order.go +++ b/builtin/logical/pki/path_acme_order.go @@ -195,7 +195,10 @@ func (b *backend) acmeNewOrderHandler(ac *acmeContext, r *logical.Request, _ *fr var authorizations []*ACMEAuthorization var authorizationIds []string for _, identifier := range identifiers { - authz := generateAuthorization(ac, account, identifier) + authz, err := generateAuthorization(ac, account, identifier) + if err != nil { + return nil, fmt.Errorf("error generating authorizations: %w", err) + } authorizations = append(authorizations, authz) err = b.acmeState.SaveAuthorization(ac, authz) @@ -295,15 +298,24 @@ func buildOrderUrl(acmeCtx *acmeContext, orderId string) string { return acmeCtx.baseUrl.JoinPath("order", orderId).String() } -func generateAuthorization(acmeCtx *acmeContext, acct *acmeAccount, identifier *ACMEIdentifier) *ACMEAuthorization { +func generateAuthorization(acmeCtx *acmeContext, acct *acmeAccount, identifier *ACMEIdentifier) (*ACMEAuthorization, error) { authId := genUuid() + var challenges []*ACMEChallenge + for _, challengeType := range []ACMEChallengeType{ACMEHTTPChallenge} { + token, err := getACMEToken() + if err != nil { + return nil, err + } - challenges := []*ACMEChallenge{ - { - Type: ACMEHTTPChallenge, - Status: ACMEChallengePending, - ChallengeFields: map[string]interface{}{}, // TODO fill this in properly - }, + challenge := &ACMEChallenge{ + Type: challengeType, + Status: ACMEChallengePending, + ChallengeFields: map[string]interface{}{ + "token": token, + }, + } + + challenges = append(challenges, challenge) } return &ACMEAuthorization{ @@ -314,7 +326,7 @@ func generateAuthorization(acmeCtx *acmeContext, acct *acmeAccount, identifier * Expires: "", // only populated when it switches to valid. Challenges: challenges, Wildcard: strings.HasPrefix(identifier.Value, "*."), - } + }, nil } func parseOptRFC3339Field(data map[string]interface{}, keyName string) (time.Time, error) { diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index 2076874bf7f8..83aedb403352 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/builtin/logical/pki/path_acme_test.go @@ -134,8 +134,7 @@ func TestAcmeBasicWorkflow(t *testing.T) { require.True(t, auth.Challenges[0].Validated.IsZero(), "validated time should be 0 on challenge") require.Equal(t, "http-01", auth.Challenges[0].Type) - // TODO: This currently does fail - // require.NotEmpty(t, auth.Challenges[0].Token, "missing challenge token") + require.NotEmpty(t, auth.Challenges[0].Token, "missing challenge token") // Load a challenge directly challenge, err := acmeClient.GetChallenge(testCtx, auth.Challenges[0].URI) @@ -144,8 +143,7 @@ func TestAcmeBasicWorkflow(t *testing.T) { require.True(t, challenge.Validated.IsZero(), "validated time should be 0 on challenge") require.Equal(t, "http-01", challenge.Type) - // TODO: This currently does fail - // require.NotEmpty(t, challenge.Token, "missing challenge token") + require.NotEmpty(t, challenge.Token, "missing challenge token") // Deactivate account t.Logf("Testing deactivate account on %s", baseAcmeURL)