-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: auth bypass #5
Conversation
@@ -284,6 +284,7 @@ local function parse_jwe(self, preshared_key, encoded_header, encoded_encrypted_ | |||
local iv = _M:jwt_decode(encoded_iv) | |||
local signature_or_tag = _M:jwt_decode(encoded_auth_tag) | |||
local basic_jwe = { | |||
typ = str_const.JWE, |
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.
Why do you add this, seems meanless
if alg == nil then | ||
jwt_obj[str_const.reason] = "No algorithm supplied" | ||
return jwt_obj | ||
end |
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.
we should not check like this, there is already corresponding judgement logic below
else
jwt_obj[str_const.reason] = "Unsupported algorithm " .. alg
end
We just fix the bug when alg == nil
else
jwt_obj[str_const.reason] = "Unsupported algorithm " .. (alg or "nil")
end
if jwt_obj.typ == str_const.JWE or (jwt_obj.typ == nil and (typ == str_const.JWE or jwt_obj.header.enc)) then | ||
return sign_jwe(self, secret_key, jwt_obj) | ||
end |
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.
The way to check if the obj is jwe is not correct. The jwe obj has five dot in the body, i think checking this is more appropriate
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.
And the function sign_jwe
is not correct too, you should refactor the function
@@ -322,6 +323,7 @@ local function parse_jwt(encoded_header, encoded_payload, signature) | |||
end | |||
|
|||
local basic_jwt = { | |||
typ = str_const.JWT, |
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.
ditto
This PR fixes the authentication bypass issue in cdbattags/lua-resty-jwt#61.