diff --git a/acme/api/order.go b/acme/api/order.go index 708c35632..befcb541d 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -3,6 +3,7 @@ package api import ( "encoding/base64" "errors" + "fmt" "net" "time" @@ -105,7 +106,7 @@ func (o *OrderService) UpdateForCSR(orderURL string, csr []byte) (acme.ExtendedO } if order.Status == acme.StatusInvalid { - return acme.ExtendedOrder{}, order.Error + return acme.ExtendedOrder{}, fmt.Errorf("invalid order: %w", order.Err()) } return acme.ExtendedOrder{Order: order}, nil diff --git a/acme/commons.go b/acme/commons.go index 918cc605f..d22ea96ae 100644 --- a/acme/commons.go +++ b/acme/commons.go @@ -200,6 +200,14 @@ type Order struct { Replaces string `json:"replaces,omitempty"` } +func (r *Order) Err() error { + if r.Error != nil { + return r.Error + } + + return nil +} + // Authorization the ACME authorization object. // - https://www.rfc-editor.org/rfc/rfc8555.html#section-7.1.4 type Authorization struct { @@ -285,6 +293,14 @@ type Challenge struct { KeyAuthorization string `json:"keyAuthorization"` } +func (c *Challenge) Err() error { + if c.Error != nil { + return c.Error + } + + return nil +} + // Identifier the ACME identifier object. // - https://www.rfc-editor.org/rfc/rfc8555.html#section-9.7.7 type Identifier struct { diff --git a/acme/errors.go b/acme/errors.go index acaea5f65..6cd74bfb8 100644 --- a/acme/errors.go +++ b/acme/errors.go @@ -25,15 +25,7 @@ type ProblemDetails struct { URL string `json:"url,omitempty"` } -// SubProblem a "subproblems". -// - https://www.rfc-editor.org/rfc/rfc8555.html#section-6.7.1 -type SubProblem struct { - Type string `json:"type,omitempty"` - Detail string `json:"detail,omitempty"` - Identifier Identifier `json:"identifier,omitempty"` -} - -func (p ProblemDetails) Error() string { +func (p *ProblemDetails) Error() string { msg := fmt.Sprintf("acme: error: %d", p.HTTPStatus) if p.Method != "" || p.URL != "" { msg += fmt.Sprintf(" :: %s :: %s", p.Method, p.URL) @@ -51,6 +43,14 @@ func (p ProblemDetails) Error() string { return msg } +// SubProblem a "subproblems". +// - https://www.rfc-editor.org/rfc/rfc8555.html#section-6.7.1 +type SubProblem struct { + Type string `json:"type,omitempty"` + Detail string `json:"detail,omitempty"` + Identifier Identifier `json:"identifier,omitempty"` +} + // NonceError represents the error which is returned // if the nonce sent by the client was not accepted by the server. type NonceError struct { diff --git a/certificate/certificates.go b/certificate/certificates.go index 56c081701..68972478f 100644 --- a/certificate/certificates.go +++ b/certificate/certificates.go @@ -700,7 +700,7 @@ func checkOrderStatus(order acme.ExtendedOrder) (bool, error) { case acme.StatusValid: return true, nil case acme.StatusInvalid: - return false, order.Error + return false, fmt.Errorf("invalid order: %w", order.Err()) default: return false, nil } diff --git a/certificate/certificates_test.go b/certificate/certificates_test.go index bff66429d..da35dbd1a 100644 --- a/certificate/certificates_test.go +++ b/certificate/certificates_test.go @@ -389,6 +389,51 @@ func Test_Get(t *testing.T) { assert.Equal(t, issuerMock, string(certRes.IssuerCertificate), "IssuerCertificate") } +func Test_checkOrderStatus(t *testing.T) { + testCases := []struct { + desc string + order acme.Order + requireErr require.ErrorAssertionFunc + expected bool + }{ + { + desc: "status valid", + order: acme.Order{Status: acme.StatusValid}, + requireErr: require.NoError, + expected: true, + }, + { + desc: "status invalid", + order: acme.Order{Status: acme.StatusInvalid}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status invalid with error", + order: acme.Order{Status: acme.StatusInvalid, Error: &acme.ProblemDetails{}}, + requireErr: require.Error, + expected: false, + }, + { + desc: "unknown status", + order: acme.Order{Status: "foo"}, + requireErr: require.NoError, + expected: false, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + status, err := checkOrderStatus(acme.ExtendedOrder{Order: test.order}) + test.requireErr(t, err) + + assert.Equal(t, test.expected, status) + }) + } +} + type resolverMock struct { error error } diff --git a/challenge/resolver/solver_manager.go b/challenge/resolver/solver_manager.go index ee6bafb49..95db0af54 100644 --- a/challenge/resolver/solver_manager.go +++ b/challenge/resolver/solver_manager.go @@ -124,7 +124,7 @@ func validate(core *api.Core, domain string, chlg acme.Challenge) error { return nil } - return errors.New("the server didn't respond to our request") + return fmt.Errorf("the server didn't respond to our request (status=%s)", authz.Status) } return backoff.Retry(operation, bo) @@ -137,9 +137,9 @@ func checkChallengeStatus(chlng acme.ExtendedChallenge) (bool, error) { case acme.StatusPending, acme.StatusProcessing: return false, nil case acme.StatusInvalid: - return false, chlng.Error + return false, fmt.Errorf("invalid challenge: %w", chlng.Err()) default: - return false, errors.New("the server returned an unexpected state") + return false, fmt.Errorf("the server returned an unexpected challenge status: %s", chlng.Status) } } @@ -154,11 +154,11 @@ func checkAuthorizationStatus(authz acme.Authorization) (bool, error) { case acme.StatusInvalid: for _, chlg := range authz.Challenges { if chlg.Status == acme.StatusInvalid && chlg.Error != nil { - return false, chlg.Error + return false, fmt.Errorf("invalid authorization: %w", chlg.Err()) } } - return false, fmt.Errorf("the authorization state %s", authz.Status) + return false, errors.New("invalid authorization") default: - return false, errors.New("the server returned an unexpected state") + return false, fmt.Errorf("the server returned an unexpected authorization status: %s", authz.Status) } } diff --git a/challenge/resolver/solver_manager_test.go b/challenge/resolver/solver_manager_test.go index 9249beeba..113f4963e 100644 --- a/challenge/resolver/solver_manager_test.go +++ b/challenge/resolver/solver_manager_test.go @@ -55,9 +55,6 @@ func TestValidate(t *testing.T) { statuses = statuses[1:] chlg := &acme.Challenge{Type: "http-01", Status: st, URL: "http://example.com/", Token: "token"} - if st == acme.StatusInvalid { - chlg.Error = &acme.ProblemDetails{} - } err := tester.WriteJSONResponse(w, chlg) if err != nil { @@ -83,7 +80,6 @@ func TestValidate(t *testing.T) { if st == acme.StatusInvalid { chlg := acme.Challenge{ Status: acme.StatusInvalid, - Error: &acme.ProblemDetails{}, } authorization.Challenges = append(authorization.Challenges, chlg) } @@ -106,7 +102,7 @@ func TestValidate(t *testing.T) { { name: "POST-unexpected", statuses: []string{"weird"}, - want: "unexpected", + want: "the server returned an unexpected challenge status: weird", }, { name: "POST-valid", @@ -115,12 +111,12 @@ func TestValidate(t *testing.T) { { name: "POST-invalid", statuses: []string{acme.StatusInvalid}, - want: "error", + want: "invalid challenge:", }, { name: "POST-pending-unexpected", statuses: []string{acme.StatusPending, "weird"}, - want: "unexpected", + want: "the server returned an unexpected authorization status: weird", }, { name: "POST-pending-valid", @@ -129,7 +125,7 @@ func TestValidate(t *testing.T) { { name: "POST-pending-invalid", statuses: []string{acme.StatusPending, acme.StatusInvalid}, - want: "error", + want: "invalid authorization", }, } @@ -148,6 +144,126 @@ func TestValidate(t *testing.T) { } } +func Test_checkChallengeStatus(t *testing.T) { + testCases := []struct { + desc string + challenge acme.Challenge + requireErr require.ErrorAssertionFunc + expected bool + }{ + { + desc: "status valid", + challenge: acme.Challenge{Status: acme.StatusValid}, + requireErr: require.NoError, + expected: true, + }, + { + desc: "status invalid", + challenge: acme.Challenge{Status: acme.StatusInvalid}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status invalid with error", + challenge: acme.Challenge{Status: acme.StatusInvalid, Error: &acme.ProblemDetails{}}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status pending", + challenge: acme.Challenge{Status: acme.StatusPending}, + requireErr: require.NoError, + expected: false, + }, + { + desc: "status processing", + challenge: acme.Challenge{Status: acme.StatusProcessing}, + requireErr: require.NoError, + expected: false, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + status, err := checkChallengeStatus(acme.ExtendedChallenge{Challenge: test.challenge}) + test.requireErr(t, err) + + assert.Equal(t, test.expected, status) + }) + } +} + +func Test_checkAuthorizationStatus(t *testing.T) { + testCases := []struct { + desc string + authorization acme.Authorization + requireErr require.ErrorAssertionFunc + expected bool + }{ + { + desc: "status valid", + authorization: acme.Authorization{Status: acme.StatusValid}, + requireErr: require.NoError, + expected: true, + }, + { + desc: "status invalid", + authorization: acme.Authorization{Status: acme.StatusInvalid}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status invalid with error", + authorization: acme.Authorization{Status: acme.StatusInvalid, Challenges: []acme.Challenge{{Error: &acme.ProblemDetails{}}}}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status pending", + authorization: acme.Authorization{Status: acme.StatusPending}, + requireErr: require.NoError, + expected: false, + }, + { + desc: "status processing", + authorization: acme.Authorization{Status: acme.StatusProcessing}, + requireErr: require.NoError, + expected: false, + }, + { + desc: "status deactivated", + authorization: acme.Authorization{Status: acme.StatusDeactivated}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status expired", + authorization: acme.Authorization{Status: acme.StatusExpired}, + requireErr: require.Error, + expected: false, + }, + { + desc: "status revoked", + authorization: acme.Authorization{Status: acme.StatusRevoked}, + requireErr: require.Error, + expected: false, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + status, err := checkAuthorizationStatus(test.authorization) + test.requireErr(t, err) + + assert.Equal(t, test.expected, status) + }) + } +} + // validateNoBody reads the http.Request POST body, parses the JWS and validates it to read the body. // If there is an error doing this, // or if the JWS body is not the empty JSON payload "{}" or a POST-as-GET payload "" an error is returned. diff --git a/registration/registar.go b/registration/registar.go index 78e0ce7d8..15c28bad6 100644 --- a/registration/registar.go +++ b/registration/registar.go @@ -60,7 +60,7 @@ func (r *Registrar) Register(options RegisterOptions) (*Resource, error) { account, err := r.core.Accounts.New(accMsg) if err != nil { // seems impossible - var errorDetails acme.ProblemDetails + errorDetails := &acme.ProblemDetails{} if !errors.As(err, &errorDetails) || errorDetails.HTTPStatus != http.StatusConflict { return nil, err } @@ -84,7 +84,7 @@ func (r *Registrar) RegisterWithExternalAccountBinding(options RegisterEABOption account, err := r.core.Accounts.NewEAB(accMsg, options.Kid, options.HmacEncoded) if err != nil { // seems impossible - var errorDetails acme.ProblemDetails + errorDetails := &acme.ProblemDetails{} if !errors.As(err, &errorDetails) || errorDetails.HTTPStatus != http.StatusConflict { return nil, err }