jwt icon indicating copy to clipboard operation
jwt copied to clipboard

Token used before issued

Open JorritSalverda opened this issue 3 years ago • 11 comments

Having switched from github.com/dgrijalva/jwt-go to github.com/golang-jwt/jwt/v4 I still get token used before issued errors due to clock skew, even though @dgrijalva said this check would be removed from v4, in line with jwt spec. See https://github.com/golang-jwt/jwt/blob/3f50a786ff28f918ba8e7fb9183390b11483e755/claims.go#L63-L66

However I see that check is still executed at https://github.com/dgrijalva/jwt-go/issues/314#issuecomment-481789374. Is this dependent on https://github.com/golang-jwt/jwt/issues/16 getting resolved? Until that's resolved can the VerifyIssuedAt check just be dropped? Or any other suggested workarounds?

JorritSalverda avatar Aug 30 '21 07:08 JorritSalverda

Yes, this is somewhat dependent on #16. Meanwhile you can use jwt.TimeFunc to account for clock skew.

https://github.com/golang-jwt/jwt/blob/2ebb50f957d606de5909fcf9ed49f9af3bc35e97/token.go#L10-L13

oxisto avatar Aug 30 '21 08:08 oxisto

Thanks, I'll test with

jwt.TimeFunc = func() time.Time {
		return time.Now().UTC().Add(time.Second * 20)
	}

to see if it gets rid of these errors.

JorritSalverda avatar Aug 30 '21 08:08 JorritSalverda

Hey @JorritSalverda did that solve your issue?

@oxisto Were we still interested in https://github.com/golang-jwt/jwt/issues/16

mfridman avatar Sep 10 '21 21:09 mfridman

Hi @mfridman for some reason it didn't help and i resorted to jumping through a lot of hoops to ignore this type of validation error, see https://github.com/estafette/estafette-ci-api/blob/main/pkg/clients/bitbucketapi/client.go#L277-L309

Looking at my logs the now value is actually larger than iat, so have to figure out how to get the actual value for now used during evaluation. I think though it's probably only off by a couple of milliseconds so not sure why overriding the TimeFunc didn't do it's job.

JorritSalverda avatar Sep 13 '21 08:09 JorritSalverda

Thanks for the insight. I think introducing (configurable) leeway could help with these sorts of issues.

Side note, every-time I see folks having to resort binary operators to determine the error it makes me cringe, because it's so prone to error.

mfridman avatar Sep 13 '21 12:09 mfridman

Is the error "token used before issued" really needed?

RFC7519 says that:

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4 The "exp" (expiration time) claim identifies the expiration time on or after which the JWT MUST NOT be accepted for processing.

RFC7519 also says that:

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5 The "nbf" (not before) claim identifies the time before which the JWT MUST NOT be accepted for processing.

however, RFC7519 DOES NOT say that the "iat" (issued at) claim identifies the time before which the JWT MUST NOT be accepted for processing. it just says that:

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.6 The "iat" (issued at) claim identifies the time at which the JWT was issued. This claim can be used to determine the age of the JWT. Its value MUST be a number containing a NumericDate value. Use of this claim is OPTIONAL.

Some implementations of JWT have the maxAge option instead of the error "token used before issued".

https://github.com/auth0/node-jsonwebtoken/blob/74d5719bd03993fcf71e3b176621f133eb6138c0/verify.js#L199-L211

shogo82148 avatar Sep 27 '21 10:09 shogo82148

Is the error "token used before issued" really needed?

RFC7519 says that:

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4 The "exp" (expiration time) claim identifies the expiration time on or after which the JWT MUST NOT be accepted for processing.

RFC7519 also says that:

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5 The "nbf" (not before) claim identifies the time before which the JWT MUST NOT be accepted for processing.

however, RFC7519 DOES NOT say that the "iat" (issued at) claim identifies the time before which the JWT MUST NOT be accepted for processing. it just says that:

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.6 The "iat" (issued at) claim identifies the time at which the JWT was issued. This claim can be used to determine the age of the JWT. Its value MUST be a number containing a NumericDate value. Use of this claim is OPTIONAL.

Some implementations of JWT have the maxAge option instead of the error "token used before issued".

https://github.com/auth0/node-jsonwebtoken/blob/74d5719bd03993fcf71e3b176621f133eb6138c0/verify.js#L199-L211

Yup, it is not really necessary to enforce it according to the specification. That is why in the long run it would be good to implement #16.

oxisto avatar Sep 27 '21 11:09 oxisto

@JorritSalverda Could you have a look at #139 to see if that mitigates your problem? Once we have that merged in, it will also pave way to a backwards-compatible way of configuration the validation. This was sort of a chicken-egg problem and we choose to merge in the egg first.

oxisto avatar Feb 18 '22 09:02 oxisto

~Is there a fix or workaround for this issue I can use before the various PRs are merged? I'm not sure I completely understand how my local system issuing the token and then validating the token in a subsequent request is subject to clock skew, but really hoping to find a workaround or solution here.~ disregard. I'm still new here; updating to v4 resolved the issue.

rcmaples avatar Feb 26 '22 17:02 rcmaples

@oxisto : For v4, Do you think we can also add an option to enable the issued at validation (should be disabled by default) to #139 ?

Something along the lines of:

func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...validationOption) bool {
	validator := validator{}
	for _, o := range opts {
		o(&validator)
	}
	
	if validator.validateIAT {
		if c.IssuedAt == nil {
			return verifyIat(nil, cmp, req)
		}
		return verifyIat(&c.IssuedAt.Time, cmp, req)
	}
	
	return true
}

I don't think I'd want a leeway option for Issued at. I'd just want to turn it off. But, we can combine the two as well:

func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...validationOption) bool {
	validator := validator{}
	for _, o := range opts {
		o(&validator)
	}
	
	if validator.validateIAT {
		if c.IssuedAt == nil {
			return verifyIat(nil, cmp, req, validator.leeway)
		}
		return verifyIat(&c.IssuedAt.Time, cmp, req, validator.leeway)
	}
	
	return true
}

daniel-cohen avatar Mar 07 '22 04:03 daniel-cohen

@oxisto : For v4, Do you think we can also add an option to enable the issued at validation (should be disabled by default) to #139 ?

Something along the lines of:

func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...validationOption) bool {
	validator := validator{}
	for _, o := range opts {
		o(&validator)
	}
	
	if validator.validateIAT {
		if c.IssuedAt == nil {
			return verifyIat(nil, cmp, req)
		}
		return verifyIat(&c.IssuedAt.Time, cmp, req)
	}
	
	return true
}

I don't think I'd want a leeway option for Issued at. I'd just want to turn it off. But, we can combine the two as well:

func (c *RegisteredClaims) VerifyIssuedAt(cmp time.Time, req bool, opts ...validationOption) bool {
	validator := validator{}
	for _, o := range opts {
		o(&validator)
	}
	
	if validator.validateIAT {
		if c.IssuedAt == nil {
			return verifyIat(nil, cmp, req, validator.leeway)
		}
		return verifyIat(&c.IssuedAt.Time, cmp, req, validator.leeway)
	}
	
	return true
}

We are still waiting for some minor last fixes in that PR. Once this PR is merged in, we would welcome any additional PRs on this validation topic.

oxisto avatar Mar 07 '22 09:03 oxisto

I landed here after getting "token used before issued" from this library when consuming the OIDC token from GitHub Actions. I'm not sure what kind of skew there may be, but the receiving machine has a correct date/time AFAIK.

What's the best workaround for this?

alexellis avatar Oct 05 '22 17:10 alexellis

@alexellis I ran in to this problem earlier this week and solved it by making a custom Claims type like this:

type Claims struct {
	ValidFunc func() error
	jwt.StandardClaims
}

func (c Claims) Valid() error {
	if c.ValidFunc == nil {
		return c.StandardClaims.Valid()
	}
	return c.ValidFunc()
}

func main() {
	claims := Claims{}

	claims.ValidFunc = func() error {
		claims.IssuedAt = 0
		return claims.StandardClaims.Valid()
	}

	jwt.ParseWithClaims(tokenString, &claims, keyFunc)
}

The reason this works is that iat is not marked as required in StandardClaims Valid() function and if set to 0 it will be ignored. This way I don't have to reimplement all the other validations myself.

https://github.com/golang-jwt/jwt/blob/2ebb50f957d606de5909fcf9ed49f9af3bc35e97/claims.go#L122-L127

mdjarv avatar Oct 12 '22 06:10 mdjarv

@alexellis I ran in to this problem earlier this week and solved it by making a custom Claims type like this:

type Claims struct {
	ValidFunc func() error
	jwt.StandardClaims
}

func (c Claims) Valid() error {
	if c.ValidFunc == nil {
		return c.StandardClaims.Valid()
	}
	return c.ValidFunc()
}

func main() {
	claims := Claims{}

	claims.ValidFunc = func() error {
		claims.IssuedAt = 0
		return claims.StandardClaims.Valid()
	}

	jwt.ParseWithClaims(tokenString, &claims, keyFunc)
}

The reason this works is that iat is not marked as required in StandardClaims Valid() function and if set to 0 it will be ignored. This way I don't have to reimplement all the other validations myself.

https://github.com/golang-jwt/jwt/blob/2ebb50f957d606de5909fcf9ed49f9af3bc35e97/claims.go#L122-L127

While this works, I personally would not recommend that as this is going a route that will probably be changed in the upcoming v5 version. If you are interested, you can check out the branch of the PR mentioned above. It has support for skipping IAT / currently defaults to skipping IAT.

oxisto avatar Oct 12 '22 09:10 oxisto

Fixed by #234

oxisto avatar Feb 21 '23 18:02 oxisto