Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected Token Expiration #236

Closed
n-iwamoto opened this issue Jul 30, 2022 · 9 comments
Closed

Unexpected Token Expiration #236

n-iwamoto opened this issue Jul 30, 2022 · 9 comments
Labels

Comments

@n-iwamoto
Copy link

n-iwamoto commented Jul 30, 2022

Very occasionally, we are getting a "token expiration" error when validating a JWT token (the log times on the left column are in UTC). Is there anything obviously wrong that we're doing, or is there a possible issue here?

2022-07-29 23:04:54 (error) ValidateToken error: token expired
2022-07-29 23:04:54 (debug)ValidateToken error(tokenPayload): 
{
    "sub": "testaccount010@zzz.com",
    "aud": "zzz",
    "scope": "openid profile",
    "iss": "https:\/\/...",
    "exp": 1659136195,
    "iat": 1659135895,
    "client_id": zzz",
    "jti": "MjU4Y2JkYWQtZTNjMi00Yjc5LThhOTItMjIyOWU5NjNmN2I2LTYzZXhGQ25VbGxSUTBWdlYwWndmMzRncnBHTT0="
}
2022-07-29 23:04:54 (warning) accessToken validation failed; return error

The version of JWT-CPP we're using is from the tag v0.6.0. The following are the pertinent sections of code we're using:

// switch from default picojson to nlohmann
#define JWT_DISABLE_PICOJSON 

#include "jwt-cpp/traits/nlohmann-json/defaults.h"

using traits = jwt::traits::nlohmann_json;
using claim = jwt::basic_claim<traits>;
using json = nlohmann::json;

auto verifySSO = jwt::verify().allow_algorithm(jwt::algorithm::rs256(m_rsa_pub_key, "", "", "")).with_issuer(m_issuer);

bool JWTUtility::ValidateToken(jwt::verifier<jwt::default_clock, traits> verifier, std::string token) {
	string tokenPayload;

	try {
		auto decoded = jwt::decode(token);

		tokenPayload = decoded.get_payload();

		verifier.verify(decoded);	// verifier = verifySSO

		return true;
	}
	catch (exception& e) {
		g_logger.Log("ValidateToken error: " + string(e.what()), GlobalSettings::Error);
		g_logger.Log("ValidateToken error (tokenPayload): " + tokenPayload, GlobalSettings::Debug);
		return false;
	}
}
@Thalhammer
Copy link
Owner

Thalhammer commented Jul 30, 2022

I just checked the token in your log and it seems like the error isn't that the token is not valid anymore but instead it is not yet valid.
JWT's can contain upto three timestamps, with iat and exp being the most common.
iat This is the timestamp when the token was created. In your case the unix timestamp resolves to Friday, 29 July 2022 23:04:55 UTC.
nbf This (if present) indicates that the token should be accepted before a certain time, its not used here.
expThe is the expiry date. The token wont be accepted after this.
As you can see the token claims to be created ~1 second after it was checked, so it obviously doesn't pass the validation.
There can be a number of cases for this but the most common one would be a clock skew between the system time of the device creating the token and the device checking it.

The proper way to fix this would be to make sure the servers have matching time, however this is not always possible.
Another way is the leeway option on the verifier object.
When building the verifier you can use .leeway(number in seconds) to change the global default leeway, or one of .expires_at_leeway(number), .not_before_leeway(number and .issued_at_leeway(number) to change the leeway value only for a single claim. Leeway basically adds a bit of wiggle room to account for clock differences and transmission times.
For iat and nbf this means that the timestamp inside the token might be up to leeway seconds in the future and the token is still considered valid. For exp it works the other way and cases tokens to be accepted that would have already been expired.

If you can't guarantee synced clocks settings a couple seconds of leeway (given your small token lifetime I'd probably go with something like 5s) is a good idea.

Let me know if you need more information, or feel free to close the ticket if this solved your problem.
In case your company/application is public feel free to post details to #203 so we can inform you incase theres ever a security issue or similar.

@n-iwamoto
Copy link
Author

n-iwamoto commented Jul 30, 2022 via email

@n-iwamoto
Copy link
Author

Will close ticket, but since the problem is very intermittent, it's tough to test for (other than in a testing environment). However, it's very likely that the few other occurrences of this (no output of the JWT in log at the time) were for the same problem.

@Thalhammer
Copy link
Owner

Is there a "usual/common" number used for leeway as standard practice? I'm thinking 5-10 seconds?

Not really. In a perfect world you wouldn't need leeway at all, but saidly the real world isn't perfect. Usually you wanna keep the value fairly low, cause adding leeway makes tokens valid that technically aren't valid.

For me, it had one of the cleanest, and easiest to use syntax of the ones I looked at, as well as providing good standards coverage.

Happy to hear that. If you have any other issues or improvements/feature requests, feel free to open issues for them.

@n-iwamoto
Copy link
Author

Sounds good, thanks Dominik, decided to go w/10 seconds for now to see how it goes.

Will do on the suggestions, thanks again!

@n-iwamoto
Copy link
Author

Hi Dominik,

Would you happen to have an example of the verifier using a network time server vs the internal clock?

Thanks,
Neil

@Thalhammer
Copy link
Owner

Would you happen to have an example of the verifier using a network time server vs the internal clock?

I don't, cause normally you want the time of the entire system synced using something like NTP and not just a single part of the program.
You can however overwrite the clock used by the verifier by passing a template type to the verify function.
The clock type needs to implement an interface similar to this:

struct default_clock {
	date now() const;
};

Where date is a std::chrono::system_clock::time_point.
You could probably implement one that uses some kind of ntp library/code to get an accurate time, but I'd advise against it, cause it will probably cause you other problems in the future.

@n-iwamoto
Copy link
Author

Yeah, that was my preference as well; i.e. why just get one section of code to work and not the rest of the system. However, thought I'd ask just in case.

Thanks for the pointer

@Fransferdy
Copy link

I had the same issue and scratched my head until I went bald, until I got to read this page that is, thanks for the well made explanation, I would just suggest(if you haven't done it already), updating the error message from "token expired" in this case, to "token not yet valid(issue_date)", it would have helped me a ton figuring this problem by myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants