From 9e4b47dcd6b559315f237971712680d93b2d7cdb Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 11:30:24 -0800
Subject: [PATCH 01/16] va: compute maxRemoteFailures based on MPIC

Previously this was a configuration field.
---
 cmd/boulder-va/main.go |  6 +++---
 cmd/remoteva/main.go   |  1 -
 docs/multi-va.md       |  2 +-
 va/caa_test.go         |  3 ++-
 va/va.go               | 21 +++++++++++++++++++--
 va/va_test.go          |  6 +-----
 6 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/cmd/boulder-va/main.go b/cmd/boulder-va/main.go
index 60353424abd..88aaff28b77 100644
--- a/cmd/boulder-va/main.go
+++ b/cmd/boulder-va/main.go
@@ -18,8 +18,9 @@ import (
 type Config struct {
 	VA struct {
 		vaConfig.Common
-		RemoteVAs                   []cmd.GRPCClientConfig `validate:"omitempty,dive"`
-		MaxRemoteValidationFailures int                    `validate:"omitempty,min=0,required_with=RemoteVAs"`
+		RemoteVAs []cmd.GRPCClientConfig `validate:"omitempty,dive"`
+		// Deprecated and ignored
+		MaxRemoteValidationFailures int `validate:"omitempty,min=0,required_with=RemoteVAs"`
 		Features                    features.Config
 	}
 
@@ -109,7 +110,6 @@ func main() {
 	vai, err := va.NewValidationAuthorityImpl(
 		resolver,
 		remotes,
-		c.VA.MaxRemoteValidationFailures,
 		c.VA.UserAgent,
 		c.VA.IssuerDomain,
 		scope,
diff --git a/cmd/remoteva/main.go b/cmd/remoteva/main.go
index 49db5c17953..97320f9710f 100644
--- a/cmd/remoteva/main.go
+++ b/cmd/remoteva/main.go
@@ -135,7 +135,6 @@ func main() {
 	vai, err := va.NewValidationAuthorityImpl(
 		resolver,
 		nil, // Our RVAs will never have RVAs of their own.
-		0,   // Only the VA is concerned with max validation failures
 		c.RVA.UserAgent,
 		c.RVA.IssuerDomain,
 		scope,
diff --git a/docs/multi-va.md b/docs/multi-va.md
index 4c8df880daa..d1d0b044f7d 100644
--- a/docs/multi-va.md
+++ b/docs/multi-va.md
@@ -38,7 +38,7 @@ and
 as their config files.
 
 We require that almost all remote validation requests succeed; the exact number
-is controlled by the VA's `maxRemoteFailures` config variable. If the number of
+is controlled by the VA based on the thresholds required by MPIC. If the number of
 failing remote VAs exceeds that threshold, validation is terminated. If the
 number of successful remote VAs is high enough that it would be impossible for
 the outstanding remote VAs to exceed that threshold, validation immediately
diff --git a/va/caa_test.go b/va/caa_test.go
index 477d3b84b3d..f6495d239da 100644
--- a/va/caa_test.go
+++ b/va/caa_test.go
@@ -858,7 +858,6 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "1 hijacked RVA, CAA issuewild type present, 1 failure allowed",
 			domains:                  "satisfiable-wildcard.com",
-			maxLookupFailures:        1,
 			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`,
 			localDNSClient:           caaMockDNS{},
 			remoteVAs: []RemoteVA{
@@ -920,8 +919,10 @@ func TestMultiCAARechecking(t *testing.T) {
 			test.AssertNotError(t, err, "Should not have errored, but did")
 
 			if tc.expectedProbSubstring != "" {
+				test.AssertNotNil(t, isValidRes.Problem, "IsCAAValidRequest returned nil problem, but should not have")
 				test.AssertContains(t, isValidRes.Problem.Detail, tc.expectedProbSubstring)
 			} else if isValidRes.Problem != nil {
+				t.Errorf("Problem: %v", isValidRes.Problem)
 				test.AssertBoxedNil(t, isValidRes.Problem, "IsCAAValidRequest returned a problem, but should not have")
 			}
 
diff --git a/va/va.go b/va/va.go
index 17c03cf6e01..13c4c47a26c 100644
--- a/va/va.go
+++ b/va/va.go
@@ -271,7 +271,6 @@ var _ vapb.CAAServer = (*ValidationAuthorityImpl)(nil)
 func NewValidationAuthorityImpl(
 	resolver bdns.Client,
 	remoteVAs []RemoteVA,
-	maxRemoteFailures int,
 	userAgent string,
 	issuerDomain string,
 	stats prometheus.Registerer,
@@ -299,7 +298,7 @@ func NewValidationAuthorityImpl(
 		clk:                clk,
 		metrics:            initMetrics(stats),
 		remoteVAs:          remoteVAs,
-		maxRemoteFailures:  maxRemoteFailures,
+		maxRemoteFailures:  maxAllowedFailures(len(remoteVAs)),
 		accountURIPrefixes: accountURIPrefixes,
 		// singleDialTimeout specifies how long an individual `DialContext` operation may take
 		// before timing out. This timeout ignores the base RPC timeout and is strictly
@@ -313,6 +312,24 @@ func NewValidationAuthorityImpl(
 	return va, nil
 }
 
+// maxAllowedFailures returns the maximum number of allowed failures
+// for a given number of remote perspectives, according to the "Quorum
+// Requirements" table in BRs Section 3.2.2.9, as follows:
+//
+//	| # of Distinct Remote Network Perspectives Used | # of Allowed non-Corroborations |
+//	| --- | --- |
+//	| 2-5 |  1  |
+//	| 6+  |  2  |
+func maxAllowedFailures(perspectiveCount int) int {
+	if perspectiveCount < 2 {
+		return 0
+	}
+	if perspectiveCount < 6 {
+		return 1
+	}
+	return 2
+}
+
 // Used for audit logging
 type verificationRequestEvent struct {
 	ID                string         `json:",omitempty"`
diff --git a/va/va_test.go b/va/va_test.go
index 8a5c07776a2..7db1c189cc4 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -115,8 +115,7 @@ func setup(srv *httptest.Server, maxRemoteFailures int, userAgent string, remote
 
 	va, err := NewValidationAuthorityImpl(
 		&bdns.MockClient{Log: logger},
-		nil,
-		maxRemoteFailures,
+		remoteVAs,
 		userAgent,
 		"letsencrypt.org",
 		metrics.NoopRegisterer,
@@ -142,9 +141,6 @@ func setup(srv *httptest.Server, maxRemoteFailures int, userAgent string, remote
 	if err != nil {
 		panic(fmt.Sprintf("Failed to create validation authority: %v", err))
 	}
-	if remoteVAs != nil {
-		va.remoteVAs = remoteVAs
-	}
 	return va, logger
 }
 

From 9165051c7d87a476258c051fb0c88b0ebac71fea Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 11:38:38 -0800
Subject: [PATCH 02/16] va: update tests to not set maxRemoteFailures

---
 va/caa_test.go     | 20 ++++++++------------
 va/dns_test.go     | 20 ++++++++++----------
 va/http_test.go    | 32 ++++++++++++++++----------------
 va/tlsalpn_test.go | 40 ++++++++++++++++++++--------------------
 va/va_test.go      | 28 ++++++++++++++--------------
 5 files changed, 68 insertions(+), 72 deletions(-)

diff --git a/va/caa_test.go b/va/caa_test.go
index f6495d239da..4c4f4d26ac0 100644
--- a/va/caa_test.go
+++ b/va/caa_test.go
@@ -190,7 +190,7 @@ func (mock caaMockDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA,
 }
 
 func TestCAATimeout(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, caaMockDNS{})
+	va, _ := setup(nil, "", nil, caaMockDNS{})
 
 	params := &caaParams{
 		accountURIID:     12345,
@@ -407,7 +407,7 @@ func TestCAAChecking(t *testing.T) {
 	method := core.ChallengeTypeHTTP01
 	params := &caaParams{accountURIID: accountURIID, validationMethod: method}
 
-	va, _ := setup(nil, 0, "", nil, caaMockDNS{})
+	va, _ := setup(nil, "", nil, caaMockDNS{})
 	va.accountURIPrefixes = []string{"https://letsencrypt.org/acct/reg/"}
 
 	for _, caaTest := range testCases {
@@ -430,7 +430,7 @@ func TestCAAChecking(t *testing.T) {
 }
 
 func TestCAALogging(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, caaMockDNS{})
+	va, _ := setup(nil, "", nil, caaMockDNS{})
 
 	testCases := []struct {
 		Name            string
@@ -520,7 +520,7 @@ func TestCAALogging(t *testing.T) {
 // TestIsCAAValidErrMessage tests that an error result from `va.IsCAAValid`
 // includes the domain name that was being checked in the failure detail.
 func TestIsCAAValidErrMessage(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, caaMockDNS{})
+	va, _ := setup(nil, "", nil, caaMockDNS{})
 
 	// Call IsCAAValid with a domain we know fails with a generic error from the
 	// caaMockDNS.
@@ -545,7 +545,7 @@ func TestIsCAAValidErrMessage(t *testing.T) {
 // which do not have the necessary parameters to do CAA Account and Method
 // Binding checks.
 func TestIsCAAValidParams(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, caaMockDNS{})
+	va, _ := setup(nil, "", nil, caaMockDNS{})
 
 	// Calling IsCAAValid without a ValidationMethod should fail.
 	_, err := va.IsCAAValid(ctx, &vapb.IsCAAValidRequest{
@@ -591,7 +591,7 @@ func (b caaBrokenDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, s
 func TestDisabledMultiCAARechecking(t *testing.T) {
 	brokenRVA, _ := setupRemote(nil, "broken", caaBrokenDNS{}, "", "")
 	remoteVAs := []RemoteVA{{brokenRVA, "broken"}}
-	va, _ := setup(nil, 0, "local", remoteVAs, nil)
+	va, _ := setup(nil, "local", remoteVAs, nil)
 
 	features.Set(features.Config{
 		EnforceMultiCAA:     false,
@@ -670,7 +670,6 @@ func TestMultiCAARechecking(t *testing.T) {
 
 	testCases := []struct {
 		name                     string
-		maxLookupFailures        int
 		domains                  string
 		remoteVAs                []RemoteVA
 		expectedProbSubstring    string
@@ -869,7 +868,6 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "2 hijacked RVAs, CAA issuewild type present, 1 failure allowed",
 			domains:                  "satisfiable-wildcard.com",
-			maxLookupFailures:        1,
 			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
 			expectedProbType:         probs.CAAProblem,
 			expectedDiffLogSubstring: `RemoteSuccesses":1,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`,
@@ -883,7 +881,6 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "3 hijacked RVAs, CAA issuewild type present, 1 failure allowed",
 			domains:                  "satisfiable-wildcard.com",
-			maxLookupFailures:        1,
 			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
 			expectedProbType:         probs.CAAProblem,
 			expectedDiffLogSubstring: `RemoteSuccesses":0,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`,
@@ -898,7 +895,7 @@ func TestMultiCAARechecking(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			va, mockLog := setup(nil, tc.maxLookupFailures, localUA, tc.remoteVAs, tc.localDNSClient)
+			va, mockLog := setup(nil, localUA, tc.remoteVAs, tc.localDNSClient)
 			defer mockLog.Clear()
 
 			// MultiCAAFullResults: false is inherently flaky because of the
@@ -922,7 +919,6 @@ func TestMultiCAARechecking(t *testing.T) {
 				test.AssertNotNil(t, isValidRes.Problem, "IsCAAValidRequest returned nil problem, but should not have")
 				test.AssertContains(t, isValidRes.Problem.Detail, tc.expectedProbSubstring)
 			} else if isValidRes.Problem != nil {
-				t.Errorf("Problem: %v", isValidRes.Problem)
 				test.AssertBoxedNil(t, isValidRes.Problem, "IsCAAValidRequest returned a problem, but should not have")
 			}
 
@@ -963,7 +959,7 @@ func TestCAAFailure(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
 
-	va, _ := setup(hs, 0, "", nil, caaMockDNS{})
+	va, _ := setup(hs, "", nil, caaMockDNS{})
 
 	err := va.checkCAA(ctx, dnsi("reserved.com"), &caaParams{1, core.ChallengeTypeHTTP01})
 	if err == nil {
diff --git a/va/dns_test.go b/va/dns_test.go
index a545228a47f..71c45b58276 100644
--- a/va/dns_test.go
+++ b/va/dns_test.go
@@ -19,7 +19,7 @@ import (
 )
 
 func TestDNSValidationEmpty(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	// This test calls PerformValidation directly, because that is where the
 	// metrics checked below are incremented.
@@ -36,7 +36,7 @@ func TestDNSValidationEmpty(t *testing.T) {
 }
 
 func TestDNSValidationWrong(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 	_, err := va.validateDNS01(context.Background(), dnsi("wrong-dns01.com"), expectedKeyAuthorization)
 	if err == nil {
 		t.Fatalf("Successful DNS validation with wrong TXT record")
@@ -46,7 +46,7 @@ func TestDNSValidationWrong(t *testing.T) {
 }
 
 func TestDNSValidationWrongMany(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, err := va.validateDNS01(context.Background(), dnsi("wrong-many-dns01.com"), expectedKeyAuthorization)
 	if err == nil {
@@ -57,7 +57,7 @@ func TestDNSValidationWrongMany(t *testing.T) {
 }
 
 func TestDNSValidationWrongLong(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, err := va.validateDNS01(context.Background(), dnsi("long-dns01.com"), expectedKeyAuthorization)
 	if err == nil {
@@ -68,7 +68,7 @@ func TestDNSValidationWrongLong(t *testing.T) {
 }
 
 func TestDNSValidationFailure(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, err := va.validateDNS01(ctx, dnsi("localhost"), expectedKeyAuthorization)
 	prob := detailedError(err)
@@ -82,7 +82,7 @@ func TestDNSValidationInvalid(t *testing.T) {
 		Value: "790DB180-A274-47A4-855F-31C428CB1072",
 	}
 
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, err := va.validateDNS01(ctx, notDNS, expectedKeyAuthorization)
 	prob := detailedError(err)
@@ -91,7 +91,7 @@ func TestDNSValidationInvalid(t *testing.T) {
 }
 
 func TestDNSValidationServFail(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, err := va.validateDNS01(ctx, dnsi("servfail.com"), expectedKeyAuthorization)
 
@@ -100,7 +100,7 @@ func TestDNSValidationServFail(t *testing.T) {
 }
 
 func TestDNSValidationNoServer(t *testing.T) {
-	va, log := setup(nil, 0, "", nil, nil)
+	va, log := setup(nil, "", nil, nil)
 	staticProvider, err := bdns.NewStaticProvider([]string{})
 	test.AssertNotError(t, err, "Couldn't make new static provider")
 
@@ -119,7 +119,7 @@ func TestDNSValidationNoServer(t *testing.T) {
 }
 
 func TestDNSValidationOK(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, prob := va.validateDNS01(ctx, dnsi("good-dns01.com"), expectedKeyAuthorization)
 
@@ -127,7 +127,7 @@ func TestDNSValidationOK(t *testing.T) {
 }
 
 func TestDNSValidationNoAuthorityOK(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, prob := va.validateDNS01(ctx, dnsi("no-authority-dns01.com"), expectedKeyAuthorization)
 
diff --git a/va/http_test.go b/va/http_test.go
index 2885a7382aa..835c7fbc295 100644
--- a/va/http_test.go
+++ b/va/http_test.go
@@ -71,7 +71,7 @@ func (mock dnsMockReturnsUnroutable) LookupHost(_ context.Context, hostname stri
 // the appropriate "Timeout during connect" error message, which helps clients
 // distinguish between firewall problems and server problems.
 func TestDialerTimeout(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 	// Timeouts below 50ms tend to be flaky.
 	va.singleDialTimeout = 50 * time.Millisecond
 
@@ -167,7 +167,7 @@ func TestHTTPValidationTarget(t *testing.T) {
 		exampleQuery = "my-path=was&my=own"
 	)
 
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
 			target, err := va.newHTTPValidationTarget(
@@ -298,7 +298,7 @@ func TestExtractRequestTarget(t *testing.T) {
 		},
 	}
 
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
 			host, port, err := va.extractRequestTarget(tc.Req)
@@ -320,7 +320,7 @@ func TestExtractRequestTarget(t *testing.T) {
 // generates a DNS error, and checks that a log line with the detailed error is
 // generated.
 func TestHTTPValidationDNSError(t *testing.T) {
-	va, mockLog := setup(nil, 0, "", nil, nil)
+	va, mockLog := setup(nil, "", nil, nil)
 
 	_, _, prob := va.fetchHTTP(ctx, "always.error", "/.well-known/acme-challenge/whatever")
 	test.AssertError(t, prob, "Expected validation fetch to fail")
@@ -336,7 +336,7 @@ func TestHTTPValidationDNSError(t *testing.T) {
 // the mock resolver results in valid query/response data being logged in
 // a format we can decode successfully.
 func TestHTTPValidationDNSIdMismatchError(t *testing.T) {
-	va, mockLog := setup(nil, 0, "", nil, nil)
+	va, mockLog := setup(nil, "", nil, nil)
 
 	_, _, prob := va.fetchHTTP(ctx, "id.mismatch", "/.well-known/acme-challenge/whatever")
 	test.AssertError(t, prob, "Expected validation fetch to fail")
@@ -375,7 +375,7 @@ func TestHTTPValidationDNSIdMismatchError(t *testing.T) {
 }
 
 func TestSetupHTTPValidation(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	mustTarget := func(t *testing.T, host string, port int, path string) *httpValidationTarget {
 		target, err := va.newHTTPValidationTarget(
@@ -745,7 +745,7 @@ func TestFetchHTTP(t *testing.T) {
 
 	// Setup a VA. By providing the testSrv to setup the VA will use the testSrv's
 	// randomly assigned port as its HTTP port.
-	va, _ := setup(testSrv, 0, "", nil, nil)
+	va, _ := setup(testSrv, "", nil, nil)
 
 	// We need to know the randomly assigned HTTP port for testcases as well
 	httpPort := getPort(testSrv)
@@ -1216,7 +1216,7 @@ func TestHTTPBadPort(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	// Pick a random port between 40000 and 65000 - with great certainty we won't
 	// have an HTTP server listening on this port and the test will fail as
@@ -1243,7 +1243,7 @@ func TestHTTPKeyAuthorizationFileMismatch(t *testing.T) {
 	})
 	hs.Start()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 	_, err := va.validateHTTP01(ctx, dnsi("localhost.com"), expectedToken, expectedKeyAuthorization)
 
 	if err == nil {
@@ -1259,7 +1259,7 @@ func TestHTTP(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
 
-	va, log := setup(hs, 0, "", nil, nil)
+	va, log := setup(hs, "", nil, nil)
 
 	_, err := va.validateHTTP01(ctx, dnsi("localhost.com"), expectedToken, expectedKeyAuthorization)
 	if err != nil {
@@ -1323,7 +1323,7 @@ func TestHTTPTimeout(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	started := time.Now()
 	timeout := 250 * time.Millisecond
@@ -1351,7 +1351,7 @@ func TestHTTPTimeout(t *testing.T) {
 func TestHTTPRedirectLookup(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
-	va, log := setup(hs, 0, "", nil, nil)
+	va, log := setup(hs, "", nil, nil)
 
 	_, err := va.validateHTTP01(ctx, dnsi("localhost.com"), pathMoved, ka(pathMoved))
 	if err != nil {
@@ -1413,7 +1413,7 @@ func TestHTTPRedirectLookup(t *testing.T) {
 func TestHTTPRedirectLoop(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateHTTP01(ctx, dnsi("localhost"), "looper", ka("looper"))
 	if prob == nil {
@@ -1424,7 +1424,7 @@ func TestHTTPRedirectLoop(t *testing.T) {
 func TestHTTPRedirectUserAgent(t *testing.T) {
 	hs := httpSrv(t, expectedToken)
 	defer hs.Close()
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 	va.userAgent = rejectUserAgent
 
 	_, prob := va.validateHTTP01(ctx, dnsi("localhost"), pathMoved, ka(pathMoved))
@@ -1460,7 +1460,7 @@ func TestValidateHTTP(t *testing.T) {
 	hs := httpSrv(t, token)
 	defer hs.Close()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateHTTP01(ctx, dnsi("localhost"), token, ka(token))
 	test.Assert(t, prob == nil, "validation failed")
@@ -1470,7 +1470,7 @@ func TestLimitedReader(t *testing.T) {
 	token := core.NewToken()
 
 	hs := httpSrv(t, "012345\xff67890123456789012345678901234567890123456789012345678901234567890123456789")
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 	defer hs.Close()
 
 	_, err := va.validateHTTP01(ctx, dnsi("localhost"), token, ka(token))
diff --git a/va/tlsalpn_test.go b/va/tlsalpn_test.go
index ce68008727d..fc8d5385997 100644
--- a/va/tlsalpn_test.go
+++ b/va/tlsalpn_test.go
@@ -137,7 +137,7 @@ func TestTLSALPN01FailIP(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, 0, "expected")
 	test.AssertNotError(t, err, "Error creating test server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err = va.validateTLSALPN01(ctx, identifier.NewIP(netip.MustParseAddr("127.0.0.1")), expectedKeyAuthorization)
 	if err == nil {
@@ -162,7 +162,7 @@ func slowTLSSrv() *httptest.Server {
 
 func TestTLSALPNTimeoutAfterConnect(t *testing.T) {
 	hs := slowTLSSrv()
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	timeout := 50 * time.Millisecond
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
@@ -199,7 +199,7 @@ func TestTLSALPNTimeoutAfterConnect(t *testing.T) {
 
 func TestTLSALPN01DialTimeout(t *testing.T) {
 	hs := slowTLSSrv()
-	va, _ := setup(hs, 0, "", nil, dnsMockReturnsUnroutable{&bdns.MockClient{}})
+	va, _ := setup(hs, "", nil, dnsMockReturnsUnroutable{&bdns.MockClient{}})
 	started := time.Now()
 
 	timeout := 50 * time.Millisecond
@@ -249,7 +249,7 @@ func TestTLSALPN01Refused(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, 0, "expected")
 	test.AssertNotError(t, err, "Error creating test server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 	// Take down validation server and check that validation fails.
 	hs.Close()
 	_, err = va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
@@ -268,7 +268,7 @@ func TestTLSALPN01TalkingToHTTP(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, 0, "expected")
 	test.AssertNotError(t, err, "Error creating test server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 	httpOnly := httpSrv(t, "")
 	va.tlsPort = getPort(httpOnly)
 
@@ -295,7 +295,7 @@ func brokenTLSSrv() *httptest.Server {
 func TestTLSError(t *testing.T) {
 	hs := brokenTLSSrv()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	if err == nil {
@@ -311,7 +311,7 @@ func TestTLSError(t *testing.T) {
 func TestDNSError(t *testing.T) {
 	hs := brokenTLSSrv()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err := va.validateTLSALPN01(ctx, dnsi("always.invalid"), expectedKeyAuthorization)
 	if err == nil {
@@ -386,7 +386,7 @@ func TestTLSALPN01Success(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, 0, "expected")
 	test.AssertNotError(t, err, "Error creating test server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	if prob != nil {
@@ -411,7 +411,7 @@ func TestTLSALPN01ObsoleteFailure(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifierV1Obsolete, 0, "expected")
 	test.AssertNotError(t, err, "Error creating test server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertNotNil(t, prob, "expected validation to fail")
@@ -423,7 +423,7 @@ func TestValidateTLSALPN01BadChallenge(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, badKeyAuthorization, IdPeAcmeIdentifier, 0, "expected")
 	test.AssertNotError(t, err, "Error creating test server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err = va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 
@@ -445,7 +445,7 @@ func TestValidateTLSALPN01BadChallenge(t *testing.T) {
 func TestValidateTLSALPN01BrokenSrv(t *testing.T) {
 	hs := brokenTLSSrv()
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	if err == nil {
@@ -458,7 +458,7 @@ func TestValidateTLSALPN01BrokenSrv(t *testing.T) {
 func TestValidateTLSALPN01UnawareSrv(t *testing.T) {
 	hs := tlssniSrvWithNames(t, "expected")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	if err == nil {
@@ -508,7 +508,7 @@ func TestValidateTLSALPN01MalformedExtnValue(t *testing.T) {
 		}
 
 		hs := tlsalpn01SrvWithCert(t, acmeCert, 0)
-		va, _ := setup(hs, 0, "", nil, nil)
+		va, _ := setup(hs, "", nil, nil)
 
 		_, err := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 		hs.Close()
@@ -547,7 +547,7 @@ func TestTLSALPN01TLSVersion(t *testing.T) {
 		hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, tc.version, "expected")
 		test.AssertNotError(t, err, "Error creating test server")
 
-		va, _ := setup(hs, 0, "", nil, nil)
+		va, _ := setup(hs, "", nil, nil)
 
 		_, prob := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 		if !tc.expectError {
@@ -572,7 +572,7 @@ func TestTLSALPN01WrongName(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, tls.VersionTLS12, "incorrect")
 	test.AssertNotError(t, err, "failed to set up tls-alpn-01 server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertError(t, prob, "validation should have failed")
@@ -583,7 +583,7 @@ func TestTLSALPN01ExtraNames(t *testing.T) {
 	hs, err := tlsalpn01Srv(t, expectedKeyAuthorization, IdPeAcmeIdentifier, tls.VersionTLS12, "expected", "extra")
 	test.AssertNotError(t, err, "failed to set up tls-alpn-01 server")
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertError(t, prob, "validation should have failed")
@@ -639,7 +639,7 @@ func TestTLSALPN01NotSelfSigned(t *testing.T) {
 
 	hs := tlsalpn01SrvWithCert(t, acmeCert, tls.VersionTLS12)
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err = va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertError(t, err, "validation should have failed")
@@ -684,7 +684,7 @@ func TestTLSALPN01ExtraIdentifiers(t *testing.T) {
 
 	hs := tlsalpn01SrvWithCert(t, acmeCert, tls.VersionTLS12)
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, prob := va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertError(t, prob, "validation should have failed")
@@ -742,7 +742,7 @@ func TestTLSALPN01ExtraSANs(t *testing.T) {
 
 	hs := tlsalpn01SrvWithCert(t, acmeCert, tls.VersionTLS12)
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err = va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertError(t, err, "validation should have failed")
@@ -796,7 +796,7 @@ func TestTLSALPN01ExtraAcmeExtensions(t *testing.T) {
 
 	hs := tlsalpn01SrvWithCert(t, acmeCert, tls.VersionTLS12)
 
-	va, _ := setup(hs, 0, "", nil, nil)
+	va, _ := setup(hs, "", nil, nil)
 
 	_, err = va.validateTLSALPN01(ctx, dnsi("expected"), expectedKeyAuthorization)
 	test.AssertError(t, err, "validation should have failed")
diff --git a/va/va_test.go b/va/va_test.go
index 7db1c189cc4..542a2b4f5bd 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -103,7 +103,7 @@ func createValidationRequest(domain string, challengeType core.AcmeChallenge) *v
 
 // setup returns an in-memory VA and a mock logger. The default resolver client
 // is MockClient{}, but can be overridden.
-func setup(srv *httptest.Server, maxRemoteFailures int, userAgent string, remoteVAs []RemoteVA, mockDNSClientOverride bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) {
+func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNSClientOverride bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) {
 	features.Reset()
 	fc := clock.NewFake()
 
@@ -145,7 +145,7 @@ func setup(srv *httptest.Server, maxRemoteFailures int, userAgent string, remote
 }
 
 func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) (RemoteClients, *blog.Mock) {
-	rva, log := setup(srv, 0, userAgent, nil, mockDNSClientOverride)
+	rva, log := setup(srv, userAgent, nil, mockDNSClientOverride)
 	rva.perspective = perspective
 	rva.rir = rir
 
@@ -239,7 +239,7 @@ func (inmem inMemVA) IsCAAValid(ctx context.Context, req *vapb.IsCAAValidRequest
 }
 
 func TestValidateMalformedChallenge(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	_, err := va.validateChallenge(ctx, dnsi("example.com"), "fake-type-01", expectedToken, expectedKeyAuthorization)
 
@@ -248,7 +248,7 @@ func TestValidateMalformedChallenge(t *testing.T) {
 }
 
 func TestPerformValidationInvalid(t *testing.T) {
-	va, _ := setup(nil, 0, "", nil, nil)
+	va, _ := setup(nil, "", nil, nil)
 
 	req := createValidationRequest("foo.com", core.ChallengeTypeDNS01)
 	res, _ := va.PerformValidation(context.Background(), req)
@@ -262,7 +262,7 @@ func TestPerformValidationInvalid(t *testing.T) {
 }
 
 func TestInternalErrorLogged(t *testing.T) {
-	va, mockLog := setup(nil, 0, "", nil, nil)
+	va, mockLog := setup(nil, "", nil, nil)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond)
 	defer cancel()
@@ -275,7 +275,7 @@ func TestInternalErrorLogged(t *testing.T) {
 }
 
 func TestPerformValidationValid(t *testing.T) {
-	va, mockLog := setup(nil, 0, "", nil, nil)
+	va, mockLog := setup(nil, "", nil, nil)
 
 	// create a challenge with well known token
 	req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01)
@@ -299,7 +299,7 @@ func TestPerformValidationValid(t *testing.T) {
 // TestPerformValidationWildcard tests that the VA properly strips the `*.`
 // prefix from a wildcard name provided to the PerformValidation function.
 func TestPerformValidationWildcard(t *testing.T) {
-	va, mockLog := setup(nil, 0, "", nil, nil)
+	va, mockLog := setup(nil, "", nil, nil)
 
 	// create a challenge with well known token
 	req := createValidationRequest("*.good-dns01.com", core.ChallengeTypeDNS01)
@@ -329,7 +329,7 @@ func TestPerformValidationWildcard(t *testing.T) {
 }
 
 func TestDCVAndCAASequencing(t *testing.T) {
-	va, mockLog := setup(nil, 0, "", nil, nil)
+	va, mockLog := setup(nil, "", nil, nil)
 
 	// When validation succeeds, CAA should be checked.
 	mockLog.Clear()
@@ -468,7 +468,7 @@ func TestMultiVA(t *testing.T) {
 			ms.setAllowedUAs(tc.AllowedUAs)
 
 			// Configure a primary VA with testcase remote VAs.
-			localVA, mockLog := setup(ms.Server, 0, localUA, tc.RemoteVAs, nil)
+			localVA, mockLog := setup(ms.Server, localUA, tc.RemoteVAs, nil)
 
 			// Perform all validations
 			res, _ := localVA.PerformValidation(ctx, req)
@@ -516,7 +516,7 @@ func TestMultiVAEarlyReturn(t *testing.T) {
 	}
 
 	// Create a local test VA with the two remote VAs
-	localVA, _ := setup(ms.Server, 0, localUA, remoteVAs, nil)
+	localVA, _ := setup(ms.Server, localUA, remoteVAs, nil)
 
 	// Perform all validations
 	start := time.Now()
@@ -566,7 +566,7 @@ func TestMultiVAPolicy(t *testing.T) {
 	}
 
 	// Create a local test VA with the two remote VAs
-	localVA, _ := setup(ms.Server, 0, localUA, remoteVAs, nil)
+	localVA, _ := setup(ms.Server, localUA, remoteVAs, nil)
 
 	// Perform validation for a domain not in the disabledDomains list
 	req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01)
@@ -594,7 +594,7 @@ func TestMultiVALogging(t *testing.T) {
 		{rva1, rva1UA},
 		{rva2, rva2UA},
 	}
-	va, vaLog := setup(ms.Server, 0, localUA, remoteVAs, nil)
+	va, vaLog := setup(ms.Server, localUA, remoteVAs, nil)
 	req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01)
 	res, err := va.PerformValidation(ctx, req)
 	test.Assert(t, res.Problems == nil, fmt.Sprintf("validation failed with: %#v", res.Problems))
@@ -683,14 +683,14 @@ func TestLogRemoteDifferentials(t *testing.T) {
 	remoteVA1, _ := setupRemote(nil, "remote 1", nil, "", "")
 	remoteVA2, _ := setupRemote(nil, "remote 2", nil, "", "")
 	remoteVA3, _ := setupRemote(nil, "remote 3", nil, "", "")
+	// The VA will allow a max of 1 remote failure based on MPIC.
 	remoteVAs := []RemoteVA{
 		{remoteVA1, "remote 1"},
 		{remoteVA2, "remote 2"},
 		{remoteVA3, "remote 3"},
 	}
 
-	// Set up a local VA that allows a max of 2 remote failures.
-	localVA, mockLog := setup(nil, 2, "local 1", remoteVAs, nil)
+	localVA, mockLog := setup(nil, "local 1", remoteVAs, nil)
 
 	egProbA := probs.DNS("root DNS servers closed at 4:30pm")
 	egProbB := probs.OrderNotReady("please take a number")

From eaf880e3c271ba655c5a1e0a5dfd03eb73b8e5ea Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 12:11:28 -0800
Subject: [PATCH 03/16] Some more test updates

---
 va/caa_test.go | 35 +++++++++++++++++++++++++++--------
 va/va_test.go  | 31 +++++++++++++++++++++++++------
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/va/caa_test.go b/va/caa_test.go
index 4c4f4d26ac0..cc0cda9c528 100644
--- a/va/caa_test.go
+++ b/va/caa_test.go
@@ -702,13 +702,24 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "functional localVA, 1 broken RVA, no CAA records",
 			domains:                  "present-dns-only.com",
+			localDNSClient:           caaMockDNS{},
+			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"broken","Problem":{"type":"dns","detail":"While processing CAA for`,
+			remoteVAs: []RemoteVA{
+				{brokenVA, brokenUA},
+				{remoteVA, remoteUA},
+				{remoteVA, remoteUA},
+			},
+		},
+		{
+			name:                     "functional localVA, 2 broken RVAs, no CAA records",
+			domains:                  "present-dns-only.com",
 			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
 			expectedProbType:         probs.DNSProblem,
-			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"broken","Problem":{"type":"dns","detail":"While processing CAA for`,
+			expectedDiffLogSubstring: `RemoteSuccesses":1,"RemoteFailures":[{"VAHostname":"broken","Problem":{"type":"dns","detail":"While processing CAA for`,
 			localDNSClient:           caaMockDNS{},
 			remoteVAs: []RemoteVA{
 				{brokenVA, brokenUA},
-				{remoteVA, remoteUA},
+				{brokenVA, brokenUA},
 				{remoteVA, remoteUA},
 			},
 		},
@@ -738,8 +749,6 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "functional localVA, 1 broken RVA, CAA issue type present",
 			domains:                  "present.com",
-			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
-			expectedProbType:         probs.DNSProblem,
 			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"broken","Problem":{"type":"dns","detail":"While processing CAA for`,
 			localDNSClient:           caaMockDNS{},
 			remoteVAs: []RemoteVA{
@@ -748,6 +757,19 @@ func TestMultiCAARechecking(t *testing.T) {
 				{remoteVA, remoteUA},
 			},
 		},
+		{
+			name:                     "functional localVA, 2 broken RVA, CAA issue type present",
+			domains:                  "present.com",
+			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
+			expectedProbType:         probs.DNSProblem,
+			expectedDiffLogSubstring: `RemoteSuccesses":1,"RemoteFailures":[{"VAHostname":"broken","Problem":{"type":"dns","detail":"While processing CAA for`,
+			localDNSClient:           caaMockDNS{},
+			remoteVAs: []RemoteVA{
+				{brokenVA, brokenUA},
+				{brokenVA, brokenUA},
+				{remoteVA, remoteUA},
+			},
+		},
 		{
 			name:                     "functional localVA, all broken RVAs, CAA issue type present",
 			domains:                  "present.com",
@@ -779,8 +801,6 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "1 hijacked RVA, CAA issue type present",
 			domains:                  "present.com",
-			expectedProbSubstring:    "CAA record for present.com prevents issuance",
-			expectedProbType:         probs.CAAProblem,
 			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`,
 			localDNSClient:           caaMockDNS{},
 			remoteVAs: []RemoteVA{
@@ -818,8 +838,6 @@ func TestMultiCAARechecking(t *testing.T) {
 		{
 			name:                     "1 hijacked RVA, CAA issuewild type present",
 			domains:                  "satisfiable-wildcard.com",
-			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
-			expectedProbType:         probs.CAAProblem,
 			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`,
 			localDNSClient:           caaMockDNS{},
 			remoteVAs: []RemoteVA{
@@ -923,6 +941,7 @@ func TestMultiCAARechecking(t *testing.T) {
 			}
 
 			if tc.expectedProbType != "" {
+				test.AssertNotNil(t, isValidRes.Problem, "IsCAAValidRequest returned nil problem, but should not have")
 				test.AssertEquals(t, string(tc.expectedProbType), isValidRes.Problem.ProblemType)
 			}
 
diff --git a/va/va_test.go b/va/va_test.go
index 542a2b4f5bd..d7e763cc6c6 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -357,6 +357,7 @@ func TestMultiVA(t *testing.T) {
 	const (
 		remoteUA1 = "remote 1"
 		remoteUA2 = "remote 2"
+		remoteUA3 = "remote 3"
 		localUA   = "local 1"
 	)
 	allowedUAs := map[string]bool{
@@ -371,9 +372,11 @@ func TestMultiVA(t *testing.T) {
 
 	remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
 	remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
+	remoteVA3, _ := setupRemote(ms.Server, remoteUA3, nil, "", "")
 	remoteVAs := []RemoteVA{
 		{remoteVA1, remoteUA1},
 		{remoteVA2, remoteUA2},
+		{remoteVA3, remoteUA3},
 	}
 	brokenVA := RemoteClients{
 		VAClient:  brokenRemoteVA{},
@@ -398,7 +401,7 @@ func TestMultiVA(t *testing.T) {
 		ExpectedLog  string
 	}{
 		{
-			// With local and both remote VAs working there should be no problem.
+			// With local and all remote VAs working there should be no problem.
 			Name:       "Local and remote VAs OK",
 			RemoteVAs:  remoteVAs,
 			AllowedUAs: allowedUAs,
@@ -411,10 +414,21 @@ func TestMultiVA(t *testing.T) {
 			ExpectedProb: unauthorized,
 		},
 		{
-			// If a remote VA fails with an internal err it should fail
-			Name: "Local VA ok, remote VA internal err",
+			// If a remote VA fails with an internal err it should succeed
+			Name: "Local VA ok, one remote VA internal err",
 			RemoteVAs: []RemoteVA{
 				{remoteVA1, remoteUA1},
+				{remoteVA2, remoteUA2},
+				{brokenVA, "broken"},
+			},
+			AllowedUAs: allowedUAs,
+		},
+		{
+			// If two remote VAs fail with an internal err it should fail
+			Name: "Local VA ok, 2 remote VAs internal err",
+			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
+				{brokenVA, "broken"},
 				{brokenVA, "broken"},
 			},
 			AllowedUAs:   allowedUAs,
@@ -437,6 +451,7 @@ func TestMultiVA(t *testing.T) {
 			RemoteVAs: []RemoteVA{
 				{remoteVA1, remoteUA1},
 				{cancelledVA, remoteUA2},
+				{remoteVA3, remoteUA3},
 			},
 			AllowedUAs:   allowedUAs,
 			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"),
@@ -484,8 +499,8 @@ func TestMultiVA(t *testing.T) {
 
 			if tc.ExpectedLog != "" {
 				lines := mockLog.GetAllMatching(tc.ExpectedLog)
-				if len(lines) != 1 {
-					t.Fatalf("Got log %v; expected %q", mockLog.GetAll(), tc.ExpectedLog)
+				if len(lines) < 1 {
+					t.Fatalf("Got log %v; expected %s", strings.Join(mockLog.GetAll(), "\n")+"\n", tc.ExpectedLog)
 				}
 			}
 		})
@@ -496,12 +511,14 @@ func TestMultiVAEarlyReturn(t *testing.T) {
 	const (
 		remoteUA1 = "remote 1"
 		remoteUA2 = "slow remote"
+		remoteUA3 = "remote 3"
 		localUA   = "local 1"
 	)
 	allowedUAs := map[string]bool{
 		localUA:   true,
-		remoteUA1: false, // forbid UA 1 to provoke early return
+		remoteUA1: false, // forbid UA 1 and 3 to provoke early return
 		remoteUA2: true,
+		remoteUA3: false,
 	}
 
 	ms := httpMultiSrv(t, expectedToken, allowedUAs)
@@ -509,10 +526,12 @@ func TestMultiVAEarlyReturn(t *testing.T) {
 
 	remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
 	remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
+	remoteVA3, _ := setupRemote(ms.Server, remoteUA3, nil, "", "")
 
 	remoteVAs := []RemoteVA{
 		{remoteVA1, remoteUA1},
 		{remoteVA2, remoteUA2},
+		{remoteVA3, remoteUA3},
 	}
 
 	// Create a local test VA with the two remote VAs

From 83428e9c67224c3dc1b7b06b185bd22f351f3096 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 12:24:55 -0800
Subject: [PATCH 04/16] Add test cases

---
 va/va_test.go | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index d7e763cc6c6..491c7964b42 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -414,8 +414,17 @@ func TestMultiVA(t *testing.T) {
 			ExpectedProb: unauthorized,
 		},
 		{
-			// If a remote VA fails with an internal err it should succeed
-			Name: "Local VA ok, one remote VA internal err",
+			// If one out of two remote VAs fail with an internal err it should succeed
+			Name: "Local VA ok, 1/2 remote VA internal err",
+			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
+				{brokenVA, "broken"},
+			},
+			AllowedUAs: allowedUAs,
+		},
+		{
+			// If one out of three remote VAs fails with an internal err it should succeed
+			Name: "Local VA ok, 1/3 remote VA internal err",
 			RemoteVAs: []RemoteVA{
 				{remoteVA1, remoteUA1},
 				{remoteVA2, remoteUA2},
@@ -424,8 +433,8 @@ func TestMultiVA(t *testing.T) {
 			AllowedUAs: allowedUAs,
 		},
 		{
-			// If two remote VAs fail with an internal err it should fail
-			Name: "Local VA ok, 2 remote VAs internal err",
+			// If two out of three remote VAs fail with an internal err it should fail
+			Name: "Local VA ok, 2/3 remote VAs internal err",
 			RemoteVAs: []RemoteVA{
 				{remoteVA1, remoteUA1},
 				{brokenVA, "broken"},

From ea102b592f31b609799a6beaaa4ec01ab73b8552 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 12:36:40 -0800
Subject: [PATCH 05/16] config: remove maxRemoteValidationFailures

---
 test/config-next/va.json | 1 -
 test/config/va.json      | 1 -
 2 files changed, 2 deletions(-)

diff --git a/test/config-next/va.json b/test/config-next/va.json
index e725c2be0eb..e3d3526f6e4 100644
--- a/test/config-next/va.json
+++ b/test/config-next/va.json
@@ -54,7 +54,6 @@
 				"hostOverride": "rva1.boulder"
 			}
 		],
-		"maxRemoteValidationFailures": 1,
 		"accountURIPrefixes": [
 			"http://boulder.service.consul:4000/acme/reg/",
 			"http://boulder.service.consul:4001/acme/acct/"
diff --git a/test/config/va.json b/test/config/va.json
index 8a8275c6f5a..76de0e9d048 100644
--- a/test/config/va.json
+++ b/test/config/va.json
@@ -51,7 +51,6 @@
 				"hostOverride": "rva1.boulder"
 			}
 		],
-		"maxRemoteValidationFailures": 1,
 		"accountURIPrefixes": [
 			"http://boulder.service.consul:4000/acme/reg/",
 			"http://boulder.service.consul:4001/acme/acct/"

From 8d6015a015609f38ca9177195c5c1f44bcda5258 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 12:38:21 -0800
Subject: [PATCH 06/16] features: remove maxValidationFailures

---
 features/features.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/features/features.go b/features/features.go
index 1f4c594e846..36ca9db76e1 100644
--- a/features/features.go
+++ b/features/features.go
@@ -53,8 +53,8 @@ type Config struct {
 
 	// MultiCAAFullResults will cause the main VA to block and wait for all of
 	// the remote VA CAA recheck results instead of returning early if the
-	// number of failures is greater than the configured
-	// maxRemoteValidationFailures. Only used when EnforceMultiCAA is true.
+	// number of failures is greater than the number allowed by MPIC.
+	// Only used when EnforceMultiCAA is true.
 	MultiCAAFullResults bool
 
 	// MultipleCertificateProfiles, when enabled, triggers the following

From 9ca06936b9e06f7c6c197186cc5d833979744a59 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Thu, 14 Nov 2024 12:39:47 -0800
Subject: [PATCH 07/16] Revert testing diff.

---
 va/va_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index 491c7964b42..12612ab05b8 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -508,8 +508,8 @@ func TestMultiVA(t *testing.T) {
 
 			if tc.ExpectedLog != "" {
 				lines := mockLog.GetAllMatching(tc.ExpectedLog)
-				if len(lines) < 1 {
-					t.Fatalf("Got log %v; expected %s", strings.Join(mockLog.GetAll(), "\n")+"\n", tc.ExpectedLog)
+				if len(lines) == 0 {
+					t.Fatalf("Got log %v; expected %q", mockLog.GetAll(), tc.ExpectedLog)
 				}
 			}
 		})

From 40ed92edf2a439d4bc9759abb8b901a7eaa196f8 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 11:10:07 -0800
Subject: [PATCH 08/16] Remove test for logging of perspective

---
 va/va_test.go | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index 12612ab05b8..4213fa8985f 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -627,31 +627,6 @@ func TestMultiVALogging(t *testing.T) {
 	res, err := va.PerformValidation(ctx, req)
 	test.Assert(t, res.Problems == nil, fmt.Sprintf("validation failed with: %#v", res.Problems))
 	test.AssertNotError(t, err, "performing validation")
-
-	// We do not log perspective or RIR for the local VAs.
-	// We expect these log lines to be available immediately.
-	test.Assert(t, len(vaLog.GetAllMatching(`"Perspective"`)) == 0, "expected no logged perspective for primary")
-	test.Assert(t, len(vaLog.GetAllMatching(`"RIR"`)) == 0, "expected no logged RIR for primary")
-
-	// We do log perspective and RIR for the remote VAs.
-	//
-	// Because the remote VAs are operating on different goroutines, we aren't guaranteed their
-	// log lines have arrived yet. Give it a few tries.
-	for i := 0; i < 10; i++ {
-		if len(rva1Log.GetAllMatching(`"Perspective":"dev-arin"`)) >= 1 &&
-			len(rva1Log.GetAllMatching(`"RIR":"ARIN"`)) >= 1 &&
-			len(rva2Log.GetAllMatching(`"Perspective":"dev-ripe"`)) >= 1 &&
-			len(rva2Log.GetAllMatching(`"RIR":"RIPE"`)) >= 1 {
-			break
-		}
-		if i == 9 {
-			t.Logf("VA:\n%s\n", strings.Join(vaLog.GetAll(), "\n"))
-			t.Logf("RVA 1:\n%s\n", strings.Join(rva1Log.GetAll(), "\n"))
-			t.Logf("RVA 2:\n%s\n", strings.Join(rva2Log.GetAll(), "\n"))
-			t.Errorf("expected perspective and RIR logs for remote VAs, but they never arrived")
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
 }
 
 func TestDetailedError(t *testing.T) {

From af7bcc761603aadc14ef83e8f0bc6d2e69ca8ed4 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 11:37:43 -0800
Subject: [PATCH 09/16] Remove spurious log variables

---
 va/va_test.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index 4213fa8985f..eeaadacf624 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -615,14 +615,14 @@ func TestMultiVALogging(t *testing.T) {
 	ms := httpMultiSrv(t, expectedToken, map[string]bool{localUA: true, rva1UA: true, rva2UA: true})
 	defer ms.Close()
 
-	rva1, rva1Log := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN")
-	rva2, rva2Log := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE")
+	rva1, _ := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN")
+	rva2, _ := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE")
 
 	remoteVAs := []RemoteVA{
 		{rva1, rva1UA},
 		{rva2, rva2UA},
 	}
-	va, vaLog := setup(ms.Server, localUA, remoteVAs, nil)
+	va, _ := setup(ms.Server, localUA, remoteVAs, nil)
 	req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01)
 	res, err := va.PerformValidation(ctx, req)
 	test.Assert(t, res.Problems == nil, fmt.Sprintf("validation failed with: %#v", res.Problems))

From 517b03f783eec7fa1e03b9d9a165058f2eec7a89 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 12:16:08 -0800
Subject: [PATCH 10/16] Fix merge conflicts

---
 va/va_test.go | 29 ++++-------------------------
 1 file changed, 4 insertions(+), 25 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index 12b42782048..c47b2c336d3 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -144,13 +144,8 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS
 	return va, logger
 }
 
-<<<<<<< HEAD
-func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) (RemoteClients, *blog.Mock) {
-	rva, log := setup(srv, userAgent, nil, mockDNSClientOverride)
-=======
 func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) RemoteClients {
-	rva, _ := setup(srv, 0, userAgent, nil, mockDNSClientOverride)
->>>>>>> 2502113ac38a36d1b5a7729f3cee58bf359dab4d
+	rva, _ := setup(srv, userAgent, nil, mockDNSClientOverride)
 	rva.perspective = perspective
 	rva.rir = rir
 
@@ -375,14 +370,9 @@ func TestMultiVA(t *testing.T) {
 	ms := httpMultiSrv(t, expectedToken, allowedUAs)
 	defer ms.Close()
 
-<<<<<<< HEAD
-	remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
-	remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
-	remoteVA3, _ := setupRemote(ms.Server, remoteUA3, nil, "", "")
-=======
 	remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
 	remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")
->>>>>>> 2502113ac38a36d1b5a7729f3cee58bf359dab4d
+	remoteVA3 := setupRemote(ms.Server, remoteUA3, nil, "", "")
 	remoteVAs := []RemoteVA{
 		{remoteVA1, remoteUA1},
 		{remoteVA2, remoteUA2},
@@ -543,14 +533,9 @@ func TestMultiVAEarlyReturn(t *testing.T) {
 	ms := httpMultiSrv(t, expectedToken, allowedUAs)
 	defer ms.Close()
 
-<<<<<<< HEAD
-	remoteVA1, _ := setupRemote(ms.Server, remoteUA1, nil, "", "")
-	remoteVA2, _ := setupRemote(ms.Server, remoteUA2, nil, "", "")
-	remoteVA3, _ := setupRemote(ms.Server, remoteUA3, nil, "", "")
-=======
 	remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
 	remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")
->>>>>>> 2502113ac38a36d1b5a7729f3cee58bf359dab4d
+	remoteVA3 := setupRemote(ms.Server, remoteUA3, nil, "", "")
 
 	remoteVAs := []RemoteVA{
 		{remoteVA1, remoteUA1},
@@ -698,16 +683,10 @@ func TestDetailedError(t *testing.T) {
 
 func TestLogRemoteDifferentials(t *testing.T) {
 	// Create some remote VAs
-<<<<<<< HEAD
-	remoteVA1, _ := setupRemote(nil, "remote 1", nil, "", "")
-	remoteVA2, _ := setupRemote(nil, "remote 2", nil, "", "")
-	remoteVA3, _ := setupRemote(nil, "remote 3", nil, "", "")
-	// The VA will allow a max of 1 remote failure based on MPIC.
-=======
 	remoteVA1 := setupRemote(nil, "remote 1", nil, "", "")
 	remoteVA2 := setupRemote(nil, "remote 2", nil, "", "")
 	remoteVA3 := setupRemote(nil, "remote 3", nil, "", "")
->>>>>>> 2502113ac38a36d1b5a7729f3cee58bf359dab4d
+	// The VA will allow a max of 1 remote failure based on MPIC.
 	remoteVAs := []RemoteVA{
 		{remoteVA1, "remote 1"},
 		{remoteVA2, "remote 2"},

From 03a80c6e76a32497b4d6985626753ffaeb16fdf8 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 12:53:41 -0800
Subject: [PATCH 11/16] Restore maxRemoteValidationFailures to va.json

---
 test/config/va.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/config/va.json b/test/config/va.json
index 76de0e9d048..8a8275c6f5a 100644
--- a/test/config/va.json
+++ b/test/config/va.json
@@ -51,6 +51,7 @@
 				"hostOverride": "rva1.boulder"
 			}
 		],
+		"maxRemoteValidationFailures": 1,
 		"accountURIPrefixes": [
 			"http://boulder.service.consul:4000/acme/reg/",
 			"http://boulder.service.consul:4001/acme/acct/"

From 82bf0c04cd853bbe2baa955fade47cc70534509a Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 14:22:51 -0800
Subject: [PATCH 12/16] Fix tests

---
 va/caa_test.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/va/caa_test.go b/va/caa_test.go
index b1f06cfad47..7a94d9f10db 100644
--- a/va/caa_test.go
+++ b/va/caa_test.go
@@ -729,8 +729,8 @@ func TestMultiCAARechecking(t *testing.T) {
 				"operation":      opCAA,
 				"perspective":    allPerspectives,
 				"challenge_type": string(core.ChallengeTypeDNS01),
-				"problem_type":   string(probs.DNSProblem),
-				"result":         fail,
+				"problem_type":   "",
+				"result":         pass,
 			},
 		},
 		{
@@ -797,8 +797,8 @@ func TestMultiCAARechecking(t *testing.T) {
 				"operation":      opCAA,
 				"perspective":    allPerspectives,
 				"challenge_type": string(core.ChallengeTypeDNS01),
-				"problem_type":   string(probs.DNSProblem),
-				"result":         fail,
+				"problem_type":   "",
+				"result":         pass,
 			},
 		},
 		{

From 38a27d2c05df7ebd8c7ffb2ce215e609ed667eea Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 14:25:58 -0800
Subject: [PATCH 13/16] Add more stat checks

---
 va/caa_test.go | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/va/caa_test.go b/va/caa_test.go
index 7a94d9f10db..4ec4ce06d2e 100644
--- a/va/caa_test.go
+++ b/va/caa_test.go
@@ -745,6 +745,13 @@ func TestMultiCAARechecking(t *testing.T) {
 				{brokenVA, brokenUA},
 				{remoteVA, remoteUA},
 			},
+			expectedLabels: prometheus.Labels{
+				"operation":      opCAA,
+				"perspective":    allPerspectives,
+				"challenge_type": string(core.ChallengeTypeDNS01),
+				"problem_type":   string(probs.DNSProblem),
+				"result":         fail,
+			},
 		},
 		{
 			name:                     "functional localVA, all broken RVAs, no CAA records",
@@ -813,6 +820,13 @@ func TestMultiCAARechecking(t *testing.T) {
 				{brokenVA, brokenUA},
 				{remoteVA, remoteUA},
 			},
+			expectedLabels: prometheus.Labels{
+				"operation":      opCAA,
+				"perspective":    allPerspectives,
+				"challenge_type": string(core.ChallengeTypeDNS01),
+				"problem_type":   string(probs.DNSProblem),
+				"result":         fail,
+			},
 		},
 		{
 			name:                     "functional localVA, all broken RVAs, CAA issue type present",

From 297493eafb38083192114a5b103a24e16a2e02b5 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Fri, 15 Nov 2024 14:55:48 -0800
Subject: [PATCH 14/16] Remove "n failures allowed" from test names

---
 va/caa_test.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/va/caa_test.go b/va/caa_test.go
index 4ec4ce06d2e..4b1d49ef265 100644
--- a/va/caa_test.go
+++ b/va/caa_test.go
@@ -938,7 +938,7 @@ func TestMultiCAARechecking(t *testing.T) {
 			},
 		},
 		{
-			name:                     "1 hijacked RVA, CAA issuewild type present, 1 failure allowed",
+			name:                     "1 hijacked RVA, CAA issuewild type present",
 			domains:                  "satisfiable-wildcard.com",
 			expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`,
 			localDNSClient:           caaMockDNS{},
@@ -949,7 +949,7 @@ func TestMultiCAARechecking(t *testing.T) {
 			},
 		},
 		{
-			name:                     "2 hijacked RVAs, CAA issuewild type present, 1 failure allowed",
+			name:                     "2 hijacked RVAs, CAA issuewild type present",
 			domains:                  "satisfiable-wildcard.com",
 			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
 			expectedProbType:         probs.CAAProblem,
@@ -962,7 +962,7 @@ func TestMultiCAARechecking(t *testing.T) {
 			},
 		},
 		{
-			name:                     "3 hijacked RVAs, CAA issuewild type present, 1 failure allowed",
+			name:                     "3 hijacked RVAs, CAA issuewild type present",
 			domains:                  "satisfiable-wildcard.com",
 			expectedProbSubstring:    "During secondary CAA checking: While processing CAA",
 			expectedProbType:         probs.CAAProblem,

From f151151bb5c0c406646ae7974ff667934716ee25 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Mon, 18 Nov 2024 12:44:10 -0800
Subject: [PATCH 15/16] Test 5 and 6 backends in TestMultiVAEarlyReturn

---
 va/va_test.go | 79 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index d3bd20e3745..e341b6f6757 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -522,55 +522,62 @@ func TestMultiVA(t *testing.T) {
 }
 
 func TestMultiVAEarlyReturn(t *testing.T) {
-	const (
-		remoteUA1 = "remote 1"
-		remoteUA2 = "slow remote"
-		remoteUA3 = "remote 3"
-		localUA   = "local 1"
-	)
+	const passUA = "pass"
+	const failUA = "fail"
+	// httpMultiSrv handles this specially by being slow
+	const slowRemoteUA = "slow remote"
 	allowedUAs := map[string]bool{
-		localUA:   true,
-		remoteUA1: false, // forbid UA 1 and 3 to provoke early return
-		remoteUA2: true,
-		remoteUA3: false,
+		passUA: true,
 	}
 
 	ms := httpMultiSrv(t, expectedToken, allowedUAs)
 	defer ms.Close()
 
-	remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
-	remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")
-	remoteVA3 := setupRemote(ms.Server, remoteUA3, nil, "", "")
+	makeRemotes := func(userAgent ...string) []RemoteVA {
+		var rvas []RemoteVA
+		for i, ua := range userAgent {
+			clients := setupRemote(ms.Server, ua, nil, "", "")
+			rva := RemoteVA{clients, fmt.Sprintf("remote VA %d hostname", i)}
+			rvas = append(rvas, rva)
+		}
+		return rvas
+	}
 
-	remoteVAs := []RemoteVA{
-		{remoteVA1, remoteUA1},
-		{remoteVA2, remoteUA2},
-		{remoteVA3, remoteUA3},
+	testCases := []struct {
+		remoteUserAgents []string
+	}{
+		{remoteUserAgents: []string{slowRemoteUA, passUA, failUA}},
+		{remoteUserAgents: []string{slowRemoteUA, slowRemoteUA, passUA, passUA, failUA}},
+		{remoteUserAgents: []string{slowRemoteUA, slowRemoteUA, passUA, passUA, failUA, failUA}},
 	}
 
-	// Create a local test VA with the two remote VAs
-	localVA, _ := setup(ms.Server, localUA, remoteVAs, nil)
+	for i, tc := range testCases {
+		t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
+			rvas := makeRemotes(tc.remoteUserAgents...)
+			localVA, _ := setup(ms.Server, pass, rvas, nil)
 
-	// Perform all validations
-	start := time.Now()
-	req := createValidationRequest("localhost", core.ChallengeTypeHTTP01)
-	res, _ := localVA.PerformValidation(ctx, req)
+			// Perform all validations
+			start := time.Now()
+			req := createValidationRequest("localhost", core.ChallengeTypeHTTP01)
+			res, _ := localVA.PerformValidation(ctx, req)
 
-	// It should always fail
-	if res.Problems == nil {
-		t.Error("expected prob from PerformValidation, got nil")
-	}
+			// It should always fail
+			if res.Problems == nil {
+				t.Error("expected prob from PerformValidation, got nil")
+			}
 
-	elapsed := time.Since(start).Round(time.Millisecond).Milliseconds()
+			elapsed := time.Since(start).Round(time.Millisecond).Milliseconds()
 
-	// The slow UA should sleep for `slowRemoteSleepMillis`. But the first remote
-	// VA should fail quickly and the early-return code should cause the overall
-	// overall validation to return a prob quickly (i.e. in less than half of
-	// `slowRemoteSleepMillis`).
-	if elapsed > slowRemoteSleepMillis/2 {
-		t.Errorf(
-			"Expected an early return from PerformValidation in < %d ms, took %d ms",
-			slowRemoteSleepMillis/2, elapsed)
+			// The slow UA should sleep for `slowRemoteSleepMillis`. But the first remote
+			// VA should fail quickly and the early-return code should cause the overall
+			// overall validation to return a prob quickly (i.e. in less than half of
+			// `slowRemoteSleepMillis`).
+			if elapsed > slowRemoteSleepMillis/2 {
+				t.Errorf(
+					"Expected an early return from PerformValidation in < %d ms, took %d ms",
+					slowRemoteSleepMillis/2, elapsed)
+			}
+		})
 	}
 }
 

From b2aee9ba2d02c784316751d729b63717ce3b2a4e Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Mon, 18 Nov 2024 13:06:44 -0800
Subject: [PATCH 16/16] Add test cases for 5 and 6 backends

---
 va/va_test.go | 68 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 4 deletions(-)

diff --git a/va/va_test.go b/va/va_test.go
index e341b6f6757..8f9368a1fc5 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -363,12 +363,15 @@ func TestMultiVA(t *testing.T) {
 		remoteUA1 = "remote 1"
 		remoteUA2 = "remote 2"
 		remoteUA3 = "remote 3"
+		remoteUA4 = "remote 4"
 		localUA   = "local 1"
 	)
 	allowedUAs := map[string]bool{
 		localUA:   true,
 		remoteUA1: true,
 		remoteUA2: true,
+		remoteUA3: true,
+		remoteUA4: true,
 	}
 
 	// Create an IPv4 test server
@@ -378,6 +381,7 @@ func TestMultiVA(t *testing.T) {
 	remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "")
 	remoteVA2 := setupRemote(ms.Server, remoteUA2, nil, "", "")
 	remoteVA3 := setupRemote(ms.Server, remoteUA3, nil, "", "")
+	remoteVA4 := setupRemote(ms.Server, remoteUA4, nil, "", "")
 	remoteVAs := []RemoteVA{
 		{remoteVA1, remoteUA1},
 		{remoteVA2, remoteUA2},
@@ -450,6 +454,62 @@ func TestMultiVA(t *testing.T) {
 			// The real failure cause should be logged
 			ExpectedLog: expectedInternalErrLine,
 		},
+		{
+			// If one out of five remote VAs fail with an internal err it should succeed
+			Name: "Local VA ok, 1/5 remote VAs internal err",
+			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
+				{remoteVA2, remoteUA2},
+				{remoteVA3, remoteUA3},
+				{remoteVA4, remoteUA4},
+				{brokenVA, "broken"},
+			},
+			AllowedUAs: allowedUAs,
+		},
+		{
+			// If two out of five remote VAs fail with an internal err it should fail
+			Name: "Local VA ok, 2/5 remote VAs internal err",
+			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
+				{remoteVA2, remoteUA2},
+				{remoteVA3, remoteUA3},
+				{brokenVA, "broken"},
+				{brokenVA, "broken"},
+			},
+			AllowedUAs:   allowedUAs,
+			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
+			// The real failure cause should be logged
+			ExpectedLog: expectedInternalErrLine,
+		},
+		{
+			// If two out of six remote VAs fail with an internal err it should succeed
+			Name: "Local VA ok, 2/6 remote VAs internal err",
+			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
+				{remoteVA2, remoteUA2},
+				{remoteVA3, remoteUA3},
+				{remoteVA4, remoteUA4},
+				{brokenVA, "broken"},
+				{brokenVA, "broken"},
+			},
+			AllowedUAs: allowedUAs,
+		},
+		{
+			// If three out of six remote VAs fail with an internal err it should fail
+			Name: "Local VA ok, 4/6 remote VAs internal err",
+			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
+				{remoteVA2, remoteUA2},
+				{remoteVA3, remoteUA3},
+				{brokenVA, "broken"},
+				{brokenVA, "broken"},
+				{brokenVA, "broken"},
+			},
+			AllowedUAs:   allowedUAs,
+			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"),
+			// The real failure cause should be logged
+			ExpectedLog: expectedInternalErrLine,
+		},
 		{
 			// With only one working remote VA there should be a validation failure
 			Name:       "Local VA and one remote VA OK",
@@ -460,20 +520,20 @@ func TestMultiVA(t *testing.T) {
 				expectedKeyAuthorization)),
 		},
 		{
-			// Any remote VA cancellations are a problem.
+			// If one remote VA cancels, it should succeed
 			Name: "Local VA and one remote VA OK, one cancelled VA",
 			RemoteVAs: []RemoteVA{
 				{remoteVA1, remoteUA1},
 				{cancelledVA, remoteUA2},
 				{remoteVA3, remoteUA3},
 			},
-			AllowedUAs:   allowedUAs,
-			ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"),
+			AllowedUAs: allowedUAs,
 		},
 		{
-			// Any remote VA cancellations are a problem.
+			// If two remote VA cancels, it should fail
 			Name: "Local VA OK, two cancelled remote VAs",
 			RemoteVAs: []RemoteVA{
+				{remoteVA1, remoteUA1},
 				{cancelledVA, remoteUA1},
 				{cancelledVA, remoteUA2},
 			},