Add an extra timeout waiting for RVAs (#8434)

Once we have enough RVA results, set a tighter timeout on the remaining
ones so we don't hold up issuance on a single slow perspective. This
strikes a balance between our previous strategy (cancelling all dangling
remotes immediately upon getting enough results) and our current
strategy (don't cancel any remotes unless we hit our global timeout).

Add a new VA config field to control how long we wait for the lagging
perspectives. If this config field is not set, default to not cancelling
early, which is our current behavior.

Author: mcpherrinm

cmd/boulder-va/main.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/letsencrypt/boulder/bdns"
 	"github.com/letsencrypt/boulder/cmd"
+	"github.com/letsencrypt/boulder/config"
 	"github.com/letsencrypt/boulder/features"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
 	"github.com/letsencrypt/boulder/iana"
@@ -51,9 +52,12 @@ type Config struct {
 	VA struct {
 		vaConfig.Common
 		RemoteVAs []RemoteVAGRPCClientConfig `validate:"omitempty,dive"`
-		// Deprecated and ignored
-		MaxRemoteValidationFailures int `validate:"omitempty,min=0,required_with=RemoteVAs"`
-		Features                    features.Config
+		// SlowRemoteTimeout sets how long the VA is willing to wait for slow
+		// RemoteVA instances to finish their work. It starts counting from
+		// when the VA first gets a quorum of (un)successful remote results.
+		// Leaving this value zero means the VA won't early-cancel slow remotes.
+		SlowRemoteTimeout config.Duration
+		Features          features.Config
 	}
 
 	Syslog        cmd.SyslogConfig
@@ -149,7 +153,9 @@ func main() {
 		c.VA.AccountURIPrefixes,
 		va.PrimaryPerspective,
 		"",
-		iana.IsReservedAddr)
+		iana.IsReservedAddr,
+		c.VA.SlowRemoteTimeout.Duration,
+	)
 	cmd.FailOnError(err, "Unable to create VA server")
 
 	start, err := bgrpc.NewServer(c.VA.GRPC, logger).Add(

cmd/remoteva/main.go

@@ -140,7 +140,9 @@ func main() {
 		c.RVA.AccountURIPrefixes,
 		c.RVA.Perspective,
 		c.RVA.RIR,
-		iana.IsReservedAddr)
+		iana.IsReservedAddr,
+		0,
+	)
 	cmd.FailOnError(err, "Unable to create Remote-VA server")
 
 	start, err := bgrpc.NewServer(c.RVA.GRPC, logger).Add(

test/config-next/va.json

@@ -63,6 +63,7 @@
 			"http://boulder.service.consul:4001/acme/acct/",
 			"http://boulder.service.consul:4000/acme/reg/"
 		],
+		"slowRemoteTimeout": "2s",
 		"features": {
 			"DNSAccount01Enabled": true
 		}

va/va.go

@@ -216,6 +216,7 @@ type ValidationAuthorityImpl struct {
 	maxRemoteFailures  int
 	accountURIPrefixes []string
 	singleDialTimeout  time.Duration
+	slowRemoteTimeout  time.Duration
 	perspective        string
 	rir                string
 	isReservedIPFunc   func(netip.Addr) error
@@ -239,6 +240,7 @@ func NewValidationAuthorityImpl(
 	perspective string,
 	rir string,
 	reservedIPChecker func(netip.Addr) error,
+	slowRemoteTimeout time.Duration,
 ) (*ValidationAuthorityImpl, error) {
 
 	if len(accountURIPrefixes) == 0 {
@@ -620,6 +622,16 @@ func (va *ValidationAuthorityImpl) doRemoteOperation(ctx context.Context, op rem
 			firstProb = currProb
 		}
 
+		if va.slowRemoteTimeout != 0 {
+			// If enough perspectives have passed, or enough perspectives have
+			// failed, set a tighter deadline for the remaining perspectives.
+			if (len(passed) >= required && len(passedRIRs) >= requiredRIRs) ||
+				(len(failed) > remoteVACount-required) {
+				timer := time.AfterFunc(va.slowRemoteTimeout, cancel)
+				defer timer.Stop()
+			}
+		}
+
 		// Once all the VAs have returned a result, break the loop.
 		if len(passed)+len(failed) >= remoteVACount {
 			break

va/va_test.go

@@ -145,6 +145,7 @@ func setup(srv *httptest.Server, userAgent string, remoteVAs []RemoteVA, mockDNS
 		perspective,
 		"",
 		isNonLoopbackReservedIP,
+		time.Second,
 	)
 	if err != nil {
 		panic(fmt.Sprintf("Failed to create validation authority: %v", err))
@@ -323,6 +324,7 @@ func TestNewValidationAuthorityImplWithDuplicateRemotes(t *testing.T) {
 		"example perspective",
 		"",
 		isNonLoopbackReservedIP,
+		time.Second,
 	)
 	test.AssertError(t, err, "NewValidationAuthorityImpl allowed duplicate remote perspectives")
 	test.AssertContains(t, err.Error(), "duplicate remote VA perspective \"dadaist\"")