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

JWKS Uri containing an encryption key (besides signature keys) will fail io.jwt.decode_verify #4699

Closed
lenalebt opened this issue May 17, 2022 · 9 comments · Fixed by #4725
Closed
Labels

Comments

@lenalebt
Copy link

lenalebt commented May 17, 2022

Short description

We use Keycloak, and it was configured to also have an encryption key exposed on the JWKS URI, using an algorithm that OPA does not support. OPA fails, but you cannot easily see that if you do not use the --strict-builtin-errors flag, which is not set on the Rego playground at https://play.openpolicyagent.org/.

OPA should ignore keys it does not support instead of failing, offer options for filtering, or better document that behaviour. In the above case, it's enough to only feed signature keys to io.jwt.decode_verify.

  • OPA version: 0.40.0-envoy-1
  • used in k8s, together with istio external authorization, but the issue also is there in the playground and locally

Steps To Reproduce

Use a JWKS like this:

{
  "keys": [
    {
      "kid": "DsmD8hx6.....Z_ThSPY",
      "kty": "RSA",
      "alg": "RS256",
      "use": "sig",
      "n": "iVJfdNtA7wiT91cnG3uDrq80akzb0nm5q.....5Bw1ij5FyHULUnCRXXZeXZKYNZx08fr1T1q4Dvw",
      "e": "AQAB",
      "x5c": [
        "MIICnTCCAYUCBgGAmcGzvzANBgkqh.....zqnm8BWAH3zR6Ho7Cxghf63i79o1MDZ2S0DNTXg8Rw=="
      ],
      "x5t": "C1Qht.....ZaY72S4",
      "x5t#S256": "4KN412_TA.....xXpnFgpWJ8"
    },
    {
      "kid": "tRA344j.....Bau2GII",
      "kty": "RSA",
      "alg": "RSA-OAEP",
      "use": "enc",
      "n": "onlqv4UZx5ZabJ3TCq-IO0s0xaOwo6fWl9o4SzLXPbG.....tvxonQhoYOeMlS0XkdEdLzB-eqh_hkQ",
      "e": "AQAB",
      "x5c": [
        "MIICnTCCAYUCBgGAmcG0xjANBgkqhkiG9w0BAQsFADASMRAwDgYDVQ.....Q2YVaQn47Eew=="
      ],
      "x5t": "WKfdwb.....dQkg",
      "x5t#S256": "2_FidAw.....jlCQl20"
    }
  ]
}

Call io.jwt.decode_verify as described in the example from https://www.openpolicyagent.org/docs/v0.39.0/policy-reference/#using-jwks :

jwks = `{
    "keys": [{
        "kty":"EC",
        "crv":"P-256",
        "x":"z8J91ghFy5o6f2xZ4g8LsLH7u2wEpT2ntj8loahnlsE",
        "y":"7bdeXLH61KrGWRdh7ilnbcGQACxykaPKfmBccTHIOUo"
    }]
}`
token = "some_JWT_token"

[valid, header, payload] := io.jwt.decode_verify(token, {
    "cert": jwks,
    "iss": "xxx",
})

-> OPA fails to validate the token, even if it is signed with the key OPA supports.

Expected behavior

  • OPA should ignore keys it does not support, or at least offer options to filter for signature keys only.
  • The OPA playground should either run with --strict-builtin-errors by default (any reason not to?), or at least offer an option to enable that

We implemented this manually via this code (for others to use):

jwks_request(url) = http.send({
      "url": url,
      "method": "GET",
      "force_cache": true,
      "force_cache_duration_seconds": 3600 # Cache response for an hour
})

jwks := jwks_request("YOUR_JWKS_URI").raw_body
filtered_jwks := [ key |
      keys := json.unmarshal(jwks)["keys"]
      key := keys[_]
      key["use"] == "sig"
    ]
compatible_jwks := json.marshal({"keys": filtered_jwks})

Additional context

We did not configure our keycloak to explicitly contain an encryption key in the JWKS endpoint - we're using a SaaS provider, and that was the default configuration over there.

@lenalebt lenalebt added the bug label May 17, 2022
@srenatus
Copy link
Contributor

The OPA playground should either run with --strict-builtin-errors by default (any reason not to?), or at least offer an option to enable that

Thanks for bringing that up! We've been discussing a revamp of some of the playground before, since there are a few controls to expose: strict mode, coverage, strict builtin errors; and adding further buttons makes the UI too convoluted, so some sort of ⚙️-menu with all the knobs would be useful...hoping this may happen sometime 🤞

However, defaulting to strict builtin errors is a more debatable thing -- it's not the evaluation mode used by the server, or opa eval, by default; so why make it the default for the playground? 🤔

Thanks for providing that workaround! ✨ A tiny nitpick, if you pass "force_json": true to http.send, you can use jwks_request("...").body and have the evaluator also cache the unmarshalled JSON response. It'll save you the time of doing json.unmarshal(jwks) for a jwks that doesn't change:

jwks := jwks_request("YOUR_JWKS_URI").body
filtered_jwks := [ key |
      some key in jwks.keys # assuming `import future.keywords.in`
      key.use == "sig"
    ]

@lenalebt
Copy link
Author

lenalebt commented May 17, 2022

Yeah, tbh I'm not sure about the details of how it behaves differently if you provide --strict-builtin-errors - it's just that with the current setup, OPA just stops working and it really is not easy to find out what happens underneath. The playground also does not let me do HTTP calls for example, so I think it behaves differently anyways to the server, and it usually is humans looking for guidance who use the playground - that's why I thought it makes sense.

I totally get that it might get convoluted with more buttons. While it would have taken more time, eventually I would have tried all the options from a ⚙️ menu I guess, so it would definitely be an improvement :-).

Thanks for the caching hint! Is there a way to also cache the filtered jwks? I also like the syntax you provided, so I'll use it for our code as well :-).

EDIT: I took the raw_body from this example, maybe it makes sense changing that as well?

@srenatus
Copy link
Contributor

Is there a way to also cache the filtered jwks? I also like the syntax you provided, so I'll use it for our code as well :-).

Nope you can't cache the intermediate result there... but having less to do when re-calculating it for evert policy eval will be better 😅 And yes, updating those docs would be a good call.

Of course you could use jwks from data and have some other recurring process keep that part of OPA's storage up-to-date using the Data API. When it would only ever have what you need. But I don't think the hassle is worth it.

Speaking if more hassle than it's worth, I've often found that it's more straightforward to use the JWT built-in functions other than decode_verify. The latter has many assumptions builtin, it's a bit if a kitchen-sink built-in trying to do all things at once, with bad error reporting. I.e. it's checking a bunch of things, but the only response is valid: true/false.

@lenalebt
Copy link
Author

Just an update for anybody reading this thread in the future, trying what I tried: The parameter to force json evaluation is named force_json_decode, not force_json, took me a while to see that THIS was the problem :-).

@srenatus
Copy link
Contributor

Ugh sorry about that. 🤦

@lenalebt
Copy link
Author

No problem, let's simply say that there is more room for better error reporting also in that area :-). Still, thanks a lot for helping me over here, it brought it forward and I learned a few new things :-)

@srenatus
Copy link
Contributor

Glad you're unstuck. However, there are a few things to improve mentioned in your original issue, and the comments. So let's keep this open for tracking.

@anderseknert
Copy link
Member

Thanks for reporting this @lenalebt!

We should indeed be smarter about signature verification. In addition to not fail when encountering keys of unknown type, the verification could be improved by:

  1. Check the kid value of the token header, and if present, verify using only that key from the JWKS.
  2. In the absence of kid, check the alg value of the token header, and verify only using keys of that type.

anderseknert added a commit to anderseknert/opa that referenced this issue Jun 1, 2022
Additionally, improve the JWT verification process when a JWKS
is provided:

* If `kid` is present in JWT header, and exists in JWKS — verify using
  that key only.
* If `kid` not present in JWT header, try verification only using keys
  matching the `alg` provided in the JWT header (mandatory claim).

Fixes open-policy-agent#4699

Signed-off-by: Anders Eknert <anders@eknert.com>
anderseknert added a commit that referenced this issue Jun 2, 2022
Additionally, improve the JWT verification process when a JWKS
is provided:

* If `kid` is present in JWT header, and exists in JWKS — verify using
  that key only.
* If `kid` not present in JWT header, try verification only using keys
  matching the `alg` provided in the JWT header (mandatory claim).

Fixes #4699

Signed-off-by: Anders Eknert <anders@eknert.com>
@lenalebt
Copy link
Author

lenalebt commented Jun 7, 2022

Thanks for the changes, I think this will really make things easier to use 😄 !

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

Successfully merging a pull request may close this issue.

3 participants