VA: Make PerformValidation more like DoDCV

core/objects.go

@@ -150,24 +150,6 @@ type ValidationRecord struct {
 	// lookup for AddressUsed. During recursive A and AAAA lookups, a record may
 	// instead look like A:host:port or AAAA:host:port
 	ResolverAddrs []string `json:"resolverAddrs,omitempty"`
-
-	// Perspective uniquely identifies the Network Perspective used to perform
-	// the validation, as specified in BRs Section 5.4.1, Requirement 2.7
-	// ("Multi-Perspective Issuance Corroboration attempts from each Network
-	// Perspective"). It should uniquely identify either the Primary Perspective
-	// (VA) or a group of RVAs deployed in the same datacenter.
-	Perspective string `json:"perspective,omitempty"`
-
-	// RIR indicates the Regional Internet Registry where this RVA is located.
-	// This field is used to identify the RIR region from which a given
-	// validation was performed, as specified in the "Phased Implementation
-	// Timeline" in BRs Section 3.2.2.9. It must be one of the following values:
-	//   - ARIN
-	//   - RIPE
-	//   - APNIC
-	//   - LACNIC
-	//   - AfriNIC
-	RIR string `json:"rir,omitempty"`
 }
 
 // Challenge is an aggregate of all data needed for any challenges.

grpc/pb-marshalling.go

@@ -179,7 +179,7 @@ func PBToValidationRecord(in *corepb.ValidationRecord) (record core.ValidationRe
 	}, nil
 }
 
-func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDetails) (*vapb.ValidationResult, error) {
+func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDetails, perspective, rir string) (*vapb.ValidationResult, error) {
 	recordAry := make([]*corepb.ValidationRecord, len(records))
 	var err error
 	for i, v := range records {
@@ -193,8 +193,10 @@ func ValidationResultToPB(records []core.ValidationRecord, prob *probs.ProblemDe
 		return nil, err
 	}
 	return &vapb.ValidationResult{
-		Records:  recordAry,
-		Problems: marshalledProbs,
+		Records:     recordAry,
+		Problems:    marshalledProbs,
+		Perspective: perspective,
+		Rir:         rir,
 	}, nil
 }
 

grpc/pb-marshalling_test.go

@@ -154,9 +154,11 @@ func TestValidationResult(t *testing.T) {
 	result := []core.ValidationRecord{vrA, vrB}
 	prob := &probs.ProblemDetails{Type: probs.TLSProblem, Detail: "asd", HTTPStatus: 200}
 
-	pb, err := ValidationResultToPB(result, prob)
+	pb, err := ValidationResultToPB(result, prob, "surreal", "ARIN")
 	test.AssertNotError(t, err, "ValidationResultToPB failed")
 	test.Assert(t, pb != nil, "Returned vapb.ValidationResult is nil")
+	test.AssertEquals(t, pb.Perspective, "surreal")
+	test.AssertEquals(t, pb.Rir, "ARIN")
 
 	reconResult, reconProb, err := pbToValidationResult(pb)
 	test.AssertNotError(t, err, "pbToValidationResult failed")

ra/ra.go

@@ -1792,7 +1792,7 @@ func (ra *RegistrationAuthorityImpl) recordValidation(ctx context.Context, authI
 	} else {
 		expires = ra.clk.Now().Add(ra.authorizationLifetime)
 	}
-	vr, err := bgrpc.ValidationResultToPB(challenge.ValidationRecord, challenge.Error)
+	vr, err := bgrpc.ValidationResultToPB(challenge.ValidationRecord, challenge.Error, "", "")
 	if err != nil {
 		return err
 	}

test/v2_integration.py

@@ -1083,8 +1083,8 @@ def test_http_multiva_threshold_fail():
         raise(Exception("no HTTP-01 challenge in failed authz"))
     if httpChall.error.typ != "urn:ietf:params:acme:error:unauthorized":
         raise(Exception("expected unauthorized prob, found {0}".format(httpChall.error.typ)))
-    if not httpChall.error.detail.startswith("During secondary validation: "):
-        raise(Exception("expected 'During secondary validation' problem detail, found {0}".format(httpChall.error.detail)))
+    if not httpChall.error.detail.startswith("During secondary domain validation: "):
+        raise(Exception("expected 'During secondary domain validation' problem detail, found {0}".format(httpChall.error.detail)))
 
 class FakeH2ServerHandler(socketserver.BaseRequestHandler):
     """

va/caa.go

@@ -36,10 +36,10 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC
 		return nil, berrors.InternalServerError("incomplete IsCAAValid request")
 	}
 	logEvent := verificationRequestEvent{
-		// TODO(#7061) Plumb req.Authz.Id as "ID:" through from the RA to
+		// TODO(#7061) Plumb req.Authz.Id as "AuthzID:" through from the RA to
 		// correlate which authz triggered this request.
-		Requester: req.AccountURIID,
-		Hostname:  req.Domain,
+		Requester:  req.AccountURIID,
+		Identifier: req.Domain,
 	}
 
 	challType := core.AcmeChallenge(req.ValidationMethod)
@@ -76,7 +76,7 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC
 			va.observeLatency(opCAA, allPerspectives, string(challType), probType, outcome, va.clk.Since(start))
 		}
 		// Log the total check latency.
-		logEvent.ValidationLatency = va.clk.Since(start).Round(time.Millisecond).Seconds()
+		logEvent.Latency = va.clk.Since(start).Round(time.Millisecond).Seconds()
 
 		va.log.AuditObject("CAA check result", logEvent)
 	}()

va/va.go

@@ -281,15 +281,16 @@ func maxAllowedFailures(perspectiveCount int) int {
 	return 2
 }
 
-// Used for audit logging
+// verificationRequestEvent is logged once for each validation attempt. Its
+// fields are exported for logging purposes.
 type verificationRequestEvent struct {
-	ID                string         `json:",omitempty"`
-	Requester         int64          `json:",omitempty"`
-	Hostname          string         `json:",omitempty"`
-	Challenge         core.Challenge `json:",omitempty"`
-	ValidationLatency float64
-	Error             string `json:",omitempty"`
-	InternalError     string `json:",omitempty"`
+	AuthzID       string
+	Requester     int64
+	Identifier    string
+	Challenge     core.Challenge
+	Error         string `json:",omitempty"`
+	InternalError string `json:",omitempty"`
+	Latency       float64
 }
 
 // ipError is an error type used to pass though the IP address of the remote
@@ -463,14 +464,10 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
 
 	responses := make(chan *response, remoteVACount)
 	for _, i := range rand.Perm(remoteVACount) {
-		go func(rva RemoteVA, out chan<- *response) {
+		go func(rva RemoteVA) {
 			res, err := rva.PerformValidation(ctx, req)
-			out <- &response{
-				addr:   rva.Address,
-				result: res,
-				err:    err,
-			}
-		}(va.remoteVAs[i], responses)
+			responses <- &response{rva.Address, res, err}
+		}(va.remoteVAs[i])
 	}
 
 	required := remoteVACount - va.maxRemoteFailures
@@ -486,10 +483,10 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
 			failed = append(failed, resp.addr)
 
 			if canceled.Is(resp.err) {
-				currProb = probs.ServerInternal("Remote PerformValidation RPC canceled")
+				currProb = probs.ServerInternal("Secondary domain validation RPC canceled")
 			} else {
 				va.log.Errf("Remote VA %q.PerformValidation failed: %s", resp.addr, resp.err)
-				currProb = probs.ServerInternal("Remote PerformValidation RPC failed")
+				currProb = probs.ServerInternal("Secondary domain validation RPC failed")
 			}
 		} else if resp.result.Problems != nil {
 			// The remote VA returned a problem.
@@ -499,7 +496,7 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
 			currProb, err = bgrpc.PBToProblemDetails(resp.result.Problems)
 			if err != nil {
 				va.log.Errf("Remote VA %q.PerformValidation returned malformed problem: %s", resp.addr, err)
-				currProb = probs.ServerInternal("Remote PerformValidation RPC returned malformed result")
+				currProb = probs.ServerInternal("Secondary domain validation RPC returned malformed result")
 			}
 		} else {
 			// The remote VA returned a successful result.
@@ -517,7 +514,7 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
 		}
 		if len(failed) > va.maxRemoteFailures {
 			// Too many failed responses to reach quorum.
-			firstProb.Detail = fmt.Sprintf("During secondary validation: %s", firstProb.Detail)
+			firstProb.Detail = fmt.Sprintf("During secondary domain validation: %s", firstProb.Detail)
 			return firstProb
 		}
 
@@ -623,9 +620,16 @@ func (va *ValidationAuthorityImpl) performLocalValidation(
 	return records, nil
 }
 
-// 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.
+// PerformValidation conducts a local Domain Control Validation (DCV) and CAA
+// check for the specified challenge and dnsName. When invoked on the primary
+// Validation Authority (VA) and the local validation succeeds, it also performs
+// DCV and CAA checks using the configured remote VAs. Failed validations are
+// indicated by a non-nil Problems in the returned ValidationResult.
+// PerformValidation returns error only for internal logic errors (and the
+// client may receive errors from gRPC in the event of a communication problem).
+// ValidationResult always includes a list of ValidationRecords, even when it
+// also contains Problems. This method does NOT implement Multi-Perspective
+// Issuance Corroboration as defined in BRs Sections 3.2.2.9 and 5.4.1.
 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")
@@ -648,10 +652,10 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
 	var localLatency time.Duration
 	start := va.clk.Now()
 	logEvent := verificationRequestEvent{
-		ID:        req.Authz.Id,
-		Requester: req.Authz.RegID,
-		Hostname:  req.DnsName,
-		Challenge: chall,
+		AuthzID:    req.Authz.Id,
+		Requester:  req.Authz.RegID,
+		Identifier: req.DnsName,
+		Challenge:  chall,
 	}
 	defer func() {
 		probType := ""
@@ -673,7 +677,7 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
 		}
 
 		// Log the total validation latency.
-		logEvent.ValidationLatency = time.Since(start).Round(time.Millisecond).Seconds()
+		logEvent.Latency = time.Since(start).Round(time.Millisecond).Seconds()
 		va.log.AuditObject("Validation result", logEvent)
 	}()
 
@@ -701,7 +705,7 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
 	if err != nil {
 		logEvent.InternalError = err.Error()
 		prob = detailedError(err)
-		return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob))
+		return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir)
 	}
 
 	// Do remote validation. We do this after local validation is complete to
@@ -710,5 +714,5 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
 	// own validation records, and it's not helpful to present multiple large
 	// errors to the end user.
 	prob = va.performRemoteValidation(ctx, req)
-	return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob))
+	return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir)
 }

va/va_test.go

@@ -304,8 +304,8 @@ func TestPerformValidationValid(t *testing.T) {
 	if len(resultLog) != 1 {
 		t.Fatalf("Wrong number of matching lines for 'Validation result'")
 	}
-	if !strings.Contains(resultLog[0], `"Hostname":"good-dns01.com"`) {
-		t.Error("PerformValidation didn't log validation hostname.")
+	if !strings.Contains(resultLog[0], `"Identifier":"good-dns01.com"`) {
+		t.Error("PerformValidation didn't log validation identifier.")
 	}
 }
 
@@ -332,9 +332,9 @@ func TestPerformValidationWildcard(t *testing.T) {
 		t.Fatalf("Wrong number of matching lines for 'Validation result'")
 	}
 
-	// We expect that the top level Hostname reflect the wildcard name
-	if !strings.Contains(resultLog[0], `"Hostname":"*.good-dns01.com"`) {
-		t.Errorf("PerformValidation didn't log correct validation hostname.")
+	// We expect that the top level Identifier reflect the wildcard name
+	if !strings.Contains(resultLog[0], `"Identifier":"*.good-dns01.com"`) {
+		t.Errorf("PerformValidation didn't log correct validation identifier.")
 	}
 	// We expect that the ValidationRecord contain the correct non-wildcard
 	// hostname that was validated
@@ -451,7 +451,7 @@ func TestMultiVA(t *testing.T) {
 				{brokenVA, "broken"},
 			},
 			AllowedUAs:   allowedUAs,
-			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
+			ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC failed"),
 			// The real failure cause should be logged
 			ExpectedLog: expectedInternalErrLine,
 		},
@@ -478,7 +478,7 @@ func TestMultiVA(t *testing.T) {
 				{brokenVA, "broken"},
 			},
 			AllowedUAs:   allowedUAs,
-			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
+			ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC failed"),
 			// The real failure cause should be logged
 			ExpectedLog: expectedInternalErrLine,
 		},
@@ -507,7 +507,7 @@ func TestMultiVA(t *testing.T) {
 				{brokenVA, "broken"},
 			},
 			AllowedUAs:   allowedUAs,
-			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
+			ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC failed"),
 			// The real failure cause should be logged
 			ExpectedLog: expectedInternalErrLine,
 		},
@@ -517,7 +517,7 @@ func TestMultiVA(t *testing.T) {
 			RemoteVAs:  remoteVAs,
 			AllowedUAs: map[string]bool{localUA: true, remoteUA2: true},
 			ExpectedProb: probs.Unauthorized(fmt.Sprintf(
-				`During secondary validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
+				`During secondary domain validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
 				expectedKeyAuthorization)),
 		},
 		{
@@ -539,15 +539,15 @@ func TestMultiVA(t *testing.T) {
 				{cancelledVA, remoteUA3},
 			},
 			AllowedUAs:   allowedUAs,
-			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"),
+			ExpectedProb: probs.ServerInternal("During secondary domain validation: Secondary domain validation RPC canceled"),
 		},
 		{
 			// With the local and remote VAs seeing diff problems, we expect a problem.
 			Name:       "Local and remote VA differential, full results, enforce multi VA",
 			RemoteVAs:  remoteVAs,
 			AllowedUAs: map[string]bool{localUA: true},
 			ExpectedProb: probs.Unauthorized(fmt.Sprintf(
-				`During secondary validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
+				`During secondary domain validation: The key authorization file from the server did not match this challenge. Expected %q (got "???")`,
 				expectedKeyAuthorization)),
 		},
 	}