RVA: Recheck CAA records #7221
RVA: Recheck CAA records #7221
Conversation
There was a problem hiding this comment.
I've only reviewed the production code so far, haven't taken a look at tests yet, but figured I should send comments so you'd have them quickly anyway.
High level comment: it's slightly surprising for the two processRemote...Results functions to call through to a shared helper with some metadata for log/error messages, but for the two performRemote... functions (which are similarly identical) to be wholly independent. I don't have a strong opinion on which (or leaving things as-is!) is better -- on the one hand, sharing code is generally good; on the other hand, the metadata construct is a little awkward; on the third hand, simply the fact that the two performRemote... functions call a different underlying gRPC method may be enough argument that they should remain separate. I just want to make sure we think about it.
| }).Observe(recheckLatency.Seconds()) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Question for the future (not this PR): do we want IsCAAValid to be producing and audit logging a structured logEvent, just like PerformValidation does? I think we will want that (or something similar) eventually, given the MPIC ballot's logging requirements.
There was a problem hiding this comment.
This is a great idea. We could edit the caa-log-checker to handle this case too in a followup PR.
There was a problem hiding this comment.
The authz ID is not currently passed to IsCAAValidRequest unlike PerformValidationRequest ([1] and [2]) which allows logging enough data to determine which sufficiently old authz triggered the CAA check. I think we do want that, but it would need to be done in a separate PR, get deployed to staging/prod, then accessed here once the protobuf change has been successfully rolled out.
There was a problem hiding this comment.
From sprint planning, I'll make the protobuf change as a followup PR.
|
@pgporada, this PR appears to contain configuration changes. Please ensure that a corresponding deployment ticket has been filed with the new configuration values. |
Co-authored-by: Samantha <hello@entropy.cat>
Previously,
va.IsCAAValidwould only check CAA records from the primary VA during initial domain control validation, completely ignoring any configured RVAs. The upcoming MPIC ballot will require that it be done from multiple perspectives. With the currently deployed Multi-Perspective Validation in staging and production, this change brings us in line with the proposed phase 3. This change reuses the existing MaxRemoteValidationFailures variable for the required non-corroboration quorum.Adds new VA feature flags:
EnforceMultiCAAinstructs a primary VA to command each of its configured RVAs to perform a CAA recheck.MultiCAAFullResultscauses the primary VA to block waiting for all RVA CAA recheck results to arrive.Renamed
va.logRemoteValidationDifferentialstova.logRemoteDifferentialsbecause it can handle initial domain control validations and CAA rechecking with minimal editing.Part of #7061