More changes to bring DoDCV into better alignment for PerformValidation

va/va.go

@@ -97,7 +97,7 @@ type RemoteVA struct {
 type vaMetrics struct {
 	// validationLatency is a histogram of the latency to perform validations
 	// from the primary and remote VA perspectives. It's labelled by:
-	//   - operation: VA.ValidateChallenge or VA.CheckCAA as [challenge|caa|challenge+caa]
+	//   - operation: VA.DoDCV or VA.CheckCAA as [challenge|caa|challenge+caa]
 	//   - perspective: ValidationAuthorityImpl.perspective
 	//   - challenge_type: core.Challenge.Type
 	//   - problem_type: probs.ProblemType
@@ -428,7 +428,7 @@ func (va *ValidationAuthorityImpl) validateChallenge(
 // observeLatency records entries in the validationLatency histogram of the
 // latency to perform validations from the primary and remote VA perspectives.
 // The labels are:
-//   - operation: VA.ValidateChallenge or VA.CheckCAA as [challenge|caa]
+//   - operation: VA.DoDCV or VA.CheckCAA as [challenge|caa]
 //   - perspective: [ValidationAuthorityImpl.perspective|all]
 //   - challenge_type: core.Challenge.Type
 //   - problem_type: probs.ProblemType
@@ -468,11 +468,7 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
 	for _, i := range rand.Perm(remoteVACount) {
 		go func(rva RemoteVA, out chan<- *response) {
 			res, err := rva.PerformValidation(ctx, req)
-			out <- &response{
-				addr:   rva.Address,
-				result: res,
-				err:    err,
-			}
+			out <- &response{rva.Address, res, err}
 		}(va.remoteVAs[i], responses)
 	}
 
@@ -626,13 +622,9 @@ func (va *ValidationAuthorityImpl) performLocalValidation(
 	return records, nil
 }
 
-// PerformValidation performs a local Domain Control Validation (DCV) and CAA
-// check for the provided challenge and dnsName. If called on the primary VA and
-// local validation passes, it will also perform DCV and CAA checks using the
-// configured remote VAs. It returns a validation result and an error if the
-// validation failed. The returned result will always contain a list of
-// validation records, even when it also contains a problem. This method is not
-// MPIC-compliant.
+// PerformValidation validates the challenge for the domain in the request.
+// The returned result will always contain a list of validation records, even
+// when it also contains a problem.
 func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) {
 	if core.IsAnyNilOrZero(req, req.DnsName, req.Challenge, req.Authz, req.ExpectedKeyAuthorization) {
 		return nil, berrors.InternalServerError("Incomplete validation request")

va/vampic.go

@@ -9,6 +9,7 @@ import (
 	"slices"
 	"time"
 
+	"github.com/letsencrypt/boulder/canceled"
 	"github.com/letsencrypt/boulder/core"
 	berrors "github.com/letsencrypt/boulder/errors"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
@@ -97,7 +98,7 @@ func (va *ValidationAuthorityImpl) remoteDoDCV(ctx context.Context, req *vapb.DC
 		err    error
 	}
 
-	responses := make(chan *response, remoteVACount)
+	var responses = make(chan *response, remoteVACount)
 	for _, i := range rand.Perm(remoteVACount) {
 		go func(rva RemoteVA) {
 			res, err := rva.DoDCV(ctx, req)
@@ -109,20 +110,22 @@ func (va *ValidationAuthorityImpl) remoteDoDCV(ctx context.Context, req *vapb.DC
 		}(va.remoteVAs[i])
 	}
 
+	required := remoteVACount - va.maxRemoteFailures
 	var passed []string
 	var failed []string
 	passedRIRs := make(map[string]struct{})
 
 	var firstProb *probs.ProblemDetails
-	for resp := range responses {
+	for i := 0; i < remoteVACount; i++ {
+		resp := <-responses
 		var currProb *probs.ProblemDetails
 		if resp.err != nil {
 			// Failed to communicate with the remote VA.
 			failed = append(failed, resp.addr)
-			if errors.Is(resp.err, context.Canceled) {
+			if canceled.Is(resp.err) {
 				currProb = probs.ServerInternal("Secondary domain validation RPC canceled")
 			} else {
-				va.log.Errf("Remote VA %q.ValidateChallenge failed: %s", resp.addr, resp.err)
+				va.log.Errf("Remote VA %q.DoDCV failed: %s", resp.addr, resp.err)
 				currProb = probs.ServerInternal("Secondary domain validation RPC failed")
 			}
 
@@ -133,7 +136,7 @@ func (va *ValidationAuthorityImpl) remoteDoDCV(ctx context.Context, req *vapb.DC
 			var err error
 			currProb, err = bgrpc.PBToProblemDetails(resp.result.Problems)
 			if err != nil {
-				va.log.Errf("Remote VA %q.ValidateChallenge returned a malformed problem: %s", resp.addr, err)
+				va.log.Errf("Remote VA %q.DoDCV returned malformed problem: %s", resp.addr, err)
 				currProb = probs.ServerInternal("Secondary domain validation RPC returned malformed result")
 			}
 
@@ -152,8 +155,7 @@ func (va *ValidationAuthorityImpl) remoteDoDCV(ctx context.Context, req *vapb.DC
 	// Prepare the summary, this MUST be returned even if the validation failed.
 	summary := prepareSummary(passed, failed, passedRIRs, remoteVACount)
 
-	maxRemoteFailures := maxAllowedFailures(remoteVACount)
-	if len(failed) > maxRemoteFailures {
+	if len(failed) > va.maxRemoteFailures {
 		// Too many failures to reach quorum.
 		if firstProb != nil {
 			firstProb.Detail = fmt.Sprintf("During secondary domain validation: %s", firstProb.Detail)
@@ -162,7 +164,7 @@ func (va *ValidationAuthorityImpl) remoteDoDCV(ctx context.Context, req *vapb.DC
 		return summary, probs.ServerInternal("Secondary domain validation failed due to too many failures")
 	}
 
-	if len(passed) < (remoteVACount - maxRemoteFailures) {
+	if len(passed) < required {
 		// Too few successful responses to reach quorum.
 		if firstProb != nil {
 			firstProb.Detail = fmt.Sprintf("During secondary domain validation: %s", firstProb.Detail)