-
Notifications
You must be signed in to change notification settings - Fork 4.2k
First Draft for JWT Best Practices Doc #1182
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
base: master
Are you sure you want to change the base?
Conversation
* Ignores .idea files (pycharm ide) * Index updated by make generate-site * Added assets/JWTCSA as a place for assets and snippets * Added a JWT Cheat Sheet Doc * Fixed google_analytics in mkdocs.yaml * Added pymdownx plugins for: * Admonitions (`blocks.details`) * Code Snippets (`snippets`) * Tabbed Content (`tabbed`) * Pinned modern minimum versions on requirements.txt
I marked this PR as a draft so nobody does by mistake. |
I am super eager to see this fleshed out, can I help? Wanna meet and discuss? |
Honestly we probably should, but I'm super busy this week. I might have some time next. |
* Still not ready * Taking some feedback from our attempt to implement a jwt based auth scheme professionally. * Recommends jwt symmetrically signed with jwks certificates as it seems like it will be the best supported and offers an upgrade/portability path to OIDC.
Slow going but was able to get a little more work on this. Have a JWKS + JWT example in python in there atm. |
Hello, @chalbersma , just wanted to check where things stand on this or if you would like any assistance? I am familiar with JWT security and multiple programming languages, so I would be happy to assist if it wanted. |
Hello @EbonyAdder At the company I work for we were looking to adopt a JWT auth-based standard. I was going to use the lessons learned and example code from that to populate this best practices document. Unfortunately in the process of building, I think we're learning an unfortunate less for "naked" jwts authentication/authorization; that it might be best to use a "fuller" service mesh style system (think envoy) to manage these tokens and connections. This is still on my to-do list; I've just lost confidence that the approach recommended is still what should be recommended at this time. |
Can we revisit this? Here are some of the ASVS requirements for ASVS 5.0. V9.1 Token source and integrityThis section includes requirements that ensure that the token has been produced by a trusted party and that it has not been tampered with.
V9.2 Token contentBefore making security decisions based on the content of a self-contained token, it is necessary to validate that the token has been presented within its validity period and that it is meant for use by the receiving service and for the purpose for which it was presented. This is to avoid insecure cross-usage between different services or with different token types from the same issuer. Specific requirements for OAuth and OIDC are covered in the dedicated chapter.
|
|
||
## Issues | ||
|
||
### None Hashing Algorithm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None is not an "hashing" algorithm. It is an (not really) "authentication algorithm".
<!-- --8<-- [start:pyjwt] --> | ||
``` | ||
try: | ||
pyjwt.decode(encoded, key, algorithms=["HS256","ES256"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is weird to support HS256 (a MAC) and ES256 (a signature) at the same time. Either your key is a shared secret and you will support HS256 or this is a public key and you support ES256. I think ES256 is spurious here. This would be consistent with other examples.
import uuid | ||
from jwcrypto import jwk, jwt | ||
|
||
unique_kid = uuid.uuid4() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm always wondering why everyone keeps insisting on using UUID for this kind of things when you can just use a random token :) What about unique_kid = secrets.token_urlsafe(20)
? It is shorter than a uuid.uuid4()
but still has more entropy.
|
||
# Publish to Datastore in Multinode system | ||
|
||
this_token = jwt.JWT(header={"alg": "RS256"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a JWT but at the same time there is not expiration. There should probably at least be some expiration in the "default example"?
|
||
## Introduction | ||
|
||
Many applications use **JSON Web Tokens** (JWT) to allow the client to indicate its identity for further exchange after |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"to allow the client", I guess you mean the user. JWT can definitely be used to represent claims about the client, I think you are actually thinking more about the user.
#### Symptom | ||
|
||
This attack occurs when a token has been intercepted/stolen by an attacker and they use it to gain access to the system | ||
using targeted user identity. In part, this attack really can't be mitigated "in platform" as JWTs are generically stateless |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be mitigated using some proof-of-possession such as:
- sender-constrained JWT such as TLS-bound access tokens and DPoP access tokens;
- SD-JWT bound to a public key pair
Any thoughts on this, @chalbersma ? Also, there are new ASVS requirements on JWTs to consider. https://github.com/OWASP/ASVS/blob/master/5.0/en/0x18-V9-Tokens.md |
There is a lot going on here. Hey @randomstuff do you have desire to take this on and drive it home? I trust your talent from ASVS to finish this - even if its not complete, just a cheat will do - so long as its accurate. |
Here is the latest ASVS Content on tokens: V9.1 Token source and integrityThis section includes requirements that ensure that the token has been produced by a trusted party and that it has not been tampered with.
V9.2 Token contentBefore making security decisions based on the content of a self-contained token, it is necessary to validate that the token has been presented within its validity period and that it is meant for use by the receiving service and for the purpose for which it was presented. This is to avoid insecure cross-usage between different services or with different token types from the same issuer. Specific requirements for OAuth and OIDC are covered in the dedicated chapter.
|
### Verification | ||
|
||
Your application, absolutely should verify that the tokens it recieves are valid. You can do this with a symmetric scheme, where | ||
a secret between all parties out of band. For more on how to manage secrets please refer to the [secrets management cheat sheet](Secrets_Management_Cheat_Sheet.md). Or you can use an asymmetric key to signe your tokens. you have the option of signing your |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a [shared] secret between all parties out of band
It is dangerous to share a secret between more than two parties. I would say "between two parties".
blocks.details
)snippets
)tabbed
)This PR covers issue #1176 .
Please do not merge yet. This is a work in progress at best.