Skip to content

Commit

Permalink
feat: periodic key revocation (#401)
Browse files Browse the repository at this point in the history 
This replaces Relay's key rotation system with a new abstraction:
`credential.Rotator`

Previously, key rotation was the concern of the stream message handler,
project manager, environment manager, and environments. This was because
SDK key expiry was handled by the stream manager at the highest level,
even though the actual credential handling needed to be delegated to the
lowest level (environments.)

Now, key rotation happens locally within an environment. Key deprecation
messages are pushed down the stack until they get to an environment,
which hands it off to the `Rotator`.

A `Rotator` can be queried at any time to determine what keys should be
added or removed from an environment. It is purely logical and doesn't
deal with goroutines or channels, making it easy to test.

---------

**Behavioral Change**
Key expiration is no longer instantaneous, it now happens on a
configurable interval - for example, expired keys are checked every
minute. The tradeoff is higher latency, but easier to test and predict
the behavior.

The interval can be controlled with config option
`EXPIRED_CREDENTIAL_CLEANUP_INTERVAL`.
  • Loading branch information
@cwaldren-ld
cwaldren-ld authored Jun 25, 2024
1 parent 3c12e10 commit 92033e9
Show file tree
Hide file tree
Showing 30 changed files with 1,044 additions and 760 deletions.
41 changes: 23 additions & 18 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ const (
// the CPU time it takes to read the archive over and over again. It is somewhat arbitrary.
// It likely doesn't make sense to use an interval this frequent in production use-cases.
minimumFileDataSourceMonitoringInterval = 100 * time.Millisecond
// This minimum was chosen to protect the host system from unnecessary work, while also allowing expired
// credentials to be revoked nearly instantaneously. It is not necessarily a recommendation.
// It likely doesn't make sense to use an interval this frequent in production use-cases.
minimumCredentialCleanupInterval = 100 * time.Millisecond
)

// DefaultLoggers is the default logging configuration used by Relay.
Expand Down Expand Up @@ -134,24 +138,25 @@ type Config struct {
// variables, individual fields are not documented here; instead, see the `README.md` section on
// configuration.
type MainConfig struct {
ExitOnError bool `conf:"EXIT_ON_ERROR"`
ExitAlways bool `conf:"EXIT_ALWAYS"`
IgnoreConnectionErrors bool `conf:"IGNORE_CONNECTION_ERRORS"`
StreamURI ct.OptURLAbsolute `conf:"STREAM_URI"`
BaseURI ct.OptURLAbsolute `conf:"BASE_URI"`
ClientSideBaseURI ct.OptURLAbsolute `conf:"CLIENT_SIDE_BASE_URI"`
Port ct.OptIntGreaterThanZero `conf:"PORT"`
InitTimeout ct.OptDuration `conf:"INIT_TIMEOUT"`
HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"`
MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"`
DisconnectedStatusTime ct.OptDuration `conf:"DISCONNECTED_STATUS_TIME"`
TLSEnabled bool `conf:"TLS_ENABLED"`
TLSCert string `conf:"TLS_CERT"`
TLSKey string `conf:"TLS_KEY"`
TLSMinVersion OptTLSVersion `conf:"TLS_MIN_VERSION"`
LogLevel OptLogLevel `conf:"LOG_LEVEL"`
BigSegmentsStaleAsDegraded bool `conf:"BIG_SEGMENTS_STALE_AS_DEGRADED"`
BigSegmentsStaleThreshold ct.OptDuration `conf:"BIG_SEGMENTS_STALE_THRESHOLD"`
ExitOnError bool `conf:"EXIT_ON_ERROR"`
ExitAlways bool `conf:"EXIT_ALWAYS"`
IgnoreConnectionErrors bool `conf:"IGNORE_CONNECTION_ERRORS"`
StreamURI ct.OptURLAbsolute `conf:"STREAM_URI"`
BaseURI ct.OptURLAbsolute `conf:"BASE_URI"`
ClientSideBaseURI ct.OptURLAbsolute `conf:"CLIENT_SIDE_BASE_URI"`
Port ct.OptIntGreaterThanZero `conf:"PORT"`
InitTimeout ct.OptDuration `conf:"INIT_TIMEOUT"`
HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"`
MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"`
DisconnectedStatusTime ct.OptDuration `conf:"DISCONNECTED_STATUS_TIME"`
TLSEnabled bool `conf:"TLS_ENABLED"`
TLSCert string `conf:"TLS_CERT"`
TLSKey string `conf:"TLS_KEY"`
TLSMinVersion OptTLSVersion `conf:"TLS_MIN_VERSION"`
LogLevel OptLogLevel `conf:"LOG_LEVEL"`
BigSegmentsStaleAsDegraded bool `conf:"BIG_SEGMENTS_STALE_AS_DEGRADED"`
BigSegmentsStaleThreshold ct.OptDuration `conf:"BIG_SEGMENTS_STALE_THRESHOLD"`
ExpiredCredentialCleanupInterval ct.OptDuration `conf:"EXPIRED_CREDENTIAL_CLEANUP_INTERVAL"`
}

// AutoConfigConfig contains configuration parameters for the auto-configuration feature.
Expand Down
45 changes: 13 additions & 32 deletions config/config_field_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"fmt"
"strings"

"github.com/launchdarkly/ld-relay/v8/internal/credential"

"github.com/launchdarkly/go-sdk-common/v3/ldlog"
)

Expand Down Expand Up @@ -44,6 +42,13 @@ type FilterKey string
// DefaultFilter represents the lack of a filter, meaning a full LaunchDarkly environment.
const DefaultFilter = FilterKey("")

func last4Chars(s string) string {
if len(s) < 4 { // COVERAGE: doesn't happen in unit tests, also can't happen with real environments
return s
}
return s[len(s)-4:]
}

// GetAuthorizationHeaderValue for SDKKey returns the same string, since SDK keys are passed in
// the Authorization header.
func (k SDKKey) GetAuthorizationHeaderValue() string {
Expand All @@ -57,21 +62,7 @@ func (k SDKKey) Defined() bool {
func (k SDKKey) String() string {
return string(k)
}

func (k SDKKey) Compare(cr credential.AutoConfig) (credential.SDKCredential, credential.Status) {
if cr.SDKKey == k {
return nil, credential.Unchanged
}
if cr.ExpiringSDKKey == k {
// If the AutoConfig update contains an ExpiringSDKKey that is equal to *this* key, then it means
// this key is now considered deprecated.
return cr.SDKKey, credential.Deprecated
} else {
// Otherwise if the AutoConfig update contains *some other* key, then it means this one must be considered
// expired.
return cr.SDKKey, credential.Expired
}
}
func (k SDKKey) Masked() string { return "..." + last4Chars(k.String()) }

// GetAuthorizationHeaderValue for MobileKey returns the same string, since mobile keys are passed in the
// Authorization header.
Expand All @@ -87,12 +78,7 @@ func (k MobileKey) String() string {
return string(k)
}

func (k MobileKey) Compare(cr credential.AutoConfig) (credential.SDKCredential, credential.Status) {
if cr.MobileKey == k {
return nil, credential.Unchanged
}
return cr.MobileKey, credential.Expired
}
func (k MobileKey) Masked() string { return "..." + last4Chars(k.String()) }

// GetAuthorizationHeaderValue for EnvironmentID returns an empty string, since environment IDs are not
// passed in a header but as part of the request URL.
Expand All @@ -108,10 +94,8 @@ func (k EnvironmentID) String() string {
return string(k)
}

func (k EnvironmentID) Compare(_ credential.AutoConfig) (credential.SDKCredential, credential.Status) {
// EnvironmentIDs should not change.
return nil, credential.Unchanged
}
// Masked is an alias for String(), because EnvironmentIDs are considered non-sensitive public information.
func (k EnvironmentID) Masked() string { return k.String() }

// GetAuthorizationHeaderValue for AutoConfigKey returns the same string, since these keys are passed in
// the Authorization header. Note that unlike the other kinds of authorization keys, this one is never
Expand All @@ -120,15 +104,12 @@ func (k AutoConfigKey) GetAuthorizationHeaderValue() string {
return string(k)
}

func (k AutoConfigKey) Compare(_ credential.AutoConfig) (credential.SDKCredential, credential.Status) {
// AutoConfigKeys should not change.
return nil, credential.Unchanged
}

func (k AutoConfigKey) String() string {
return string(k)
}

func (k AutoConfigKey) Masked() string { return last4Chars(string(k)) }

// UnmarshalText allows the SDKKey type to be set from environment variables.
func (k *SDKKey) UnmarshalText(data []byte) error {
*k = SDKKey(string(data))
Expand Down
10 changes: 10 additions & 0 deletions config/config_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ var (
errAutoConfWithFilters = errors.New("cannot configure filters if auto-configuration is enabled")
errMissingProjKey = errors.New("when filters are configured, all environments must specify a 'projKey'")
errInvalidFileDataSourceMonitoringInterval = fmt.Errorf("file data source monitoring interval must be >= %s", minimumFileDataSourceMonitoringInterval)
errInvalidCredentialCleanupInterval = fmt.Errorf("expired credential cleanup interval must be >= %s", minimumCredentialCleanupInterval)
)

func errEnvironmentWithNoSDKKey(envName string) error {
Expand Down Expand Up @@ -78,6 +79,7 @@ func ValidateConfig(c *Config, loggers ldlog.Loggers) error {
validateConfigDatabases(&result, c, loggers)
validateConfigFilters(&result, c)
validateOfflineMode(&result, c)
validateCredentialCleanupInterval(&result, c)

return result.GetError()
}
Expand Down Expand Up @@ -196,6 +198,14 @@ func validateOfflineMode(result *ct.ValidationResult, c *Config) {
}
}

func validateCredentialCleanupInterval(result *ct.ValidationResult, c *Config) {
if c.Main.ExpiredCredentialCleanupInterval.IsDefined() {
interval := c.Main.ExpiredCredentialCleanupInterval.GetOrElse(0)
if interval < minimumCredentialCleanupInterval {
result.AddError(nil, errInvalidCredentialCleanupInterval)
}
}
}
func validateConfigDatabases(result *ct.ValidationResult, c *Config, loggers ldlog.Loggers) {
normalizeRedisConfig(result, c)

Expand Down
13 changes: 13 additions & 0 deletions config/test_data_configs_invalid_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ type testDataInvalidConfig struct {
func makeInvalidConfigs() []testDataInvalidConfig {
return []testDataInvalidConfig{
makeInvalidConfigMissingSDKKey(),
makeInvalidConfigCredentialCleanupInterval("0s"),
makeInvalidConfigCredentialCleanupInterval("-1s"),
makeInvalidConfigCredentialCleanupInterval("99ms"),
makeInvalidConfigTLSWithNoCertOrKey(),
makeInvalidConfigTLSWithNoCert(),
makeInvalidConfigTLSWithNoKey(),
Expand Down Expand Up @@ -255,6 +258,16 @@ fileDataSourceMonitoringInterval = ` + interval + `
return c
}

func makeInvalidConfigCredentialCleanupInterval(interval string) testDataInvalidConfig {
c := testDataInvalidConfig{name: "credential cleanup interval with invalid value"}
c.fileError = errInvalidCredentialCleanupInterval.Error()
c.fileContent = `
[Main]
expiredCredentialCleanupInterval = ` + interval + `
`
return c
}

func makeInvalidConfigRedisInvalidHostname() testDataInvalidConfig {
c := testDataInvalidConfig{name: "Redis - invalid hostname"}
c.envVarsError = "invalid Redis hostname"
Expand Down
111 changes: 57 additions & 54 deletions config/test_data_configs_valid_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,23 +98,24 @@ func makeValidConfigAllBaseProperties() testDataValidConfig {
c := testDataValidConfig{name: "all base properties"}
c.makeConfig = func(c *Config) {
c.Main = MainConfig{
Port: mustOptIntGreaterThanZero(8333),
BaseURI: newOptURLAbsoluteMustBeValid("http://base"),
ClientSideBaseURI: newOptURLAbsoluteMustBeValid("http://clientbase"),
StreamURI: newOptURLAbsoluteMustBeValid("http://stream"),
ExitOnError: true,
ExitAlways: true,
IgnoreConnectionErrors: true,
HeartbeatInterval: ct.NewOptDuration(90 * time.Second),
MaxClientConnectionTime: ct.NewOptDuration(30 * time.Minute),
DisconnectedStatusTime: ct.NewOptDuration(3 * time.Minute),
TLSEnabled: true,
TLSCert: "cert",
TLSKey: "key",
TLSMinVersion: NewOptTLSVersion(tls.VersionTLS12),
LogLevel: NewOptLogLevel(ldlog.Warn),
BigSegmentsStaleAsDegraded: true,
BigSegmentsStaleThreshold: ct.NewOptDuration(10 * time.Minute),
Port: mustOptIntGreaterThanZero(8333),
BaseURI: newOptURLAbsoluteMustBeValid("http://base"),
ClientSideBaseURI: newOptURLAbsoluteMustBeValid("http://clientbase"),
StreamURI: newOptURLAbsoluteMustBeValid("http://stream"),
ExitOnError: true,
ExitAlways: true,
IgnoreConnectionErrors: true,
HeartbeatInterval: ct.NewOptDuration(90 * time.Second),
MaxClientConnectionTime: ct.NewOptDuration(30 * time.Minute),
DisconnectedStatusTime: ct.NewOptDuration(3 * time.Minute),
TLSEnabled: true,
TLSCert: "cert",
TLSKey: "key",
TLSMinVersion: NewOptTLSVersion(tls.VersionTLS12),
LogLevel: NewOptLogLevel(ldlog.Warn),
BigSegmentsStaleAsDegraded: true,
BigSegmentsStaleThreshold: ct.NewOptDuration(10 * time.Minute),
ExpiredCredentialCleanupInterval: ct.NewOptDuration(1 * time.Minute),
}
c.Events = EventsConfig{
SendEvents: true,
Expand Down Expand Up @@ -146,43 +147,44 @@ func makeValidConfigAllBaseProperties() testDataValidConfig {
}
}
c.envVars = map[string]string{
"PORT": "8333",
"BASE_URI": "http://base",
"CLIENT_SIDE_BASE_URI": "http://clientbase",
"STREAM_URI": "http://stream",
"EXIT_ON_ERROR": "1",
"EXIT_ALWAYS": "1",
"IGNORE_CONNECTION_ERRORS": "1",
"HEARTBEAT_INTERVAL": "90s",
"MAX_CLIENT_CONNECTION_TIME": "30m",
"DISCONNECTED_STATUS_TIME": "3m",
"TLS_ENABLED": "1",
"TLS_CERT": "cert",
"TLS_KEY": "key",
"TLS_MIN_VERSION": "1.2",
"LOG_LEVEL": "warn",
"BIG_SEGMENTS_STALE_AS_DEGRADED": "true",
"BIG_SEGMENTS_STALE_THRESHOLD": "10m",
"USE_EVENTS": "1",
"EVENTS_HOST": "http://events",
"EVENTS_FLUSH_INTERVAL": "120s",
"EVENTS_CAPACITY": "500",
"EVENTS_INLINE_USERS": "1",
"LD_ENV_earth": "earth-sdk",
"LD_MOBILE_KEY_earth": "earth-mob",
"LD_CLIENT_SIDE_ID_earth": "earth-env",
"LD_PREFIX_earth": "earth-",
"LD_TABLE_NAME_earth": "earth-table",
"LD_LOG_LEVEL_earth": "debug",
"LD_ENV_krypton": "krypton-sdk",
"LD_MOBILE_KEY_krypton": "krypton-mob",
"LD_CLIENT_SIDE_ID_krypton": "krypton-env",
"LD_SECURE_MODE_krypton": "1",
"LD_PREFIX_krypton": "krypton-",
"LD_TABLE_NAME_krypton": "krypton-table",
"LD_ALLOWED_ORIGIN_krypton": "https://oa,https://rann",
"LD_ALLOWED_HEADER_krypton": "Timestamp-Valid,Random-Id-Valid",
"LD_TTL_krypton": "5m",
"PORT": "8333",
"BASE_URI": "http://base",
"CLIENT_SIDE_BASE_URI": "http://clientbase",
"STREAM_URI": "http://stream",
"EXIT_ON_ERROR": "1",
"EXIT_ALWAYS": "1",
"IGNORE_CONNECTION_ERRORS": "1",
"HEARTBEAT_INTERVAL": "90s",
"MAX_CLIENT_CONNECTION_TIME": "30m",
"DISCONNECTED_STATUS_TIME": "3m",
"TLS_ENABLED": "1",
"TLS_CERT": "cert",
"TLS_KEY": "key",
"TLS_MIN_VERSION": "1.2",
"LOG_LEVEL": "warn",
"BIG_SEGMENTS_STALE_AS_DEGRADED": "true",
"BIG_SEGMENTS_STALE_THRESHOLD": "10m",
"USE_EVENTS": "1",
"EVENTS_HOST": "http://events",
"EVENTS_FLUSH_INTERVAL": "120s",
"EVENTS_CAPACITY": "500",
"EVENTS_INLINE_USERS": "1",
"LD_ENV_earth": "earth-sdk",
"LD_MOBILE_KEY_earth": "earth-mob",
"LD_CLIENT_SIDE_ID_earth": "earth-env",
"LD_PREFIX_earth": "earth-",
"LD_TABLE_NAME_earth": "earth-table",
"LD_LOG_LEVEL_earth": "debug",
"LD_ENV_krypton": "krypton-sdk",
"LD_MOBILE_KEY_krypton": "krypton-mob",
"LD_CLIENT_SIDE_ID_krypton": "krypton-env",
"LD_SECURE_MODE_krypton": "1",
"LD_PREFIX_krypton": "krypton-",
"LD_TABLE_NAME_krypton": "krypton-table",
"LD_ALLOWED_ORIGIN_krypton": "https://oa,https://rann",
"LD_ALLOWED_HEADER_krypton": "Timestamp-Valid,Random-Id-Valid",
"LD_TTL_krypton": "5m",
"EXPIRED_CREDENTIAL_CLEANUP_INTERVAL": "1m",
}
c.fileContent = `
[Main]
Expand All @@ -203,6 +205,7 @@ TLSMinVersion = "1.2"
LogLevel = "warn"
BigSegmentsStaleAsDegraded = 1
BigSegmentsStaleThreshold = 10m
ExpiredCredentialCleanupInterval = 1m
[Events]
SendEvents = 1
Expand Down
Loading

0 comments on commit 92033e9

Please sign in to comment.