Skip to content

Commit

Permalink
Run expiration check with extended time for all tokens (#12245)
Browse files Browse the repository at this point in the history
* Remove unused flags
Print oidc verifier config in log messages

* Skip expiry check and run expiration check with 10 minutes expiration time by default for all tokens.
Added options to set expiry check and expiration time values.
Verify method runs own expiration check if upstream expiry check is skipped.
Added expiration check verification.
Applying options only if provided. This allows better logging.
  • Loading branch information
dekiel authored Oct 28, 2024
1 parent 3dc38c7 commit 82cd35f
Show file tree
Hide file tree
Showing 4 changed files with 280 additions and 104 deletions.
61 changes: 10 additions & 51 deletions cmd/oidc-token-verifier/main.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package main

import (
"errors"
"fmt"
"os"

"github.com/coreos/go-oidc/v3/oidc"
"github.com/kyma-project/test-infra/pkg/logging"
tioidc "github.com/kyma-project/test-infra/pkg/oidc"
"github.com/spf13/cobra"
Expand All @@ -24,9 +22,6 @@ type Logger interface {
type options struct {
token string
clientID string
outputPath string
publicKeyPath string
newPublicKeysVarName string
trustedWorkflows []string
debug bool
oidcTokenExpirationTime int // OIDC token expiration time in minutes
Expand All @@ -46,15 +41,13 @@ func NewRootCmd() *cobra.Command {
It uses OIDC discovery to get the public keys and verify the token whenever the public keys are not cached or expired.`,
}
rootCmd.PersistentFlags().StringVarP(&opts.token, "token", "t", "", "OIDC token to verify")
rootCmd.PersistentFlags().StringVarP(&opts.newPublicKeysVarName, "new-keys-var", "n", "OIDC_NEW_PUBLIC_KEYS", "Name of the environment variable to set when new public keys are fetched")
// This flag should be enabled once we add support for it in the code.
// rootCmd.PersistentFlags().StringSliceVarP(&opts.trustedWorkflows, "trusted-workflows", "w", []string{}, "List of trusted workflows")
// err := rootCmd.MarkPersistentFlagRequired("trusted-workflows")
// if err != nil {
// panic(err)
// }
rootCmd.PersistentFlags().StringVarP(&opts.clientID, "client-id", "c", "image-builder", "OIDC token client ID, this is used to verify the audience claim in the token. The value should be the same as the audience claim value in the token.")
rootCmd.PersistentFlags().StringVarP(&opts.publicKeyPath, "public-key-path", "p", "", "Path to the cached public keys directory")
rootCmd.PersistentFlags().BoolVarP(&opts.debug, "debug", "d", false, "Enable debug mode")
rootCmd.PersistentFlags().IntVarP(&opts.oidcTokenExpirationTime, "oidc-token-expiration-time", "e", 10, "OIDC token expiration time in minutes")
return rootCmd
Expand All @@ -65,7 +58,7 @@ func NewVerifyCmd() *cobra.Command {
Use: "verify",
Short: "Verify token and expected claims values",
RunE: func(_ *cobra.Command, _ []string) error {
if err := opts.extractClaims(); err != nil {
if err := opts.verifyToken(); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -105,12 +98,11 @@ func isTokenProvided(logger Logger, opts *options) error {
// It verifies the token signature and expiration time, verifies if the token is issued by a trusted issuer,
// and the claims have expected values.
// It uses OIDC discovery to get the identity provider public keys.
func (opts *options) extractClaims() error {
func (opts *options) verifyToken() error {
var (
zapLogger *zap.Logger
err error
tokenExpiredError *oidc.TokenExpiredError
token *tioidc.Token
zapLogger *zap.Logger
err error
token *tioidc.Token
)
if opts.debug {
zapLogger, err = zap.NewDevelopment()
Expand All @@ -130,13 +122,10 @@ func (opts *options) extractClaims() error {
// Print used options values.
logger.Infow("Using the following trusted workflows", "trusted-workflows", opts.trustedWorkflows)
logger.Infow("Using the following client ID", "client-id", opts.clientID)
logger.Infow("Using the following public key path", "public-key-path", opts.publicKeyPath)
logger.Infow("Using the following new public keys environment variable", "new-keys-var", opts.newPublicKeysVarName)
logger.Infow("Using the following claims output path", "claims-output-path", opts.outputPath)

// Create a new verifier config that will be used to verify the token.
// The clientID is used to verify the audience claim in the token.
verifyConfig, err := tioidc.NewVerifierConfig(logger, opts.clientID)
verifyConfig, err := tioidc.NewVerifierConfig(logger, opts.clientID, tioidc.SkipExpiryCheck())
if err != nil {
return err
}
Expand Down Expand Up @@ -164,20 +153,14 @@ func (opts *options) extractClaims() error {

// Create a new verifier using the provider and the verifier config.
// The verifier is used to verify the token signature, expiration time and execute standard OIDC validation.
verifier := provider.NewVerifier(logger, verifyConfig)
verifier, err := provider.NewVerifier(logger, verifyConfig, tioidc.WithExtendedExpiration(opts.oidcTokenExpirationTime))
if err != nil {
return err
}
logger.Infow("New verifier created")

// Verify the token
token, err = verifier.Verify(ctx, opts.token)
if errors.As(err, &tokenExpiredError) {
err = verifier.VerifyExtendedExpiration(err.(*oidc.TokenExpiredError).Expiry, opts.oidcTokenExpirationTime)
if err != nil {
return err
}
verifyConfig.SkipExpiryCheck = false
verifierWithoutExpiration := provider.NewVerifier(logger, verifyConfig)
token, err = verifierWithoutExpiration.Verify(ctx, opts.token)
}
if err != nil {
return err
}
Expand All @@ -199,30 +182,6 @@ func (opts *options) extractClaims() error {
return nil
}

// If the public keys are not cached or expired, it uses OIDC discovery to get the public keys.
// New public keys are written to the file specified by the --public-key-path flag.
// If new public keys are fetched, it sets ado environment variable to true.

// loadPublicKeysFromLocal loads the public keys from the file specified by the --public-key-path flag.
// example implementation https://gist.github.com/nilsmagnus/199d56ce849b83bdd7df165b25cb2f56
// func (opts *options) loadPublicKeysFromLocal() error {
//
// }
//

// savePublicKeysFromRemote fetches the public keys from the OIDC discovery endpoint.
// It writes the public keys to the file specified by the --public-key-path flag.
// It sets the environment variable specified by --new-public-keys-var-name to true to indicate that new public keys are fetched.
// func (opts *options) savePublicKeysFromRemote(issuer string) error {
//
// }

// setAdoEnvVar sets the Azure DevOps pipeline environment variable to true.
// Environment variable name is specified by --new-public-keys-var-name flag.
// func (opts *options) setAdoEnvVar() error {
//
// }

func main() {
if err := rootCmd.Execute(); err != nil {
panic(err)
Expand Down
122 changes: 91 additions & 31 deletions pkg/oidc/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@ type VerifierConfig struct {
oidc.Config
}

// String returns the string representation of the VerifierConfig.
// It's used for logging purposes.
func (config *VerifierConfig) String() string {
return fmt.Sprintf("ClientID: %s, SkipClientIDCheck: %t, SkipExpiryCheck: %t, SkipIssuerCheck: %t, InsecureSkipSignatureCheck: %t, SupportedSigningAlgs: %v, Now: %T",
config.ClientID, config.SkipClientIDCheck, config.SkipExpiryCheck, config.SkipIssuerCheck, config.InsecureSkipSignatureCheck, config.SupportedSigningAlgs, config.Now)
}

// TokenProcessor is responsible for processing the token.
type TokenProcessor struct {
rawToken string
Expand Down Expand Up @@ -134,6 +141,16 @@ type GithubClaims struct {
// VerifierConfigOption is a function that modifies the VerifierConfig.
type VerifierConfigOption func(*VerifierConfig) error

// SkipExpiryCheck set verifier config to skip the token expiration check.
// It's used to allow longer token expiration time.
// The Verifier must run its own expiration check for extended expiration time.
func SkipExpiryCheck() VerifierConfigOption {
return func(config *VerifierConfig) error {
config.SkipExpiryCheck = true
return nil
}
}

// Provider is the OIDC provider.
// It abstracts the provider implementation.
// VerifierProvider provides the OIDC token verifier.
Expand All @@ -145,16 +162,31 @@ type Provider struct {
// Token is the OIDC token.
// It abstracts the IDToken implementation.
type Token struct {
Token ClaimsReader
Token ClaimsReader
IssuedAt time.Time
}

// TokenVerifier is the OIDC token verifier.
// It abstracts the Verifier implementation.
type TokenVerifier struct {
Verifier Verifier
Logger LoggerInterface
Config VerifierConfig
ExpirationTimeMinutes int
Verifier Verifier
Logger LoggerInterface
}

// TokenVerifierOption is a function that modifies the TokenVerifier.
type TokenVerifierOption func(verifier *TokenVerifier) error

// WithExtendedExpiration sets the custom expiration time used by the TokenVerifier.
func WithExtendedExpiration(expirationTimeMinutes int) TokenVerifierOption {
return func(verifier *TokenVerifier) error {
verifier.ExpirationTimeMinutes = expirationTimeMinutes
return nil
}
}

// maskToken masks the token value. It's used for debug logging purposes.
func maskToken(token string) string {
if len(token) < 15 {
return "********"
Expand Down Expand Up @@ -205,42 +237,57 @@ func NewVerifierConfig(logger LoggerInterface, clientID string, options ...Verif

// Verify verifies the raw OIDC token.
// It returns a Token struct which contains the verified token if successful.
// Verify allow checking extended expiration time for the token.
// It runs the token expiration check if the upstream verifier is configured to skip it.
func (tokenVerifier *TokenVerifier) Verify(ctx context.Context, rawToken string) (*Token, error) {
logger := tokenVerifier.Logger
logger.Debugw("Verifying token")
logger.Debugw("Got raw token value", "rawToken", maskToken(rawToken))
logger.Debugw("verifying token", "rawToken", maskToken(rawToken))
idToken, err := tokenVerifier.Verifier.Verify(ctx, rawToken)
if err != nil {
token := Token{}
return &token, fmt.Errorf("failed to verify token: %w", err)
return nil, fmt.Errorf("failed to verify token: %w", err)
}
logger.Debugw("Token verified successfully")
logger.Debugw("upstream verifier checks finished")
token := Token{
Token: idToken,
Token: idToken,
IssuedAt: idToken.IssuedAt,
}
logger.Debugw("checking if upstream verifier is configured to skip token expiration check", "SkipExpiryCheck", tokenVerifier.Config.SkipExpiryCheck)
if tokenVerifier.Config.SkipExpiryCheck {
logger.Debugw("upstream verifier configured to skip token expiration check, running our own check", "expirationTimeMinutes", tokenVerifier.ExpirationTimeMinutes, "tokenIssuedAt", token.IssuedAt)
err = token.IsTokenExpired(logger, tokenVerifier.ExpirationTimeMinutes)
logger.Debugw("finished token expiration check")
}
if err != nil {
return nil, fmt.Errorf("failed to verify token: %w", err)
}
logger.Debugw("token verified successfully")
return &token, nil
}

// VerifyExtendedExpiration checks the OIDC token expiration timestamp against the provided expiration time.
// It allows to accept tokens after the token original expiration time elapsed.
// The other aspects of the token must be verified separately with expiration check disabled.
func (tokenVerifier *TokenVerifier) VerifyExtendedExpiration(expirationTimestamp time.Time, gracePeriodMinutes int) error {
logger := tokenVerifier.Logger
logger.Debugw("Verifying token expiration time", "expirationTimestamp", expirationTimestamp, "gracePeriodMinutes", gracePeriodMinutes)
// IsTokenExpired checks the OIDC token expiration timestamp against the provided expiration time.
// It allows accepting tokens after the token original expiration time elapsed.
// The other aspects of the token must be verified separately with the expiration check disabled.
func (token *Token) IsTokenExpired(logger LoggerInterface, expirationTimeMinutes int) error {
logger.Debugw("verifying token expiration time", "tokenIssuedAt", token.IssuedAt, "expirationTimeMinutes", expirationTimeMinutes)
now := time.Now()
// check if expirationTimestamp is in the future
if expirationTimestamp.After(now) {
return fmt.Errorf("token expiration time is in the future: %v", expirationTimestamp)
if token.IssuedAt.After(now) {
return fmt.Errorf("token issued in the future, tokenIssuedAt: %v, now: %v", token.IssuedAt, now)
}
elapsed := now.Sub(expirationTimestamp)
gracePeriod := time.Minute
if elapsed <= gracePeriod {
logger.Debugw("token issued in the past")
expirationTime := token.IssuedAt.Add(time.Minute * time.Duration(expirationTimeMinutes))
logger.Debugw("computed expiration time", "expirationTime", expirationTime)
if expirationTime.After(now) || expirationTime.Equal(now) {
logger.Debugw("token not expired")
return nil
}
return fmt.Errorf("token expired more than %v ago", gracePeriod)
return fmt.Errorf("token expired, tokenIssuedAt: %v, expired at %v", token.IssuedAt, expirationTime)
}

// Claims gets the claims from the token and unmarshal them into the provided claims struct.
// TODO: Should we have a tests for this method?
//
// We can test for returned error.
// We can test the claims struct is populated with the expected values.
func (token *Token) Claims(claims interface{}) error {
return token.Token.Claims(claims)
}
Expand Down Expand Up @@ -284,15 +331,26 @@ func NewProviderFromDiscovery(ctx context.Context, logger LoggerInterface, issue

// NewVerifier creates a new TokenVerifier for provider.
// It returns a TokenVerifier struct containing the new Verifier if successful.
func (provider *Provider) NewVerifier(logger LoggerInterface, verifierConfig VerifierConfig) TokenVerifier {
func (provider *Provider) NewVerifier(logger LoggerInterface, verifierConfig VerifierConfig, options ...TokenVerifierOption) (TokenVerifier, error) {
logger.Debugw("Creating new verifier with config", "config", fmt.Sprintf("%+v", verifierConfig))
verifier := provider.VerifierProvider.Verifier(&verifierConfig.Config)
tokenVerifier := TokenVerifier{
Config: verifierConfig,
Verifier: verifier,
Logger: logger,
}
if len(options) > 0 {
logger.Debugw("Applying TokenVerifierOptions")
for _, option := range options {
err := option(&tokenVerifier)
if err != nil {
return TokenVerifier{}, fmt.Errorf("failed to apply TokenVerifierOption: %w", err)
}
}
logger.Debugw("Applied all TokenVerifierOptions")
}
logger.Debugw("Created new verifier")
return tokenVerifier
return tokenVerifier, nil
}

// NewTokenProcessor creates a new TokenProcessor for trusted issuers.
Expand Down Expand Up @@ -339,14 +397,16 @@ func NewTokenProcessor(
tokenProcessor.issuer = trustedIssuer
logger.Debugw("Added trusted issuer to TokenProcessor", "issuer", tokenProcessor.issuer)

logger.Debugw("Applying TokenProcessorOptions")
for _, option := range options {
err := option(&tokenProcessor)
if err != nil {
return TokenProcessor{}, fmt.Errorf("failed to apply TokenProcessorOption: %w", err)
if len(options) > 0 {
logger.Debugw("Applying TokenProcessorOptions")
for _, option := range options {
err := option(&tokenProcessor)
if err != nil {
return TokenProcessor{}, fmt.Errorf("failed to apply TokenProcessorOption: %w", err)
}
}
logger.Debugw("Applied all TokenProcessorOptions")
}
logger.Debugw("Applied all TokenProcessorOptions")

logger.Debugw("Created token processor", "issuer", tokenProcessor.issuer)
return tokenProcessor, nil
Expand Down Expand Up @@ -396,8 +456,8 @@ func (tokenProcessor *TokenProcessor) isTrustedIssuer(issuer string, trustedIssu
return Issuer{}, fmt.Errorf("issuer %s is not trusted", issuer)
}

// TODO(dekiel): This should return Issuer struct or has a different name.
// Issuer returns the issuer of the token.
// TODO(dekiel): This should return Issuer struct or has a different name.
func (tokenProcessor *TokenProcessor) Issuer() string {
return tokenProcessor.issuer.IssuerURL
}
Expand Down
Loading

0 comments on commit 82cd35f

Please sign in to comment.