[management] Make the certificate challenge window one knob to turn

Renewal was timed against the window in two different ways: the period derived
from it, the sweep interval did not. Shortening the window to watch a renewal in
an end-to-end run would have left the refresher still looking for due accounts
every quarter of an hour, so nothing would have been renewed in time and the
test would have reported the feature broken.

Derive the sweep from the period, within bounds that keep a very short window
from spinning and a normal one from checking less often than is useful, and
allow the window itself to be set through the environment so a run can take
seconds instead of half a day. A value that cannot be parsed or falls outside
the bounds keeps the default, because a window nobody intended is a security
property nobody chose, and an override is logged at warning level since it sets
how long a device keeps passing the check after its key is gone.

Every instance has to be given the same value: the window is part of the nonce,
so instances that disagree reject each other's.

management/server/certificate_challenge.go

@@ -15,19 +15,27 @@ import (
 )
 
 const (
-	// certChallengePeriod is how often an account whose policies carry a certificate
-	// posture check is pushed a fresh challenge. A nonce is accepted for its own window
-	// and the one before it, so one issued at the very end of a window lives only
-	// certposture.Window. A third of that leaves a missed run well clear of the edge,
-	// where a half would put it exactly on it.
-	certChallengePeriod = certposture.Window / 3
-
-	// certChallengeTick is how often the refresher looks for accounts that are due. The
-	// period is measured in hours, so this only has to be fine enough to spread the
-	// accounts over it.
-	certChallengeTick = 15 * time.Minute
+	minCertChallengeTick = time.Second
+	maxCertChallengeTick = 15 * time.Minute
 )
 
+// certChallengePeriod is how often an account whose policies carry a certificate
+// posture check is pushed a fresh challenge. A nonce is accepted for its own window and
+// the one before it, so one issued at the very end of a window lives only one window. A
+// third of that leaves a missed run well clear of the edge, where a half would put it
+// exactly on it.
+func certChallengePeriod() time.Duration {
+	return certposture.EffectiveWindow() / 3
+}
+
+// certChallengeTick is how often the refresher looks for accounts that are due. It is
+// derived from the period rather than fixed, so shortening the challenge window for a
+// test shortens this with it; the bounds keep a tiny window from spinning and a normal
+// one from checking less often than is useful.
+func certChallengeTick(period time.Duration) time.Duration {
+	return min(max(period/10, minCertChallengeTick), maxCertChallengeTick)
+}
+
 // certChallengeRefresher pushes a fresh certificate challenge to the peers of every
 // account that needs one, from a single goroutine.
 //
@@ -52,10 +60,11 @@ type certChallengeRefresher struct {
 }
 
 func newCertChallengeRefresher(refresh func(ctx context.Context, accountID string) bool) *certChallengeRefresher {
+	period := certChallengePeriod()
 	return &certChallengeRefresher{
 		due:     map[string]time.Time{},
-		period:  certChallengePeriod,
-		tick:    certChallengeTick,
+		period:  period,
+		tick:    certChallengeTick(period),
 		now:     time.Now,
 		refresh: refresh,
 	}

management/server/certificate_challenge_test.go

@@ -58,10 +58,24 @@ func TestCertChallengePeriod_LeavesRoomForAMissedRun(t *testing.T) {
 	// A nonce issued at the very end of a window is accepted for that window and the
 	// next one only, so the shortest life a peer can be handed is one window. Renewing
 	// has to stay clear of that edge even when a run is missed.
-	assert.Less(t, 2*certChallengePeriod, certposture.Window,
+	assert.Less(t, 2*certChallengePeriod(), certposture.EffectiveWindow(),
 		"a missed refresh must still leave the peer's nonce valid, with margin")
 }
 
+func TestCertChallengeTick_FollowsTheWindow(t *testing.T) {
+	// An end-to-end test shortens the challenge window to watch a renewal happen. The
+	// tick has to come down with it, or the refresher would still be looking for due
+	// accounts every quarter of an hour and never renew anything in time.
+	short := certChallengeTick(30 * time.Second)
+	assert.Less(t, short, 30*time.Second, "the tick must be shorter than the period it serves")
+	assert.GreaterOrEqual(t, short, minCertChallengeTick, "the tick must not spin")
+
+	assert.Equal(t, maxCertChallengeTick, certChallengeTick(24*time.Hour),
+		"a long period must not stretch the tick without bound")
+	assert.Equal(t, minCertChallengeTick, certChallengeTick(time.Millisecond),
+		"a tiny period must not drive the tick below its floor")
+}
+
 func TestCertChallengeRefresher_SpreadsAccountsOverThePeriod(t *testing.T) {
 	// Every account on an instance shares the same global challenge window, so a
 	// restart arms them all at once. The offset is what keeps them from fanning out
@@ -85,9 +99,9 @@ func TestCertChallengeRefresher_SpreadsAccountsOverThePeriod(t *testing.T) {
 }
 
 func TestCertChallengeOffset_IsStablePerAccount(t *testing.T) {
-	assert.Equal(t, offsetWithin("account-a", certChallengePeriod), offsetWithin("account-a", certChallengePeriod),
+	assert.Equal(t, offsetWithin("account-a", certChallengePeriod()), offsetWithin("account-a", certChallengePeriod()),
 		"the same account must keep its slot across restarts")
-	assert.NotEqual(t, offsetWithin("account-a", certChallengePeriod), offsetWithin("account-b", certChallengePeriod),
+	assert.NotEqual(t, offsetWithin("account-a", certChallengePeriod()), offsetWithin("account-b", certChallengePeriod()),
 		"two accounts must not share a slot")
 }
 
@@ -124,7 +138,7 @@ func TestCertChallengeRefresher_TrackIsIdempotent(t *testing.T) {
 	r.Track(context.Background(), "account-a")
 	first := r.due["account-a"]
 
-	clock.Advance(certChallengePeriod)
+	clock.Advance(certChallengePeriod())
 	r.Track(context.Background(), "account-a")
 
 	assert.Equal(t, first, r.due["account-a"], "re-tracking must not push the next run further out")