diff --git a/cmd/boulder-va/main.go b/cmd/boulder-va/main.go index f2c2c848766..04da53c2d86 100644 --- a/cmd/boulder-va/main.go +++ b/cmd/boulder-va/main.go @@ -30,9 +30,7 @@ type RemoteVAGRPCClientConfig struct { // Requirement 2.7 ("Multi-Perspective Issuance Corroboration attempts // from each Network Perspective"). It should uniquely identify a group // of RVAs deployed in the same datacenter. - // - // TODO(#7615): Make mandatory. - Perspective string `validate:"omitempty"` + Perspective string `validate:"required"` // RIR indicates the Regional Internet Registry where this RVA is // located. This field is used to identify the RIR region from which a @@ -44,9 +42,7 @@ type RemoteVAGRPCClientConfig struct { // - APNIC // - LACNIC // - AFRINIC - // - // TODO(#7615): Make mandatory. - RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AFRINIC"` + RIR string `validate:"required,oneof=ARIN RIPE APNIC LACNIC AFRINIC"` } type Config struct { diff --git a/cmd/remoteva/main.go b/cmd/remoteva/main.go index fa83487503e..bf236ddc619 100644 --- a/cmd/remoteva/main.go +++ b/cmd/remoteva/main.go @@ -25,9 +25,7 @@ type Config struct { // Requirement 2.7 ("Multi-Perspective Issuance Corroboration attempts // from each Network Perspective"). It should uniquely identify a group // of RVAs deployed in the same datacenter. - // - // TODO(#7615): Make mandatory. - Perspective string `omitempty:"omitempty"` + Perspective string `omitempty:"required"` // RIR indicates the Regional Internet Registry where this RVA is // located. This field is used to identify the RIR region from which a @@ -39,9 +37,7 @@ type Config struct { // - APNIC // - LACNIC // - AFRINIC - // - // TODO(#7615): Make mandatory. - RIR string `validate:"omitempty,oneof=ARIN RIPE APNIC LACNIC AFRINIC"` + RIR string `validate:"required,oneof=ARIN RIPE APNIC LACNIC AFRINIC"` // SkipGRPCClientCertVerification, when disabled as it should typically // be, will cause the remoteva server (which receives gRPCs from a diff --git a/features/features.go b/features/features.go index a167a981eb3..cd131679b51 100644 --- a/features/features.go +++ b/features/features.go @@ -21,6 +21,8 @@ type Config struct { DisableLegacyLimitWrites bool MultipleCertificateProfiles bool InsertAuthzsIndividually bool + EnforceMultiCAA bool + EnforceMPIC bool // ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for // GET requests. WARNING: This feature is a draft and highly unstable. @@ -52,11 +54,6 @@ type Config struct { // DOH enables DNS-over-HTTPS queries for validation DOH bool - // EnforceMultiCAA causes the VA to kick off remote CAA rechecks when true. - // When false, no remote CAA rechecks will be performed. The primary VA will - // make a valid/invalid decision with the results. - EnforceMultiCAA bool - // CheckIdentifiersPaused checks if any of the identifiers in the order are // currently paused at NewOrder time. If any are paused, an error is // returned to the Subscriber indicating that the order cannot be processed @@ -83,24 +80,6 @@ type Config struct { // removing pending authz reuse. NoPendingAuthzReuse bool - // EnforceMPIC enforces SC-067 V3: Require Multi-Perspective Issuance - // Corroboration by: - // - Requiring at least three distinct perspectives, as outlined in the - // "Phased Implementation Timeline" in BRs section 3.2.2.9 ("Effective - // March 15, 2025"). - // - Ensuring that corroborating (passing) perspectives reside in at least - // 2 distinct Regional Internet Registries (RIRs) per the "Phased - // Implementation Timeline" in BRs section 3.2.2.9 ("Effective March 15, - // 2026"). - // - Including an MPIC summary consisting of: passing perspectives, failing - // perspectives, passing RIRs, and a quorum met for issuance (e.g., 2/3 - // or 3/3) in each validation audit log event, per BRs Section 5.4.1, - // Requirement 2.8. - // - // This feature flag also causes CAA checks to happen after all remote VAs - // have passed DCV. - EnforceMPIC bool - // UnsplitIssuance causes the RA to make a single call to the CA for issuance, // calling the new `IssueCertificate` instead of the old `IssuePrecertficate` / // `IssueCertificateForPrecertificate` pair. diff --git a/ra/ra.go b/ra/ra.go index a93337245c3..5d4ec3d06a0 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -855,19 +855,11 @@ func (ra *RegistrationAuthorityImpl) recheckCAA(ctx context.Context, authzs []*c } var resp *vapb.IsCAAValidResponse var err error - if !features.Get().EnforceMPIC { - resp, err = ra.VA.IsCAAValid(ctx, &vapb.IsCAAValidRequest{ - Domain: name, - ValidationMethod: method, - AccountURIID: authz.RegistrationID, - }) - } else { - resp, err = ra.VA.DoCAA(ctx, &vapb.IsCAAValidRequest{ - Domain: name, - ValidationMethod: method, - AccountURIID: authz.RegistrationID, - }) - } + resp, err = ra.VA.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Domain: name, + ValidationMethod: method, + AccountURIID: authz.RegistrationID, + }) if err != nil { ra.log.AuditErrf("Rechecking CAA: %s", err) err = berrors.InternalServerError( @@ -1535,33 +1527,23 @@ func (ra *RegistrationAuthorityImpl) resetAccountPausingLimit(ctx context.Contex } } -// doDCVAndCAA performs DCV and CAA checks. When EnforceMPIC is enabled, the -// checks are executed sequentially: DCV is performed first and CAA is only -// checked if DCV is successful. Validation records from the DCV check are -// returned even if the CAA check fails. When EnforceMPIC is disabled, DCV and -// CAA checks are performed in the same request. +// doDCVAndCAA performs DCV and CAA checks sequentially: DCV is performed first +// and CAA is only checked if DCV is successful. Validation records from the DCV +// check are returned even if the CAA check fails. func (ra *RegistrationAuthorityImpl) checkDCVAndCAA(ctx context.Context, dcvReq *vapb.PerformValidationRequest, caaReq *vapb.IsCAAValidRequest) (*corepb.ProblemDetails, []*corepb.ValidationRecord, error) { - if !features.Get().EnforceMPIC { - performValidationRes, err := ra.VA.PerformValidation(ctx, dcvReq) - if err != nil { - return nil, nil, err - } - return performValidationRes.Problem, performValidationRes.Records, nil - } else { - doDCVRes, err := ra.VA.DoDCV(ctx, dcvReq) - if err != nil { - return nil, nil, err - } - if doDCVRes.Problem != nil { - return doDCVRes.Problem, doDCVRes.Records, nil - } + doDCVRes, err := ra.VA.DoDCV(ctx, dcvReq) + if err != nil { + return nil, nil, err + } + if doDCVRes.Problem != nil { + return doDCVRes.Problem, doDCVRes.Records, nil + } - doCAAResp, err := ra.VA.DoCAA(ctx, caaReq) - if err != nil { - return nil, nil, err - } - return doCAAResp.Problem, doDCVRes.Records, nil + doCAAResp, err := ra.VA.DoCAA(ctx, caaReq) + if err != nil { + return nil, nil, err } + return doCAAResp.Problem, doDCVRes.Records, nil } // PerformValidation initiates validation for a specific challenge associated diff --git a/test/config-next/ra.json b/test/config-next/ra.json index c2170b65ea2..bc79d9b270c 100644 --- a/test/config-next/ra.json +++ b/test/config-next/ra.json @@ -143,8 +143,7 @@ "AsyncFinalize": true, "AutomaticallyPauseZombieClients": true, "NoPendingAuthzReuse": true, - "UnsplitIssuance": true, - "EnforceMPIC": true + "UnsplitIssuance": true }, "ctLogs": { "stagger": "500ms", diff --git a/test/config-next/va.json b/test/config-next/va.json index 514317f2a91..f9ea246f54e 100644 --- a/test/config-next/va.json +++ b/test/config-next/va.json @@ -38,7 +38,6 @@ } }, "features": { - "EnforceMultiCAA": true, "DOH": true }, "remoteVAs": [ diff --git a/test/config/remoteva-a.json b/test/config/remoteva-a.json index 8725cda2f3c..06c43ea8e27 100644 --- a/test/config/remoteva-a.json +++ b/test/config/remoteva-a.json @@ -27,6 +27,11 @@ "va.boulder" ] }, + "va.CAA": { + "clientNames": [ + "va.boulder" + ] + }, "grpc.health.v1.Health": { "clientNames": [ "health-checker.boulder" diff --git a/test/config/remoteva-b.json b/test/config/remoteva-b.json index 5089b7a452c..a7dd04b6118 100644 --- a/test/config/remoteva-b.json +++ b/test/config/remoteva-b.json @@ -27,6 +27,11 @@ "va.boulder" ] }, + "va.CAA": { + "clientNames": [ + "va.boulder" + ] + }, "grpc.health.v1.Health": { "clientNames": [ "health-checker.boulder" diff --git a/test/config/remoteva-c.json b/test/config/remoteva-c.json index 5234429f5b2..77f2e825c78 100644 --- a/test/config/remoteva-c.json +++ b/test/config/remoteva-c.json @@ -27,6 +27,11 @@ "va.boulder" ] }, + "va.CAA": { + "clientNames": [ + "va.boulder" + ] + }, "grpc.health.v1.Health": { "clientNames": [ "health-checker.boulder" diff --git a/va/caa.go b/va/caa.go index 0b27c90cac1..624a5e6e8fe 100644 --- a/va/caa.go +++ b/va/caa.go @@ -16,7 +16,6 @@ import ( "github.com/letsencrypt/boulder/core" corepb "github.com/letsencrypt/boulder/core/proto" berrors "github.com/letsencrypt/boulder/errors" - "github.com/letsencrypt/boulder/features" "github.com/letsencrypt/boulder/identifier" "github.com/letsencrypt/boulder/probs" vapb "github.com/letsencrypt/boulder/va/proto" @@ -27,15 +26,20 @@ type caaParams struct { validationMethod core.AcmeChallenge } -// IsCAAValid checks requested CAA records from a VA, and recursively any RVAs -// configured in the VA. It returns a response or an error. -func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) { +// DoCAA conducts a CAA check for the specified dnsName. When invoked on the +// primary Validation Authority (VA) and the local check succeeds, it also +// performs CAA checks using the configured remote VAs. Failed checks are +// indicated by a non-nil Problems in the returned ValidationResult. DoCAA +// returns error only for internal logic errors (and the client may receive +// errors from gRPC in the event of a communication problem). This method +// implements the CAA portion of Multi-Perspective Issuance Corroboration as +// defined in BRs Sections 3.2.2.9 and 5.4.1. +func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) { if core.IsAnyNilOrZero(req.Domain, req.ValidationMethod, req.AccountURIID) { return nil, berrors.InternalServerError("incomplete IsCAAValid request") } - logEvent := verificationRequestEvent{ - // TODO(#7061) Plumb req.Authz.Id as "AuthzID:" through from the RA to - // correlate which authz triggered this request. + logEvent := validationLogEvent{ + AuthzID: req.AuthzID, Requester: req.AccountURIID, Identifier: req.Domain, } @@ -51,7 +55,11 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC validationMethod: challType, } + // Initialize variables and a deferred function to handle check latency + // metrics, log check errors, and log an MPIC summary. Avoid using := to + // redeclare `prob`, `localLatency`, or `summary` below this point. var prob *probs.ProblemDetails + var summary *mpicSummary var internalErr error var localLatency time.Duration start := va.clk.Now() @@ -72,6 +80,7 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC if va.isPrimaryVA() { // Observe total check latency (primary+remote). va.observeLatency(opCAA, allPerspectives, string(challType), probType, outcome, va.clk.Since(start)) + logEvent.Summary = summary } // Log the total check latency. logEvent.Latency = va.clk.Since(start).Round(time.Millisecond).Seconds() @@ -90,15 +99,16 @@ func (va *ValidationAuthorityImpl) IsCAAValid(ctx context.Context, req *vapb.IsC prob.Detail = fmt.Sprintf("While processing CAA for %s: %s", req.Domain, prob.Detail) } - if features.Get().EnforceMultiCAA { + if va.isPrimaryVA() { op := func(ctx context.Context, remoteva RemoteVA, req proto.Message) (remoteResult, error) { checkRequest, ok := req.(*vapb.IsCAAValidRequest) if !ok { return nil, fmt.Errorf("got type %T, want *vapb.IsCAAValidRequest", req) } - return remoteva.IsCAAValid(ctx, checkRequest) + return remoteva.DoCAA(ctx, checkRequest) } - remoteProb := va.performRemoteOperation(ctx, op, req) + var remoteProb *probs.ProblemDetails + summary, remoteProb = va.doRemoteOperation(ctx, op, req) // If the remote result was a non-nil problem then fail the CAA check if remoteProb != nil { prob = remoteProb diff --git a/va/caa_test.go b/va/caa_test.go index c65270bcc1f..b85ffb7ad8e 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -521,107 +521,59 @@ func TestCAALogging(t *testing.T) { } } -type caaCheckFuncRunner func(context.Context, *ValidationAuthorityImpl, *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) - -var runIsCAAValid = func(ctx context.Context, va *ValidationAuthorityImpl, req *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) { - return va.IsCAAValid(ctx, req) -} - -var runDoCAA = func(ctx context.Context, va *ValidationAuthorityImpl, req *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) { - return va.DoCAA(ctx, req) -} - -// TestIsCAAValidErrMessage tests that an error result from `va.IsCAAValid` +// TestDoCAAErrMessage 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) { +func TestDoCAAErrMessage(t *testing.T) { t.Parallel() va, _ := setup(nil, "", nil, caaMockDNS{}) - testCases := []struct { - name string - caaCheckFunc caaCheckFuncRunner - }{ - { - name: "IsCAAValid", - caaCheckFunc: runIsCAAValid, - }, - { - name: "DoCAA", - caaCheckFunc: runDoCAA, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - // Call the operation with a domain we know fails with a generic error from the - // caaMockDNS. - domain := "caa-timeout.com" - resp, err := tc.caaCheckFunc(ctx, va, &vapb.IsCAAValidRequest{ - Domain: domain, - ValidationMethod: string(core.ChallengeTypeHTTP01), - AccountURIID: 12345, - }) + // Call the operation with a domain we know fails with a generic error from the + // caaMockDNS. + domain := "caa-timeout.com" + resp, err := va.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Domain: domain, + ValidationMethod: string(core.ChallengeTypeHTTP01), + AccountURIID: 12345, + }) - // The lookup itself should not return an error - test.AssertNotError(t, err, "Unexpected error calling IsCAAValidRequest") - // The result should not be nil - test.AssertNotNil(t, resp, "Response to IsCAAValidRequest was nil") - // The result's Problem should not be nil - test.AssertNotNil(t, resp.Problem, "Response Problem was nil") - // The result's Problem should be an error message that includes the domain. - test.AssertEquals(t, resp.Problem.Detail, fmt.Sprintf("While processing CAA for %s: error", domain)) - }) - } + // The lookup itself should not return an error + test.AssertNotError(t, err, "Unexpected error calling IsCAAValidRequest") + // The result should not be nil + test.AssertNotNil(t, resp, "Response to IsCAAValidRequest was nil") + // The result's Problem should not be nil + test.AssertNotNil(t, resp.Problem, "Response Problem was nil") + // The result's Problem should be an error message that includes the domain. + test.AssertEquals(t, resp.Problem.Detail, fmt.Sprintf("While processing CAA for %s: error", domain)) } -// TestIsCAAValidParams tests that the IsCAAValid method rejects any requests +// TestDoCAAParams tests that the IsCAAValid method rejects any requests // which do not have the necessary parameters to do CAA Account and Method // Binding checks. -func TestIsCAAValidParams(t *testing.T) { +func TestDoCAAParams(t *testing.T) { t.Parallel() va, _ := setup(nil, "", nil, caaMockDNS{}) - testCases := []struct { - name string - caaCheckFunc caaCheckFuncRunner - }{ - { - name: "IsCAAValid", - caaCheckFunc: runIsCAAValid, - }, - { - name: "DoCAA", - caaCheckFunc: runDoCAA, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - // Calling IsCAAValid without a ValidationMethod should fail. - _, err := tc.caaCheckFunc(ctx, va, &vapb.IsCAAValidRequest{ - Domain: "present.com", - AccountURIID: 12345, - }) - test.AssertError(t, err, "calling IsCAAValid without a ValidationMethod") + // Calling IsCAAValid without a ValidationMethod should fail. + _, err := va.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Domain: "present.com", + AccountURIID: 12345, + }) + test.AssertError(t, err, "calling IsCAAValid without a ValidationMethod") - // Calling IsCAAValid with an invalid ValidationMethod should fail. - _, err = tc.caaCheckFunc(ctx, va, &vapb.IsCAAValidRequest{ - Domain: "present.com", - ValidationMethod: "tls-sni-01", - AccountURIID: 12345, - }) - test.AssertError(t, err, "calling IsCAAValid with a bad ValidationMethod") + // Calling IsCAAValid with an invalid ValidationMethod should fail. + _, err = va.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Domain: "present.com", + ValidationMethod: "tls-sni-01", + AccountURIID: 12345, + }) + test.AssertError(t, err, "calling IsCAAValid with a bad ValidationMethod") - // Calling IsCAAValid without an AccountURIID should fail. - _, err = tc.caaCheckFunc(ctx, va, &vapb.IsCAAValidRequest{ - Domain: "present.com", - ValidationMethod: string(core.ChallengeTypeHTTP01), - }) - test.AssertError(t, err, "calling IsCAAValid without an AccountURIID") - }) - } + // Calling IsCAAValid without an AccountURIID should fail. + _, err = va.DoCAA(ctx, &vapb.IsCAAValidRequest{ + Domain: "present.com", + ValidationMethod: string(core.ChallengeTypeHTTP01), + }) + test.AssertError(t, err, "calling IsCAAValid without an AccountURIID") } var errCAABrokenDNSClient = errors.New("dnsClient is broken") @@ -642,28 +594,6 @@ func (b caaBrokenDNS) LookupCAA(_ context.Context, domain string) ([]*dns.CAA, s return nil, "", bdns.ResolverAddrs{"caaBrokenDNS"}, errCAABrokenDNSClient } -func TestDisabledMultiCAARechecking(t *testing.T) { - remoteVAs := []remoteConf{{ua: "broken", rir: arin, dns: caaBrokenDNS{}}} - va, _ := setupWithRemotes(nil, "local", remoteVAs, nil) - - features.Set(features.Config{ - EnforceMultiCAA: false, - }) - defer features.Reset() - - isValidRes, err := va.IsCAAValid(context.TODO(), &vapb.IsCAAValidRequest{ - Domain: "present.com", - ValidationMethod: string(core.ChallengeTypeDNS01), - AccountURIID: 1, - }) - test.AssertNotError(t, err, "Error during IsCAAValid") - // The primary VA can successfully recheck the CAA record and is allowed to - // issue for this domain. If `EnforceMultiCAA`` was enabled, the configured - // remote VA with broken dns.Client would fail the check and return a - // Problem, but that code path could never trigger. - test.AssertBoxedNil(t, isValidRes.Problem, "IsCAAValid returned a problem, but should not have") -} - // caaHijackedDNS implements the `dns.DNSClient` interface with a set of useful // test answers for CAA queries. It returns alternate CAA records than what // caaMockDNS returns simulating either a BGP hijack or DNS records that have @@ -735,26 +665,8 @@ func TestMultiCAARechecking(t *testing.T) { hijackedUA = "hijacked" ) - type testFunc struct { - name string - impl caaCheckFuncRunner - } - - testFuncs := []testFunc{ - { - name: "IsCAAValid", - impl: runIsCAAValid, - }, - { - name: "DoCAA", - impl: runDoCAA, - }, - } - testCases := []struct { - name string - // method is only set inside of the test loop. - methodName string + name string domains string remoteVAs []remoteConf expectedProbSubstring string @@ -1151,77 +1063,55 @@ func TestMultiCAARechecking(t *testing.T) { } for _, tc := range testCases { - for _, testFunc := range testFuncs { - t.Run(tc.name+"_"+testFunc.name, func(t *testing.T) { - va, mockLog := setupWithRemotes(nil, localUA, tc.remoteVAs, tc.localDNSClient) - defer mockLog.Clear() - - features.Set(features.Config{ - EnforceMultiCAA: true, - }) - defer features.Reset() - - isValidRes, err := testFunc.impl(context.TODO(), va, &vapb.IsCAAValidRequest{ - Domain: tc.domains, - ValidationMethod: string(core.ChallengeTypeDNS01), - AccountURIID: 1, - }) - 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) - } - - if testFunc.name == "IsCAAValid" { - var invalidRVACount int - for _, x := range tc.remoteVAs { - if x.ua == brokenUA || x.ua == hijackedUA { - invalidRVACount++ - } - } - - gotRequestProbs := mockLog.GetAllMatching(" returned a problem: ") - test.AssertEquals(t, len(gotRequestProbs), invalidRVACount) - - gotDifferential := mockLog.GetAllMatching("remoteVADifferentials JSON=.*") - if tc.expectedDiffLogSubstring != "" { - test.AssertEquals(t, len(gotDifferential), 1) - test.AssertContains(t, gotDifferential[0], tc.expectedDiffLogSubstring) - } else { - test.AssertEquals(t, len(gotDifferential), 0) - } - } - - if testFunc.name == "DoCAA" && tc.expectedSummary != nil { - gotAuditLog := parseValidationLogEvent(t, mockLog.GetAllMatching("JSON=.*")) - slices.Sort(tc.expectedSummary.Passed) - slices.Sort(tc.expectedSummary.Failed) - slices.Sort(tc.expectedSummary.PassedRIRs) - test.AssertDeepEquals(t, gotAuditLog.Summary, tc.expectedSummary) - } - - gotAnyRemoteFailures := mockLog.GetAllMatching("CAA check failed due to remote failures:") - if len(gotAnyRemoteFailures) >= 1 { - // The primary VA only emits this line once. - test.AssertEquals(t, len(gotAnyRemoteFailures), 1) - } else { - test.AssertEquals(t, len(gotAnyRemoteFailures), 0) - } - - if tc.expectedLabels != nil { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, tc.expectedLabels, 1) - } + t.Run(tc.name, func(t *testing.T) { + va, mockLog := setupWithRemotes(nil, localUA, tc.remoteVAs, tc.localDNSClient) + defer mockLog.Clear() + features.Set(features.Config{ + EnforceMultiCAA: true, }) - } + defer features.Reset() + + isValidRes, err := va.DoCAA(context.TODO(), &vapb.IsCAAValidRequest{ + Domain: tc.domains, + ValidationMethod: string(core.ChallengeTypeDNS01), + AccountURIID: 1, + }) + 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) + } + + if tc.expectedSummary != nil { + gotAuditLog := parseValidationLogEvent(t, mockLog.GetAllMatching("JSON=.*")) + slices.Sort(tc.expectedSummary.Passed) + slices.Sort(tc.expectedSummary.Failed) + slices.Sort(tc.expectedSummary.PassedRIRs) + test.AssertDeepEquals(t, gotAuditLog.Summary, tc.expectedSummary) + } + + gotAnyRemoteFailures := mockLog.GetAllMatching("CAA check failed due to remote failures:") + if len(gotAnyRemoteFailures) >= 1 { + // The primary VA only emits this line once. + test.AssertEquals(t, len(gotAnyRemoteFailures), 1) + } else { + test.AssertEquals(t, len(gotAnyRemoteFailures), 0) + } + + if tc.expectedLabels != nil { + test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, tc.expectedLabels, 1) + } + + }) } } diff --git a/va/dns_test.go b/va/dns_test.go index 58f157647e8..812e1252cde 100644 --- a/va/dns_test.go +++ b/va/dns_test.go @@ -8,35 +8,14 @@ import ( "time" "github.com/jmhodges/clock" - "github.com/prometheus/client_golang/prometheus" "github.com/letsencrypt/boulder/bdns" - "github.com/letsencrypt/boulder/core" "github.com/letsencrypt/boulder/identifier" "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/test" ) -func TestDNSValidationEmpty(t *testing.T) { - va, _ := setup(nil, "", nil, nil) - - // This test calls PerformValidation directly, because that is where the - // metrics checked below are incremented. - req := createValidationRequest("empty-txts.com", core.ChallengeTypeDNS01) - res, _ := va.PerformValidation(context.Background(), req) - test.AssertEquals(t, res.Problem.ProblemType, "unauthorized") - test.AssertEquals(t, res.Problem.Detail, "No TXT record found at _acme-challenge.empty-txts.com") - - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCVAndCAA, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": string(probs.UnauthorizedProblem), - "result": fail, - }, 1) -} - func TestDNSValidationWrong(t *testing.T) { va, _ := setup(nil, "", nil, nil) _, err := va.validateDNS01(context.Background(), dnsi("wrong-dns01.com"), expectedKeyAuthorization) diff --git a/va/proto/va.pb.go b/va/proto/va.pb.go index abc195705a3..70ed9c5d461 100644 --- a/va/proto/va.pb.go +++ b/va/proto/va.pb.go @@ -402,27 +402,19 @@ var file_va_proto_rawDesc = []byte{ 0x07, 0x70, 0x72, 0x6f, 0x62, 0x6c, 0x65, 0x6d, 0x12, 0x20, 0x0a, 0x0b, 0x70, 0x65, 0x72, 0x73, 0x70, 0x65, 0x63, 0x74, 0x69, 0x76, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0b, 0x70, 0x65, 0x72, 0x73, 0x70, 0x65, 0x63, 0x74, 0x69, 0x76, 0x65, 0x12, 0x10, 0x0a, 0x03, 0x72, 0x69, - 0x72, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x72, 0x69, 0x72, 0x32, 0x8e, 0x01, 0x0a, - 0x02, 0x56, 0x41, 0x12, 0x49, 0x0a, 0x11, 0x50, 0x65, 0x72, 0x66, 0x6f, 0x72, 0x6d, 0x56, 0x61, - 0x6c, 0x69, 0x64, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x1c, 0x2e, 0x76, 0x61, 0x2e, 0x50, 0x65, - 0x72, 0x66, 0x6f, 0x72, 0x6d, 0x56, 0x61, 0x6c, 0x69, 0x64, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, - 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x14, 0x2e, 0x76, 0x61, 0x2e, 0x56, 0x61, 0x6c, 0x69, - 0x64, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, 0x00, 0x12, 0x3d, - 0x0a, 0x05, 0x44, 0x6f, 0x44, 0x43, 0x56, 0x12, 0x1c, 0x2e, 0x76, 0x61, 0x2e, 0x50, 0x65, 0x72, - 0x66, 0x6f, 0x72, 0x6d, 0x56, 0x61, 0x6c, 0x69, 0x64, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, - 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x14, 0x2e, 0x76, 0x61, 0x2e, 0x56, 0x61, 0x6c, 0x69, 0x64, - 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, 0x00, 0x32, 0x7e, 0x0a, - 0x03, 0x43, 0x41, 0x41, 0x12, 0x3d, 0x0a, 0x0a, 0x49, 0x73, 0x43, 0x41, 0x41, 0x56, 0x61, 0x6c, - 0x69, 0x64, 0x12, 0x15, 0x2e, 0x76, 0x61, 0x2e, 0x49, 0x73, 0x43, 0x41, 0x41, 0x56, 0x61, 0x6c, - 0x69, 0x64, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x16, 0x2e, 0x76, 0x61, 0x2e, 0x49, - 0x73, 0x43, 0x41, 0x41, 0x56, 0x61, 0x6c, 0x69, 0x64, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, - 0x65, 0x22, 0x00, 0x12, 0x38, 0x0a, 0x05, 0x44, 0x6f, 0x43, 0x41, 0x41, 0x12, 0x15, 0x2e, 0x76, - 0x61, 0x2e, 0x49, 0x73, 0x43, 0x41, 0x41, 0x56, 0x61, 0x6c, 0x69, 0x64, 0x52, 0x65, 0x71, 0x75, - 0x65, 0x73, 0x74, 0x1a, 0x16, 0x2e, 0x76, 0x61, 0x2e, 0x49, 0x73, 0x43, 0x41, 0x41, 0x56, 0x61, - 0x6c, 0x69, 0x64, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, 0x29, 0x5a, - 0x27, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x6c, 0x65, 0x74, 0x73, - 0x65, 0x6e, 0x63, 0x72, 0x79, 0x70, 0x74, 0x2f, 0x62, 0x6f, 0x75, 0x6c, 0x64, 0x65, 0x72, 0x2f, - 0x76, 0x61, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x72, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x72, 0x69, 0x72, 0x32, 0x43, 0x0a, 0x02, + 0x56, 0x41, 0x12, 0x3d, 0x0a, 0x05, 0x44, 0x6f, 0x44, 0x43, 0x56, 0x12, 0x1c, 0x2e, 0x76, 0x61, + 0x2e, 0x50, 0x65, 0x72, 0x66, 0x6f, 0x72, 0x6d, 0x56, 0x61, 0x6c, 0x69, 0x64, 0x61, 0x74, 0x69, + 0x6f, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x14, 0x2e, 0x76, 0x61, 0x2e, 0x56, + 0x61, 0x6c, 0x69, 0x64, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x22, + 0x00, 0x32, 0x3f, 0x0a, 0x03, 0x43, 0x41, 0x41, 0x12, 0x38, 0x0a, 0x05, 0x44, 0x6f, 0x43, 0x41, + 0x41, 0x12, 0x15, 0x2e, 0x76, 0x61, 0x2e, 0x49, 0x73, 0x43, 0x41, 0x41, 0x56, 0x61, 0x6c, 0x69, + 0x64, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x16, 0x2e, 0x76, 0x61, 0x2e, 0x49, 0x73, + 0x43, 0x41, 0x41, 0x56, 0x61, 0x6c, 0x69, 0x64, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, + 0x22, 0x00, 0x42, 0x29, 0x5a, 0x27, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, + 0x2f, 0x6c, 0x65, 0x74, 0x73, 0x65, 0x6e, 0x63, 0x72, 0x79, 0x70, 0x74, 0x2f, 0x62, 0x6f, 0x75, + 0x6c, 0x64, 0x65, 0x72, 0x2f, 0x76, 0x61, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, + 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -454,16 +446,12 @@ var file_va_proto_depIdxs = []int32{ 3, // 2: va.PerformValidationRequest.authz:type_name -> va.AuthzMeta 7, // 3: va.ValidationResult.records:type_name -> core.ValidationRecord 5, // 4: va.ValidationResult.problem:type_name -> core.ProblemDetails - 2, // 5: va.VA.PerformValidation:input_type -> va.PerformValidationRequest - 2, // 6: va.VA.DoDCV:input_type -> va.PerformValidationRequest - 0, // 7: va.CAA.IsCAAValid:input_type -> va.IsCAAValidRequest - 0, // 8: va.CAA.DoCAA:input_type -> va.IsCAAValidRequest - 4, // 9: va.VA.PerformValidation:output_type -> va.ValidationResult - 4, // 10: va.VA.DoDCV:output_type -> va.ValidationResult - 1, // 11: va.CAA.IsCAAValid:output_type -> va.IsCAAValidResponse - 1, // 12: va.CAA.DoCAA:output_type -> va.IsCAAValidResponse - 9, // [9:13] is the sub-list for method output_type - 5, // [5:9] is the sub-list for method input_type + 2, // 5: va.VA.DoDCV:input_type -> va.PerformValidationRequest + 0, // 6: va.CAA.DoCAA:input_type -> va.IsCAAValidRequest + 4, // 7: va.VA.DoDCV:output_type -> va.ValidationResult + 1, // 8: va.CAA.DoCAA:output_type -> va.IsCAAValidResponse + 7, // [7:9] is the sub-list for method output_type + 5, // [5:7] is the sub-list for method input_type 5, // [5:5] is the sub-list for extension type_name 5, // [5:5] is the sub-list for extension extendee 0, // [0:5] is the sub-list for field type_name diff --git a/va/proto/va.proto b/va/proto/va.proto index 44fa5c6e6e1..d5e2019b7d5 100644 --- a/va/proto/va.proto +++ b/va/proto/va.proto @@ -6,12 +6,10 @@ option go_package = "github.com/letsencrypt/boulder/va/proto"; import "core/proto/core.proto"; service VA { - rpc PerformValidation(PerformValidationRequest) returns (ValidationResult) {} rpc DoDCV(PerformValidationRequest) returns (ValidationResult) {} } service CAA { - rpc IsCAAValid(IsCAAValidRequest) returns (IsCAAValidResponse) {} rpc DoCAA(IsCAAValidRequest) returns (IsCAAValidResponse) {} } diff --git a/va/proto/va_grpc.pb.go b/va/proto/va_grpc.pb.go index 55eced18465..ed04bb450a5 100644 --- a/va/proto/va_grpc.pb.go +++ b/va/proto/va_grpc.pb.go @@ -19,15 +19,13 @@ import ( const _ = grpc.SupportPackageIsVersion9 const ( - VA_PerformValidation_FullMethodName = "/va.VA/PerformValidation" - VA_DoDCV_FullMethodName = "/va.VA/DoDCV" + VA_DoDCV_FullMethodName = "/va.VA/DoDCV" ) // VAClient is the client API for VA service. // // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. type VAClient interface { - PerformValidation(ctx context.Context, in *PerformValidationRequest, opts ...grpc.CallOption) (*ValidationResult, error) DoDCV(ctx context.Context, in *PerformValidationRequest, opts ...grpc.CallOption) (*ValidationResult, error) } @@ -39,16 +37,6 @@ func NewVAClient(cc grpc.ClientConnInterface) VAClient { return &vAClient{cc} } -func (c *vAClient) PerformValidation(ctx context.Context, in *PerformValidationRequest, opts ...grpc.CallOption) (*ValidationResult, error) { - cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) - out := new(ValidationResult) - err := c.cc.Invoke(ctx, VA_PerformValidation_FullMethodName, in, out, cOpts...) - if err != nil { - return nil, err - } - return out, nil -} - func (c *vAClient) DoDCV(ctx context.Context, in *PerformValidationRequest, opts ...grpc.CallOption) (*ValidationResult, error) { cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) out := new(ValidationResult) @@ -63,7 +51,6 @@ func (c *vAClient) DoDCV(ctx context.Context, in *PerformValidationRequest, opts // All implementations must embed UnimplementedVAServer // for forward compatibility type VAServer interface { - PerformValidation(context.Context, *PerformValidationRequest) (*ValidationResult, error) DoDCV(context.Context, *PerformValidationRequest) (*ValidationResult, error) mustEmbedUnimplementedVAServer() } @@ -72,9 +59,6 @@ type VAServer interface { type UnimplementedVAServer struct { } -func (UnimplementedVAServer) PerformValidation(context.Context, *PerformValidationRequest) (*ValidationResult, error) { - return nil, status.Errorf(codes.Unimplemented, "method PerformValidation not implemented") -} func (UnimplementedVAServer) DoDCV(context.Context, *PerformValidationRequest) (*ValidationResult, error) { return nil, status.Errorf(codes.Unimplemented, "method DoDCV not implemented") } @@ -91,24 +75,6 @@ func RegisterVAServer(s grpc.ServiceRegistrar, srv VAServer) { s.RegisterService(&VA_ServiceDesc, srv) } -func _VA_PerformValidation_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { - in := new(PerformValidationRequest) - if err := dec(in); err != nil { - return nil, err - } - if interceptor == nil { - return srv.(VAServer).PerformValidation(ctx, in) - } - info := &grpc.UnaryServerInfo{ - Server: srv, - FullMethod: VA_PerformValidation_FullMethodName, - } - handler := func(ctx context.Context, req interface{}) (interface{}, error) { - return srv.(VAServer).PerformValidation(ctx, req.(*PerformValidationRequest)) - } - return interceptor(ctx, in, info, handler) -} - func _VA_DoDCV_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { in := new(PerformValidationRequest) if err := dec(in); err != nil { @@ -134,10 +100,6 @@ var VA_ServiceDesc = grpc.ServiceDesc{ ServiceName: "va.VA", HandlerType: (*VAServer)(nil), Methods: []grpc.MethodDesc{ - { - MethodName: "PerformValidation", - Handler: _VA_PerformValidation_Handler, - }, { MethodName: "DoDCV", Handler: _VA_DoDCV_Handler, @@ -148,15 +110,13 @@ var VA_ServiceDesc = grpc.ServiceDesc{ } const ( - CAA_IsCAAValid_FullMethodName = "/va.CAA/IsCAAValid" - CAA_DoCAA_FullMethodName = "/va.CAA/DoCAA" + CAA_DoCAA_FullMethodName = "/va.CAA/DoCAA" ) // CAAClient is the client API for CAA service. // // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. type CAAClient interface { - IsCAAValid(ctx context.Context, in *IsCAAValidRequest, opts ...grpc.CallOption) (*IsCAAValidResponse, error) DoCAA(ctx context.Context, in *IsCAAValidRequest, opts ...grpc.CallOption) (*IsCAAValidResponse, error) } @@ -168,16 +128,6 @@ func NewCAAClient(cc grpc.ClientConnInterface) CAAClient { return &cAAClient{cc} } -func (c *cAAClient) IsCAAValid(ctx context.Context, in *IsCAAValidRequest, opts ...grpc.CallOption) (*IsCAAValidResponse, error) { - cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) - out := new(IsCAAValidResponse) - err := c.cc.Invoke(ctx, CAA_IsCAAValid_FullMethodName, in, out, cOpts...) - if err != nil { - return nil, err - } - return out, nil -} - func (c *cAAClient) DoCAA(ctx context.Context, in *IsCAAValidRequest, opts ...grpc.CallOption) (*IsCAAValidResponse, error) { cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) out := new(IsCAAValidResponse) @@ -192,7 +142,6 @@ func (c *cAAClient) DoCAA(ctx context.Context, in *IsCAAValidRequest, opts ...gr // All implementations must embed UnimplementedCAAServer // for forward compatibility type CAAServer interface { - IsCAAValid(context.Context, *IsCAAValidRequest) (*IsCAAValidResponse, error) DoCAA(context.Context, *IsCAAValidRequest) (*IsCAAValidResponse, error) mustEmbedUnimplementedCAAServer() } @@ -201,9 +150,6 @@ type CAAServer interface { type UnimplementedCAAServer struct { } -func (UnimplementedCAAServer) IsCAAValid(context.Context, *IsCAAValidRequest) (*IsCAAValidResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "method IsCAAValid not implemented") -} func (UnimplementedCAAServer) DoCAA(context.Context, *IsCAAValidRequest) (*IsCAAValidResponse, error) { return nil, status.Errorf(codes.Unimplemented, "method DoCAA not implemented") } @@ -220,24 +166,6 @@ func RegisterCAAServer(s grpc.ServiceRegistrar, srv CAAServer) { s.RegisterService(&CAA_ServiceDesc, srv) } -func _CAA_IsCAAValid_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { - in := new(IsCAAValidRequest) - if err := dec(in); err != nil { - return nil, err - } - if interceptor == nil { - return srv.(CAAServer).IsCAAValid(ctx, in) - } - info := &grpc.UnaryServerInfo{ - Server: srv, - FullMethod: CAA_IsCAAValid_FullMethodName, - } - handler := func(ctx context.Context, req interface{}) (interface{}, error) { - return srv.(CAAServer).IsCAAValid(ctx, req.(*IsCAAValidRequest)) - } - return interceptor(ctx, in, info, handler) -} - func _CAA_DoCAA_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { in := new(IsCAAValidRequest) if err := dec(in); err != nil { @@ -263,10 +191,6 @@ var CAA_ServiceDesc = grpc.ServiceDesc{ ServiceName: "va.CAA", HandlerType: (*CAAServer)(nil), Methods: []grpc.MethodDesc{ - { - MethodName: "IsCAAValid", - Handler: _CAA_IsCAAValid_Handler, - }, { MethodName: "DoCAA", Handler: _CAA_DoCAA_Handler, diff --git a/va/va.go b/va/va.go index a1e2cd4492e..c93baed6f9a 100644 --- a/va/va.go +++ b/va/va.go @@ -4,14 +4,15 @@ import ( "bytes" "context" "crypto/tls" - "encoding/json" "errors" "fmt" + "maps" "math/rand/v2" "net" "net/url" "os" "regexp" + "slices" "strings" "syscall" "time" @@ -242,8 +243,7 @@ func NewValidationAuthorityImpl( for i, va1 := range remoteVAs { for j, va2 := range remoteVAs { - // TODO(#7615): Remove the != "" check once perspective is required. - if i != j && va1.Perspective == va2.Perspective && va1.Perspective != "" { + if i != j && va1.Perspective == va2.Perspective { return nil, fmt.Errorf("duplicate remote VA perspective %q", va1.Perspective) } } @@ -294,18 +294,6 @@ func maxAllowedFailures(perspectiveCount int) int { return 2 } -// verificationRequestEvent is logged once for each validation attempt. Its -// fields are exported for logging purposes. -type verificationRequestEvent struct { - AuthzID string - Requester int64 - Identifier string - Challenge core.Challenge - Error string `json:",omitempty"` - InternalError string `json:",omitempty"` - Latency float64 -} - // ipError is an error type used to pass though the IP address of the remote // host when an error occurs during HTTP-01 and TLS-ALPN domain validation. type ipError struct { @@ -472,23 +460,82 @@ type remoteResult interface { GetRir() string } -var _ remoteResult = (*vapb.ValidationResult)(nil) -var _ remoteResult = (*vapb.IsCAAValidResponse)(nil) +const ( + // requiredRIRs is the minimum number of distinct Regional Internet + // Registries required for MPIC-compliant validation. Per BRs Section + // 3.2.2.9, starting March 15, 2026, the required number is 2. + requiredRIRs = 2 +) + +// mpicSummary is returned by doRemoteOperation and contains a summary of the +// validation results for logging purposes. To ensure that the JSON output does +// not contain nil slices, and to ensure deterministic output use the +// summarizeMPIC function to prepare an mpicSummary. +type mpicSummary struct { + // Passed are the perspectives that passed validation. + Passed []string `json:"passedPerspectives"` + + // Failed are the perspectives that failed validation. + Failed []string `json:"failedPerspectives"` + + // PassedRIRs are the Regional Internet Registries that the passing + // perspectives reside in. + PassedRIRs []string `json:"passedRIRs"` + + // QuorumResult is the Multi-Perspective Issuance Corroboration quorum + // result, per BRs Section 5.4.1, Requirement 2.7 (i.e., "3/4" which should + // be interpreted as "Three (3) out of four (4) attempted Network + // Perspectives corroborated the determinations made by the Primary Network + // Perspective". + QuorumResult string `json:"quorumResult"` +} + +// summarizeMPIC prepares an *mpicSummary for logging, ensuring there are no nil +// slices and output is deterministic. +func summarizeMPIC(passed, failed []string, passedRIRSet map[string]struct{}) *mpicSummary { + if passed == nil { + passed = []string{} + } + slices.Sort(passed) + if failed == nil { + failed = []string{} + } + slices.Sort(failed) -// performRemoteOperation concurrently calls the provided operation with `req` and a -// RemoteVA once for each configured RemoteVA. It cancels remaining operations and returns -// early if either the required number of successful results is obtained or the number of -// failures exceeds va.maxRemoteFailures. + passedRIRs := []string{} + if passedRIRSet != nil { + for rir := range maps.Keys(passedRIRSet) { + passedRIRs = append(passedRIRs, rir) + } + } + slices.Sort(passedRIRs) + + return &mpicSummary{ + Passed: passed, + Failed: failed, + PassedRIRs: passedRIRs, + QuorumResult: fmt.Sprintf("%d/%d", len(passed), len(passed)+len(failed)), + } +} + +// doRemoteOperation concurrently calls the provided operation with `req` and a +// RemoteVA once for each configured RemoteVA. It cancels remaining operations +// and returns early if either the required number of successful results is +// obtained or the number of failures exceeds va.maxRemoteFailures. // // Internal logic errors are logged. If the number of operation failures exceeds // va.maxRemoteFailures, the first encountered problem is returned as a // *probs.ProblemDetails. -func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, op remoteOperation, req proto.Message) *probs.ProblemDetails { +func (va *ValidationAuthorityImpl) doRemoteOperation(ctx context.Context, op remoteOperation, req proto.Message) (*mpicSummary, *probs.ProblemDetails) { remoteVACount := len(va.remoteVAs) - if remoteVACount == 0 { - return nil + // - Mar 15, 2026: MUST implement using at least 3 perspectives + // - Jun 15, 2026: MUST implement using at least 4 perspectives + // - Dec 15, 2026: MUST implement using at least 5 perspectives + // See "Phased Implementation Timeline" in + // https://github.com/cabforum/servercert/blob/main/docs/BR.md#3229-multi-perspective-issuance-corroboration + if remoteVACount < 3 { + return nil, probs.ServerInternal("Insufficient remote perspectives: need at least 3") } - isCAAValidReq, isCAACheck := req.(*vapb.IsCAAValidRequest) type response struct { addr string @@ -509,9 +556,7 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} return } - // TODO(#7615): Remove the != "" checks once perspective and rir are required. - if (rva.Perspective != "" && res.GetPerspective() != "" && res.GetPerspective() != rva.Perspective) || - (rva.RIR != "" && res.GetRir() != "" && res.GetRir() != rva.RIR) { + if res.GetPerspective() != rva.Perspective || res.GetRir() != rva.RIR { err = fmt.Errorf( "Expected perspective %q (%q) but got reply from %q (%q) - misconfiguration likely", rva.Perspective, rva.RIR, res.GetPerspective(), res.GetRir(), ) @@ -525,6 +570,7 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o required := remoteVACount - va.maxRemoteFailures var passed []string var failed []string + var passedRIRs = map[string]struct{}{} var firstProb *probs.ProblemDetails for resp := range responses { @@ -550,13 +596,10 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o va.log.Errf("Operation on Remote VA (%s) returned malformed problem: %s", resp.addr, err) currProb = probs.ServerInternal("Secondary validation RPC returned malformed result") } - if isCAACheck { - // We're checking CAA, log the problem. - va.log.Errf("Operation on Remote VA (%s) returned a problem: %s", resp.addr, currProb) - } } else { // The remote VA returned a successful result. passed = append(passed, resp.perspective) + passedRIRs[resp.rir] = struct{}{} } if firstProb == nil && currProb != nil { @@ -567,7 +610,7 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o // To respond faster, if we get enough successes or too many failures, we cancel remaining RPCs. // Finish the loop to collect remaining responses into `failed` so we can rely on having a response // for every request we made. - if len(passed) >= required { + if len(passed) >= required && len(passedRIRs) >= requiredRIRs { cancel() } if len(failed) > va.maxRemoteFailures { @@ -579,103 +622,43 @@ func (va *ValidationAuthorityImpl) performRemoteOperation(ctx context.Context, o break } } - - if isCAACheck { - // We're checking CAA, log the results. - va.logRemoteResults(isCAAValidReq, len(passed), len(failed)) - } - - if len(passed) >= required { - return nil - } else if len(failed) > va.maxRemoteFailures { - firstProb.Detail = fmt.Sprintf("During secondary validation: %s", firstProb.Detail) - return firstProb - } else { - // This condition should not occur - it indicates the passed/failed counts - // neither met the required threshold nor the maxRemoteFailures threshold. - return probs.ServerInternal("Too few remote RPC results") + if len(passed) >= required && len(passedRIRs) >= requiredRIRs { + return summarizeMPIC(passed, failed, passedRIRs), nil } -} - -// logRemoteResults is called by performRemoteOperation when the request passed -// is *vapb.IsCAAValidRequest. -func (va *ValidationAuthorityImpl) logRemoteResults(req *vapb.IsCAAValidRequest, passed int, failed int) { - if failed == 0 { - // There's no point logging a differential line if everything succeeded. - return - } - - logOb := struct { - Domain string - AccountID int64 - ChallengeType string - RemoteSuccesses int - RemoteFailures int - }{ - Domain: req.Domain, - AccountID: req.AccountURIID, - ChallengeType: req.ValidationMethod, - RemoteSuccesses: passed, - RemoteFailures: failed, - } - - logJSON, err := json.Marshal(logOb) - if err != nil { - // log a warning - a marshaling failure isn't expected given the data - // isn't critical enough to break validation by returning an error the - // caller. - va.log.Warningf("Could not marshal log object in "+ - "logRemoteDifferential: %s", err) - return + if firstProb == nil { + // This should never happen. If we didn't meet the thresholds above we + // should have seen at least one error. + return summarizeMPIC(passed, failed, passedRIRs), probs.ServerInternal( + "During secondary validation: validation failed but the problem is unavailable") } - - va.log.Infof("remoteVADifferentials JSON=%s", string(logJSON)) + firstProb.Detail = fmt.Sprintf("During secondary validation: %s", firstProb.Detail) + return summarizeMPIC(passed, failed, passedRIRs), firstProb } -// performLocalValidation performs primary domain control validation and then -// checks CAA. If either step fails, it immediately returns a bare error so -// that our audit logging can include the underlying error. -func (va *ValidationAuthorityImpl) performLocalValidation( - ctx context.Context, - ident identifier.ACMEIdentifier, - regid int64, - kind core.AcmeChallenge, - token string, - keyAuthorization string, -) ([]core.ValidationRecord, error) { - // Do primary domain control validation. Any kind of error returned by this - // counts as a validation error, and will be converted into an appropriate - // probs.ProblemDetails by the calling function. - records, err := va.validateChallenge(ctx, ident, kind, token, keyAuthorization) - if err != nil { - return records, err - } - - // Do primary CAA checks. Any kind of error returned by this counts as not - // receiving permission to issue, and will be converted into an appropriate - // probs.ProblemDetails by the calling function. - err = va.checkCAA(ctx, ident, &caaParams{ - accountURIID: regid, - validationMethod: kind, - }) - if err != nil { - return records, err - } - - return records, nil +// validationLogEvent is a struct that contains the information needed to log +// the results of DoCAA and DoDCV. +type validationLogEvent struct { + AuthzID string + Requester int64 + Identifier string + Challenge core.Challenge + Error string `json:",omitempty"` + InternalError string `json:",omitempty"` + Latency float64 + Summary *mpicSummary `json:",omitempty"` } -// PerformValidation conducts a local Domain Control Validation (DCV) and CAA -// check for the specified challenge and dnsName. When invoked on the primary -// Validation Authority (VA) and the local validation succeeds, it also performs -// DCV and CAA checks using the configured remote VAs. Failed validations are -// indicated by a non-nil Problems in the returned ValidationResult. -// PerformValidation returns error only for internal logic errors (and the -// client may receive errors from gRPC in the event of a communication problem). -// ValidationResult always includes a list of ValidationRecords, even when it -// also contains Problems. This method does NOT implement Multi-Perspective -// Issuance Corroboration as defined in BRs Sections 3.2.2.9 and 5.4.1. -func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) { +// DoDCV conducts a local Domain Control Validation (DCV) for the specified +// challenge. When invoked on the primary Validation Authority (VA) and the +// local validation succeeds, it also performs DCV validations using the +// configured remote VAs. Failed validations are indicated by a non-nil Problems +// in the returned ValidationResult. DoDCV returns error only for internal logic +// errors (and the client may receive errors from gRPC in the event of a +// communication problem). ValidationResult always includes a list of +// ValidationRecords, even when it also contains Problems. This method +// implements the DCV portion of Multi-Perspective Issuance Corroboration as +// defined in BRs Sections 3.2.2.9 and 5.4.1. +func (va *ValidationAuthorityImpl) DoDCV(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) { if core.IsAnyNilOrZero(req, req.DnsName, req.Challenge, req.Authz, req.ExpectedKeyAuthorization) { return nil, berrors.InternalServerError("Incomplete validation request") } @@ -690,13 +673,14 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v return nil, berrors.MalformedError("challenge failed consistency check: %s", err) } - // Set up variables and a deferred closure to report validation latency - // metrics and log validation errors. Below here, do not use := to redeclare - // `prob`, or this will fail. + // Initialize variables and a deferred function to handle validation latency + // metrics, log validation errors, and log an MPIC summary. Avoid using := + // to redeclare `prob`, `localLatency`, or `summary` below this point. var prob *probs.ProblemDetails + var summary *mpicSummary var localLatency time.Duration start := va.clk.Now() - logEvent := verificationRequestEvent{ + logEvent := validationLogEvent{ AuthzID: req.Authz.Id, Requester: req.Authz.RegID, Identifier: req.DnsName, @@ -715,10 +699,11 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v outcome = pass } // Observe local validation latency (primary|remote). - va.observeLatency(opDCVAndCAA, va.perspective, string(chall.Type), probType, outcome, localLatency) + va.observeLatency(opDCV, va.perspective, string(chall.Type), probType, outcome, localLatency) if va.isPrimaryVA() { // Observe total validation latency (primary+remote). - va.observeLatency(opDCVAndCAA, allPerspectives, string(chall.Type), probType, outcome, va.clk.Since(start)) + va.observeLatency(opDCV, allPerspectives, string(chall.Type), probType, outcome, va.clk.Since(start)) + logEvent.Summary = summary } // Log the total validation latency. @@ -730,13 +715,13 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v // *before* checking whether it returned an error. These few checks are // carefully written to ensure that they work whether the local validation // was successful or not, and cannot themselves fail. - records, err := va.performLocalValidation( + records, err := va.validateChallenge( ctx, identifier.NewDNS(req.DnsName), - req.Authz.RegID, chall.Type, chall.Token, - req.ExpectedKeyAuthorization) + req.ExpectedKeyAuthorization, + ) // Stop the clock for local validation latency. localLatency = va.clk.Since(start) @@ -753,18 +738,20 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir) } - // Do remote validation. We do this after local validation is complete to - // avoid wasting work when validation will fail anyway. This only returns a - // singular problem, because the remote VAs have already audit-logged their - // own validation records, and it's not helpful to present multiple large - // errors to the end user. - op := func(ctx context.Context, remoteva RemoteVA, req proto.Message) (remoteResult, error) { - validationRequest, ok := req.(*vapb.PerformValidationRequest) - if !ok { - return nil, fmt.Errorf("got type %T, want *vapb.PerformValidationRequest", req) + if va.isPrimaryVA() { + // Do remote validation. We do this after local validation is complete + // to avoid wasting work when validation will fail anyway. This only + // returns a singular problem, because the remote VAs have already + // logged their own validationLogEvent, and it's not helpful to present + // multiple large errors to the end user. + op := func(ctx context.Context, remoteva RemoteVA, req proto.Message) (remoteResult, error) { + validationRequest, ok := req.(*vapb.PerformValidationRequest) + if !ok { + return nil, fmt.Errorf("got type %T, want *vapb.PerformValidationRequest", req) + } + return remoteva.DoDCV(ctx, validationRequest) } - return remoteva.PerformValidation(ctx, validationRequest) + summary, prob = va.doRemoteOperation(ctx, op, req) } - prob = va.performRemoteOperation(ctx, op, req) return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir) } diff --git a/va/va_test.go b/va/va_test.go index a15eb044206..d94e394de48 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -21,7 +21,6 @@ import ( "github.com/jmhodges/clock" "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc" - "google.golang.org/protobuf/proto" "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/core" @@ -203,15 +202,7 @@ func setupRemotes(confs []remoteConf, srv *httptest.Server) []RemoteVA { if c.impl != (RemoteClients{}) { clients = c.impl } - var address string - if c.ua == "broken" { - address = "broken" - } - if c.ua == "hijacked" { - address = "hijacked" - } remoteVAs = append(remoteVAs, RemoteVA{ - Address: address, RemoteClients: clients, Perspective: perspective, RIR: c.rir, @@ -268,18 +259,10 @@ func httpMultiSrv(t *testing.T, token string, allowedUAs map[string]bool) *multi // PerformValidation calls type cancelledVA struct{} -func (v cancelledVA) PerformValidation(_ context.Context, _ *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { - return nil, context.Canceled -} - func (v cancelledVA) DoDCV(_ context.Context, _ *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { return nil, context.Canceled } -func (v cancelledVA) IsCAAValid(_ context.Context, _ *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { - return nil, context.Canceled -} - func (v cancelledVA) DoCAA(_ context.Context, _ *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { return nil, context.Canceled } @@ -292,20 +275,11 @@ type brokenRemoteVA struct{} // PerformValidation and IsSafeDomain functions. var errBrokenRemoteVA = errors.New("brokenRemoteVA is broken") -// PerformValidation returns errBrokenRemoteVA unconditionally -func (b brokenRemoteVA) PerformValidation(_ context.Context, _ *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { - return nil, errBrokenRemoteVA -} - // DoDCV returns errBrokenRemoteVA unconditionally func (b brokenRemoteVA) DoDCV(_ context.Context, _ *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { return nil, errBrokenRemoteVA } -func (b brokenRemoteVA) IsCAAValid(_ context.Context, _ *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { - return nil, errBrokenRemoteVA -} - func (b brokenRemoteVA) DoCAA(_ context.Context, _ *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { return nil, errBrokenRemoteVA } @@ -318,18 +292,10 @@ type inMemVA struct { rva *ValidationAuthorityImpl } -func (inmem *inMemVA) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { - return inmem.rva.PerformValidation(ctx, req) -} - func (inmem *inMemVA) DoDCV(ctx context.Context, req *vapb.PerformValidationRequest, _ ...grpc.CallOption) (*vapb.ValidationResult, error) { return inmem.rva.DoDCV(ctx, req) } -func (inmem *inMemVA) IsCAAValid(ctx context.Context, req *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { - return inmem.rva.IsCAAValid(ctx, req) -} - func (inmem *inMemVA) DoCAA(ctx context.Context, req *vapb.IsCAAValidRequest, _ ...grpc.CallOption) (*vapb.IsCAAValidResponse, error) { return inmem.rva.DoCAA(ctx, req) } @@ -360,16 +326,6 @@ func TestNewValidationAuthorityImplWithDuplicateRemotes(t *testing.T) { test.AssertContains(t, err.Error(), "duplicate remote VA perspective \"dadaist\"") } -type validationFuncRunner func(context.Context, *ValidationAuthorityImpl, *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) - -var runPerformValidation = func(ctx context.Context, va *ValidationAuthorityImpl, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) { - return va.PerformValidation(ctx, req) -} - -var runDoDCV = func(ctx context.Context, va *ValidationAuthorityImpl, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) { - return va.DoDCV(ctx, req) -} - func TestPerformValidationWithMismatchedRemoteVAPerspectives(t *testing.T) { t.Parallel() @@ -386,30 +342,11 @@ func TestPerformValidationWithMismatchedRemoteVAPerspectives(t *testing.T) { remoteVAs := setupRemotes([]remoteConf{{rir: ripe}}, nil) remoteVAs = append(remoteVAs, mismatched1, mismatched2) - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - va, mockLog := setup(nil, "", remoteVAs, nil) - req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) - res, _ := tc.validationFunc(context.Background(), va, req) - test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") - test.AssertEquals(t, len(mockLog.GetAllMatching("Expected perspective")), 2) - }) - } + va, mockLog := setup(nil, "", remoteVAs, nil) + req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) + res, _ := va.DoDCV(context.Background(), req) + test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") + test.AssertEquals(t, len(mockLog.GetAllMatching("Expected perspective")), 2) } func TestPerformValidationWithMismatchedRemoteVARIRs(t *testing.T) { @@ -428,31 +365,11 @@ func TestPerformValidationWithMismatchedRemoteVARIRs(t *testing.T) { remoteVAs := setupRemotes([]remoteConf{{rir: ripe}}, nil) remoteVAs = append(remoteVAs, mismatched1, mismatched2) - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - va, mockLog := setup(nil, "", remoteVAs, nil) - req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) - res, _ := tc.validationFunc(context.Background(), va, req) - test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") - test.AssertEquals(t, len(mockLog.GetAllMatching("Expected perspective")), 2) - }) - } + va, mockLog := setup(nil, "", remoteVAs, nil) + req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) + res, _ := va.DoDCV(context.Background(), req) + test.AssertNotNil(t, res.GetProblem(), "validation succeeded with mismatched remote VA perspectives") + test.AssertEquals(t, len(mockLog.GetAllMatching("Expected perspective")), 2) } func TestValidateMalformedChallenge(t *testing.T) { @@ -468,135 +385,55 @@ func TestPerformValidationInvalid(t *testing.T) { t.Parallel() va, _ := setup(nil, "", nil, nil) - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - req := createValidationRequest("foo.com", core.ChallengeTypeDNS01) - res, _ := tc.validationFunc(context.Background(), va, req) - test.Assert(t, res.Problem != nil, "validation succeeded") - if tc.validationFuncName == "PerformValidation" { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCVAndCAA, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": string(probs.UnauthorizedProblem), - "result": fail, - }, 1) - } else { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCV, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": string(probs.UnauthorizedProblem), - "result": fail, - }, 1) - } - }) - } + req := createValidationRequest("foo.com", core.ChallengeTypeDNS01) + res, _ := va.DoDCV(context.Background(), req) + test.Assert(t, res.Problem != nil, "validation succeeded") + test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ + "operation": opDCV, + "perspective": va.perspective, + "challenge_type": string(core.ChallengeTypeDNS01), + "problem_type": string(probs.UnauthorizedProblem), + "result": fail, + }, 1) } func TestInternalErrorLogged(t *testing.T) { t.Parallel() - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - va, mockLog := setup(nil, "", nil, nil) + va, mockLog := setup(nil, "", nil, nil) - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) - defer cancel() - req := createValidationRequest("nonexistent.com", core.ChallengeTypeHTTP01) - _, err := tc.validationFunc(ctx, va, req) - test.AssertNotError(t, err, "failed validation should not be an error") - matchingLogs := mockLog.GetAllMatching( - `Validation result JSON=.*"InternalError":"127.0.0.1: Get.*nonexistent.com/\.well-known.*: context deadline exceeded`) - test.AssertEquals(t, len(matchingLogs), 1) - }) - } + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) + defer cancel() + req := createValidationRequest("nonexistent.com", core.ChallengeTypeHTTP01) + _, err := va.DoDCV(ctx, req) + test.AssertNotError(t, err, "failed validation should not be an error") + matchingLogs := mockLog.GetAllMatching( + `Validation result JSON=.*"InternalError":"127.0.0.1: Get.*nonexistent.com/\.well-known.*: context deadline exceeded`) + test.AssertEquals(t, len(matchingLogs), 1) } func TestPerformValidationValid(t *testing.T) { t.Parallel() - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() + va, mockLog := setup(nil, "", nil, nil) - va, mockLog := setup(nil, "", nil, nil) - - // create a challenge with well known token - req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) - res, _ := tc.validationFunc(context.Background(), va, req) - test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed: %#v", res.Problem)) - if tc.validationFuncName == "PerformValidation" { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCVAndCAA, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": "", - "result": pass, - }, 1) - } else { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCV, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": "", - "result": pass, - }, 1) - } - resultLog := mockLog.GetAllMatching(`Validation result`) - if len(resultLog) != 1 { - t.Fatalf("Wrong number of matching lines for 'Validation result'") - } - if !strings.Contains(resultLog[0], `"Identifier":"good-dns01.com"`) { - t.Error("PerformValidation didn't log validation identifier.") - } - }) + // create a challenge with well known token + req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) + res, _ := va.DoDCV(context.Background(), req) + test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed: %#v", res.Problem)) + test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ + "operation": opDCV, + "perspective": va.perspective, + "challenge_type": string(core.ChallengeTypeDNS01), + "problem_type": "", + "result": pass, + }, 1) + resultLog := mockLog.GetAllMatching(`Validation result`) + if len(resultLog) != 1 { + t.Fatalf("Wrong number of matching lines for 'Validation result'") + } + if !strings.Contains(resultLog[0], `"Identifier":"good-dns01.com"`) { + t.Error("PerformValidation didn't log validation identifier.") } } @@ -605,149 +442,33 @@ func TestPerformValidationValid(t *testing.T) { func TestPerformValidationWildcard(t *testing.T) { t.Parallel() - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - va, mockLog := setup(nil, "", nil, nil) - - // create a challenge with well known token - req := createValidationRequest("*.good-dns01.com", core.ChallengeTypeDNS01) - // perform a validation for a wildcard name - res, _ := tc.validationFunc(context.Background(), va, req) - test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed: %#v", res.Problem)) - if tc.validationFuncName == "PerformValidation" { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCVAndCAA, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": "", - "result": pass, - }, 1) - } else { - test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ - "operation": opDCV, - "perspective": va.perspective, - "challenge_type": string(core.ChallengeTypeDNS01), - "problem_type": "", - "result": pass, - }, 1) - } - resultLog := mockLog.GetAllMatching(`Validation result`) - if len(resultLog) != 1 { - t.Fatalf("Wrong number of matching lines for 'Validation result'") - } - - // We expect that the top level Identifier reflect the wildcard name - if !strings.Contains(resultLog[0], `"Identifier":"*.good-dns01.com"`) { - t.Errorf("PerformValidation didn't log correct validation identifier.") - } - // We expect that the ValidationRecord contain the correct non-wildcard - // hostname that was validated - if !strings.Contains(resultLog[0], `"hostname":"good-dns01.com"`) { - t.Errorf("PerformValidation didn't log correct validation record hostname.") - } - }) - } -} - -func TestDCVAndCAASequencing(t *testing.T) { va, mockLog := setup(nil, "", nil, nil) - // When validation succeeds, CAA should be checked. - mockLog.Clear() - req := createValidationRequest("good-dns01.com", core.ChallengeTypeDNS01) - res, err := va.PerformValidation(context.Background(), req) - test.AssertNotError(t, err, "performing validation") + // create a challenge with well known token + req := createValidationRequest("*.good-dns01.com", core.ChallengeTypeDNS01) + // perform a validation for a wildcard name + res, _ := va.DoDCV(context.Background(), req) test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed: %#v", res.Problem)) - caaLog := mockLog.GetAllMatching(`Checked CAA records for`) - test.AssertEquals(t, len(caaLog), 1) - - // When validation fails, CAA should be skipped. - mockLog.Clear() - req = createValidationRequest("bad-dns01.com", core.ChallengeTypeDNS01) - res, err = va.PerformValidation(context.Background(), req) - test.AssertNotError(t, err, "performing validation") - test.Assert(t, res.Problem != nil, "validation succeeded") - caaLog = mockLog.GetAllMatching(`Checked CAA records for`) - test.AssertEquals(t, len(caaLog), 0) -} - -func TestPerformRemoteOperation(t *testing.T) { - va, _ := setupWithRemotes(nil, "", []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, - }, nil) - - testCases := []struct { - name string - req proto.Message - expectedType string - expectedDetail string - op func(ctx context.Context, rva RemoteVA, req proto.Message) (remoteResult, error) - }{ - { - name: "ValidationResult", - req: &vapb.PerformValidationRequest{}, - expectedType: string(probs.BadNonceProblem), - expectedDetail: "quite surprising", - op: func(ctx context.Context, rva RemoteVA, req proto.Message) (remoteResult, error) { - prob := &corepb.ProblemDetails{ - ProblemType: string(probs.BadNonceProblem), - Detail: "quite surprising", - } - return &vapb.ValidationResult{Problem: prob, Perspective: rva.Perspective, Rir: rva.RIR}, nil - }, - }, - { - name: "IsCAAValidResponse", - req: &vapb.IsCAAValidRequest{}, - expectedType: string(probs.PausedProblem), - expectedDetail: "quite surprising, indeed", - op: func(ctx context.Context, rva RemoteVA, req proto.Message) (remoteResult, error) { - prob := &corepb.ProblemDetails{ - ProblemType: string(probs.PausedProblem), - Detail: "quite surprising, indeed", - } - return &vapb.IsCAAValidResponse{Problem: prob, Perspective: rva.Perspective, Rir: rva.RIR}, nil - }, - }, - { - name: "IsCAAValidRequestWithValidationResult", - req: &vapb.IsCAAValidRequest{}, - expectedType: string(probs.BadPublicKeyProblem), - expectedDetail: "a shocking result", - op: func(ctx context.Context, rva RemoteVA, req proto.Message) (remoteResult, error) { - prob := &corepb.ProblemDetails{ - ProblemType: string(probs.BadPublicKeyProblem), - Detail: "a shocking result", - } - return &vapb.ValidationResult{Problem: prob, Perspective: rva.Perspective, Rir: rva.RIR}, nil - }, - }, + test.AssertMetricWithLabelsEquals(t, va.metrics.validationLatency, prometheus.Labels{ + "operation": opDCV, + "perspective": va.perspective, + "challenge_type": string(core.ChallengeTypeDNS01), + "problem_type": "", + "result": pass, + }, 1) + resultLog := mockLog.GetAllMatching(`Validation result`) + if len(resultLog) != 1 { + t.Fatalf("Wrong number of matching lines for 'Validation result'") } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - prob := va.performRemoteOperation(context.Background(), tc.op, tc.req) - test.AssertEquals(t, string(prob.Type), tc.expectedType) - test.AssertContains(t, prob.Detail, tc.expectedDetail) - }) + // We expect that the top level Identifier reflect the wildcard name + if !strings.Contains(resultLog[0], `"Identifier":"*.good-dns01.com"`) { + t.Errorf("PerformValidation didn't log correct validation identifier.") + } + // We expect that the ValidationRecord contain the correct non-wildcard + // hostname that was validated + if !strings.Contains(resultLog[0], `"hostname":"good-dns01.com"`) { + t.Errorf("PerformValidation didn't log correct validation record hostname.") } } @@ -766,22 +487,6 @@ func TestMultiVA(t *testing.T) { CAAClient: cancelledVA{}, } - type testFunc struct { - name string - impl validationFuncRunner - } - - testFuncs := []testFunc{ - { - name: "PerformValidation", - impl: runPerformValidation, - }, - { - name: "DoDCV", - impl: runDoDCV, - }, - } - testCases := []struct { Name string Remotes []remoteConf @@ -938,62 +643,46 @@ func TestMultiVA(t *testing.T) { } for _, tc := range testCases { - for _, testFunc := range testFuncs { - t.Run(tc.Name+"_"+testFunc.name, func(t *testing.T) { - t.Parallel() - - // Configure one test server per test case so that all tests can run in parallel. - ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) - defer ms.Close() - - // Configure a primary VA with testcase remote VAs. - localVA, mockLog := setupWithRemotes(ms.Server, tc.PrimaryUA, tc.Remotes, nil) - - // Perform all validations - res, _ := testFunc.impl(ctx, localVA, req) - if res.Problem == nil && tc.ExpectedProbType != "" { - t.Errorf("expected prob %v, got nil", tc.ExpectedProbType) - } else if res.Problem != nil && tc.ExpectedProbType == "" { - t.Errorf("expected no prob, got %v", res.Problem) - } else if res.Problem != nil && tc.ExpectedProbType != "" { - // That result should match expected. - test.AssertEquals(t, res.Problem.ProblemType, tc.ExpectedProbType) - } + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() - if tc.ExpectedLogContains != "" { - lines := mockLog.GetAllMatching(tc.ExpectedLogContains) - if len(lines) == 0 { - t.Fatalf("Got log %v; expected %q", mockLog.GetAll(), tc.ExpectedLogContains) - } + // Configure one test server per test case so that all tests can run in parallel. + ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) + defer ms.Close() + + // Configure a primary VA with testcase remote VAs. + localVA, mockLog := setupWithRemotes(ms.Server, tc.PrimaryUA, tc.Remotes, nil) + + // Perform all validations + res, _ := localVA.DoDCV(ctx, req) + if res.Problem == nil && tc.ExpectedProbType != "" { + t.Errorf("expected prob %v, got nil", tc.ExpectedProbType) + } else if res.Problem != nil && tc.ExpectedProbType == "" { + t.Errorf("expected no prob, got %v", res.Problem) + } else if res.Problem != nil && tc.ExpectedProbType != "" { + // That result should match expected. + test.AssertEquals(t, res.Problem.ProblemType, tc.ExpectedProbType) + } + + if tc.ExpectedLogContains != "" { + lines := mockLog.GetAllMatching(tc.ExpectedLogContains) + if len(lines) == 0 { + t.Fatalf("Got log %v; expected %q", mockLog.GetAll(), tc.ExpectedLogContains) } - }) - } + } + }) } } func TestMultiVAEarlyReturn(t *testing.T) { t.Parallel() - type testFunc struct { - name string - impl validationFuncRunner - } - - testFuncs := []testFunc{ - { - name: "PerformValidation", - impl: runPerformValidation, - }, - { - name: "DoDCV", - impl: runDoDCV, - }, - } - testCases := []struct { + name string remoteConfs []remoteConf }{ { + name: "One slow, one pass, one fail", remoteConfs: []remoteConf{ {ua: slowUA, rir: arin}, {ua: pass, rir: ripe}, @@ -1001,6 +690,7 @@ func TestMultiVAEarlyReturn(t *testing.T) { }, }, { + name: "Two slow, two pass, one fail", remoteConfs: []remoteConf{ {ua: slowUA, rir: arin}, {ua: slowUA, rir: ripe}, @@ -1010,6 +700,7 @@ func TestMultiVAEarlyReturn(t *testing.T) { }, }, { + name: "Two slow, two pass, two fail", remoteConfs: []remoteConf{ {ua: slowUA, rir: arin}, {ua: slowUA, rir: ripe}, @@ -1022,39 +713,37 @@ func TestMultiVAEarlyReturn(t *testing.T) { } for i, tc := range testCases { - for _, testFunc := range testFuncs { - t.Run(fmt.Sprintf("case %d"+"_"+testFunc.name, i), func(t *testing.T) { - t.Parallel() + t.Run(fmt.Sprintf("TestCase%d", i), func(t *testing.T) { + t.Parallel() - // Configure one test server per test case so that all tests can run in parallel. - ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) - defer ms.Close() + // Configure one test server per test case so that all tests can run in parallel. + ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) + defer ms.Close() - localVA, _ := setupWithRemotes(ms.Server, pass, tc.remoteConfs, nil) + localVA, _ := setupWithRemotes(ms.Server, pass, tc.remoteConfs, nil) - // Perform all validations - start := time.Now() - req := createValidationRequest("localhost", core.ChallengeTypeHTTP01) - res, _ := testFunc.impl(ctx, localVA, req) + // Perform all validations + start := time.Now() + req := createValidationRequest("localhost", core.ChallengeTypeHTTP01) + res, _ := localVA.DoDCV(ctx, req) - // It should always fail - if res.Problem == nil { - t.Error("expected prob from PerformValidation, got nil") - } + // It should always fail + if res.Problem == 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) + } + }) } } @@ -1067,39 +756,20 @@ func TestMultiVAPolicy(t *testing.T) { {ua: fail, rir: apnic}, } - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) - defer ms.Close() + ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) + defer ms.Close() - // Create a local test VA with the remote VAs - localVA, _ := setupWithRemotes(ms.Server, pass, remoteConfs, nil) + // Create a local test VA with the remote VAs + localVA, _ := setupWithRemotes(ms.Server, pass, remoteConfs, nil) - // Perform validation for a domain not in the disabledDomains list - req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01) - res, _ := tc.validationFunc(ctx, localVA, req) - // It should fail - if res.Problem == nil { - t.Error("expected prob from PerformValidation, got nil") - } - }) + // Perform validation for a domain not in the disabledDomains list + req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01) + res, _ := localVA.DoDCV(ctx, req) + // It should fail + if res.Problem == nil { + t.Error("expected prob from PerformValidation, got nil") } + } func TestMultiVALogging(t *testing.T) { @@ -1111,34 +781,14 @@ func TestMultiVALogging(t *testing.T) { {ua: pass, rir: apnic}, } - testCases := []struct { - validationFuncName string - validationFunc validationFuncRunner - }{ - { - validationFuncName: "PerformValidation", - validationFunc: runPerformValidation, - }, - { - validationFuncName: "DoDCV", - validationFunc: runDoDCV, - }, - } - - for _, tc := range testCases { - t.Run(tc.validationFuncName, func(t *testing.T) { - t.Parallel() - - ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) - defer ms.Close() + ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) + defer ms.Close() - va, _ := setupWithRemotes(ms.Server, pass, remoteConfs, nil) - req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01) - res, err := tc.validationFunc(ctx, va, req) - test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed with: %#v", res.Problem)) - test.AssertNotError(t, err, "performing validation") - }) - } + va, _ := setupWithRemotes(ms.Server, pass, remoteConfs, nil) + req := createValidationRequest("letsencrypt.org", core.ChallengeTypeHTTP01) + res, err := va.DoDCV(ctx, req) + test.Assert(t, res.Problem == nil, fmt.Sprintf("validation failed with: %#v", res.Problem)) + test.AssertNotError(t, err, "performing validation") } func TestDetailedError(t *testing.T) { @@ -1192,66 +842,3 @@ func TestDetailedError(t *testing.T) { } } } - -func TestLogRemoteDifferentials(t *testing.T) { - t.Parallel() - - remoteConfs := []remoteConf{ - {ua: pass, rir: arin}, - {ua: pass, rir: ripe}, - {ua: pass, rir: apnic}, - } - - testCases := []struct { - name string - req *vapb.IsCAAValidRequest - passed int - failed int - expectedLog string - }{ - { - name: "all results equal (nil)", - passed: 0, - failed: 3, - expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","AccountID":1999,"ChallengeType":"blorpus-01","RemoteSuccesses":0,"RemoteFailures":3}`, - }, - { - name: "all results equal (not nil)", - passed: 0, - failed: 3, - expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","AccountID":1999,"ChallengeType":"blorpus-01","RemoteSuccesses":0,"RemoteFailures":3}`, - }, - { - name: "differing results, some non-nil", - passed: 2, - failed: 1, - expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","AccountID":1999,"ChallengeType":"blorpus-01","RemoteSuccesses":2,"RemoteFailures":1}`, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - // Configure one test server per test case so that all tests can run in parallel. - ms := httpMultiSrv(t, expectedToken, map[string]bool{pass: true, fail: false}) - defer ms.Close() - - localVA, mockLog := setupWithRemotes(ms.Server, pass, remoteConfs, nil) - - localVA.logRemoteResults(&vapb.IsCAAValidRequest{ - Domain: "example.com", - AccountURIID: 1999, - ValidationMethod: "blorpus-01", - }, tc.passed, tc.failed) - - lines := mockLog.GetAllMatching("remoteVADifferentials JSON=.*") - if tc.expectedLog != "" { - test.AssertEquals(t, len(lines), 1) - test.AssertEquals(t, lines[0], tc.expectedLog) - } else { - test.AssertEquals(t, len(lines), 0) - } - }) - } -} diff --git a/va/vampic.go b/va/vampic.go deleted file mode 100644 index 28b6036f7d8..00000000000 --- a/va/vampic.go +++ /dev/null @@ -1,428 +0,0 @@ -package va - -import ( - "context" - "errors" - "fmt" - "maps" - "math/rand/v2" - "slices" - "time" - - "github.com/letsencrypt/boulder/core" - corepb "github.com/letsencrypt/boulder/core/proto" - berrors "github.com/letsencrypt/boulder/errors" - bgrpc "github.com/letsencrypt/boulder/grpc" - "github.com/letsencrypt/boulder/identifier" - "github.com/letsencrypt/boulder/probs" - vapb "github.com/letsencrypt/boulder/va/proto" - "google.golang.org/protobuf/proto" -) - -const ( - // requiredRIRs is the minimum number of distinct Regional Internet - // Registries required for MPIC-compliant validation. Per BRs Section - // 3.2.2.9, starting March 15, 2026, the required number is 2. - requiredRIRs = 2 -) - -// mpicSummary is returned by doRemoteOperation and contains a summary of the -// validation results for logging purposes. To ensure that the JSON output does -// not contain nil slices, and to ensure deterministic output use the -// summarizeMPIC function to prepare an mpicSummary. -type mpicSummary struct { - // Passed are the perspectives that passed validation. - Passed []string `json:"passedPerspectives"` - - // Failed are the perspectives that failed validation. - Failed []string `json:"failedPerspectives"` - - // PassedRIRs are the Regional Internet Registries that the passing - // perspectives reside in. - PassedRIRs []string `json:"passedRIRs"` - - // QuorumResult is the Multi-Perspective Issuance Corroboration quorum - // result, per BRs Section 5.4.1, Requirement 2.7 (i.e., "3/4" which should - // be interpreted as "Three (3) out of four (4) attempted Network - // Perspectives corroborated the determinations made by the Primary Network - // Perspective". - QuorumResult string `json:"quorumResult"` -} - -// summarizeMPIC prepares an *mpicSummary for logging, ensuring there are no nil -// slices and output is deterministic. -func summarizeMPIC(passed, failed []string, passedRIRSet map[string]struct{}) *mpicSummary { - if passed == nil { - passed = []string{} - } - slices.Sort(passed) - if failed == nil { - failed = []string{} - } - slices.Sort(failed) - - passedRIRs := []string{} - if passedRIRSet != nil { - for rir := range maps.Keys(passedRIRSet) { - passedRIRs = append(passedRIRs, rir) - } - } - slices.Sort(passedRIRs) - - return &mpicSummary{ - Passed: passed, - Failed: failed, - PassedRIRs: passedRIRs, - QuorumResult: fmt.Sprintf("%d/%d", len(passed), len(passed)+len(failed)), - } -} - -// doRemoteOperation concurrently calls the provided operation with `req` and a -// RemoteVA once for each configured RemoteVA. It cancels remaining operations -// and returns early if either the required number of successful results is -// obtained or the number of failures exceeds va.maxRemoteFailures. -// -// Internal logic errors are logged. If the number of operation failures exceeds -// va.maxRemoteFailures, the first encountered problem is returned as a -// *probs.ProblemDetails. -func (va *ValidationAuthorityImpl) doRemoteOperation(ctx context.Context, op remoteOperation, req proto.Message) (*mpicSummary, *probs.ProblemDetails) { - remoteVACount := len(va.remoteVAs) - // - Mar 15, 2026: MUST implement using at least 3 perspectives - // - Jun 15, 2026: MUST implement using at least 4 perspectives - // - Dec 15, 2026: MUST implement using at least 5 perspectives - // See "Phased Implementation Timeline" in - // https://github.com/cabforum/servercert/blob/main/docs/BR.md#3229-multi-perspective-issuance-corroboration - if remoteVACount < 3 { - return nil, probs.ServerInternal("Insufficient remote perspectives: need at least 3") - } - - type response struct { - addr string - perspective string - rir string - result remoteResult - err error - } - - subCtx, cancel := context.WithCancel(ctx) - defer cancel() - - responses := make(chan *response, remoteVACount) - for _, i := range rand.Perm(remoteVACount) { - go func(rva RemoteVA) { - res, err := op(subCtx, rva, req) - if err != nil { - responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} - return - } - if res.GetPerspective() != rva.Perspective || res.GetRir() != rva.RIR { - err = fmt.Errorf( - "Expected perspective %q (%q) but got reply from %q (%q) - misconfiguration likely", rva.Perspective, rva.RIR, res.GetPerspective(), res.GetRir(), - ) - responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} - return - } - responses <- &response{rva.Address, rva.Perspective, rva.RIR, res, err} - }(va.remoteVAs[i]) - } - - required := remoteVACount - va.maxRemoteFailures - var passed []string - var failed []string - var passedRIRs = map[string]struct{}{} - var firstProb *probs.ProblemDetails - - for resp := range responses { - var currProb *probs.ProblemDetails - - if resp.err != nil { - // Failed to communicate with the remote VA. - failed = append(failed, resp.perspective) - - if core.IsCanceled(resp.err) { - currProb = probs.ServerInternal("Secondary validation RPC canceled") - } else { - va.log.Errf("Operation on remote VA (%s) failed: %s", resp.addr, resp.err) - currProb = probs.ServerInternal("Secondary validation RPC failed") - } - } else if resp.result.GetProblem() != nil { - // The remote VA returned a problem. - failed = append(failed, resp.perspective) - - var err error - currProb, err = bgrpc.PBToProblemDetails(resp.result.GetProblem()) - if err != nil { - va.log.Errf("Operation on Remote VA (%s) returned malformed problem: %s", resp.addr, err) - currProb = probs.ServerInternal("Secondary validation RPC returned malformed result") - } - } else { - // The remote VA returned a successful result. - passed = append(passed, resp.perspective) - passedRIRs[resp.rir] = struct{}{} - } - - if firstProb == nil && currProb != nil { - // A problem was encountered for the first time. - firstProb = currProb - } - - // To respond faster, if we get enough successes or too many failures, we cancel remaining RPCs. - // Finish the loop to collect remaining responses into `failed` so we can rely on having a response - // for every request we made. - if len(passed) >= required && len(passedRIRs) >= requiredRIRs { - cancel() - } - if len(failed) > va.maxRemoteFailures { - cancel() - } - - // Once all the VAs have returned a result, break the loop. - if len(passed)+len(failed) >= remoteVACount { - break - } - } - if len(passed) >= required && len(passedRIRs) >= requiredRIRs { - return summarizeMPIC(passed, failed, passedRIRs), nil - } - if firstProb == nil { - // This should never happen. If we didn't meet the thresholds above we - // should have seen at least one error. - return summarizeMPIC(passed, failed, passedRIRs), probs.ServerInternal( - "During secondary validation: validation failed but the problem is unavailable") - } - firstProb.Detail = fmt.Sprintf("During secondary validation: %s", firstProb.Detail) - return summarizeMPIC(passed, failed, passedRIRs), firstProb -} - -// validationLogEvent is a struct that contains the information needed to log -// the results of DoCAA and DoDCV. -type validationLogEvent struct { - AuthzID string - Requester int64 - Identifier string - Challenge core.Challenge - Error string `json:",omitempty"` - InternalError string `json:",omitempty"` - Latency float64 - Summary *mpicSummary `json:",omitempty"` -} - -// DoDCV conducts a local Domain Control Validation (DCV) for the specified -// challenge. When invoked on the primary Validation Authority (VA) and the -// local validation succeeds, it also performs DCV validations using the -// configured remote VAs. Failed validations are indicated by a non-nil Problems -// in the returned ValidationResult. DoDCV returns error only for internal logic -// errors (and the client may receive errors from gRPC in the event of a -// communication problem). ValidationResult always includes a list of -// ValidationRecords, even when it also contains Problems. This method -// implements the DCV portion of Multi-Perspective Issuance Corroboration as -// defined in BRs Sections 3.2.2.9 and 5.4.1. -func (va *ValidationAuthorityImpl) DoDCV(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) { - if core.IsAnyNilOrZero(req, req.DnsName, req.Challenge, req.Authz, req.ExpectedKeyAuthorization) { - return nil, berrors.InternalServerError("Incomplete validation request") - } - - chall, err := bgrpc.PBToChallenge(req.Challenge) - if err != nil { - return nil, errors.New("challenge failed to deserialize") - } - - err = chall.CheckPending() - if err != nil { - return nil, berrors.MalformedError("challenge failed consistency check: %s", err) - } - - // Initialize variables and a deferred function to handle validation latency - // metrics, log validation errors, and log an MPIC summary. Avoid using := - // to redeclare `prob`, `localLatency`, or `summary` below this point. - var prob *probs.ProblemDetails - var summary *mpicSummary - var localLatency time.Duration - start := va.clk.Now() - logEvent := validationLogEvent{ - AuthzID: req.Authz.Id, - Requester: req.Authz.RegID, - Identifier: req.DnsName, - Challenge: chall, - } - defer func() { - probType := "" - outcome := fail - if prob != nil { - probType = string(prob.Type) - logEvent.Error = prob.Error() - logEvent.Challenge.Error = prob - logEvent.Challenge.Status = core.StatusInvalid - } else { - logEvent.Challenge.Status = core.StatusValid - outcome = pass - } - // Observe local validation latency (primary|remote). - va.observeLatency(opDCV, va.perspective, string(chall.Type), probType, outcome, localLatency) - if va.isPrimaryVA() { - // Observe total validation latency (primary+remote). - va.observeLatency(opDCV, allPerspectives, string(chall.Type), probType, outcome, va.clk.Since(start)) - logEvent.Summary = summary - } - - // Log the total validation latency. - logEvent.Latency = va.clk.Since(start).Round(time.Millisecond).Seconds() - va.log.AuditObject("Validation result", logEvent) - }() - - // Do local validation. Note that we process the result in a couple ways - // *before* checking whether it returned an error. These few checks are - // carefully written to ensure that they work whether the local validation - // was successful or not, and cannot themselves fail. - records, err := va.validateChallenge( - ctx, - identifier.NewDNS(req.DnsName), - chall.Type, - chall.Token, - req.ExpectedKeyAuthorization, - ) - - // Stop the clock for local validation latency. - localLatency = va.clk.Since(start) - - // Check for malformed ValidationRecords - logEvent.Challenge.ValidationRecord = records - if err == nil && !logEvent.Challenge.RecordsSane() { - err = errors.New("records from local validation failed sanity check") - } - - if err != nil { - logEvent.InternalError = err.Error() - prob = detailedError(err) - return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir) - } - - if va.isPrimaryVA() { - // Do remote validation. We do this after local validation is complete - // to avoid wasting work when validation will fail anyway. This only - // returns a singular problem, because the remote VAs have already - // logged their own validationLogEvent, and it's not helpful to present - // multiple large errors to the end user. - op := func(ctx context.Context, remoteva RemoteVA, req proto.Message) (remoteResult, error) { - validationRequest, ok := req.(*vapb.PerformValidationRequest) - if !ok { - return nil, fmt.Errorf("got type %T, want *vapb.PerformValidationRequest", req) - } - return remoteva.DoDCV(ctx, validationRequest) - } - summary, prob = va.doRemoteOperation(ctx, op, req) - } - return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir) -} - -// DoCAA conducts a CAA check for the specified dnsName. When invoked on the -// primary Validation Authority (VA) and the local check succeeds, it also -// performs CAA checks using the configured remote VAs. Failed checks are -// indicated by a non-nil Problems in the returned ValidationResult. DoCAA -// returns error only for internal logic errors (and the client may receive -// errors from gRPC in the event of a communication problem). This method -// implements the CAA portion of Multi-Perspective Issuance Corroboration as -// defined in BRs Sections 3.2.2.9 and 5.4.1. -func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAValidRequest) (*vapb.IsCAAValidResponse, error) { - if core.IsAnyNilOrZero(req.Domain, req.ValidationMethod, req.AccountURIID) { - return nil, berrors.InternalServerError("incomplete IsCAAValid request") - } - logEvent := validationLogEvent{ - AuthzID: req.AuthzID, - Requester: req.AccountURIID, - Identifier: req.Domain, - } - - challType := core.AcmeChallenge(req.ValidationMethod) - if !challType.IsValid() { - return nil, berrors.InternalServerError("unrecognized validation method %q", req.ValidationMethod) - } - - acmeID := identifier.NewDNS(req.Domain) - params := &caaParams{ - accountURIID: req.AccountURIID, - validationMethod: challType, - } - - // Initialize variables and a deferred function to handle check latency - // metrics, log check errors, and log an MPIC summary. Avoid using := to - // redeclare `prob`, `localLatency`, or `summary` below this point. - var prob *probs.ProblemDetails - var summary *mpicSummary - var internalErr error - var localLatency time.Duration - start := va.clk.Now() - - defer func() { - probType := "" - outcome := fail - if prob != nil { - // CAA check failed. - probType = string(prob.Type) - logEvent.Error = prob.Error() - } else { - // CAA check passed. - outcome = pass - } - // Observe local check latency (primary|remote). - va.observeLatency(opCAA, va.perspective, string(challType), probType, outcome, localLatency) - if va.isPrimaryVA() { - // Observe total check latency (primary+remote). - va.observeLatency(opCAA, allPerspectives, string(challType), probType, outcome, va.clk.Since(start)) - logEvent.Summary = summary - } - // Log the total check latency. - logEvent.Latency = va.clk.Since(start).Round(time.Millisecond).Seconds() - - va.log.AuditObject("CAA check result", logEvent) - }() - - internalErr = va.checkCAA(ctx, acmeID, params) - - // Stop the clock for local check latency. - localLatency = va.clk.Since(start) - - if internalErr != nil { - logEvent.InternalError = internalErr.Error() - prob = detailedError(internalErr) - prob.Detail = fmt.Sprintf("While processing CAA for %s: %s", req.Domain, prob.Detail) - } - - if va.isPrimaryVA() { - op := func(ctx context.Context, remoteva RemoteVA, req proto.Message) (remoteResult, error) { - checkRequest, ok := req.(*vapb.IsCAAValidRequest) - if !ok { - return nil, fmt.Errorf("got type %T, want *vapb.IsCAAValidRequest", req) - } - return remoteva.DoCAA(ctx, checkRequest) - } - var remoteProb *probs.ProblemDetails - summary, remoteProb = va.doRemoteOperation(ctx, op, req) - // If the remote result was a non-nil problem then fail the CAA check - if remoteProb != nil { - prob = remoteProb - va.log.Infof("CAA check failed due to remote failures: identifier=%v err=%s", - req.Domain, remoteProb) - } - } - - if prob != nil { - // The ProblemDetails will be serialized through gRPC, which requires UTF-8. - // It will also later be serialized in JSON, which defaults to UTF-8. Make - // sure it is UTF-8 clean now. - prob = filterProblemDetails(prob) - return &vapb.IsCAAValidResponse{ - Problem: &corepb.ProblemDetails{ - ProblemType: string(prob.Type), - Detail: replaceInvalidUTF8([]byte(prob.Detail)), - }, - Perspective: va.perspective, - Rir: va.rir, - }, nil - } else { - return &vapb.IsCAAValidResponse{ - Perspective: va.perspective, - Rir: va.rir, - }, nil - } -}