Question About LRU Cache Key Implementation

[HsinHeng]

Hi All, Thank you to contribute this awesome libs.

I have found that cache is use hash(token) as key, not direct to use token as key.
hash(token) spend a little time complexity, can we directly use token as key?

[simoneb]

@HsinHeng have you tested what kind of performance degrade the hashing is introducing?

[HsinHeng]

@HsinHeng have you tested what kind of performance degrade the hashing is introducing?

I create a simple script within 3 cases:

1. verify
2. verify cache with hash key
3. verify cache without hash key: i warp verify. i don't want to do monkey patch.

const {
	createVerifier,
	createSigner,
} = require('fast-jwt');
const LruCache = require('mnemonist/lru-cache');

const KEY = Buffer.from('c97SjV4RqgCtpeyuFBJEpe8qobHJ2suWAxgbGjLYP3g=', 'base64');
const ALGORITHM = 'HS256';
const PAYLOAD = {
	hello: 'world',
};
const TOKEN = createSigner({
	key: KEY,
	algorithm: ALGORITHM,
})(PAYLOAD);

const RUN = 1000000;

function verify(token) {
	const verify = createVerifier({
		key: KEY,
		algorithms: [ALGORITHM],
		complete: false,
		cache: false,
	});

	const startedAt = Date.now();

	for (let i = 0; i < RUN; i++) {
		verify(token);
	}

	const finishedAt = Date.now();
	const seconds = (finishedAt - startedAt) / 1000;

	console.log(`------- verify:                            ${parseInt(RUN / seconds)} op/sec --------`);
}

function verifyCache(token) {
	const verify = createVerifier({
		key: KEY,
		algorithms: [ALGORITHM],
		complete: false,
		cache: true,
	});

	const startedAt = Date.now();

	for (let i = 0; i < RUN; i++) {
		verify(token);
	}

	const finishedAt = Date.now();
	const seconds = (finishedAt - startedAt) / 1000;

	console.log(`------- verify (cache):                    ${parseInt(RUN / seconds)} op/sec --------`);
}

function verifyCacheWithoutHashKey(token) {
	const cache = new LruCache(1000);

	const verify = createVerifier({
		key: KEY,
		algorithms: [ALGORITHM],
		complete: false,
		cache: false,
	});

	const wrapVerify = function (token) {
		const result = cache.get(token);

		if (result !== undefined) {
			const now = Date.now();
			const {
				startedAt,
				expiredAt,
				payload,
			} = result;

			if (now < startedAt) {
				throw new Error(`The token is valid at ${startedAt}`);
			}

			if (now >= expiredAt) {
				throw new Error(`The token is expired at ${expiredAt}`);
			}

			return payload;
		}

		const payload = verify(token);

		const {
			iat,
			exp,
			nbf,
		} = payload;

		let startedAt = 0;

		if (typeof nbf === 'number') {
			startedAt = new Date(nbf * 1000).getTime();
		} else {
			startedAt = new Date(iat * 1000).getTime();
		}

		let expiredAt = 0;

		if (typeof exp === 'number') {
			expiredAt = new Date(exp * 1000).getTime();
		} else {
			expiredAt = new Date('9999-12-31').getTime();
		}

		cache.set(token, { payload, startedAt, expiredAt });

		return payload;
	};

	const startedAt = Date.now();

	for (let i = 0; i < RUN; i++) {
		wrapVerify(token);
	}

	const finishedAt = Date.now();
	const seconds = (finishedAt - startedAt) / 1000;

	console.log(`------- verify (cache without hash key): ${parseInt(RUN / seconds)} op/sec --------`);
}

verify(TOKEN);
verifyCache(TOKEN);
verifyCacheWithoutHashKey(TOKEN);

The benchmark in my mac:

------- verify:                            204457 op/sec --------
------- verify (cache):                    481927 op/sec --------
------- verify (cache without hash key): 17857142 op/sec --------

As I know, crypto is cpu-intensive, the result is expected.
In my opinion, JWT already contain hash value, I think hash(token) do twice hash a little wired..

The other sides, we care about security & token collision.

1. Security: In my opinion, sensitive data in server-side local memory is ok.
2. Token Collision: If token collision by using diff key/algorithm/payload, I think it's ok. Identical token means payload is identical (base64).

Anyway, thank you for your time.
Please let me know your concerns or design.

[simoneb]

Thanks for the detailed feedback, much appreciated!

Ok so, based on your benchmarks above, it definitely does look like getting rid of the hashing would definitely improve the performance, so I'm happy to do that.

We discussed it offline with @ilteoood and what we propose to do is allowing to override the cache key generation algorithm, but still defaulting to the existing one, primarily to avoid breaking changes and by staying conservative in terms of security and collision.

Would that work for you? Basically, if we do this, what you'd do in order to skip the hashing would be to pass the identity function to the cache key generator option.

[HsinHeng]

Thanks for the detailed feedback, much appreciated!

Ok so, based on your benchmarks above, it definitely does look like getting rid of the hashing would definitely improve the performance, so I'm happy to do that.

We discussed it offline with @ilteoood and what we propose to do is allowing to override the cache key generation algorithm, but still defaulting to the existing one, primarily to avoid breaking changes and by staying conservative in terms of security and collision.

Would that work for you? Basically, if we do this, what you'd do in order to skip the hashing would be to pass the identity function to the cache key generator option.

Sometime I design jwt verify in node.js or gateway like kong ingress/nginx-plus based on different products.
When I design jwt verify in gateway, my node.js app just only do jwt decode.

Actually I am looking for decode cache.😆 (At least to do twice JSON.parse)

But I am inspired of your libs, that's awesome.
It's useful to me. Thanks a lot.

[github-actions]

🎉 This issue has been resolved in version 4.0.6 🎉

The release is available on:

• npm package
• GitHub release

Your optic bot 📦🚀