From a46c388f664cdab5cb2f38ae7a064d3593776933 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 18 Nov 2024 15:36:09 -0800 Subject: [PATCH] va: compute maxRemoteFailures based on MPIC (#7810) Previously this was a configuration field. Ports `maxAllowedFailures()` from `determineMaxAllowedFailures()` in #7794. Test updates: Remove the `maxRemoteFailures` param from `setup` in all VA tests. Some tests were depending on setting this param directly to provoke failures. For example, `TestMultiVAEarlyReturn` previously relied on "zero allowed failures". Since the number of allowed failures is now 1 for the number of remote VAs we were testing (2), the VA wasn't returning early with an error; it was succeeding! To fix that, make sure there are two failures. Since two failures from two RVAs wouldn't exercise the right situation, add a third RVA, so we get two failures from three RVAs. Similarly, TestMultiCAARechecking had several test cases that omitted this field, effectively setting it to zero allowed failures. I updated the "1 RVA failure" test case to expect overall success and added a "2 RVA failures" test case to expect overall failure (we previously expected overall failure from a single RVA failing). In TestMultiVA I had to change a test for `len(lines) != 1` to `len(lines) == 0`, because with more backends we were now logging more errors, and finding e.g. `len(lines)` to be 2. --- cmd/boulder-va/main.go | 6 +- cmd/remoteva/main.go | 1 - docs/multi-va.md | 2 +- features/features.go | 4 +- test/config-next/va.json | 1 - va/caa_test.go | 76 +++++++++----- va/dns_test.go | 20 ++-- va/http_test.go | 32 +++--- va/tlsalpn_test.go | 40 ++++---- va/va.go | 21 +++- va/va_test.go | 207 ++++++++++++++++++++++++++++----------- 11 files changed, 273 insertions(+), 137 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/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 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/va/caa_test.go b/va/caa_test.go index 802dd01e004..4b1d49ef265 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -191,7 +191,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, @@ -408,7 +408,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 { @@ -431,7 +431,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 @@ -521,7 +521,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. @@ -546,7 +546,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{ @@ -592,7 +592,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, @@ -671,7 +671,6 @@ func TestMultiCAARechecking(t *testing.T) { testCases := []struct { name string - maxLookupFailures int domains string remoteVAs []RemoteVA expectedProbSubstring string @@ -719,13 +718,31 @@ 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}, + }, + expectedLabels: prometheus.Labels{ + "operation": opCAA, + "perspective": allPerspectives, + "challenge_type": string(core.ChallengeTypeDNS01), + "problem_type": "", + "result": pass, + }, + }, + { + 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}, }, expectedLabels: prometheus.Labels{ @@ -776,8 +793,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{ @@ -785,6 +800,26 @@ func TestMultiCAARechecking(t *testing.T) { {remoteVA, remoteUA}, {remoteVA, remoteUA}, }, + expectedLabels: prometheus.Labels{ + "operation": opCAA, + "perspective": allPerspectives, + "challenge_type": string(core.ChallengeTypeDNS01), + "problem_type": "", + "result": pass, + }, + }, + { + 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}, + }, expectedLabels: prometheus.Labels{ "operation": opCAA, "perspective": allPerspectives, @@ -831,8 +866,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{ @@ -870,8 +903,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{ @@ -907,9 +938,8 @@ 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", - maxLookupFailures: 1, expectedDiffLogSubstring: `RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"hijacked","Problem":{"type":"caa","detail":"While processing CAA for`, localDNSClient: caaMockDNS{}, remoteVAs: []RemoteVA{ @@ -919,9 +949,8 @@ 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", - 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`, @@ -933,9 +962,8 @@ 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", - 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`, @@ -950,7 +978,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 @@ -971,12 +999,14 @@ 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 { test.AssertBoxedNil(t, isValidRes.Problem, "IsCAAValidRequest returned a problem, but should not have") } 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) } @@ -1017,7 +1047,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 d9fc776bd10..ef7d9cfd54e 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. @@ -38,7 +38,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") @@ -48,7 +48,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 { @@ -59,7 +59,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 { @@ -70,7 +70,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) @@ -84,7 +84,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) @@ -93,7 +93,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) @@ -102,7 +102,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") @@ -121,7 +121,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) @@ -129,7 +129,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.go b/va/va.go index 22c2a60b601..34cc3b68b01 100644 --- a/va/va.go +++ b/va/va.go @@ -222,7 +222,6 @@ var _ vapb.CAAServer = (*ValidationAuthorityImpl)(nil) func NewValidationAuthorityImpl( resolver bdns.Client, remoteVAs []RemoteVA, - maxRemoteFailures int, userAgent string, issuerDomain string, stats prometheus.Registerer, @@ -250,7 +249,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 @@ -264,6 +263,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 f65d31f0342..8f9368a1fc5 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() @@ -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,14 +141,11 @@ 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 } func setupRemote(srv *httptest.Server, userAgent string, mockDNSClientOverride bdns.Client, perspective, rir string) RemoteClients { - rva, _ := setup(srv, 0, userAgent, nil, mockDNSClientOverride) + rva, _ := setup(srv, userAgent, nil, mockDNSClientOverride) rva.perspective = perspective rva.rir = rir @@ -243,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) @@ -252,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) @@ -267,7 +263,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() @@ -280,7 +276,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) @@ -306,7 +302,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) @@ -338,7 +334,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() @@ -366,12 +362,16 @@ func TestMultiVA(t *testing.T) { const ( 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 @@ -380,9 +380,12 @@ 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}, + {remoteVA3, remoteUA3}, } brokenVA := RemoteClients{ VAClient: brokenRemoteVA{}, @@ -407,7 +410,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, @@ -420,10 +423,86 @@ 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 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}, + {brokenVA, "broken"}, + }, + AllowedUAs: allowedUAs, + }, + { + // 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"}, + {brokenVA, "broken"}, + }, + AllowedUAs: allowedUAs, + ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC failed"), + // 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, @@ -441,19 +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}, }, @@ -477,7 +557,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) @@ -493,7 +573,7 @@ func TestMultiVA(t *testing.T) { if tc.ExpectedLog != "" { lines := mockLog.GetAllMatching(tc.ExpectedLog) - if len(lines) != 1 { + if len(lines) == 0 { t.Fatalf("Got log %v; expected %q", mockLog.GetAll(), tc.ExpectedLog) } } @@ -502,51 +582,62 @@ func TestMultiVA(t *testing.T) { } func TestMultiVAEarlyReturn(t *testing.T) { - const ( - remoteUA1 = "remote 1" - remoteUA2 = "slow remote" - 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 to provoke early return - remoteUA2: true, + passUA: true, } ms := httpMultiSrv(t, expectedToken, allowedUAs) defer ms.Close() - remoteVA1 := setupRemote(ms.Server, remoteUA1, nil, "", "") - remoteVA2 := setupRemote(ms.Server, remoteUA2, 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}, + 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, 0, 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) + } + }) } } @@ -575,7 +666,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) @@ -603,7 +694,7 @@ func TestMultiVALogging(t *testing.T) { {rva1, rva1UA}, {rva2, rva2UA}, } - va, _ := setup(ms.Server, 0, 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)) @@ -667,14 +758,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")