Why does HMACStrategy.Generate uses a lock?

mitar commented 4 months ago

Preflight checklist

[X] I could not find a solution in the existing issues, docs, nor discussions.
[X] I agree to follow this project's Code of Conduct.
[X] I have read and am following this repository's Contribution Guidelines.
[ ] I have joined the Ory Community Slack.
[ ] I am signed up to the Ory Security Patch Newsletter.

Ory Network Project

No response

Describe the bug

While reading the code, I noticed that HMACStrategy.Generate uses a mutex lock, but I do not get why it exists. No code there has any global or local state?

This was introduced in https://github.com/ory/fosite/commit/b4b9be5640c9d814b35f54b2c8621137364209ca, but I also do not get why.

Given that this is used a lot, I think mutex could be removed?

Reproducing the bug

N/A

Relevant log output

No response

Relevant configuration

No response

Version

latest master

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

No response

james-d-elliott commented 4 months ago

I suspect it may have been implemented at a time where there was a lot of information about it being unsafe for concurrent usage, which I believe from memory was false and it was based on a couple articles from prominent bloggers who misinterpreted the issues with math/rand's global Read func instead of crypto/rand's global Reader.

But I can't say for sure.

mitar commented 4 months ago

You mean RandomBytes? But then the lock could be in RandomBytes?

james-d-elliott commented 4 months ago

Yeah I'm not sure, I would have thought so too.. I can't see anything else unless the crypto module being used at the time needed it.

ory / fosite