ra/sa: render the attempted challenge as "processing"

ra/ra.go

@@ -1525,6 +1525,14 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
 		return nil, berrors.MalformedError("authorization must be pending")
 	}
 
+	// A challenge which is already processing has a validation attempt in
+	// flight. Return the same conflict error that the SA would return below,
+	// rather than letting CheckPending turn this into a less helpful
+	// "malformed" error.
+	if ch.Status == core.StatusProcessing {
+		return nil, berrors.ConflictError("Authorization is already being validated. This may indicate your client attempted the same challenge multiple times, possibly due to a client bug.")
+	}
+
 	// Compute the key authorization field based on the registration key
 	regPB, err := ra.SA.GetRegistration(ctx, &sapb.RegistrationID{Id: authz.RegistrationID})
 	if err != nil {
@@ -1544,9 +1552,11 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
 		return nil, berrors.MalformedError("cannot validate challenge: %s", cErr.Error())
 	}
 
-	// Set the authorization to "processing", to prevent parallel attempts.
+	// Set the authorization to "processing", to prevent parallel attempts, and
+	// record which challenge is being validated so that it can be rendered with
+	// the "processing" status while the authorization remains pending.
 	if features.Get().SetAuthzProcessing {
-		_, err = ra.SA.SetAuthzProcessing(ctx, &sapb.AuthorizationID2{Id: authz.ID})
+		_, err = ra.SA.SetAuthzProcessing(ctx, &sapb.SetAuthzProcessingRequest{Id: authz.ID, Attempted: string(ch.Type)})
 		if err != nil {
 			return nil, fmt.Errorf("failed to mark authz as processing: %w", err)
 		}

ra/ra_test.go

@@ -19,6 +19,7 @@ import (
 	"math/big"
 	mrand "math/rand/v2"
 	"net/netip"
+	"os"
 	"regexp"
 	"strings"
 	"sync"
@@ -594,6 +595,116 @@ func TestPerformValidationSuccess(t *testing.T) {
 	}
 }
 
+// mockSAWithProcessingCapture is a mock sapb.StorageAuthorityClient that
+// forwards all method calls to an inner SA, but also records the request
+// passed to SetAuthzProcessing and the authorization as the inner SA renders it
+// immediately afterwards (i.e. while it is still processing).
+type mockSAWithProcessingCapture struct {
+	sapb.StorageAuthorityClient
+	req   *sapb.SetAuthzProcessingRequest
+	authz *corepb.Authorization
+}
+
+func (msa *mockSAWithProcessingCapture) SetAuthzProcessing(ctx context.Context, req *sapb.SetAuthzProcessingRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
+	res, err := msa.StorageAuthorityClient.SetAuthzProcessing(ctx, req)
+	if err != nil {
+		return res, err
+	}
+	msa.req = req
+	msa.authz, err = msa.StorageAuthorityClient.GetAuthorization2(ctx, &sapb.AuthorizationID2{Id: req.Id})
+	if err != nil {
+		return nil, err
+	}
+	return res, nil
+}
+
+func TestPerformValidationSetsChallengeProcessing(t *testing.T) {
+	if os.Getenv("BOULDER_CONFIG_DIR") != "test/config-next" {
+		t.Skip("TestPerformValidationSetsChallengeProcessing requires config-next")
+	}
+
+	va, sa, ra, _, fc, registration, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	features.Set(features.Config{SetAuthzProcessing: true})
+	defer features.Reset()
+
+	msa := &mockSAWithProcessingCapture{StorageAuthorityClient: sa}
+	ra.SA = msa
+
+	authzPB := createPendingAuthorization(t, sa, registration.Id, identifier.NewDNS("example.com"), fc.Now().Add(12*time.Hour))
+
+	va.doDCVResult = &vapb.ValidationResult{Problem: nil}
+	va.doCAAResponse = &vapb.IsCAAValidResponse{Problem: nil}
+
+	challIdx := dnsChallIdx(t, authzPB.Challenges)
+	_, err := ra.PerformValidation(ctx, &rapb.PerformValidationRequest{
+		Authz:          authzPB,
+		ChallengeIndex: challIdx,
+	})
+	test.AssertNotError(t, err, "PerformValidation failed")
+
+	// The RA should have told the SA which challenge it is validating.
+	test.AssertNotNil(t, msa.req, "SetAuthzProcessing was not called")
+	test.AssertEquals(t, msa.req.Id, authzPB.Id)
+	test.AssertEquals(t, msa.req.Attempted, string(core.ChallengeTypeDNS01))
+
+	// While validation is in flight, the authz is still pending but the
+	// attempted challenge is processing.
+	test.AssertNotNil(t, msa.authz, "authz was not fetched after SetAuthzProcessing")
+	test.AssertEquals(t, msa.authz.Status, string(core.StatusPending))
+	test.AssertEquals(t, len(msa.authz.Challenges), 1)
+	test.AssertEquals(t, msa.authz.Challenges[0].Type, string(core.ChallengeTypeDNS01))
+	test.AssertEquals(t, msa.authz.Challenges[0].Status, string(core.StatusProcessing))
+
+	select {
+	case <-va.doDCVRequest:
+	case <-time.After(time.Second):
+		t.Fatal("Timed out waiting for DummyValidationAuthority.PerformValidation to complete")
+	}
+
+	// Sleep so the RA has a chance to write to the SA
+	time.Sleep(100 * time.Millisecond)
+
+	// Once validation completes, the challenge should be valid.
+	dbAuthzPB := getAuthorization(t, authzPB.Id, sa)
+	test.AssertEquals(t, dbAuthzPB.Status, string(core.StatusValid))
+	test.AssertEquals(t, dbAuthzPB.Challenges[0].Status, string(core.StatusValid))
+}
+
+func TestPerformValidationChallengeProcessing(t *testing.T) {
+	_, _, ra, _, fc, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	// An authz whose challenge is already processing has a validation attempt
+	// in flight, so a second attempt should be rejected as a conflict rather
+	// than as a malformed request.
+	exp := fc.Now().Add(10 * time.Hour)
+	authz := core.Authorization{
+		ID:             1337,
+		Identifier:     identifier.NewDNS("not-example.com"),
+		RegistrationID: 1,
+		Status:         core.StatusPending,
+		Challenges: []core.Challenge{
+			{
+				Status: core.StatusProcessing,
+				Type:   core.ChallengeTypeDNS01,
+				Token:  "exampleToken",
+			},
+		},
+		Expires: &exp,
+	}
+	authzPB, err := bgrpc.AuthzToPB(authz)
+	test.AssertNotError(t, err, "AuthzToPB failed")
+
+	_, err = ra.PerformValidation(context.Background(), &rapb.PerformValidationRequest{
+		Authz:          authzPB,
+		ChallengeIndex: 0,
+	})
+	test.AssertError(t, err, "ra.PerformValidation allowed a second attempt at a processing challenge")
+	test.AssertErrorIs(t, err, berrors.Conflict)
+}
+
 // mockSAWithSyncPause is a mock sapb.StorageAuthorityClient that forwards all
 // method calls to an inner SA, but also performs a blocking write to a channel
 // when PauseIdentifiers is called to allow the tests to synchronize.

sa/model.go

@@ -748,8 +748,18 @@ func authzPBToModel(authz *corepb.Authorization) (*authzModel, error) {
 }
 
 // populateAttemptedFields takes a challenge and populates it with the validation fields status,
-// validation records, and error (the latter only if the validation failed) from an authzModel.
+// validation records, error (the latter only if the validation failed), and validated time from an
+// authzModel. If the authorization is still pending, the challenge is the one currently being
+// validated, so it is only given the status core.StatusProcessing.
 func populateAttemptedFields(am authzModel, challenge *corepb.Challenge) error {
+	if am.Status == statusUint(core.StatusPending) {
+		// The RA has begun validating this challenge, but has not yet
+		// finalized the authorization. There are no validation results to
+		// populate yet.
+		challenge.Status = string(core.StatusProcessing)
+		return nil
+	}
+
 	if len(am.ValidationError) != 0 {
 		// If the error is non-empty the challenge must be invalid.
 		challenge.Status = string(core.StatusInvalid)
@@ -789,6 +799,12 @@ func populateAttemptedFields(am authzModel, challenge *corepb.Challenge) error {
 			return err
 		}
 	}
+	// Get the attemptedAt time and assign to the challenge validated time.
+	var validated *timestamppb.Timestamp
+	if am.AttemptedAt != nil {
+		validated = timestamppb.New(*am.AttemptedAt)
+	}
+	challenge.Validated = validated
 	return nil
 }
 
@@ -811,44 +827,33 @@ func modelToAuthzPB(am authzModel) (*corepb.Authorization, error) {
 		Expires:                timestamppb.New(am.Expires),
 		CertificateProfileName: profile,
 	}
-	// Populate authorization challenge array. We do this by iterating through
-	// the challenge type bitmap and creating a challenge of each type if its
-	// bit is set. Each of these challenges has the token from the authorization
-	// model and has its status set to core.StatusPending by default. If the
-	// challenge type is equal to that in the 'attempted' row we set the status
-	// to core.StatusValid or core.StatusInvalid depending on if there is anything
-	// in ValidationError and populate the ValidationRecord and ValidationError
-	// fields.
-	for pos := range uint8(8) {
-		if (am.Challenges>>pos)&1 == 1 {
-			challType := uintToChallType[pos]
-			challenge := &corepb.Challenge{
-				Type:   challType,
-				Status: string(core.StatusPending),
-				Token:  base64.RawURLEncoding.EncodeToString(am.Token),
-			}
-			// If the challenge type matches the attempted type it must be either
-			// valid or invalid and we need to populate extra fields.
-			// Also, once any challenge has been attempted, we consider the other
-			// challenges "gone" per https://tools.ietf.org/html/rfc8555#section-7.1.4
-			if am.Attempted != nil {
-				if uintToChallType[*am.Attempted] == challType {
-					err := populateAttemptedFields(am, challenge)
-					if err != nil {
-						return nil, err
-					}
-					// Get the attemptedAt time and assign to the challenge validated time.
-					var validated *timestamppb.Timestamp
-					if am.AttemptedAt != nil {
-						validated = timestamppb.New(*am.AttemptedAt)
-					}
-					challenge.Validated = validated
-					pb.Challenges = append(pb.Challenges, challenge)
-				}
-			} else {
-				// When no challenge has been attempted yet, all challenges are still
-				// present.
-				pb.Challenges = append(pb.Challenges, challenge)
+	// Populate authorization challenge array. Once any challenge has been
+	// attempted, we consider the other challenges "gone" per
+	// https://tools.ietf.org/html/rfc8555#section-7.1.4, so the array holds
+	// only the attempted challenge, populated with its status and validation
+	// results by populateAttemptedFields. Otherwise we iterate through the
+	// challenge type bitmap and create a pending challenge of each type whose
+	// bit is set. Each of these challenges has the token from the
+	// authorization model.
+	token := base64.RawURLEncoding.EncodeToString(am.Token)
+	if am.Attempted != nil {
+		challenge := &corepb.Challenge{
+			Type:  uintToChallType[*am.Attempted],
+			Token: token,
+		}
+		err := populateAttemptedFields(am, challenge)
+		if err != nil {
+			return nil, err
+		}
+		pb.Challenges = append(pb.Challenges, challenge)
+	} else {
+		for pos := range uint8(8) {
+			if (am.Challenges>>pos)&1 == 1 {
+				pb.Challenges = append(pb.Challenges, &corepb.Challenge{
+					Type:   uintToChallType[pos],
+					Status: string(core.StatusPending),
+					Token:  token,
+				})
 			}
 		}
 	}

sa/model_test.go

@@ -224,6 +224,48 @@ func TestAuthzModel(t *testing.T) {
 	}
 }
 
+// TestAuthzModelProcessing tests that a pending authorization whose attempted
+// column has been set (which is what SetAuthzProcessing does) renders the
+// attempted challenge with the "processing" status and no validation results,
+// while the authorization itself remains pending.
+func TestAuthzModelProcessing(t *testing.T) {
+	attempted := challTypeToUint[string(core.ChallengeTypeDNS01)]
+	model := authzModel{
+		ID:              1,
+		IdentifierType:  identifierTypeToUint[string(identifier.TypeDNS)],
+		IdentifierValue: "example.com",
+		RegistrationID:  1,
+		Status:          statusUint(core.StatusPending),
+		Expires:         time.Now().Add(time.Hour),
+		Challenges:      1<<challTypeToUint[string(core.ChallengeTypeHTTP01)] | 1<<challTypeToUint[string(core.ChallengeTypeDNS01)],
+		Token:           []byte("123"),
+	}
+
+	// Before any challenge has been attempted, every challenge is pending.
+	authzPB, err := modelToAuthzPB(model)
+	test.AssertNotError(t, err, "modelToAuthzPB failed")
+	test.AssertEquals(t, authzPB.Status, string(core.StatusPending))
+	test.AssertEquals(t, len(authzPB.Challenges), 2)
+	for _, chall := range authzPB.Challenges {
+		test.AssertEquals(t, chall.Status, string(core.StatusPending))
+	}
+
+	// Once a challenge has been attempted but the authz is still pending, only
+	// the attempted challenge is rendered, and it is processing.
+	model.Attempted = &attempted
+	authzPB, err = modelToAuthzPB(model)
+	test.AssertNotError(t, err, "modelToAuthzPB failed")
+	test.AssertEquals(t, authzPB.Status, string(core.StatusPending))
+	test.AssertEquals(t, len(authzPB.Challenges), 1)
+	chall := authzPB.Challenges[0]
+	test.AssertEquals(t, chall.Type, string(core.ChallengeTypeDNS01))
+	test.AssertEquals(t, chall.Status, string(core.StatusProcessing))
+	test.AssertEquals(t, chall.Token, "MTIz")
+	test.Assert(t, chall.Validated == nil, "processing challenge should not have a validated timestamp")
+	test.Assert(t, chall.Error == nil, "processing challenge should not have an error")
+	test.AssertEquals(t, len(chall.Validationrecords), 0)
+}
+
 // TestModelToOrderBADJSON tests that converting an order model with an invalid
 // validation error JSON field to an Order produces the expected bad JSON error.
 func TestModelToOrderBadJSON(t *testing.T) {
@@ -286,12 +328,14 @@ func TestPopulateAttemptedFieldsBadJSON(t *testing.T) {
 		{
 			Name: "Bad validation error field",
 			Model: &authzModel{
+				Status:          statusUint(core.StatusInvalid),
 				ValidationError: badJSON,
 			},
 		},
 		{
 			Name: "Bad validation record field",
 			Model: &authzModel{
+				Status:           statusUint(core.StatusInvalid),
 				ValidationRecord: badJSON,
 			},
 		},