SkyLothar / lua-resty-jwt

JWT For The Great Openresty
Apache License 2.0
513 stars 179 forks source link

openidc.lua:819: authenticate(): id_token 'HS256' signature verification failed, client: 10... #77

Open claudijd opened 6 years ago

claudijd commented 6 years ago

Ran into this error today, saw similar comments on https://github.com/SkyLothar/lua-resty-jwt/issues/72 as well as a release to fix this for RS256 here

My fix for this was to switch to RS and that worked just fine. I'm not that familiar with the guts on this repo, but figured I'd report this in the case it might be affecting others.

pamiel commented 6 years ago

Hi @claudijd

I'm not sure this is the exact same issue, as I think I found the root cause for RS265 signature verification failure referenced here https://github.com/SkyLothar/lua-resty-jwt/issues/72 and here https://github.com/zmartzone/lua-resty-openidc/issues/135.

The issue is indeed due to a bug in the lua-resty-openidc library you are also using: in rare cases, the public key is starting with the 0x80 byte, and the bug in lua-resty-openidc makes the DER (and then PEM) structure that is rebuilt based on the data obtained from the JWKS endpoint of the Identity Provider/OP be erroneous !

I’ve just submitted a PR to lua-resty-openidc to correct the issue: https://github.com/zmartzone/lua-resty-openidc/pull/153

However, it looks your issue is on HS256, which does not imply the same piece of code => not probably the same root cause :(

In the RS256 use case, the final error message is "too long"; is it the same for you in HS256?

claudijd commented 6 years ago

@pamiel I'm unfortunately not super familiar with the innerworkings of this so I'll apologize in advance. In my case I was integrating a Jenkins install with Auth0 and was using this library as a means of requiring authentication at nginx before any access was being provided to the app. It followed a similar pattern as described in this project.

However, when setting this up, I believe HS was the default. It worked in one instance (and still does), but when building out a new instance, it seemed to complain about signature verification. My work around was to switch to RS and that worked flawlessly. It's unfortunately not something I have a ton of time to dig into, but I did feel a duty to share this upstream in case others tripped on it and could provide a more intelligible account/assistance to sort this out for future HS users.

claudijd commented 6 years ago

whoops, sorry

gdestuynder commented 6 years ago

@pamiel the bug seems to still be there in lua-resty-openidc (1.5.4 which includes our pr) or lua-resty-jwt

openidc.lua:898: authenticate(): id_token 'HS256' signature verification failed,

using: lua-resty-openidc 1.5.4 lua-resty-jwt 0.1.11

The jwt_str if i log it seems ok (at https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L776) its just no longer ok once parsed, it looks like. I verified the secret passed for verification is also correct

Finally, loading the jwt_str manually in nodejs with jsonwebtoken verifies it successfully, so the payload looks correct

gdestuynder commented 6 years ago

after futher investigation it comes from how luarocks compiles the libraries. using opm will fix this. i didnt look at the exact problem though, but same versions, same files, no problem

zandbelt commented 6 years ago

For the record, the lua-resty-jwt README.md says:

Attention ❗️ the hmac lib used here is lua-resty-hmac, not the one in luarocks.

so it seems logical that it is related to the hmac.lua dependency.

gdestuynder commented 6 years ago

yep that seems right. @zandbelt I guess the readme in the lua-resty-openidc repos should swap luarocks instructions to use opm. I'm not really aware of the lua community discussions around this, but did notice that the openresty website mentions you should always use opm instead of luarocks now due to "issues in how luarocks packages modules", or something similar:

https://openresty.org/en/using-luarocks.html "WARNING! This page is deprecated. Use of LuaRocks with OpenResty is strongly discouraged since OpenResty provides its own package manager, OPM." and: https://github.com/3scale/apicast/issues/104

With that, I suspect this issue on lua-resty-jwt can then be closed again :)

zandbelt commented 6 years ago

lua-resty-jwt addresses this by including its own version of lua-resty-hmac: https://github.com/SkyLothar/lua-resty-jwt/blob/master/vendor/resty/hmac.lua in the rockspec: https://github.com/SkyLothar/lua-resty-jwt/blob/master/lua-resty-jwt-dev-0.rockspec#L26 which has version number 0.0.1.

But I have also experienced myself that this can be overwritten by an explicit separate manual install of lua-resty-hmac from luarocks (or perhaps it was already installed). Perhaps it is worth looking into that.

Other than that I'll swap instructions but I think usage of luarocks still exceeds opm by far.