Repository: go-acme/lego (language: go)

Issue:
Nil pointer in error interface in checkOrderStatus
### Welcome

- [x] Yes, I'm using a binary release within 2 latest releases.
- [x] Yes, I've searched similar issues on GitHub and didn't find any.
- [x] Yes, I've included all information below (version, config, etc).

### What did you expect to see?

Rate limit error should not rise an exception

### What did you see instead?

Panic, nil pointer dereference. I will try to reproduce it later on, so far it can be emulated (based on discussion here: https://github.com/go-acme/lego/pull/2426)

### How do you use lego?

Library

### Reproduction steps

See https://go.dev/play/p/WQWmhF8yF8N

### Version of lego

```console
v4.21.0
```

### Logs

<details>

```console
panic: value method main.ProblemDetails.Error called using nil *ProblemDetails pointer

goroutine 1 [running]:
main.(*ProblemDetails).Error(0x4e169f?)
	<autogenerated>:1 +0x26
main.main()
	/tmp/sandbox1881138421/prog.go:70 +0x131
```

</details>

### Go environment (if applicable)

<details>

```console
$ go version && go env
# paste output here
```

</details>


Discussion / hints:

Fix patch:
diff --git a/acme/api/order.go b/acme/api/order.go
index 708c356324..befcb541da 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 918cc605f1..d22ea96ae9 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 acaea5f651..6cd74bfb8c 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 56c0817018..68972478fe 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/challenge/resolver/solver_manager.go b/challenge/resolver/solver_manager.go
index ee6bafb49b..95db0af548 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/registration/registar.go b/registration/registar.go
index 78e0ce7d86..15c28bad67 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
 		}


Test patch:
diff --git a/certificate/certificates_test.go b/certificate/certificates_test.go
index bff66429d8..da35dbd1a2 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_test.go b/challenge/resolver/solver_manager_test.go
index 9249beebac..113f4963e6 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.