From c6190fa2bac2f27e0628138925ce0bb15eddbf3b Mon Sep 17 00:00:00 2001 From: Bethuel Date: Thu, 13 Apr 2023 20:19:04 +0300 Subject: [PATCH 1/7] add use-key-cache-headers flag to management command --- management/cmd/root.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/management/cmd/root.go b/management/cmd/root.go index 5a1c52d31..ae44f373a 100644 --- a/management/cmd/root.go +++ b/management/cmd/root.go @@ -23,6 +23,7 @@ var ( logFile string disableMetrics bool disableSingleAccMode bool + UseKeyCacheHeaders bool rootCmd = &cobra.Command{ Use: "netbird-mgmt", @@ -54,6 +55,7 @@ func init() { mgmtCmd.Flags().StringVar(&certKey, "cert-key", "", "Location of your SSL certificate private key. Can be used when you have an existing certificate and don't want a new certificate be generated automatically. If letsencrypt-domain is specified this property has no effect") mgmtCmd.Flags().BoolVar(&disableMetrics, "disable-anonymous-metrics", false, "disables push of anonymous usage metrics to NetBird") mgmtCmd.Flags().StringVar(&dnsDomain, "dns-domain", defaultSingleAccModeDomain, fmt.Sprintf("Domain used for peer resolution. This is appended to the peer's name, e.g. pi-server. %s. Max lenght is 192 characters to allow appending to a peer name with up to 63 characters.", defaultSingleAccModeDomain)) + mgmtCmd.Flags().BoolVar(&UseKeyCacheHeaders, "use-key-cache-headers", false, "Enable cache headers evaluation to determine signing key rotation period. This will refresh the signing key upon expiry.") rootCmd.MarkFlagRequired("config") //nolint rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "info", "") From a89808ecae38969c6452bdd586b531d6b4d87551 Mon Sep 17 00:00:00 2001 From: Bethuel Date: Fri, 14 Apr 2023 12:17:28 +0300 Subject: [PATCH 2/7] initialize jwt validator with keys rotation state --- management/cmd/management.go | 2 ++ management/server/grpcserver.go | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/management/cmd/management.go b/management/cmd/management.go index d956fcff5..d740423d7 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -80,6 +80,7 @@ var ( if err != nil { return fmt.Errorf("failed reading provided config file: %s: %v", mgmtConfig, err) } + config.HttpConfig.KeyRotationEnabled = UseKeyCacheHeaders tlsEnabled := false if mgmtLetsencryptDomain != "" || (config.HttpConfig.CertFile != "" && config.HttpConfig.CertKey != "") { @@ -186,6 +187,7 @@ var ( config.HttpConfig.AuthIssuer, config.GetAuthAudiences(), config.HttpConfig.AuthKeysLocation, + config.HttpConfig.KeyRotationEnabled, ) if err != nil { return fmt.Errorf("failed creating JWT validator: %v", err) diff --git a/management/server/grpcserver.go b/management/server/grpcserver.go index f63a55d65..a93a03353 100644 --- a/management/server/grpcserver.go +++ b/management/server/grpcserver.go @@ -52,7 +52,9 @@ func NewServer(config *Config, accountManager AccountManager, peersUpdateManager jwtValidator, err = jwtclaims.NewJWTValidator( config.HttpConfig.AuthIssuer, config.GetAuthAudiences(), - config.HttpConfig.AuthKeysLocation) + config.HttpConfig.AuthKeysLocation, + config.HttpConfig.KeyRotationEnabled, + ) if err != nil { return nil, status.Errorf(codes.Internal, "unable to create new jwt middleware, err: %v", err) } From 9f352c1b7e3c771495a767375af9f5ba1dfd4d62 Mon Sep 17 00:00:00 2001 From: Bethuel Date: Fri, 14 Apr 2023 12:20:34 +0300 Subject: [PATCH 3/7] validate keys for idp's with key rotation mechanism --- management/server/config.go | 2 + management/server/jwtclaims/jwtValidator.go | 53 ++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/management/server/config.go b/management/server/config.go index 9ec16b3e8..5fd7fd8d4 100644 --- a/management/server/config.go +++ b/management/server/config.go @@ -80,6 +80,8 @@ type HttpServerConfig struct { AuthKeysLocation string // OIDCConfigEndpoint is the endpoint of an IDP manager to get OIDC configuration OIDCConfigEndpoint string + // KeyRotationEnabled identifies the signing key is currently being rotated or not + KeyRotationEnabled bool } // Host represents a Wiretrustee host (e.g. STUN, TURN, Signal) diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index 147f8f2eb..d0143c3e5 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -12,6 +12,9 @@ import ( "fmt" "math/big" "net/http" + "strconv" + "strings" + "time" "github.com/golang-jwt/jwt" log "github.com/sirupsen/logrus" @@ -45,7 +48,8 @@ type Options struct { // Jwks is a collection of JSONWebKey obtained from Config.HttpServerConfig.AuthKeysLocation type Jwks struct { - Keys []JSONWebKey `json:"keys"` + Keys []JSONWebKey `json:"keys"` + expiresInTime time.Time } // JSONWebKey is a representation of a Jason Web Key @@ -64,7 +68,7 @@ type JWTValidator struct { } // NewJWTValidator constructor -func NewJWTValidator(issuer string, audienceList []string, keysLocation string) (*JWTValidator, error) { +func NewJWTValidator(issuer string, audienceList []string, keysLocation string, keyRotationEnabled bool) (*JWTValidator, error) { keys, err := getPemKeys(keysLocation) if err != nil { return nil, err @@ -89,6 +93,19 @@ func NewJWTValidator(issuer string, audienceList []string, keysLocation string) return token, errors.New("invalid issuer") } + // If keys are rotated, verify the keys prior to token validation + if keyRotationEnabled { + // If the keys are invalid, retrieve new ones + if !keys.stillValid() { + + keys, err = getPemKeys(keysLocation) + if err != nil { + log.Errorf("cannot get JSONWebKey: %v", err) + return nil, err + } + } + } + cert, err := getPemCert(token, keys) if err != nil { return nil, err @@ -154,6 +171,11 @@ func (m *JWTValidator) ValidateAndParse(token string) (*jwt.Token, error) { return parsedToken, nil } +// stillValid returns true if the JSONWebKey still valid and have enough time to be used +func (jwks *Jwks) stillValid() bool { + return jwks.expiresInTime.IsZero() && time.Now().Add(5*time.Second).Before(jwks.expiresInTime) +} + func getPemKeys(keysLocation string) (*Jwks, error) { resp, err := http.Get(keysLocation) if err != nil { @@ -167,6 +189,10 @@ func getPemKeys(keysLocation string) (*Jwks, error) { return jwks, err } + cacheControlHeader := resp.Header.Get("Cache-Control") + expiresIn := getMaxAgeFromCacheHeader(cacheControlHeader) + jwks.expiresInTime = time.Now().Add(time.Duration(expiresIn) * time.Second) + return jwks, err } @@ -248,3 +274,26 @@ func convertExponentStringToInt(stringExponent string) (int, error) { return int(exponent), nil } + +// getMaxAgeFromCacheHeader extracts max-age directive from the Cache-Control header +func getMaxAgeFromCacheHeader(cacheControl string) int { + // Split into individual directives + directives := strings.Split(cacheControl, ",") + + for _, directive := range directives { + directive = strings.TrimSpace(directive) + if strings.HasPrefix(directive, "max-age=") { + // Extract the max-age value + maxAgeStr := strings.TrimPrefix(directive, "max-age=") + maxAge, err := strconv.Atoi(maxAgeStr) + if err != nil { + log.Debugf("error parsing max-age: %v", err) + return 0 + } + + return maxAge + } + } + + return 0 +} From 53d78ad98260d0754bbd8debf22415044d084cf6 Mon Sep 17 00:00:00 2001 From: Bethuel Date: Fri, 14 Apr 2023 13:16:01 +0300 Subject: [PATCH 4/7] make variable unexported --- management/cmd/management.go | 2 +- management/cmd/root.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/management/cmd/management.go b/management/cmd/management.go index d740423d7..d95407686 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -80,7 +80,7 @@ var ( if err != nil { return fmt.Errorf("failed reading provided config file: %s: %v", mgmtConfig, err) } - config.HttpConfig.KeyRotationEnabled = UseKeyCacheHeaders + config.HttpConfig.KeyRotationEnabled = useKeyCacheHeaders tlsEnabled := false if mgmtLetsencryptDomain != "" || (config.HttpConfig.CertFile != "" && config.HttpConfig.CertKey != "") { diff --git a/management/cmd/root.go b/management/cmd/root.go index ae44f373a..399eb9b10 100644 --- a/management/cmd/root.go +++ b/management/cmd/root.go @@ -23,7 +23,7 @@ var ( logFile string disableMetrics bool disableSingleAccMode bool - UseKeyCacheHeaders bool + useKeyCacheHeaders bool rootCmd = &cobra.Command{ Use: "netbird-mgmt", @@ -55,7 +55,7 @@ func init() { mgmtCmd.Flags().StringVar(&certKey, "cert-key", "", "Location of your SSL certificate private key. Can be used when you have an existing certificate and don't want a new certificate be generated automatically. If letsencrypt-domain is specified this property has no effect") mgmtCmd.Flags().BoolVar(&disableMetrics, "disable-anonymous-metrics", false, "disables push of anonymous usage metrics to NetBird") mgmtCmd.Flags().StringVar(&dnsDomain, "dns-domain", defaultSingleAccModeDomain, fmt.Sprintf("Domain used for peer resolution. This is appended to the peer's name, e.g. pi-server. %s. Max lenght is 192 characters to allow appending to a peer name with up to 63 characters.", defaultSingleAccModeDomain)) - mgmtCmd.Flags().BoolVar(&UseKeyCacheHeaders, "use-key-cache-headers", false, "Enable cache headers evaluation to determine signing key rotation period. This will refresh the signing key upon expiry.") + mgmtCmd.Flags().BoolVar(&useKeyCacheHeaders, "use-key-cache-headers", false, "Enable cache headers evaluation to determine signing key rotation period. This will refresh the signing key upon expiry.") rootCmd.MarkFlagRequired("config") //nolint rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "info", "") From f7196cd9a57506bc58a49c823e3f41636bb16947 Mon Sep 17 00:00:00 2001 From: Bethuel Date: Sat, 15 Apr 2023 03:44:42 +0300 Subject: [PATCH 5/7] refactoring --- management/cmd/management.go | 4 ++-- management/cmd/root.go | 18 +++++++++--------- management/server/config.go | 4 ++-- management/server/grpcserver.go | 2 +- management/server/jwtclaims/jwtValidator.go | 7 +++---- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/management/cmd/management.go b/management/cmd/management.go index d95407686..5058447d8 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -80,7 +80,7 @@ var ( if err != nil { return fmt.Errorf("failed reading provided config file: %s: %v", mgmtConfig, err) } - config.HttpConfig.KeyRotationEnabled = useKeyCacheHeaders + config.HttpConfig.IdpSignKeyRefreshEnabled = idpSignKeyRefreshEnabled tlsEnabled := false if mgmtLetsencryptDomain != "" || (config.HttpConfig.CertFile != "" && config.HttpConfig.CertKey != "") { @@ -187,7 +187,7 @@ var ( config.HttpConfig.AuthIssuer, config.GetAuthAudiences(), config.HttpConfig.AuthKeysLocation, - config.HttpConfig.KeyRotationEnabled, + config.HttpConfig.IdpSignKeyRefreshEnabled, ) if err != nil { return fmt.Errorf("failed creating JWT validator: %v", err) diff --git a/management/cmd/root.go b/management/cmd/root.go index 399eb9b10..a149841c5 100644 --- a/management/cmd/root.go +++ b/management/cmd/root.go @@ -16,14 +16,14 @@ const ( ) var ( - dnsDomain string - mgmtDataDir string - mgmtConfig string - logLevel string - logFile string - disableMetrics bool - disableSingleAccMode bool - useKeyCacheHeaders bool + dnsDomain string + mgmtDataDir string + mgmtConfig string + logLevel string + logFile string + disableMetrics bool + disableSingleAccMode bool + idpSignKeyRefreshEnabled bool rootCmd = &cobra.Command{ Use: "netbird-mgmt", @@ -55,7 +55,7 @@ func init() { mgmtCmd.Flags().StringVar(&certKey, "cert-key", "", "Location of your SSL certificate private key. Can be used when you have an existing certificate and don't want a new certificate be generated automatically. If letsencrypt-domain is specified this property has no effect") mgmtCmd.Flags().BoolVar(&disableMetrics, "disable-anonymous-metrics", false, "disables push of anonymous usage metrics to NetBird") mgmtCmd.Flags().StringVar(&dnsDomain, "dns-domain", defaultSingleAccModeDomain, fmt.Sprintf("Domain used for peer resolution. This is appended to the peer's name, e.g. pi-server. %s. Max lenght is 192 characters to allow appending to a peer name with up to 63 characters.", defaultSingleAccModeDomain)) - mgmtCmd.Flags().BoolVar(&useKeyCacheHeaders, "use-key-cache-headers", false, "Enable cache headers evaluation to determine signing key rotation period. This will refresh the signing key upon expiry.") + mgmtCmd.Flags().BoolVar(&idpSignKeyRefreshEnabled, "idp-sign-key-refresh-enabled", false, "Enable cache headers evaluation to determine signing key rotation period. This will refresh the signing key upon expiry.") rootCmd.MarkFlagRequired("config") //nolint rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "info", "") diff --git a/management/server/config.go b/management/server/config.go index 5fd7fd8d4..32a468e91 100644 --- a/management/server/config.go +++ b/management/server/config.go @@ -80,8 +80,8 @@ type HttpServerConfig struct { AuthKeysLocation string // OIDCConfigEndpoint is the endpoint of an IDP manager to get OIDC configuration OIDCConfigEndpoint string - // KeyRotationEnabled identifies the signing key is currently being rotated or not - KeyRotationEnabled bool + // IdpSignKeyRefreshEnabled identifies the signing key is currently being rotated or not + IdpSignKeyRefreshEnabled bool } // Host represents a Wiretrustee host (e.g. STUN, TURN, Signal) diff --git a/management/server/grpcserver.go b/management/server/grpcserver.go index a93a03353..d27a73a8c 100644 --- a/management/server/grpcserver.go +++ b/management/server/grpcserver.go @@ -53,7 +53,7 @@ func NewServer(config *Config, accountManager AccountManager, peersUpdateManager config.HttpConfig.AuthIssuer, config.GetAuthAudiences(), config.HttpConfig.AuthKeysLocation, - config.HttpConfig.KeyRotationEnabled, + config.HttpConfig.IdpSignKeyRefreshEnabled, ) if err != nil { return nil, status.Errorf(codes.Internal, "unable to create new jwt middleware, err: %v", err) diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index d0143c3e5..2b3f23568 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -68,7 +68,7 @@ type JWTValidator struct { } // NewJWTValidator constructor -func NewJWTValidator(issuer string, audienceList []string, keysLocation string, keyRotationEnabled bool) (*JWTValidator, error) { +func NewJWTValidator(issuer string, audienceList []string, keysLocation string, idpSignkeyRefreshEnabled bool) (*JWTValidator, error) { keys, err := getPemKeys(keysLocation) if err != nil { return nil, err @@ -94,13 +94,12 @@ func NewJWTValidator(issuer string, audienceList []string, keysLocation string, } // If keys are rotated, verify the keys prior to token validation - if keyRotationEnabled { + if idpSignkeyRefreshEnabled { // If the keys are invalid, retrieve new ones if !keys.stillValid() { - keys, err = getPemKeys(keysLocation) if err != nil { - log.Errorf("cannot get JSONWebKey: %v", err) + log.Debugf("cannot get JSONWebKey: %v", err) return nil, err } } From 90c8cfd8633bceef0434673718de5f41b1635ad9 Mon Sep 17 00:00:00 2001 From: Bethuel Date: Wed, 19 Apr 2023 17:11:38 +0300 Subject: [PATCH 6/7] synchronize access to the signing keys --- management/server/jwtclaims/jwtValidator.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index 2b3f23568..3e4b5e40e 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -14,6 +14,7 @@ import ( "net/http" "strconv" "strings" + "sync" "time" "github.com/golang-jwt/jwt" @@ -74,6 +75,7 @@ func NewJWTValidator(issuer string, audienceList []string, keysLocation string, return nil, err } + var lock sync.Mutex options := Options{ ValidationKeyGetter: func(token *jwt.Token) (interface{}, error) { // Verify 'aud' claim @@ -97,6 +99,8 @@ func NewJWTValidator(issuer string, audienceList []string, keysLocation string, if idpSignkeyRefreshEnabled { // If the keys are invalid, retrieve new ones if !keys.stillValid() { + lock.Lock() + defer lock.Unlock() keys, err = getPemKeys(keysLocation) if err != nil { log.Debugf("cannot get JSONWebKey: %v", err) From 45224e76d04e1fdff7d96291ed27f08921c8ed42 Mon Sep 17 00:00:00 2001 From: Bethuel Date: Fri, 21 Apr 2023 13:34:52 +0300 Subject: [PATCH 7/7] fallback to olde keys if failing to fetch refreshed keys --- management/server/jwtclaims/jwtValidator.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index 3e4b5e40e..b206eb794 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -101,11 +101,14 @@ func NewJWTValidator(issuer string, audienceList []string, keysLocation string, if !keys.stillValid() { lock.Lock() defer lock.Unlock() - keys, err = getPemKeys(keysLocation) + + refreshedKeys, err := getPemKeys(keysLocation) if err != nil { - log.Debugf("cannot get JSONWebKey: %v", err) - return nil, err + log.Debugf("cannot get JSONWebKey: %v, falling back to old keys", err) + refreshedKeys = keys } + + keys = refreshedKeys } }