V2: implement "ready" status for Order objects (#3614)

* SA: Add Order "Ready" status, feature flag.

This commit adds the new "Ready" status to `core/objects.go` and updates
`sa.statusForOrder` to use it conditionally for orders with all valid
authorizations that haven't been finalized yet. This state is used
conditionally based on the `features.OrderReadyStatus` feature flag
since it will likely break some existing clients that expect status
"Processing" for this state. The SA unit test for `statusForOrder` is
updated with a "ready" status test case.

* RA: Enforce order ready status conditionally.

This commit updates the RA to conditionally expect orders that are being
finalized to be in the "ready" status instead of "pending". This is
conditionally enforced based on the `OrderReadyStatus` feature flag.
Along the way the SA was changed to calculate the order status for the
order returned in `sa.NewOrder` dynamically now that it could be
something other than "pending".

* WFE2: Conditionally enforce order ready status for finalization.

Similar to the RA the WFE2 should conditionally enforce that an order's
status is either "ready" or "pending" based on the "OrderReadyStatus"
feature flag.

* Integration: Fix `test_order_finalize_early`.

This commit updates the V2 `test_order_finalize_early` test for the
"ready" status. A nice side-effect of the ready state change is that we
no longer invalidate an order when it is finalized too soon because we
can reject the finalization in the WFE. Subsequently the
`test_order_finalize_early` testcase is also smaller.

* Integration: Test classic behaviour w/o feature flag.

In the previous commit I fixed the integration test for the
`config/test-next` run that has the `OrderReadyStatus` feature flag set
but broke it for the `config/test` run without the feature flag.

This commit updates the `test_order_finalize_early` test to work
correctly based on the feature flag status in both cases.

core/objects.go

@@ -36,6 +36,7 @@ const (
 	StatusUnknown     = AcmeStatus("unknown")     // Unknown status; the default
 	StatusPending     = AcmeStatus("pending")     // In process; client has next action
 	StatusProcessing  = AcmeStatus("processing")  // In process; server has next action
+	StatusReady       = AcmeStatus("ready")       // Order is ready for finalization
 	StatusValid       = AcmeStatus("valid")       // Object is valid
 	StatusInvalid     = AcmeStatus("invalid")     // Validation failed
 	StatusRevoked     = AcmeStatus("revoked")     // Object no longer valid

features/featureflag_string.go

@@ -4,9 +4,9 @@ package features
 
 import "strconv"
 
-const _FeatureFlag_name = "unusedUseAIAIssuerURLReusePendingAuthzCountCertificatesExactIPv6FirstAllowRenewalFirstRLWildcardDomainsForceConsistentStatusEnforceChallengeDisableRPCHeadroomTLSSNIRevalidationEmbedSCTsCancelCTSubmissionsVAChecksGSBEnforceV2ContentTypeEnforceOverlappingWildcards"
+const _FeatureFlag_name = "unusedUseAIAIssuerURLReusePendingAuthzCountCertificatesExactIPv6FirstAllowRenewalFirstRLWildcardDomainsForceConsistentStatusEnforceChallengeDisableRPCHeadroomTLSSNIRevalidationEmbedSCTsCancelCTSubmissionsVAChecksGSBEnforceV2ContentTypeEnforceOverlappingWildcardsOrderReadyStatus"
 
-var _FeatureFlag_index = [...]uint16{0, 6, 21, 38, 60, 69, 88, 103, 124, 147, 158, 176, 185, 204, 215, 235, 262}
+var _FeatureFlag_index = [...]uint16{0, 6, 21, 38, 60, 69, 88, 103, 124, 147, 158, 176, 185, 204, 215, 235, 262, 278}
 
 func (i FeatureFlag) String() string {
 	if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

features/features.go

@@ -36,6 +36,8 @@ const (
 	EnforceV2ContentType
 	// Reject new-orders that contain a hostname redundant with a wildcard.
 	EnforceOverlappingWildcards
+	// Set orders to status "ready" when they are awaiting finalization
+	OrderReadyStatus
 )
 
 // List of features and their default value, protected by fMu
@@ -56,6 +58,7 @@ var features = map[FeatureFlag]bool{
 	EnforceV2ContentType:        false,
 	ForceConsistentStatus:       false,
 	EnforceOverlappingWildcards: false,
+	OrderReadyStatus:            false,
 }
 
 var fMu = new(sync.RWMutex)

mocks/mocks.go

@@ -527,6 +527,11 @@ func (sa *StorageAuthority) GetOrder(_ context.Context, req *sapb.OrderRequest)
 		validOrder.Expires = &exp
 	}
 
+	if *req.Id == 8 {
+		ready := string(core.StatusReady)
+		validOrder.Status = &ready
+	}
+
 	return validOrder, nil
 }
 

ra/ra.go

@@ -884,9 +884,17 @@ func (ra *RegistrationAuthorityImpl) failOrder(
 func (ra *RegistrationAuthorityImpl) FinalizeOrder(ctx context.Context, req *rapb.FinalizeOrderRequest) (*corepb.Order, error) {
 	order := req.Order
 
-	// Only pending orders can be finalized
-	if *order.Status != string(core.StatusPending) {
-		return nil, berrors.InternalServerError("Order's status (%q) was not pending", *order.Status)
+	// Prior to ACME draft-10 the "ready" status did not exist and orders in
+	// a pending status with valid authzs were finalizable. We accept both states
+	// here for deployability ease. In the future we will only allow ready orders
+	// to be finalized.
+	// TODO(@cpu): Forbid finalizing "Pending" orders once
+	// `features.Enabled(features.OrderReadyStatus)` is deployed
+	if *order.Status != string(core.StatusPending) &&
+		*order.Status != string(core.StatusReady) {
+		return nil, berrors.MalformedError(
+			"Order's status (%q) is not acceptable for finalization",
+			*order.Status)
 	}
 
 	// There should never be an order with 0 names at the stage the RA is

ra/ra_test.go

@@ -2750,7 +2750,9 @@ func TestFinalizeOrder(t *testing.T) {
 	defer cleanUp()
 	ra.orderLifetime = time.Hour
 
-	validStatus := "valid"
+	validStatus := string(core.StatusValid)
+	readyStatus := string(core.StatusReady)
+	processingStatus := false
 
 	testKey, err := rsa.GenerateKey(rand.Reader, 2048)
 	test.AssertNotError(t, err, "error generating test key")
@@ -2817,10 +2819,26 @@ func TestFinalizeOrder(t *testing.T) {
 		Expires:        &expUnix,
 		Names:          []string{"not-example.com", "www.not-example.com"},
 		Authorizations: []string{finalAuthz.ID, finalAuthzB.ID},
-		Status:         &pendingStatus,
+		Status:         &validStatus,
 	})
 	test.AssertNotError(t, err, "Could not add test order with finalized authz IDs")
 
+	// Enable the order ready status temporarily
+	_ = features.Set(map[string]bool{"OrderReadyStatus": true})
+	// Create an order with valid authzs, it should end up status ready in the
+	// resulting returned order
+	modernFinalOrder, err := sa.NewOrder(context.Background(), &corepb.Order{
+		RegistrationID:  &Registration.ID,
+		Expires:         &expUnix,
+		Names:           []string{"not-example.com", "www.not-example.com"},
+		Authorizations:  []string{finalAuthz.ID, finalAuthzB.ID},
+		Status:          &readyStatus,
+		BeganProcessing: &processingStatus,
+	})
+	test.AssertNotError(t, err, "Could not add test order with finalized authz IDs, ready status")
+	// Disable the order ready status again
+	_ = features.Set(map[string]bool{"OrderReadyStatus": false})
+
 	// Swallowing errors here because the CSRPEM is hardcoded test data expected
 	// to parse in all instance
 	validCSRBlock, _ := pem.Decode(CSRPEM)
@@ -2873,14 +2891,14 @@ func TestFinalizeOrder(t *testing.T) {
 			ExpectedErrMsg: "Order has no associated names",
 		},
 		{
-			Name: "Wrong order state",
+			Name: "Wrong order state (valid)",
 			OrderReq: &rapb.FinalizeOrderRequest{
 				Order: &corepb.Order{
 					Status: &validStatus,
 					Names:  []string{"example.com"},
 				},
 			},
-			ExpectedErrMsg: "Order's status (\"valid\") was not pending",
+			ExpectedErrMsg: "Order's status (\"valid\") is not acceptable for finalization",
 		},
 		{
 			Name: "Invalid CSR",
@@ -2968,13 +2986,21 @@ func TestFinalizeOrder(t *testing.T) {
 			ExpectedErrMsg: "authorizations for these names not found or expired: a.com, a.org, b.com",
 		},
 		{
-			Name: "Order with correct authorizations",
+			Name: "Order with correct authorizations, pending status",
 			OrderReq: &rapb.FinalizeOrderRequest{
 				Order: finalOrder,
 				Csr:   validCSR.Raw,
 			},
 			ExpectIssuance: true,
 		},
+		{
+			Name: "Order with correct authorizations, ready status",
+			OrderReq: &rapb.FinalizeOrderRequest{
+				Order: modernFinalOrder,
+				Csr:   validCSR.Raw,
+			},
+			ExpectIssuance: true,
+		},
 	}
 
 	for _, tc := range testCases {

sa/sa.go

@@ -1461,12 +1461,27 @@ func (ssa *SQLStorageAuthority) NewOrder(ctx context.Context, req *corepb.Order)
 	createdTS := order.Created.UnixNano()
 	req.Created = &createdTS
 
-	// Update the request with pending status (No need to calculate the status
-	// based on authzs here, we know a brand new order is always pending)
-	pendingStatus := string(core.StatusPending)
-	req.Status = &pendingStatus
+	// If the OrderReadyStatus feature is enabled we need to calculate the order
+	// status before returning it. Since it may have reused all valid
+	// authorizations the order may be "born" in a ready status.
+	if features.Enabled(features.OrderReadyStatus) {
+		// Calculate the status for the order
+		status, err := ssa.statusForOrder(ctx, req)
+		if err != nil {
+			return nil, err
+		}
+		req.Status = &status
+	} else {
+		// Update the request with pending status (No need to calculate the status
+		// based on authzs here, we know a brand new order is always pending when
+		// features.OrderReadyStatus isn't enabled)
+		pendingStatus := string(core.StatusPending)
+		req.Status = &pendingStatus
+	}
+	// A new order is never processing because it can't have been finalized yet
 	processingStatus := false
 	req.BeganProcessing = &processingStatus
+	// Return the new order
 	return req, nil
 }
 
@@ -1649,7 +1664,9 @@ func (ssa *SQLStorageAuthority) GetOrder(ctx context.Context, req *sapb.OrderReq
 //   * If all of the order's authorizations are valid, and we have began
 //     processing, but there is no certificate serial, the order is processing.
 //   * If all of the order's authorizations are valid, and we haven't begun
-//     processing, then the order is pending waiting a finalization request.
+//     processing, then the order is status ready (if
+//     `features.OrderReadyStatus` is enabled) or status processing otherwise.
+//     In both cases it is awaiting a finalization request.
 // An error is returned for any other case.
 func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corepb.Order) (string, error) {
 	// Without any further work we know an order with an error is invalid
@@ -1746,9 +1763,16 @@ func (ssa *SQLStorageAuthority) statusForOrder(ctx context.Context, order *corep
 	}
 
 	// If the order is fully authorized, and we haven't begun processing it, then
-	// the order is still pending waiting a finalization request.
+	// the order is pending finalization and either status ready or status pending
+	// depending on the `OrderReadyStatus` feature flag. Historically we used
+	// "Pending" for this state but the ACME specification > draft-10 uses a new
+	// "Ready" state.
 	if fullyAuthorized && order.BeganProcessing != nil && !*order.BeganProcessing {
-		return string(core.StatusPending), nil
+		if features.Enabled(features.OrderReadyStatus) {
+			return string(core.StatusReady), nil
+		} else {
+			return string(core.StatusPending), nil
+		}
 	}
 
 	return "", berrors.InternalServerError(

sa/sa_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/letsencrypt/boulder/core"
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	berrors "github.com/letsencrypt/boulder/errors"
+	"github.com/letsencrypt/boulder/features"
 	blog "github.com/letsencrypt/boulder/log"
 	"github.com/letsencrypt/boulder/metrics"
 	"github.com/letsencrypt/boulder/revocation"
@@ -2081,7 +2082,7 @@ func TestStatusForOrder(t *testing.T) {
 	err = sa.FinalizeAuthorization(ctx, invalidAuthz)
 	test.AssertNotError(t, err, "Couldn't finalize pending authz to invalid")
 
-	// Create a deactivate authz
+	// Create a deactivated authz
 	deactivatedAuthz, err := sa.NewPendingAuthorization(ctx, newAuthz)
 	test.AssertNotError(t, err, "Couldn't create new pending authorization")
 	deactivatedAuthz.Status = core.StatusDeactivated
@@ -2103,6 +2104,7 @@ func TestStatusForOrder(t *testing.T) {
 		ExpectedStatus   string
 		SetProcessing    bool
 		Finalize         bool
+		Features         []string
 	}{
 		{
 			Name:             "Order with an invalid authz",
@@ -2141,6 +2143,20 @@ func TestStatusForOrder(t *testing.T) {
 			SetProcessing:    true,
 			ExpectedStatus:   string(core.StatusProcessing),
 		},
+		{
+			Name:             "Order with only valid authzs, not yet processed or finalized, OrderReadyStatus feature flag",
+			OrderNames:       []string{"valid.your.order.is.up"},
+			AuthorizationIDs: []string{validAuthz.ID},
+			ExpectedStatus:   string(core.StatusReady),
+			Features:         []string{"OrderReadyStatus"},
+		},
+		{
+			Name:             "Order with only valid authzs, set processing",
+			OrderNames:       []string{"valid.your.order.is.up"},
+			AuthorizationIDs: []string{validAuthz.ID},
+			SetProcessing:    true,
+			ExpectedStatus:   string(core.StatusProcessing),
+		},
 		{
 			Name:             "Order with only valid authzs, set processing and finalized",
 			OrderNames:       []string{"valid.your.order.is.up"},
@@ -2153,12 +2169,21 @@ func TestStatusForOrder(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
+			if len(tc.Features) > 0 {
+				for _, flag := range tc.Features {
+					_ = features.Set(map[string]bool{flag: true})
+				}
+				defer features.Reset()
+			}
+
 			// Add a new order with the testcase authz IDs
+			processing := false
 			newOrder, err := sa.NewOrder(ctx, &corepb.Order{
-				RegistrationID: &reg.ID,
-				Expires:        &expiresNano,
-				Authorizations: tc.AuthorizationIDs,
-				Names:          tc.OrderNames,
+				RegistrationID:  &reg.ID,
+				Expires:         &expiresNano,
+				Authorizations:  tc.AuthorizationIDs,
+				Names:           tc.OrderNames,
+				BeganProcessing: &processing,
 			})
 			test.AssertNotError(t, err, "NewOrder errored unexpectedly")
 			// If requested, set the order to processing

test/config-next/sa.json

@@ -28,7 +28,8 @@
     "features": {
       "RPCHeadroom": true,
       "WildcardDomains": true,
-      "AllowRenewalFirstRL": true
+      "AllowRenewalFirstRL": true,
+      "OrderReadyStatus": true
     }
   },
 

test/config-next/wfe2.json

@@ -35,8 +35,8 @@
       "http://127.0.0.1:4000/acme/issuer-cert": [ "test/test-ca2.pem" ]
     },
     "features": {
-      "RPCHeadroom": true,
-      "EnforceV2ContentType": true
+      "EnforceV2ContentType": true,
+      "RPCHeadroom": true
     }
   },
 

test/v2_integration.py

@@ -185,8 +185,8 @@ def test_order_finalize_early():
 
     deadline = datetime.datetime.now() + datetime.timedelta(seconds=5)
 
-    # Finalize the order without doing anything with the authorizations. YOLO
-    # We expect this to generate an unauthorized error.
+    # Finalizing an order early should generate an unauthorized error and we
+    # should check that the order is invalidated.
     chisel2.expect_problem("urn:ietf:params:acme:error:unauthorized",
         lambda: client.finalize_order(order, deadline))
 

wfe2/wfe.go

@@ -1665,11 +1665,19 @@ func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.Req
 		return
 	}
 
-	// If the order's status is not pending we can not finalize it and must
-	// return an error
-	if *order.Status != string(core.StatusPending) {
+	// Prior to ACME draft-10 the "ready" status did not exist and orders in
+	// a pending status with valid authzs were finalizable. We accept both states
+	// here for deployability ease. In the future we will only allow ready orders
+	// to be finalized.
+	// TODO(@cpu): Forbid finalizing "Pending" orders once
+	// `features.Enabled(features.OrderReadyStatus)` is deployed
+	if *order.Status != string(core.StatusPending) &&
+		*order.Status != string(core.StatusReady) {
 		wfe.sendError(response, logEvent,
-			probs.Malformed("Order's status (%q) was not pending", *order.Status), nil)
+			probs.Malformed(
+				"Order's status (%q) is not acceptable for finalization",
+				*order.Status),
+			nil)
 		return
 	}
 

wfe2/wfe_test.go

@@ -1955,7 +1955,7 @@ func TestFinalizeOrder(t *testing.T) {
 			Name: "Order is already finalized",
 			// mocks/mocks.go's StorageAuthority's GetOrder mock treats ID 1 as an Order with a Serial
 			Request:      signAndPost(t, "1/1", "http://localhost/1/1", goodCertCSRPayload, 1, wfe.nonceService),
-			ExpectedBody: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Order's status (\"valid\") was not pending","status":400}`,
+			ExpectedBody: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Order's status (\"valid\") is not acceptable for finalization","status":400}`,
 		},
 		{
 			Name: "Order is expired",
@@ -1969,7 +1969,7 @@ func TestFinalizeOrder(t *testing.T) {
 			ExpectedBody: `{"type":"` + probs.V2ErrorNS + `malformed","detail":"Error parsing certificate request: asn1: structure error: tags don't match (16 vs {class:0 tag:0 length:16 isCompound:false}) {optional:false explicit:false application:false defaultValue:\u003cnil\u003e tag:\u003cnil\u003e stringType:0 timeType:0 set:false omitEmpty:false} certificateRequest @2","status":400}`,
 		},
 		{
-			Name:            "Good CSR",
+			Name:            "Good CSR, Pending Order",
 			Request:         signAndPost(t, "1/4", "http://localhost/1/4", goodCertCSRPayload, 1, wfe.nonceService),
 			ExpectedHeaders: map[string]string{"Location": "http://localhost/acme/order/1/4"},
 			ExpectedBody: `
@@ -1983,6 +1983,23 @@ func TestFinalizeOrder(t *testing.T) {
     "http://localhost/acme/authz/hello"
   ],
   "finalize": "http://localhost/acme/finalize/1/4"
+}`,
+		},
+		{
+			Name:            "Good CSR, Ready Order",
+			Request:         signAndPost(t, "1/8", "http://localhost/1/8", goodCertCSRPayload, 1, wfe.nonceService),
+			ExpectedHeaders: map[string]string{"Location": "http://localhost/acme/order/1/8"},
+			ExpectedBody: `
+{
+  "status": "processing",
+  "expires": "1970-01-01T00:00:00.9466848Z",
+  "identifiers": [
+    {"type":"dns","value":"example.com"}
+  ],
+  "authorizations": [
+    "http://localhost/acme/authz/hello"
+  ],
+  "finalize": "http://localhost/acme/finalize/1/8"
 }`,
 		},
 	}