diff --git a/go/beacon_srv/internal/config/BUILD.bazel b/go/beacon_srv/internal/config/BUILD.bazel index 9142a6f0f0..8f55a0a83f 100644 --- a/go/beacon_srv/internal/config/BUILD.bazel +++ b/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", ], diff --git a/go/beacon_srv/internal/config/config.go b/go/beacon_srv/internal/config/config.go index 94bd5e439f..d9ce321d81 100644 --- a/go/beacon_srv/internal/config/config.go +++ b/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 } diff --git a/go/beacon_srv/internal/config/config_test.go b/go/beacon_srv/internal/config/config_test.go index 45c26fca19..535480f2c2 100644 --- a/go/beacon_srv/internal/config/config_test.go +++ b/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) } diff --git a/go/beacon_srv/internal/config/sample.go b/go/beacon_srv/internal/config/sample.go index 386abffc79..cd9fd26dfb 100644 --- a/go/beacon_srv/internal/config/sample.go +++ b/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 = "" diff --git a/go/beacon_srv/internal/ifstate/revoker.go b/go/beacon_srv/internal/ifstate/revoker.go index b7d616eabe..2244f99e16 100644 --- a/go/beacon_srv/internal/ifstate/revoker.go +++ b/go/beacon_srv/internal/ifstate/revoker.go @@ -32,10 +32,6 @@ 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 @@ -43,16 +39,10 @@ type RevInserter interface { // 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) } diff --git a/go/beacon_srv/internal/ifstate/revoker_test.go b/go/beacon_srv/internal/ifstate/revoker_test.go index e770459a0c..5997d7dbb6 100644 --- a/go/beacon_srv/internal/ifstate/revoker_test.go +++ b/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,7 +155,7 @@ 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) @@ -159,9 +163,12 @@ func TestRevokedInterfaceNotRevokedImmediately(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) @@ -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())) }) } diff --git a/go/beacon_srv/internal/revocation/handler_test.go b/go/beacon_srv/internal/revocation/handler_test.go index 60f0c8e2ba..0751bf99d1 100644 --- a/go/beacon_srv/internal/revocation/handler_test.go +++ b/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) diff --git a/go/beacon_srv/main.go b/go/beacon_srv/main.go index 0a4d5beb6b..e8d6762567 100644 --- a/go/beacon_srv/main.go +++ b/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