From 7361ce1e5783246957b9fe9ab185c0302f38a13b Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Mon, 17 Apr 2023 13:52:54 -0400 Subject: [PATCH] Add tests for fetching ACME authorizations and challenges (#20205) - Add tests to validate that we can load authorizations and challenges from the server --- builtin/logical/pki/acme_authorizations.go | 13 +++++--- builtin/logical/pki/acme_wrappers.go | 5 +-- .../logical/pki/path_acme_authorizations.go | 4 +-- builtin/logical/pki/path_acme_challenges.go | 2 +- builtin/logical/pki/path_acme_order.go | 19 +++++++---- builtin/logical/pki/path_acme_test.go | 32 ++++++++++++++++++- 6 files changed, 57 insertions(+), 18 deletions(-) diff --git a/builtin/logical/pki/acme_authorizations.go b/builtin/logical/pki/acme_authorizations.go index cdea65f6f..dd77a3388 100644 --- a/builtin/logical/pki/acme_authorizations.go +++ b/builtin/logical/pki/acme_authorizations.go @@ -58,17 +58,16 @@ const ( type ACMEChallenge struct { Type ACMEChallengeType `json:"type"` - URL string `json:"url"` Status ACMEChallengeStatusType `json:"status"` Validated string `json:"validated,optional"` Error map[string]interface{} `json:"error,optional"` ChallengeFields map[string]interface{} `json:"challenge_fields"` } -func (ac *ACMEChallenge) NetworkMarshal() map[string]interface{} { +func (ac *ACMEChallenge) NetworkMarshal(acmeCtx *acmeContext, authId string) map[string]interface{} { resp := map[string]interface{}{ "type": ac.Type, - "url": ac.URL, + "url": buildChallengeUrl(acmeCtx, authId, string(ac.Type)), "status": ac.Status, } @@ -87,6 +86,10 @@ func (ac *ACMEChallenge) NetworkMarshal() map[string]interface{} { return resp } +func buildChallengeUrl(acmeCtx *acmeContext, authId, challengeType string) string { + return acmeCtx.baseUrl.JoinPath("/challenge/", authId, challengeType).String() +} + type ACMEAuthorization struct { Id string `json:"id"` AccountId string `json:"account_id"` @@ -112,7 +115,7 @@ func (aa *ACMEAuthorization) GetExpires() (time.Time, error) { return time.Parse(time.RFC3339, aa.Expires) } -func (aa *ACMEAuthorization) NetworkMarshal() map[string]interface{} { +func (aa *ACMEAuthorization) NetworkMarshal(acmeCtx *acmeContext) map[string]interface{} { resp := map[string]interface{}{ "identifier": aa.Identifier, "status": aa.Status, @@ -126,7 +129,7 @@ func (aa *ACMEAuthorization) NetworkMarshal() map[string]interface{} { if len(aa.Challenges) > 0 { challenges := []map[string]interface{}{} for _, challenge := range aa.Challenges { - challenges = append(challenges, challenge.NetworkMarshal()) + challenges = append(challenges, challenge.NetworkMarshal(acmeCtx, aa.Id)) } resp["challenges"] = challenges } diff --git a/builtin/logical/pki/acme_wrappers.go b/builtin/logical/pki/acme_wrappers.go index 5a2ee1f06..6a45f578e 100644 --- a/builtin/logical/pki/acme_wrappers.go +++ b/builtin/logical/pki/acme_wrappers.go @@ -16,6 +16,7 @@ import ( ) type acmeContext struct { + // baseUrl is the combination of the configured cluster local URL and the acmePath up to /acme/ baseUrl *url.URL sc *storageContext } @@ -46,13 +47,13 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc { return nil, fmt.Errorf("ACME is disabled in configuration: %w", ErrServerInternal) } - baseUrl, err := getAcmeBaseUrl(sc, r.Path) + acmeBaseUrl, err := getAcmeBaseUrl(sc, r.Path) if err != nil { return nil, err } acmeCtx := &acmeContext{ - baseUrl: baseUrl, + baseUrl: acmeBaseUrl, sc: sc, } diff --git a/builtin/logical/pki/path_acme_authorizations.go b/builtin/logical/pki/path_acme_authorizations.go index 0c2ffbf25..90c3fc4a2 100644 --- a/builtin/logical/pki/path_acme_authorizations.go +++ b/builtin/logical/pki/path_acme_authorizations.go @@ -76,7 +76,7 @@ func (b *backend) acmeAuthorizationHandler(acmeCtx *acmeContext, r *logical.Requ func (b *backend) acmeAuthorizationFetchHandler(acmeCtx *acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *jwsCtx, data map[string]interface{}, authz *ACMEAuthorization) (*logical.Response, error) { return &logical.Response{ - Data: authz.NetworkMarshal(), + Data: authz.NetworkMarshal(acmeCtx), }, nil } @@ -95,6 +95,6 @@ func (b *backend) acmeAuthorizationDeactivateHandler(acmeCtx *acmeContext, r *lo } return &logical.Response{ - Data: authz.NetworkMarshal(), + Data: authz.NetworkMarshal(acmeCtx), }, nil } diff --git a/builtin/logical/pki/path_acme_challenges.go b/builtin/logical/pki/path_acme_challenges.go index 1f1432514..812c42643 100644 --- a/builtin/logical/pki/path_acme_challenges.go +++ b/builtin/logical/pki/path_acme_challenges.go @@ -90,6 +90,6 @@ func (b *backend) acmeChallengeFetchHandler(acmeCtx *acmeContext, r *logical.Req // XXX: Prompt for challenge to be tried by the server. return &logical.Response{ - Data: challenge.NetworkMarshal(), + Data: challenge.NetworkMarshal(acmeCtx, authz.Id), }, nil } diff --git a/builtin/logical/pki/path_acme_order.go b/builtin/logical/pki/path_acme_order.go index b387ed487..f722ed6ab 100644 --- a/builtin/logical/pki/path_acme_order.go +++ b/builtin/logical/pki/path_acme_order.go @@ -195,7 +195,7 @@ func (b *backend) acmeNewOrderHandler(ac *acmeContext, r *logical.Request, _ *fr var authorizations []*ACMEAuthorization var authorizationIds []string for _, identifier := range identifiers { - authz := generateAuthorization(account, identifier) + authz := generateAuthorization(ac, account, identifier) authorizations = append(authorizations, authz) err = b.acmeState.SaveAuthorization(ac, authz) @@ -263,7 +263,7 @@ func formatOrderResponse(acmeCtx *acmeContext, order *acmeOrder) *logical.Respon var authorizationUrls []string for _, authId := range order.AuthorizationIds { - authorizationUrls = append(authorizationUrls, acmeCtx.baseUrl.String()+"authz/"+authId) + authorizationUrls = append(authorizationUrls, buildAuthorizationUrl(acmeCtx, authId)) } resp := &logical.Response{ @@ -287,22 +287,27 @@ func formatOrderResponse(acmeCtx *acmeContext, order *acmeOrder) *logical.Respon return resp } -func buildOrderUrl(acmeCtx *acmeContext, orderId string) string { - return acmeCtx.baseUrl.String() + "order/" + orderId +func buildAuthorizationUrl(acmeCtx *acmeContext, authId string) string { + return acmeCtx.baseUrl.JoinPath("authorization", authId).String() } -func generateAuthorization(acct *acmeAccount, identifier *ACMEIdentifier) *ACMEAuthorization { +func buildOrderUrl(acmeCtx *acmeContext, orderId string) string { + return acmeCtx.baseUrl.JoinPath("order", orderId).String() +} + +func generateAuthorization(acmeCtx *acmeContext, acct *acmeAccount, identifier *ACMEIdentifier) *ACMEAuthorization { + authId := genUuid() + challenges := []*ACMEChallenge{ { Type: ACMEHTTPChallenge, - URL: genUuid(), Status: ACMEChallengePending, ChallengeFields: map[string]interface{}{}, // TODO fill this in properly }, } return &ACMEAuthorization{ - Id: genUuid(), + Id: authId, AccountId: acct.KeyId, Identifier: identifier, Status: ACMEAuthorizationPending, diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index 798e4cea7..2076874bf 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/builtin/logical/pki/path_acme_test.go @@ -107,8 +107,11 @@ func TestAcmeBasicWorkflow(t *testing.T) { acme.WithOrderNotAfter(time.Now().Add(7*24*time.Hour))) require.NoError(t, err, "failed creating order") require.Equal(t, acme.StatusPending, createOrder.Status) + require.Empty(t, createOrder.CertURL) + require.Equal(t, createOrder.URI+"/finalize", createOrder.FinalizeURL) + require.Len(t, createOrder.AuthzURLs, 1, "expected one authzurls") - // Get orders + // Get order t.Logf("Testing GetOrder on %s", baseAcmeURL) getOrder, err := acmeClient.GetOrder(testCtx, createOrder.URI) require.NoError(t, err, "failed fetching order") @@ -117,6 +120,33 @@ func TestAcmeBasicWorkflow(t *testing.T) { t.Fatalf("Differences exist between create and get order: \n%v", strings.Join(diffs, "\n")) } + // Load authorization + auth, err := acmeClient.GetAuthorization(testCtx, getOrder.AuthzURLs[0]) + require.NoError(t, err, "failed fetching authorization") + require.Equal(t, acme.StatusPending, auth.Status) + require.Equal(t, "dns", auth.Identifier.Type) + require.Equal(t, "www.test.com", auth.Identifier.Value) + require.False(t, auth.Wildcard, "should not be a wildcard") + require.True(t, auth.Expires.IsZero(), "authorization should only have expiry set on valid status") + + require.Len(t, auth.Challenges, 1, "expected one challenge") + require.Equal(t, acme.StatusPending, auth.Challenges[0].Status) + 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") + + // Load a challenge directly + challenge, err := acmeClient.GetChallenge(testCtx, auth.Challenges[0].URI) + require.NoError(t, err, "failed to load challenge") + require.Equal(t, acme.StatusPending, challenge.Status) + 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") + // Deactivate account t.Logf("Testing deactivate account on %s", baseAcmeURL) err = acmeClient.DeactivateReg(testCtx)