Report canceled remote validations as problems (#5011)

Previously, canceled remote validations were simply noted and then
dropped on the floor. This should be safe, as they're theoretically
only canceled when the parent span (i.e. the local PerformValidation
RPC) ends. But for the sake of defense-in-depth, it seems better to
correctly mark canceled remote validations as having Problems, so
that their results cannot be accidentally used anywhere.

This results in a test behavior change: if EnforceMultiVA is on, and
some RPCs are canceled, this now results in validation failure. This
should not have any production impact, because remote validations
should only be canceled when the parent RPC early-exits, but that only
happens when EnforceMultiVA is not enabled. These tests now test a
case where the other remote validations were canceled for some other
reason, which should result in validation failure.
This commit is contained in:
Aaron Gable 2020-08-11 09:29:49 -07:00 committed by GitHub
parent dcb42cbe66
commit 8920b698ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 11 deletions

View File

@ -388,8 +388,9 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
if err != nil && canceled.Is(err) { if err != nil && canceled.Is(err) {
// If the non-nil err was a canceled error, ignore it. That's fine: it // If the non-nil err was a canceled error, ignore it. That's fine: it
// just means we cancelled the remote VA request before it was // just means we cancelled the remote VA request before it was
// finished because we didn't care about its result. // finished because we didn't care about its result. Don't log to avoid
va.log.Infof("Remote VA %q.PerformValidation canceled: %s", rva.Address, err) // spamming the logs.
result.Problem = probs.ServerInternal("Remote PerformValidation RPC canceled")
} else if err != nil { } else if err != nil {
// This is a real error, not just a problem with the validation. // This is a real error, not just a problem with the validation.
va.log.Errf("Remote VA %q.PerformValidation failed: %s", rva.Address, err) va.log.Errf("Remote VA %q.PerformValidation failed: %s", rva.Address, err)

View File

@ -453,8 +453,7 @@ func TestMultiVA(t *testing.T) {
expectedKeyAuthorization)), expectedKeyAuthorization)),
}, },
{ {
// With one remote VA cancelled there should not be a validation failure // When enforcing multi-VA, any cancellations are a problem.
// when enforcing multi VA.
Name: "Local VA and one remote VA OK, one cancelled VA, enforce multi VA", Name: "Local VA and one remote VA OK, one cancelled VA, enforce multi VA",
RemoteVAs: []RemoteVA{ RemoteVAs: []RemoteVA{
{remoteVA1, remoteUA1}, {remoteVA1, remoteUA1},
@ -462,17 +461,18 @@ func TestMultiVA(t *testing.T) {
}, },
AllowedUAs: allowedUAs, AllowedUAs: allowedUAs,
Features: enforceMultiVA, Features: enforceMultiVA,
ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"),
}, },
{ {
// With two remote VAs cancelled there should not be a validation failure // When enforcing multi-VA, any cancellations are a problem.
// when enforcing multi VA Name: "Local VA OK, two cancelled remote VAs, enforce multi VA",
Name: "Local VA and one remote VA OK, one cancelled VA, enforce multi VA",
RemoteVAs: []RemoteVA{ RemoteVAs: []RemoteVA{
{cancelledVA{}, remoteUA1}, {cancelledVA{}, remoteUA1},
{cancelledVA{}, remoteUA2}, {cancelledVA{}, remoteUA2},
}, },
AllowedUAs: allowedUAs, AllowedUAs: allowedUAs,
Features: enforceMultiVA, Features: enforceMultiVA,
ExpectedProb: probs.ServerInternal("During secondary validation: Remote PerformValidation RPC canceled"),
}, },
{ {
// With the local and remote VAs seeing diff problems and the full results // With the local and remote VAs seeing diff problems and the full results