Improve how we create new authorizations (#7643)

Within the NewOrderAndAuthzsRequest, replace the corepb.Authorization
field with a new sapb.NewAuthzRequest message. This message has all of
the same field types and numbers, and the RA still populates all of
these fields when constructing a request, for backwards compatibility.
But it also has new fields (an Identifier carrying both type and value,
a list of challenge types, and a challenge token) which the RA
preferentially consumes if present.

This causes the content of our NewOrderAndAuthzsRequest to more closely
match the content that will be created at the database layer. Although
this may seem like a step backwards in terms of abstraction, it is also
a step forwards in terms of both efficiency (not having to transmit
multiple nearly-identical challenge objects) and correctness (being
guaranteed that the token is actually identical across all challenges).

After this change is deployed, it will be followed by a change which
removes the old fields from the NewAuthzRequest message, to realize the
efficiency gains.

Part of #5913

ra/ra.go

@@ -2611,7 +2611,7 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 
 	// Loop through each of the names missing authzs and create a new pending
 	// authorization for each.
-	var newAuthzs []*corepb.Authorization
+	var newAuthzs []*sapb.NewAuthzRequest
 	for _, name := range missingAuthzNames {
 		pb, err := ra.createPendingAuthz(newOrder.RegistrationID, identifier.ACMEIdentifier{
 			Type:  identifier.DNS,
@@ -2679,9 +2679,13 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 // createPendingAuthz checks that a name is allowed for issuance and creates the
 // necessary challenges for it and puts this and all of the relevant information
 // into a corepb.Authorization for transmission to the SA to be stored
-func (ra *RegistrationAuthorityImpl) createPendingAuthz(reg int64, identifier identifier.ACMEIdentifier) (*corepb.Authorization, error) {
-	authz := &corepb.Authorization{
-		Identifier:     identifier.Value,
+func (ra *RegistrationAuthorityImpl) createPendingAuthz(reg int64, identifier identifier.ACMEIdentifier) (*sapb.NewAuthzRequest, error) {
+	authz := &sapb.NewAuthzRequest{
+		IdentifierValue: identifier.Value,
+		Identifier: &sapb.Identifier{
+			Type:  string(identifier.Type),
+			Value: identifier.Value,
+		},
 		RegistrationID: reg,
 		Status:         string(core.StatusPending),
 		Expires:        timestamppb.New(ra.clk.Now().Add(ra.pendingAuthorizationLifetime).Truncate(time.Second)),
@@ -2696,6 +2700,8 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(reg int64, identifier id
 		return nil, berrors.InternalServerError(err.Error())
 	}
 	// Check each challenge for sanity.
+	var token string
+	var challTypes []string
 	for _, challenge := range challenges {
 		err := challenge.CheckPending()
 		if err != nil {
@@ -2704,12 +2710,31 @@ func (ra *RegistrationAuthorityImpl) createPendingAuthz(reg int64, identifier id
 			err = berrors.InternalServerError("challenge didn't pass sanity check: %+v", challenge)
 			return nil, err
 		}
+
+		if token == "" {
+			token = challenge.Token
+		} else {
+			if challenge.Token != token {
+				return nil, berrors.InternalServerError("generated different tokens for challenges within the same authz")
+			}
+		}
+
+		if slices.Contains(challTypes, string(challenge.Type)) {
+			return nil, berrors.InternalServerError("generated multiple challenges of the same type within the same authz")
+		} else {
+			challTypes = append(challTypes, string(challenge.Type))
+		}
+
 		challPB, err := bgrpc.ChallengeToPB(challenge)
 		if err != nil {
 			return nil, err
 		}
 		authz.Challenges = append(authz.Challenges, challPB)
 	}
+
+	authz.Token = token
+	authz.ChallengeTypes = challTypes
+
 	return authz, nil
 }
 

ra/ra_test.go

@@ -72,40 +72,28 @@ import (
 func createPendingAuthorization(t *testing.T, sa sapb.StorageAuthorityClient, domain string, exp time.Time) *corepb.Authorization {
 	t.Helper()
 
-	authz := core.Authorization{
-		Identifier:     identifier.DNSIdentifier(domain),
-		RegistrationID: Registration.Id,
-		Status:         "pending",
-		Expires:        &exp,
-		Challenges: []core.Challenge{
-			{
-				Token:  core.NewToken(),
-				Type:   core.ChallengeTypeHTTP01,
-				Status: core.StatusPending,
-			},
-			{
-				Token:  core.NewToken(),
-				Type:   core.ChallengeTypeDNS01,
-				Status: core.StatusPending,
+	res, err := sa.NewOrderAndAuthzs(
+		context.Background(),
+		&sapb.NewOrderAndAuthzsRequest{
+			NewOrder: &sapb.NewOrderRequest{
+				RegistrationID: Registration.Id,
+				Expires:        timestamppb.New(exp),
+				Names:          []string{domain},
 			},
-			{
-				Token:  core.NewToken(),
-				Type:   core.ChallengeTypeTLSALPN01,
-				Status: core.StatusPending,
+			NewAuthzs: []*sapb.NewAuthzRequest{
+				{
+					Identifier:     &sapb.Identifier{Type: string(core.ChallengeTypeDNS01), Value: domain},
+					RegistrationID: Registration.Id,
+					Expires:        timestamppb.New(exp),
+					ChallengeTypes: []string{
+						string(core.ChallengeTypeHTTP01),
+						string(core.ChallengeTypeDNS01),
+						string(core.ChallengeTypeTLSALPN01)},
+					Token: core.NewToken(),
+				},
 			},
 		},
-	}
-	authzPB, err := bgrpc.AuthzToPB(authz)
-	test.AssertNotError(t, err, "AuthzToPB failed")
-
-	res, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
-		NewOrder: &sapb.NewOrderRequest{
-			RegistrationID: Registration.Id,
-			Expires:        timestamppb.New(exp),
-			Names:          []string{domain},
-		},
-		NewAuthzs: []*corepb.Authorization{authzPB},
-	})
+	)
 	test.AssertNotError(t, err, "sa.NewOrderAndAuthzs failed")
 
 	return getAuthorization(t, fmt.Sprint(res.V2Authorizations[0]), sa)

sa/model.go

@@ -682,6 +682,46 @@ func hasMultipleNonPendingChallenges(challenges []*corepb.Challenge) bool {
 	return false
 }
 
+// newAuthzReqToModel converts an sapb.NewAuthzRequest to the authzModel storage
+// representation. It hardcodes the status to "pending" because it should be
+// impossible to create an authz in any other state.
+func newAuthzReqToModel(authz *sapb.NewAuthzRequest) (*authzModel, error) {
+	if authz.Token == "" && len(authz.ChallengeTypes) == 0 {
+		// This is actually a corepb.Authorization, sent to us by a not-yet-updated
+		// RA. Use the old code-path instead.
+		// TODO(#5913): Remove this fallback.
+		return authzPBToModel(&corepb.Authorization{
+			Id:             authz.Id,
+			Identifier:     authz.IdentifierValue,
+			RegistrationID: authz.RegistrationID,
+			Status:         string(core.StatusPending),
+			Expires:        authz.Expires,
+			Challenges:     authz.Challenges,
+		})
+	}
+
+	am := &authzModel{
+		IdentifierType:  identifierTypeToUint[authz.Identifier.Type],
+		IdentifierValue: authz.Identifier.Value,
+		RegistrationID:  authz.RegistrationID,
+		Status:          statusToUint[core.StatusPending],
+		Expires:         authz.Expires.AsTime(),
+	}
+
+	for _, challType := range authz.ChallengeTypes {
+		// Set the challenge type bit in the bitmap
+		am.Challenges |= 1 << challTypeToUint[challType]
+	}
+
+	token, err := base64.RawURLEncoding.DecodeString(authz.Token)
+	if err != nil {
+		return nil, err
+	}
+	am.Token = token
+
+	return am, nil
+}
+
 // authzPBToModel converts a protobuf authorization representation to the
 // authzModel storage representation.
 func authzPBToModel(authz *corepb.Authorization) (*authzModel, error) {