From 9e4b47dcd6b559315f237971712680d93b2d7cdb Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 14 Nov 2024 11:30:24 -0800 Subject: [PATCH 01/19] 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 Date: Thu, 14 Nov 2024 11:38:38 -0800 Subject: [PATCH 02/19] 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 Date: Thu, 14 Nov 2024 12:11:28 -0800 Subject: [PATCH 03/19] 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 Date: Thu, 14 Nov 2024 12:24:55 -0800 Subject: [PATCH 04/19] 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 Date: Thu, 14 Nov 2024 12:36:40 -0800 Subject: [PATCH 05/19] 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 Date: Thu, 14 Nov 2024 12:38:21 -0800 Subject: [PATCH 06/19] 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 Date: Thu, 14 Nov 2024 12:39:47 -0800 Subject: [PATCH 07/19] 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 Date: Fri, 15 Nov 2024 11:10:07 -0800 Subject: [PATCH 08/19] 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 Date: Fri, 15 Nov 2024 11:37:43 -0800 Subject: [PATCH 09/19] 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 Date: Fri, 15 Nov 2024 12:16:08 -0800 Subject: [PATCH 10/19] 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 295dc76f5986c87c0dc27b2c39354bc86d8f3245 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 14 Nov 2024 13:27:41 -0800 Subject: [PATCH 11/19] Fix tests --- va/caa_test.go | 6 ++++- va/va_test.go | 73 +++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/va/caa_test.go b/va/caa_test.go index b2030137c7c..0d3c24a3fb5 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -590,7 +590,11 @@ 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"}} + remoteVAs := []RemoteVA{ + {brokenRVA, "broken"}, + {brokenRVA, "broken"}, + {brokenRVA, "broken"}, + } va, _ := setup(nil, "local", remoteVAs, nil) features.Set(features.Config{ diff --git a/va/va_test.go b/va/va_test.go index c47b2c336d3..b182178993b 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -103,6 +103,9 @@ 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. +// +// If remoteVAs is nil, this builds a VA that acts like a remote (and does not +// perform multi-perspective validation). Otherwise it acts like a primary. func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNSClientOverride bdns.Client) (*ValidationAuthorityImpl, *blog.Mock) { features.Reset() fc := clock.NewFake() @@ -113,6 +116,11 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS userAgent = "user agent 1.0" } + perspective := PrimaryPerspective + if len(remoteVAs) == 0 { + perspective = "example perspective" + } + va, err := NewValidationAuthorityImpl( &bdns.MockClient{Log: logger}, remoteVAs, @@ -122,9 +130,12 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS fc, logger, accountURIPrefixes, - PrimaryPerspective, + perspective, "", ) + if err != nil { + panic(fmt.Sprintf("Failed to create validation authority: %v", err)) + } if mockDNSClientOverride != nil { va.dnsClient = mockDNSClientOverride @@ -138,9 +149,6 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS va.tlsPort = port } - if err != nil { - panic(fmt.Sprintf("Failed to create validation authority: %v", err)) - } return va, logger } @@ -413,15 +421,6 @@ func TestMultiVA(t *testing.T) { AllowedUAs: map[string]bool{remoteUA1: true, remoteUA2: true}, ExpectedProb: unauthorized, }, - { - // 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", @@ -467,10 +466,11 @@ func TestMultiVA(t *testing.T) { }, { // Any remote VA cancellations are a problem. - Name: "Local VA OK, two cancelled remote VAs", + Name: "Local VA OK, three cancelled remote VAs", RemoteVAs: []RemoteVA{ {cancelledVA, remoteUA1}, {cancelledVA, remoteUA2}, + {cancelledVA, remoteUA3}, }, AllowedUAs: allowedUAs, ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"), @@ -573,13 +573,15 @@ func TestMultiVAPolicy(t *testing.T) { const ( remoteUA1 = "remote 1" remoteUA2 = "remote 2" + remoteUA3 = "remote 3" localUA = "local 1" ) - // Forbid both remote UAs to ensure that multi-va fails + // Forbid all remote UAs to ensure that multi-va fails allowedUAs := map[string]bool{ localUA: true, remoteUA1: false, remoteUA2: false, + remoteUA3: false, } ms := httpMultiSrv(t, expectedToken, allowedUAs) @@ -587,10 +589,12 @@ func TestMultiVAPolicy(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 @@ -609,24 +613,63 @@ func TestMultiVALogging(t *testing.T) { const ( rva1UA = "remote 1" rva2UA = "remote 2" + rva3UA = "remote 3" localUA = "local 1" ) ms := httpMultiSrv(t, expectedToken, map[string]bool{localUA: true, rva1UA: true, rva2UA: true}) defer ms.Close() +<<<<<<< HEAD rva1 := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN") rva2 := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE") +======= + rva1, rva1Log := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN") + rva2, rva2Log := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE") + rva3, rva3Log := setupRemote(ms.Server, rva3UA, nil, "dev-ripe", "RIPE") +>>>>>>> 9085ab38f (Fix tests) remoteVAs := []RemoteVA{ {rva1, rva1UA}, {rva2, rva2UA}, + {rva3, rva3UA}, } 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)) test.AssertNotError(t, err, "performing validation") +<<<<<<< HEAD +======= + + // 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 { + len(rva3Log.GetAllMatching(`"Perspective":"dev-ripe"`)) >= 1 && + len(rva3Log.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.Logf("RVA 3:\n%s\n", strings.Join(rva3Log.GetAll(), "\n")) + t.Errorf("expected perspective and RIR logs for remote VAs, but they never arrived") + } + time.Sleep(100 * time.Millisecond) + } +>>>>>>> 9085ab38f (Fix tests) } func TestDetailedError(t *testing.T) { From 25ce2d930751ca59b9936e2d8d37883b26e6e10a Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 14 Nov 2024 13:54:04 -0800 Subject: [PATCH 12/19] Add remoteva-c --- test/config-next/remoteva-c.json | 51 ++++++++++++++++++++++++++++++++ test/config-next/va.json | 5 ++++ test/config/remoteva-c.json | 47 +++++++++++++++++++++++++++++ test/config/va.json | 5 ++++ test/consul/config.hcl | 8 +++++ test/startservers.py | 4 +++ test/v2_integration.py | 2 +- 7 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 test/config-next/remoteva-c.json create mode 100644 test/config/remoteva-c.json diff --git a/test/config-next/remoteva-c.json b/test/config-next/remoteva-c.json new file mode 100644 index 00000000000..b25440b65c7 --- /dev/null +++ b/test/config-next/remoteva-c.json @@ -0,0 +1,51 @@ +{ + "rva": { + "userAgent": "remoteva-c", + "dnsTries": 3, + "dnsStaticResolvers": [ + "10.77.77.77:8343", + "10.77.77.77:8443" + ], + "dnsTimeout": "1s", + "dnsAllowLoopbackAddresses": true, + "issuerDomain": "happy-hacker-ca.invalid", + "tls": { + "caCertfile": "test/certs/ipki/minica.pem", + "certFile": "test/certs/ipki/rva.boulder/cert.pem", + "keyFile": "test/certs/ipki/rva.boulder/key.pem" + }, + "skipGRPCClientCertVerification": true, + "grpc": { + "maxConnectionAge": "30s", + "services": { + "va.VA": { + "clientNames": [ + "va.boulder" + ] + }, + "grpc.health.v1.Health": { + "clientNames": [ + "health-checker.boulder" + ] + } + } + }, + "features": { + "DOH": true + }, + "accountURIPrefixes": [ + "http://boulder.service.consul:4000/acme/reg/", + "http://boulder.service.consul:4001/acme/acct/" + ], + "perspective": "development", + "rir": "ARIN" + }, + "syslog": { + "stdoutlevel": 4, + "sysloglevel": -1 + }, + "openTelemetry": { + "endpoint": "bjaeger:4317", + "sampleratio": 1 + } +} diff --git a/test/config-next/va.json b/test/config-next/va.json index e3d3526f6e4..7ecccc9538a 100644 --- a/test/config-next/va.json +++ b/test/config-next/va.json @@ -52,6 +52,11 @@ "serverAddress": "rva1.service.consul:9498", "timeout": "15s", "hostOverride": "rva1.boulder" + }, + { + "serverAddress": "rva1.service.consul:9499", + "timeout": "15s", + "hostOverride": "rva1.boulder" } ], "accountURIPrefixes": [ diff --git a/test/config/remoteva-c.json b/test/config/remoteva-c.json new file mode 100644 index 00000000000..27dbd79563d --- /dev/null +++ b/test/config/remoteva-c.json @@ -0,0 +1,47 @@ +{ + "rva": { + "userAgent": "remoteva-c", + "debugAddr": ":8213", + "dnsTries": 3, + "dnsProvider": { + "dnsAuthority": "consul.service.consul", + "srvLookup": { + "service": "dns", + "domain": "service.consul" + } + }, + "dnsTimeout": "1s", + "dnsAllowLoopbackAddresses": true, + "issuerDomain": "happy-hacker-ca.invalid", + "tls": { + "caCertfile": "test/certs/ipki/minica.pem", + "certFile": "test/certs/ipki/rva.boulder/cert.pem", + "keyFile": "test/certs/ipki/rva.boulder/key.pem" + }, + "grpc": { + "maxConnectionAge": "30s", + "address": ":9899", + "services": { + "va.VA": { + "clientNames": [ + "va.boulder" + ] + }, + "grpc.health.v1.Health": { + "clientNames": [ + "health-checker.boulder" + ] + } + } + }, + "features": {}, + "accountURIPrefixes": [ + "http://boulder.service.consul:4000/acme/reg/", + "http://boulder.service.consul:4001/acme/acct/" + ] + }, + "syslog": { + "stdoutlevel": 4, + "sysloglevel": 4 + } +} diff --git a/test/config/va.json b/test/config/va.json index 76de0e9d048..acce02a39ba 100644 --- a/test/config/va.json +++ b/test/config/va.json @@ -49,6 +49,11 @@ "serverAddress": "rva1.service.consul:9498", "timeout": "15s", "hostOverride": "rva1.boulder" + }, + { + "serverAddress": "rva1.service.consul:9499", + "timeout": "15s", + "hostOverride": "rva1.boulder" } ], "accountURIPrefixes": [ diff --git a/test/consul/config.hcl b/test/consul/config.hcl index 08e3c2d1d22..d00af2681de 100644 --- a/test/consul/config.hcl +++ b/test/consul/config.hcl @@ -176,6 +176,14 @@ services { tags = ["tcp"] // Required for SRV RR support in gRPC DNS resolution. } +services { + id = "rva1-c" + name = "rva1" + address = "10.77.77.77" + port = 9499 + tags = ["tcp"] // Required for SRV RR support in gRPC DNS resolution. +} + # TODO(#5294) Remove rva2-a/b in favor of rva1-a/b services { id = "rva2-a" diff --git a/test/startservers.py b/test/startservers.py index 37ba783e85d..93d0c25bcee 100644 --- a/test/startservers.py +++ b/test/startservers.py @@ -24,6 +24,10 @@ 8012, 9498, 'rva.boulder', ('./bin/boulder', 'remoteva', '--config', os.path.join(config_dir, 'remoteva-b.json'), '--addr', ':9498', '--debug-addr', ':8012'), None), + Service('remoteva-c', + 8023, 9499, 'rva.boulder', + ('./bin/boulder', 'remoteva', '--config', os.path.join(config_dir, 'remoteva-c.json'), '--addr', ':9499', '--debug-addr', ':8023'), + None), Service('boulder-sa-1', 8003, 9395, 'sa.boulder', ('./bin/boulder', 'boulder-sa', '--config', os.path.join(config_dir, 'sa.json'), '--addr', ':9395', '--debug-addr', ':8003'), diff --git a/test/v2_integration.py b/test/v2_integration.py index eaca4041bef..1b27418acb9 100644 --- a/test/v2_integration.py +++ b/test/v2_integration.py @@ -1005,7 +1005,7 @@ def test_http_multiva_threshold_pass(): # Configure a guestlist that will pass the multiVA threshold test by # allowing the primary VA at some, but not all, remotes. - guestlist = {"boulder": 1, "boulder-remoteva-a": 1, "boulder-remoteva-b": 1, "remoteva-a": 1} + guestlist = {"boulder": 1, "boulder-remoteva-a": 1, "boulder-remoteva-b": 1, "remoteva-c": 1, "remoteva-a": 1} hostname, cleanup = multiva_setup(client, guestlist) From 509a37cf037a7de81a4f5fa8cb111cd6f2d8a86f Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 14 Nov 2024 20:35:22 -0800 Subject: [PATCH 13/19] Fix bad merge --- va/va_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/va/va_test.go b/va/va_test.go index b182178993b..2808aabe7ed 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -655,7 +655,7 @@ func TestMultiVALogging(t *testing.T) { 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 { + len(rva2Log.GetAllMatching(`"RIR":"RIPE"`)) >= 1 && len(rva3Log.GetAllMatching(`"Perspective":"dev-ripe"`)) >= 1 && len(rva3Log.GetAllMatching(`"RIR":"RIPE"`)) >= 1 { break From 69655b524782b4c9473dd40b32bf0f5b6ca907c2 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 15 Nov 2024 11:14:23 -0800 Subject: [PATCH 14/19] remove test for logging of perspective --- va/va_test.go | 38 +------------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/va/va_test.go b/va/va_test.go index 2808aabe7ed..a8271e1d26c 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -620,14 +620,9 @@ func TestMultiVALogging(t *testing.T) { ms := httpMultiSrv(t, expectedToken, map[string]bool{localUA: true, rva1UA: true, rva2UA: true}) defer ms.Close() -<<<<<<< HEAD rva1 := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN") rva2 := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE") -======= - rva1, rva1Log := setupRemote(ms.Server, rva1UA, nil, "dev-arin", "ARIN") - rva2, rva2Log := setupRemote(ms.Server, rva2UA, nil, "dev-ripe", "RIPE") - rva3, rva3Log := setupRemote(ms.Server, rva3UA, nil, "dev-ripe", "RIPE") ->>>>>>> 9085ab38f (Fix tests) + rva3 := setupRemote(ms.Server, rva3UA, nil, "dev-ripe", "RIPE") remoteVAs := []RemoteVA{ {rva1, rva1UA}, @@ -639,37 +634,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") -<<<<<<< HEAD -======= - - // 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 && - len(rva3Log.GetAllMatching(`"Perspective":"dev-ripe"`)) >= 1 && - len(rva3Log.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.Logf("RVA 3:\n%s\n", strings.Join(rva3Log.GetAll(), "\n")) - t.Errorf("expected perspective and RIR logs for remote VAs, but they never arrived") - } - time.Sleep(100 * time.Millisecond) - } ->>>>>>> 9085ab38f (Fix tests) } func TestDetailedError(t *testing.T) { From 5e767383c122df60dc33e3265ffde645ee326598 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 15 Nov 2024 12:41:56 -0800 Subject: [PATCH 15/19] Review feedback --- test/v2_integration.py | 5 +++-- va/va_test.go | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/v2_integration.py b/test/v2_integration.py index 1b27418acb9..230d63ba289 100644 --- a/test/v2_integration.py +++ b/test/v2_integration.py @@ -1005,7 +1005,8 @@ def test_http_multiva_threshold_pass(): # Configure a guestlist that will pass the multiVA threshold test by # allowing the primary VA at some, but not all, remotes. - guestlist = {"boulder": 1, "boulder-remoteva-a": 1, "boulder-remoteva-b": 1, "remoteva-c": 1, "remoteva-a": 1} + # In particular, remoteva-c is missing. + guestlist = {"boulder": 1, "remoteva-a": 1, "remoteva-b": 1} hostname, cleanup = multiva_setup(client, guestlist) @@ -1021,7 +1022,7 @@ def test_http_multiva_primary_fail_remote_pass(): # Configure a guestlist that will fail the primary VA check but allow all of # the remote VAs. - guestlist = {"boulder": 0, "boulder-remoteva-a": 1, "boulder-remoteva-b": 1, "remoteva-a": 1, "remoteva-b": 1} + guestlist = {"boulder": 0, "remoteva-a": 1, "remoteva-b": 1} hostname, cleanup = multiva_setup(client, guestlist) diff --git a/va/va_test.go b/va/va_test.go index a8271e1d26c..9857c87024a 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -118,7 +118,9 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS perspective := PrimaryPerspective if len(remoteVAs) == 0 { - perspective = "example perspective" + // We're being set up as a remote. Use a distinct perspective from other remotes + // to better simulate what prod will be like. + perspective = "example perspective " + core.RandomString(4) } va, err := NewValidationAuthorityImpl( From 03a80c6e76a32497b4d6985626753ffaeb16fdf8 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 15 Nov 2024 12:53:41 -0800 Subject: [PATCH 16/19] 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 Date: Fri, 15 Nov 2024 14:22:51 -0800 Subject: [PATCH 17/19] 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 Date: Fri, 15 Nov 2024 14:25:58 -0800 Subject: [PATCH 18/19] 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 6fe8d5feb0830e256755091e18ea323d52aa1d4e Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 15 Nov 2024 14:40:16 -0800 Subject: [PATCH 19/19] Fix tests --- va/dns_test.go | 2 +- va/va_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/va/dns_test.go b/va/dns_test.go index ef7d9cfd54e..92a07a47d54 100644 --- a/va/dns_test.go +++ b/va/dns_test.go @@ -30,7 +30,7 @@ func TestDNSValidationEmpty(t *testing.T) { test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ "operation": opChallAndCAA, - "perspective": PrimaryPerspective, + "perspective": va.perspective, "challenge_type": string(core.ChallengeTypeDNS01), "problem_type": string(probs.UnauthorizedProblem), "result": fail, diff --git a/va/va_test.go b/va/va_test.go index 68c2148f12c..caf8b5f43a1 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -265,7 +265,7 @@ func TestPerformValidationInvalid(t *testing.T) { test.Assert(t, res.Problems != nil, "validation succeeded") test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ "operation": opChallAndCAA, - "perspective": PrimaryPerspective, + "perspective": va.perspective, "challenge_type": string(core.ChallengeTypeDNS01), "problem_type": string(probs.UnauthorizedProblem), "result": fail, @@ -295,7 +295,7 @@ func TestPerformValidationValid(t *testing.T) { test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ "operation": opChallAndCAA, - "perspective": PrimaryPerspective, + "perspective": va.perspective, "challenge_type": string(core.ChallengeTypeDNS01), "problem_type": "", "result": pass, @@ -322,7 +322,7 @@ func TestPerformValidationWildcard(t *testing.T) { test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ "operation": opChallAndCAA, - "perspective": PrimaryPerspective, + "perspective": va.perspective, "challenge_type": string(core.ChallengeTypeDNS01), "problem_type": "", "result": pass,