acme: include order problem in OrderError

If client.WaitOrder or client.CreateOrderCert return an acme.OrderError
it's helpful to include the order's problem field (if available). This
will often have detailed information about why a particular order
became invalid that's invaluable for debugging (e.g. a challenge
response was incorrect, a name couldn't be resolved, etc).

While it's possible for a consumer to poll the order themselves as part
of handling the order to extract a fresh Order.Error field value, it
would take an extra round-trip network request. Since we have the
underlying error in-hand when we produce the OrderError we might as well
include it directly.

Since this field is a structured object with a number of sub-fields the
OrderError.Error() function isn't updated to include the order problem
error in the String description. Interested callers should instead use
errors.Is to extract the problem information directly.

Resolves golang/go#74430

Cq-Include-Trybots: luci.golang.try:x_crypto-gotip-linux-amd64-longtest
Change-Id: I3158f064793bbfdc292dd6b5e1a6bfd7729bd980
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/681037
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Ian Stapleton Cordasco <graffatcolmingov@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

4 files changed, 28 insertions, 7 deletions

diff --git a/acme/pebble_test.go b/acme/pebble_test.go
index b633435..bb4809f 100644
--- a/acme/pebble_test.go
+++ b/acme/pebble_test.go
@@ -382,7 +382,12 @@ func testIssuance(t *testing.T, env *environment, challSrv challengeServer) {
 	// Wait for the order to become ready for finalization.
 	order, err = client.WaitOrder(ctx, order.URI)
 	if err != nil {
-		t.Fatalf("failed to wait for order %s: %s", orderURL, err)
+		var orderErr *acme.OrderError
+		if errors.Is(err, orderErr) {
+			t.Fatalf("failed to wait for order %s: %s: %s", orderURL, err, orderErr.Problem)
+		} else {
+			t.Fatalf("failed to wait for order %s: %s", orderURL, err)
+		}
 	}
 	if order.Status != acme.StatusReady {
 		t.Fatalf("expected order %s status to be ready, got %v",
diff --git a/acme/rfc8555.go b/acme/rfc8555.go
index 3152e53..fc653f3 100644
--- a/acme/rfc8555.go
+++ b/acme/rfc8555.go
@@ -272,7 +272,7 @@ func (c *Client) WaitOrder(ctx context.Context, url string) (*Order, error) {
 		case err != nil:
 			// Skip and retry.
 		case o.Status == StatusInvalid:
-			return nil, &OrderError{OrderURL: o.URI, Status: o.Status}
+			return nil, &OrderError{OrderURL: o.URI, Status: o.Status, Problem: o.Error}
 		case o.Status == StatusReady || o.Status == StatusValid:
 			return o, nil
 		}
@@ -369,7 +369,7 @@ func (c *Client) CreateOrderCert(ctx context.Context, url string, csr []byte, bu
 	}
 	// The only acceptable status post finalize and WaitOrder is "valid".
 	if o.Status != StatusValid {
-		return nil, "", &OrderError{OrderURL: o.URI, Status: o.Status}
+		return nil, "", &OrderError{OrderURL: o.URI, Status: o.Status, Problem: o.Error}
 	}
 	crt, err := c.fetchCertRFC(ctx, o.CertURL, bundle)
 	return crt, o.CertURL, err
diff --git a/acme/rfc8555_test.go b/acme/rfc8555_test.go
index d65720a..e9cedb5 100644
--- a/acme/rfc8555_test.go
+++ b/acme/rfc8555_test.go
@@ -885,11 +885,17 @@ func TestRFC_WaitOrderError(t *testing.T) {
 	s.handle("/orders/1", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Location", s.url("/orders/1"))
 		w.WriteHeader(http.StatusOK)
-		s := StatusPending
 		if count > 0 {
-			s = StatusInvalid
+			// https://www.rfc-editor.org/rfc/rfc8555#section-7.3.3
+			errorData := `{
+				"type": "urn:ietf:params:acme:error:userActionRequired",
+				"detail": "Terms of service have changed",
+				"instance": "https://example.com/acme/agreement/?token=W8Ih3PswD-8"
+			}`
+			fmt.Fprintf(w, `{"status": %q, "error": %s}`, StatusInvalid, errorData)
+		} else {
+			fmt.Fprintf(w, `{"status": %q}`, StatusPending)
 		}
-		fmt.Fprintf(w, `{"status": %q}`, s)
 		count++
 	})
 	s.start()
@@ -910,6 +916,13 @@ func TestRFC_WaitOrderError(t *testing.T) {
 	if e.Status != StatusInvalid {
 		t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid)
 	}
+	if e.Problem == nil {
+		t.Errorf("e.Problem = nil")
+	}
+	expectedProbType := "urn:ietf:params:acme:error:userActionRequired"
+	if e.Problem.ProblemType != expectedProbType {
+		t.Errorf("e.Problem.ProblemType = %q; want %q", e.Problem.ProblemType, expectedProbType)
+	}
 }
 
 func TestRFC_CreateOrderCert(t *testing.T) {
diff --git a/acme/types.go b/acme/types.go
index c466645..322640c 100644
--- a/acme/types.go
+++ b/acme/types.go
@@ -154,13 +154,16 @@ func (a *AuthorizationError) Error() string {
 
 // OrderError is returned from Client's order related methods.
 // It indicates the order is unusable and the clients should start over with
-// AuthorizeOrder.
+// AuthorizeOrder. A Problem description may be provided with details on
+// what caused the order to become unusable.
 //
 // The clients can still fetch the order object from CA using GetOrder
 // to inspect its state.
 type OrderError struct {
 	OrderURL string
 	Status   string
+	// Problem is the error that occurred while processing the order.
+	Problem *Error
 }
 
 func (oe *OrderError) Error() string {