Skip to content
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

Token verify bypass #3

Open
lordnynex opened this issue Nov 28, 2015 · 2 comments
Open

Token verify bypass #3

lordnynex opened this issue Nov 28, 2015 · 2 comments

Comments

@lordnynex
Copy link

Hello,

Thank you very much for contributing this library. All of your modules are high quality.

This module is potentially vulnerable to an algorithm/key mismatch attack. In summary, if a token producer and consumer have a mutual understanding that they will sign/verify tokens using RSA and not HMAC, it is possible in some cases to generate forged tokens to bypass token verification. I've been reading around and it seems like people are getting around this by including the fingerprint of the RSA key in the header table when signing. The verifier then validates this fingerprint against the calculated fingerprint that it already has.

Here is a busted test for jwt_spec.lua that demonstrates the problem.

  it("is not vulnerable to algorithm/key mismatch attack", function()
    local keyPair = crypto.pkey.generate("rsa", 512)

    -- The known trusted key between priver and audience
    -- Distributed to audience that expects RSA signed tokens
    --
    -- Perhaps I'm running a service and use a separate auth
    -- provider. They've given me this public key for token
    -- validation. Or the public key is known publicly.
    --
    -- Pre-requisite for this issue
    known_trusted_public_key = keyPair:to_pem()

    -- Generate a legitimate token for our audience
    function service_sign_token(claims)
      local keys = {
        private = keyPair
      }

      return jwt.encode(claims, { alg = "RS256", keys = keys })
    end

    -- Verify rsa tokens from our provider. Because....
    -- I use a provider that only generates rsa tokens?
    --
    -- In the real world, this is likely not written in the
    -- same language as our signer, let alone the same lib.
    --
    -- This makes this test a bit contrived but only trying
    -- to demonstrate the flaw. In this case, this lib is
    -- only safe from this attack if the user is feeding
    -- a crypto.pkey public key object to the decoder.
    --
    -- And thats only because crypto.hmac.digest can not
    -- handle userdata arguments for key.
    function validate_token_from_service(token)
      return jwt.decode(token, { keys = { public = known_trusted_public_key }})
    end

    -- Claim to something private that only the admin should have access to
    local claims = {
      claim_to_access_customer_data = true
    }

    local legit_token = service_sign_token(claims)
    assert.truthy(legit_token ~= nil)

    local legit_claim, err = validate_token_from_service(legit_token)
    assert.same(claims, legit_claim) -- all good

    -- Now someone has gotten a hold of the public key. Maybe it's sitting somewhere public for web traffic.
    -- Generate a forged packet and bi-pass the provider entirely
    local forged_token = jwt.encode(claims, {alg = "HS256", keys = { private = known_trusted_public_key }})
    forged_claim, err = validate_token_from_service(forged_token)
    assert.are_not.same(claims, forged_claim)
  end)
@lordnynex
Copy link
Author

I just realized this is a duplicate of #2. The above unit test indicates this is still an issue. Am I misunderstanding something?

The article linked in #2 covers 2 common security issues. I think the first is related to plain tokens and it looks like that was fixed here. Am I testing against the wrong version?

I will be implementing JWT in one of my projects and would love to use this module but I may have to fork to avoid this issue because I won't be writing the consumer, only the provider. If I end up forking, do you want a PR?

@upcFrost
Copy link

upcFrost commented Apr 22, 2016

I think it can be fixed by overriding the algorithm specified in the header when you really know which one should be used. It'll take the following modifications:

src/jwt/jws.lua::76
if not self.verify[options.alg or header.alg](rawHeader.."."..bodyStr, signature, options.keys.public) then

Then, in the test it will become

function validate_token_from_service(token)
  return jwt.decode(token, { keys = { public = known_trusted_public_key }, alg = "RS256"})
end

Also, the test should be a bit modified at the point of keyPair generation since there's an assertion that the type of the key is "string" and not "userdata" which is generated by the crypto.pkey.generate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants