lua-resty-openidc
lua-resty-openidc copied to clipboard
Add JWE support
Add JWE support that is missing currently, and fix a bug with userinfo
Thanks @chrisFrodo - as you've probably realized this is not the most active project right now. I'm sorry for that.
I've got a few comments and questions, though.
-
string_starts
is never used - The way you've added support for userinfo endpoints returning JWTs means
openidc_parse_json_response
now accepts JWTs for much more than just the userinfo endpoint. This probably should better be conditional. -
openidc_load_jwt_and_verify_crypto
already contains code to split the JWT to handle the case where the none alg has been used (only two parts of a JWT). I believe it would be cleaner and easier if we merged JWE support with the code just above - and likely we could then stick tomatch
and destructuring. - please also update the README with an example as a minimal way of documenting how to configure things when JWEs are used
- why would we want to restrict this to RSA - and only with a very specific key length and only one of the padding alternatives? Wouldn't the same work for other key sizes or even when using AES as well?
- I would love to have tests for this. I know the way testing works is a bit convoluted and it likely takes some effort to make the test user info endpoint return a JWT but most stuff should be there already
- please add yourself to the AUTHORS file
Hi @bodewig ! I'm sorry for answering this late your questions about this PR, I might have been quite busy on other projects.
Following your suggestion I've made some modification on my previous work :
- Unused function have been removed
- Added a minimal example of the JWE related config options in the
README.md
file - Moved the JWS parsing of the userinfo to the
openidc.call_userinfo_endpoint()
function - Rework the
openidc_load_jwt_and_verify_crypto()
function to use thestring.match()
function. I had to modify the logic of booth the JWS and JWT handling so each case can be handled using the same extracted token parts. I may have added too much comments though... - Included new unit tests for this PR.
About your question about the cryptographic limitations, the fun begins !
These limitations are mainly linked to the lua-resty-jwt
library and its own cryptographic constrains on decryption.
The current version of the library (v0.2.3
) only support the following settings for JWE tokens :
-
Key management algorithm (ie : "alg") :
- RSA-OAEP-256
- DIR (RFC non-compliant value) :
This value is used to tell the
parse_jwe()
function is used to directly a pre-shared secret to derivate the decryption key
-
Content encryption algorithm (ie : "enc") :
- A256GCM
- A128CBC_HS256
- A256CBC_HS256
So, as described in the OpenID Connect CORE spec, the jwt lib is not complient on the encryption/decryption aspect. It is in fact lacking the support of the RSA1_5
algorithm and should allow non signed version of the CBC mode encryption algorithms.
Here are a list of the algorithm that should be supported, the bold ones being mandatory :
alg | enc |
---|---|
RSA1_5 | A128CBC |
RSA-OAEP | A256CBC |
EDCH-ES | A128GCM |
A128KW | A256GCM |
A256KW | |
A128GCM | |
A256GCM |
Fun fact : the HMAC signature of content encrypted with CBC mode is ment be optional
That means some modifications will have to be done on the lua-resty-jwt
library in order to meet the minimun requirement of the OpenID Connect spec. I'll try to work on it this month.
Hope these answers helped you!
Thanks a lot @chrisFrodo , this looks great
apart from the small comment I left inline I wonder whether we need to do something about elliptic curves as well - see also #457 .
The EC MR makes me wonder whether it is a good idea to encode lua-resty-jwt's current limitations in lua-resty-openidc as it will always require a new release here if lua-resty-jwt learns new tricks that may be relevant to users of the library.