Storageless don't prevent replay attacks!
jensklose opened this issue · 11 comments
Attention! IMO, your approache don't work like promised.
What is the problem?
The obvious solution against replay attacks: When a request comes in, you check a token value, check that it is the current value, and if it is, process it, and then change the current value.
If you share the current value with the client without some 3th party storage, then the whole request is ready for playing again and again. All of your servers will validate the JWT and accept the included token as current.
Your solution is a JWT middleware.
This should work. The documentation promises that this is a substitute for server side storage. This is dangerous.
Any other servers can do this also with traditional sessions by just impersonating the user.
If an action is not supposed to be repeated, a separate flag should be set in place in the storage specific to this feature.
Could you please check the limitations docs and see if the scenario matches?
Sessions cannot be invalidated
This is a hint for experienced users. But your CSRF tokens don't be save.
Storing information such as the user identifier, the user data, CSRF tokens ...
If I could capture your "100 € submit" to an API with storageless session ... ;)
I like the real intention in your work. A PSR7 middleware with an easy api for JWT as Cookie solution. But I'm confused by the name and documentation.
Here another example:
when in a multi-server setup, you may allow read-only access to servers that only have access to public keys, while writes are limited to servers that have access to private keys
I think that this is a pitfall. What do you mean with "public" and "private"?
# pseudoCode
jwt(makeEncryptedPayload(pubKey), makeSignature(privateKey))
In this case your publicOnly servers can never validate the JWT. I'm wrong?
If I could capture your "100 € submit" to an API with storageless session ... ;)
This kind of stuff is extremely risky and is done with idempotent flags that prevent double submission.
But I'm confused by the name and documentation.
I'm all in for improving it! In general, this package covers well over the 90% use-case scenario, which is authentication and CSRF token generation
I think that this is a pitfall. What do you mean with "public" and "private"?
# pseudoCode jwt(makeEncryptedPayload(pubKey), makeSignature(privateKey))In this case your publicOnly servers can never validate the JWT. I'm wrong?
Correct, but the tokens are not encrypted in this library. All what consumers would do is validate the signature, which is done with a published public key. Token encryption is out of scope for now, and already goes over the 90% scenario (seriously, don't store confidential information in the token, use a vault!)
Does that clarify your doubt on this bit? How can we improve the docs about it?
@jensklose isn't that covered on https://github.com/psr7-sessions/storageless/blob/master/docs/limitations.md#sessions-cannot-be-invalidated?
I mean, there're ways to revoke tokens and don't accept a token which is no longer valid (as in doesn't contain the last state of the session) but that requires a storage system - at least a cache - to either create a whitelist or a blacklist and validate the token accordingly.
This library means to be 100% storageless and the users should use it accepting all the limitations it brings. Do you have any suggestion on how to make that clearer?
@lcobucci I know that I was not involved in the original discussion, but let me make a suggestion:
I think a lot of people coming to JWT does not (carefully) read the "cons" section cause all the hype around it. So I would've taken a high road and make a life-like examples on how that may translate to real-life scenarios and that's what I came up with:
"All-inclusive hotel" (you can change it to nightclub or whatever based on your audience):
- You check in and show your ID at the check desk (authenticate yourself)
- You are given a stamp/buckle/etc (that is JWT token) that proves you have access to services
- At this point no one cares about your ID, only the buckle
- Buckles cannot be distinguished from one another (they look the same)
- Therefore cannot be stored, and, subsequently invalidated
- Cannot be forged (to some extend but good enough for example)
- You cannot "break" them (that is unlock and put on another person)
I know the example is limited and not even 3/4 accurate, but it works for me and people I talk to and makes it a lot either to explain some implications
That's the point. People like me, looking for a session handler and then come to this useful piece of code, but there is something different in it as promised.
Again
Storing information such as ... CSRF tokens ...
CSRF tokens don't work. Without a storage your token has an unlimited validity. Maximum limited by the expiration time when set.
Don't call it session. Don't compare it to session. It is JWT!
Suggestion: Rework the whole README to better exposing the JWT character of the module. Just give an indication of the storageless variant in some applications.
Disclaimer: I know @jensklose personally.
I think the point Jens brings up is that this particular session library states that it is usable for replacing traditional PHP sessions, but eventually fails to deliver. There is a dedicated file describing all the shortcomings of this library, but this is somehow hidden and has to be discovered.
I suppose you are aware of the problems with using JWT for session data. I'd like to add two more links that state this one more time: http://cryto.net/~joepie91/blog/2016/06/13/stop-using-jwt-for-sessions/ and http://cryto.net/~joepie91/blog/2016/06/19/stop-using-jwt-for-sessions-part-2-why-your-solution-doesnt-work/
I won't tell you to delete this library because it does things some people don't like or consider dangerous.
However my point is that the untrained developer will probably miss the file describing the problems. When you come to the front page, you are greeted with a very nice README that first tells you how to install and use it, then explains that ext/session has "huge" problems (IMO this exaggerates the problems way too much, this part scares devs into not using PHP sessions). After that it explains what this library does, and lists a number of advantages. The known limitations (or disadvantages) are not listed in this document, but hidden behind a link. There is no "security considerations" section in the README or in the limitations document, and the way JWT is used to store data is never even linked to having security issues.
The fact that all data stored in the JWT is publicly accessible is a security issue.
The fact that you cannot enforce updates to the JWT session data is a security issue.
The fact that all JWT sessions ever issued can never be revoked, but will continue to grant access to the application (until they expire) is a security issue (and yes, I have seen that you are using a mechanism to expire tokens and refresh them - you still cannot "log out", tokens keep being valid).
The technical limitations like finite amount of data being stored, a certain computational overhead validating the token itself, or the problems with concurrent requests changing the cookie token are no security issues, but technical limitations that I would consider putting into that existing document.
To be fair, the impact of the security issue part depends on what kind of application makes use of this library. But the security problems should be mentioned more prominently on the front page, to make developers really consider if the supposed benefit of being independent from a persistent storage outweighs the problems of having to deal with JWT "sessions".
See also: #22 #57 (comment) #38
To be fair, the impact of the security issue part depends on what kind of application makes use of this library.
Which is what the limitations state clearly. What needs to happen here is:
- better documentation
- clarifying use-case scenarios
I already use this library in production systems that use CSRF and authentication. We already figured out the mitigation scenarios for breaches (where a breach would be a leaked token), and besides that, the "log me out from all computers" is not something that most apps that I build have (scope of leaked session or authentication data is too small, security-wise).
Therefore I'm going to suggest to stop the discussion on this thread, and instead suggest patches to the documentation.
And also, repeating myself: this is a general purpose session, where general purpose follows the Pareto principle.
Closing here meanwhile - this is solved and already documented in known limitations. Note that since 4.0.0, SameSite=Lax or SameSite=Strict can be used (Lax and SecureOnly being the current cookie defaults), so replay attacks require MITM.
TL;DR: if you need to selectively clear user sessions, this library is indeed not for you, and you should indeed use a server-side storage.