BS: Configurable RevConfig (scionproto#3111)

The values in ifstate.RevConfig are now configurable via beacon server config file.

go/beacon_srv/internal/config/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
         "//go/beacon_srv/internal/beaconstorage:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/config:go_default_library",
+        "//go/lib/ctrl/path_mgmt:go_default_library",
         "//go/lib/env:go_default_library",
         "//go/lib/infra/modules/idiscovery:go_default_library",
         "//go/lib/truststorage:go_default_library",
@@ -25,9 +26,11 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//go/beacon_srv/internal/beaconstorage/beaconstoragetest:go_default_library",
+        "//go/lib/ctrl/path_mgmt:go_default_library",
         "//go/lib/env/envtest:go_default_library",
         "//go/lib/infra/modules/idiscovery/idiscoverytest:go_default_library",
         "//go/lib/truststorage/truststoragetest:go_default_library",
+        "//go/lib/util:go_default_library",
         "@com_github_burntsushi_toml//:go_default_library",
         "@com_github_smartystreets_goconvey//convey:go_default_library",
     ],

go/beacon_srv/internal/config/config.go

@@ -22,6 +22,7 @@ import (
 	"github.com/scionproto/scion/go/beacon_srv/internal/beaconstorage"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/config"
+	"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
 	"github.com/scionproto/scion/go/lib/env"
 	"github.com/scionproto/scion/go/lib/infra/modules/idiscovery"
 	"github.com/scionproto/scion/go/lib/truststorage"
@@ -45,6 +46,11 @@ const (
 	// DefaultExpiredCheckInterval is the default interval between checking for
 	// expired interfaces.
 	DefaultExpiredCheckInterval = 200 * time.Millisecond
+	// DefaultRevTTL is the default revocation TTL.
+	DefaultRevTTL = path_mgmt.MinRevTTL
+	// DefaultRevOverlap specifies the default for how long before the expiry of an existing
+	// revocation the revoker can reissue a new revocation.
+	DefaultRevOverlap = DefaultRevTTL / 2
 )
 
 var _ config.Config = (*Config)(nil)
@@ -132,6 +138,11 @@ type BSConfig struct {
 	// ExpiredCheckInterval is the interval between checking whether interfaces
 	// have expired and should be revoked.
 	ExpiredCheckInterval util.DurWrap
+	// RevTTL is the revocation TTL. (default 10s)
+	RevTTL util.DurWrap
+	// RevOverlap specifies for how long before the expiry of an existing revocation the revoker
+	// can reissue a new revocation. (default 5s)
+	RevOverlap util.DurWrap
 	// Policies contains the policy files.
 	Policies Policies
 }
@@ -144,6 +155,8 @@ func (cfg *BSConfig) InitDefaults() {
 	initDurWrap(&cfg.PropagationInterval, DefaultPropagationInterval)
 	initDurWrap(&cfg.RegistrationInterval, DefaultRegistrationInterval)
 	initDurWrap(&cfg.ExpiredCheckInterval, DefaultExpiredCheckInterval)
+	initDurWrap(&cfg.RevTTL, DefaultRevTTL)
+	initDurWrap(&cfg.RevOverlap, DefaultRevOverlap)
 }
 
 // Validate validates that all durations are set.
@@ -166,6 +179,19 @@ func (cfg *BSConfig) Validate() error {
 	if cfg.ExpiredCheckInterval.Duration == 0 {
 		return common.NewBasicError("ExpiredCheckInterval not set", nil)
 	}
+	if cfg.RevTTL.Duration == 0 {
+		return common.NewBasicError("RevTTL is not set", nil)
+	}
+	if cfg.RevTTL.Duration < path_mgmt.MinRevTTL {
+		return common.NewBasicError("RevTTL must be equal or greater than MinRevTTL", nil,
+			"MinRevTTL", path_mgmt.MinRevTTL)
+	}
+	if cfg.RevOverlap.Duration == 0 {
+		return common.NewBasicError("RevOverlap not set", nil)
+	}
+	if cfg.RevOverlap.Duration > cfg.RevTTL.Duration {
+		return common.NewBasicError("RevOverlap cannot be greater than RevTTL", nil)
+	}
 	return nil
 }
 

go/beacon_srv/internal/config/config_test.go

@@ -17,14 +17,17 @@ package config
 import (
 	"bytes"
 	"testing"
+	"time"
 
 	"github.com/BurntSushi/toml"
 	. "github.com/smartystreets/goconvey/convey"
 
 	"github.com/scionproto/scion/go/beacon_srv/internal/beaconstorage/beaconstoragetest"
+	"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
 	"github.com/scionproto/scion/go/lib/env/envtest"
 	"github.com/scionproto/scion/go/lib/infra/modules/idiscovery/idiscoverytest"
 	"github.com/scionproto/scion/go/lib/truststorage/truststoragetest"
+	"github.com/scionproto/scion/go/lib/util"
 )
 
 func TestConfigSample(t *testing.T) {
@@ -41,6 +44,22 @@ func TestConfigSample(t *testing.T) {
 	})
 }
 
+func TestInvalidTTL(t *testing.T) {
+	Convey("Validate checks wrongly configured TTL values", t, func() {
+		cfg := BSConfig{}
+		cfg.InitDefaults()
+		err := cfg.Validate()
+		SoMsg("err", err, ShouldBeNil)
+		cfg.RevOverlap = util.DurWrap{Duration: cfg.RevTTL.Duration + time.Second}
+		err = cfg.Validate()
+		SoMsg("err", err, ShouldNotBeNil)
+		cfg.InitDefaults()
+		cfg.RevTTL = util.DurWrap{Duration: path_mgmt.MinRevTTL - time.Second}
+		err = cfg.Validate()
+		SoMsg("err", err, ShouldNotBeNil)
+	})
+}
+
 func InitTestConfig(cfg *Config) {
 	envtest.InitTest(&cfg.General, &cfg.Logging, &cfg.Metrics, &cfg.Tracing, nil)
 	truststoragetest.InitTestConfig(&cfg.TrustDB)
@@ -80,6 +99,8 @@ func CheckTestBSConfig(cfg *BSConfig) {
 		DefaultRegistrationInterval)
 	SoMsg("ExpiredCheckInterval", cfg.ExpiredCheckInterval.Duration, ShouldEqual,
 		DefaultExpiredCheckInterval)
+	SoMsg("RevTTL", cfg.RevTTL.Duration, ShouldEqual, DefaultRevTTL)
+	SoMsg("RevOverlap", cfg.RevOverlap.Duration, ShouldEqual, DefaultRevOverlap)
 	CheckTestPolicies(&cfg.Policies)
 }
 

go/beacon_srv/internal/config/sample.go

@@ -35,26 +35,33 @@ RegistrationInterval = "5s"
 
 # The interval between checking for expired interfaces to revoke. (default 200ms)
 ExpiredCheckInterval = "200ms"
+
+# The revocation TTL. (default 10s)
+RevTTL = "10s"
+
+# The amount of time before the expiry of an existing revocation where the revoker can reissue a
+# new revocation. (default 5s)
+RevOverlap = "5s"
 `
 
 const policiesSample = `
 # Output a sample policy file by providing the -help-policy flag.
 
-# The file path for the propagation policy. In case of the empty string, 
+# The file path for the propagation policy. In case of the empty string,
 # the default policy is used. (default "")
 Propagation = ""
 
-# The file path for the core registration policy. In case of the empty string, 
+# The file path for the core registration policy. In case of the empty string,
 # the default policy is used. In a non-core beacon server, this field is ignored.
 # (default "")
 CoreRegistration = ""
 
-# The file path for the up registration policy. In case of the empty string, 
+# The file path for the up registration policy. In case of the empty string,
 # the default policy is used. In a core beacon server, this field is ignored.
 # (default "")
 UpRegistration = ""
 
-# The file path for the down registration policy. In case of the empty string, 
+# The file path for the down registration policy. In case of the empty string,
 # the default policy is used. In a core beacon server, this field is ignored.
 # (default "")
 DownRegistration = ""

go/beacon_srv/internal/ifstate/revoker.go

@@ -32,27 +32,17 @@ import (
 	"github.com/scionproto/scion/go/lib/util"
 )
 
-// DefaultRevOverlap specifies the default for how long before the expiry of an existing revocation
-// the revoker can reissue a new revocation.
-const DefaultRevOverlap = path_mgmt.MinRevTTL / 2
-
 // RevInserter stores revocation into persistent storage.
 type RevInserter interface {
 	InsertRevocations(ctx context.Context, revocations ...*path_mgmt.SignedRevInfo) error
 }
 
 // RevConfig configures the parameters for revocation creation.
 type RevConfig struct {
+	RevTTL     time.Duration
 	RevOverlap time.Duration
 }
 
-// InitDefaults initializes the config fields that are not set to the default values.
-func (c *RevConfig) InitDefaults() {
-	if c.RevOverlap == 0 {
-		c.RevOverlap = DefaultRevOverlap
-	}
-}
-
 // RevokerConf is the configuration to create a new revoker.
 type RevokerConf struct {
 	Intfs        *Interfaces
@@ -74,7 +64,6 @@ type Revoker struct {
 
 // New creates a new revoker from the given arguments.
 func (cfg RevokerConf) New() *Revoker {
-	cfg.RevConfig.InitDefaults()
 	return &Revoker{
 		cfg: cfg,
 		pusher: brPusher{
@@ -139,7 +128,7 @@ func (r *Revoker) createSignedRev(ifid common.IFIDType) (*path_mgmt.SignedRevInf
 		RawIsdas:     r.cfg.TopoProvider.Get().ISD_AS.IAInt(),
 		LinkType:     r.cfg.TopoProvider.Get().IFInfoMap[ifid].LinkType,
 		RawTimestamp: now,
-		RawTTL:       uint32(path_mgmt.MinRevTTL.Seconds()),
+		RawTTL:       uint32(r.cfg.RevConfig.RevTTL.Seconds()),
 	}
 	return path_mgmt.NewSignedRevInfo(revInfo, r.cfg.Signer)
 }

go/beacon_srv/internal/ifstate/revoker_test.go

@@ -46,7 +46,8 @@ import (
 
 var (
 	timeout     = time.Second
-	overlapTime = path_mgmt.MinRevTTL / 2
+	ttl         = path_mgmt.MinRevTTL
+	overlapTime = ttl / 2
 	expireTime  = time.Second + DefaultKeepaliveTimeout
 	ia          = xtest.MustParseIA("1-ff00:0:111")
 )
@@ -118,9 +119,12 @@ func TestRevokeInterface(t *testing.T) {
 			Intfs:        intfs,
 			Msgr:         msgr,
 			Signer:       signer,
-			RevConfig:    RevConfig{RevOverlap: overlapTime},
 			TopoProvider: topoProvider,
 			RevInserter:  revInserter,
+			RevConfig: RevConfig{
+				RevTTL:     ttl,
+				RevOverlap: overlapTime,
+			},
 		}
 		revoker := cfg.New()
 		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
@@ -151,17 +155,20 @@ func TestRevokedInterfaceNotRevokedImmediately(t *testing.T) {
 			RawIsdas:     ia.IAInt(),
 			LinkType:     proto.LinkType_peer,
 			RawTimestamp: util.TimeToSecs(time.Now().Add(-500 * time.Millisecond)),
-			RawTTL:       10,
+			RawTTL:       uint32(ttl.Seconds()),
 		}, infra.NullSigner)
 		xtest.FailOnErr(t, err)
 		intfs.Get(101).Revoke(srev)
 		cfg := RevokerConf{
 			Intfs:        intfs,
 			Msgr:         msgr,
 			Signer:       signer,
-			RevConfig:    RevConfig{RevOverlap: overlapTime},
 			TopoProvider: topoProvider,
 			RevInserter:  revInserter,
+			RevConfig: RevConfig{
+				RevTTL:     ttl,
+				RevOverlap: overlapTime,
+			},
 		}
 		revoker := cfg.New()
 		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
@@ -192,8 +199,8 @@ func TestRevokedInterfaceRevokedAgain(t *testing.T) {
 			IfID:         101,
 			RawIsdas:     ia.IAInt(),
 			LinkType:     proto.LinkType_peer,
-			RawTimestamp: util.TimeToSecs(time.Now().Add(-6 * time.Second)),
-			RawTTL:       10,
+			RawTimestamp: util.TimeToSecs(time.Now().Add(-(overlapTime + 1) * time.Second)),
+			RawTTL:       uint32(ttl.Seconds()),
 		}, infra.NullSigner)
 		xtest.FailOnErr(t, err)
 		intfs.Get(101).Revoke(srev)
@@ -208,9 +215,12 @@ func TestRevokedInterfaceRevokedAgain(t *testing.T) {
 			Intfs:        intfs,
 			Msgr:         msgr,
 			Signer:       signer,
-			RevConfig:    RevConfig{RevOverlap: overlapTime},
 			TopoProvider: topoProvider,
 			RevInserter:  revInserter,
+			RevConfig: RevConfig{
+				RevTTL:     ttl,
+				RevOverlap: overlapTime,
+			},
 		}
 		revoker := cfg.New()
 		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
@@ -293,7 +303,7 @@ func checkRevocation(t *testing.T, srev *path_mgmt.SignedRevInfo,
 		rawNow := util.TimeToSecs(time.Now())
 		SoMsg("recent revocation", revInfo.RawTimestamp,
 			ShouldBeBetweenOrEqual, rawNow-1, rawNow)
-		SoMsg("minTTL", revInfo.RawTTL, ShouldEqual, uint32(path_mgmt.MinRevTTL.Seconds()))
+		SoMsg("minTTL", revInfo.RawTTL, ShouldEqual, uint32(ttl.Seconds()))
 	})
 }
 

go/beacon_srv/internal/revocation/handler_test.go

@@ -59,7 +59,7 @@ func TestHandler(t *testing.T) {
 		IfID:         101,
 		LinkType:     proto.LinkType_peer,
 		RawTimestamp: util.TimeToSecs(time.Now()),
-		RawTTL:       10,
+		RawTTL:       uint32(path_mgmt.MinRevTTL.Seconds()),
 	}
 	sRev, err := path_mgmt.NewSignedRevInfo(rev, signer)
 	xtest.FailOnErr(t, err)

go/beacon_srv/main.go

@@ -333,7 +333,10 @@ func (t *periodicTasks) startRevoker() (*periodic.Runner, error) {
 		RevInserter:  t.store,
 		Signer:       signer,
 		TopoProvider: t.topoProvider,
-		// TODO(roosd): Make RevConfig configurable
+		RevConfig: ifstate.RevConfig{
+			RevTTL:     cfg.BS.RevTTL.Duration,
+			RevOverlap: cfg.BS.RevOverlap.Duration,
+		},
 	}.New()
 	return periodic.StartPeriodicTask(r, periodic.NewTicker(cfg.BS.ExpiredCheckInterval.Duration),
 		cfg.BS.ExpiredCheckInterval.Duration), nil