diff --git a/registry/auth/token/token.go b/registry/auth/token/token.go index 2598f362a..bd6b8c6fd 100644 --- a/registry/auth/token/token.go +++ b/registry/auth/token/token.go @@ -20,6 +20,9 @@ const ( // TokenSeparator is the value which separates the header, claims, and // signature in the compact serialization of a JSON Web Token. TokenSeparator = "." + // Leeway is the Duration that will be added to NBF and EXP claim + // checks to account for clock skew as per https://tools.ietf.org/html/rfc7519#section-4.1.5 + Leeway = 60 * time.Second ) // Errors used by token parsing and verification. @@ -143,9 +146,17 @@ func (t *Token) Verify(verifyOpts VerifyOptions) error { } // Verify that the token is currently usable and not expired. - currentUnixTime := time.Now().Unix() - if !(t.Claims.NotBefore <= currentUnixTime && currentUnixTime <= t.Claims.Expiration) { - log.Errorf("token not to be used before %d or after %d - currently %d", t.Claims.NotBefore, t.Claims.Expiration, currentUnixTime) + currentTime := time.Now() + + ExpWithLeeway := time.Unix(t.Claims.Expiration, 0).Add(Leeway) + if currentTime.After(ExpWithLeeway) { + log.Errorf("token not to be used after %s - currently %s", ExpWithLeeway, currentTime) + return ErrInvalidToken + } + + NotBeforeWithLeeway := time.Unix(t.Claims.NotBefore, 0).Add(-Leeway) + if currentTime.Before(NotBeforeWithLeeway) { + log.Errorf("token not to be used before %s - currently %s", NotBeforeWithLeeway, currentTime) return ErrInvalidToken } diff --git a/registry/auth/token/token_test.go b/registry/auth/token/token_test.go index 827dbbd75..af862df7f 100644 --- a/registry/auth/token/token_test.go +++ b/registry/auth/token/token_test.go @@ -91,7 +91,7 @@ func makeTrustedKeyMap(rootKeys []libtrust.PrivateKey) map[string]libtrust.Publi return trustedKeys } -func makeTestToken(issuer, audience string, access []*ResourceActions, rootKey libtrust.PrivateKey, depth int) (*Token, error) { +func makeTestToken(issuer, audience string, access []*ResourceActions, rootKey libtrust.PrivateKey, depth int, now time.Time, exp time.Time) (*Token, error) { signingKey, err := makeSigningKeyWithChain(rootKey, depth) if err != nil { return nil, fmt.Errorf("unable to make signing key with chain: %s", err) @@ -109,8 +109,6 @@ func makeTestToken(issuer, audience string, access []*ResourceActions, rootKey l RawJWK: &rawJWK, } - now := time.Now() - randomBytes := make([]byte, 15) if _, err = rand.Read(randomBytes); err != nil { return nil, fmt.Errorf("unable to read random bytes for jwt id: %s", err) @@ -120,7 +118,7 @@ func makeTestToken(issuer, audience string, access []*ResourceActions, rootKey l Issuer: issuer, Subject: "foo", Audience: audience, - Expiration: now.Add(5 * time.Minute).Unix(), + Expiration: exp.Unix(), NotBefore: now.Unix(), IssuedAt: now.Unix(), JWTID: base64.URLEncoding.EncodeToString(randomBytes), @@ -188,7 +186,7 @@ func TestTokenVerify(t *testing.T) { tokens := make([]*Token, 0, numTokens) for i := 0; i < numTokens; i++ { - token, err := makeTestToken(issuer, audience, access, rootKeys[i], i) + token, err := makeTestToken(issuer, audience, access, rootKeys[i], i, time.Now(), time.Now().Add(5*time.Minute)) if err != nil { t.Fatal(err) } @@ -209,6 +207,78 @@ func TestTokenVerify(t *testing.T) { } } +// This tests that we don't fail tokens with nbf within +// the defined leeway in seconds +func TestLeeway(t *testing.T) { + var ( + issuer = "test-issuer" + audience = "test-audience" + access = []*ResourceActions{ + { + Type: "repository", + Name: "foo/bar", + Actions: []string{"pull", "push"}, + }, + } + ) + + rootKeys, err := makeRootKeys(1) + if err != nil { + t.Fatal(err) + } + + trustedKeys := makeTrustedKeyMap(rootKeys) + + verifyOps := VerifyOptions{ + TrustedIssuers: []string{issuer}, + AcceptedAudiences: []string{audience}, + Roots: nil, + TrustedKeys: trustedKeys, + } + + // nbf verification should pass within leeway + futureNow := time.Now().Add(time.Duration(5) * time.Second) + token, err := makeTestToken(issuer, audience, access, rootKeys[0], 0, futureNow, futureNow.Add(5*time.Minute)) + if err != nil { + t.Fatal(err) + } + + if err := token.Verify(verifyOps); err != nil { + t.Fatal(err) + } + + // nbf verification should fail with a skew larger than leeway + futureNow = time.Now().Add(time.Duration(61) * time.Second) + token, err = makeTestToken(issuer, audience, access, rootKeys[0], 0, futureNow, futureNow.Add(5*time.Minute)) + if err != nil { + t.Fatal(err) + } + + if err = token.Verify(verifyOps); err == nil { + t.Fatal("Verification should fail for token with nbf in the future outside leeway") + } + + // exp verification should pass within leeway + token, err = makeTestToken(issuer, audience, access, rootKeys[0], 0, time.Now(), time.Now().Add(-59*time.Second)) + if err != nil { + t.Fatal(err) + } + + if err = token.Verify(verifyOps); err != nil { + t.Fatal(err) + } + + // exp verification should fail with a skew larger than leeway + token, err = makeTestToken(issuer, audience, access, rootKeys[0], 0, time.Now(), time.Now().Add(-60*time.Second)) + if err != nil { + t.Fatal(err) + } + + if err = token.Verify(verifyOps); err == nil { + t.Fatal("Verification should fail for token with exp in the future outside leeway") + } +} + func writeTempRootCerts(rootKeys []libtrust.PrivateKey) (filename string, err error) { rootCerts, err := makeRootCerts(rootKeys) if err != nil { @@ -307,7 +377,7 @@ func TestAccessController(t *testing.T) { Name: testAccess.Name, Actions: []string{testAccess.Action}, }}, - rootKeys[1], 1, // Everything is valid except the key which signed it. + rootKeys[1], 1, time.Now(), time.Now().Add(5*time.Minute), // Everything is valid except the key which signed it. ) if err != nil { t.Fatal(err) @@ -333,7 +403,7 @@ func TestAccessController(t *testing.T) { token, err = makeTestToken( issuer, service, []*ResourceActions{}, // No access specified. - rootKeys[0], 1, + rootKeys[0], 1, time.Now(), time.Now().Add(5*time.Minute), ) if err != nil { t.Fatal(err) @@ -363,7 +433,7 @@ func TestAccessController(t *testing.T) { Name: testAccess.Name, Actions: []string{testAccess.Action}, }}, - rootKeys[0], 1, + rootKeys[0], 1, time.Now(), time.Now().Add(5*time.Minute), ) if err != nil { t.Fatal(err)