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

feat(jwt): Support JWKS endpoint and improve JWT aud claim handling #15657

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

catFurr
Copy link

@catFurr catFurr commented Feb 22, 2025

This PR addresses issue #15182 by adding support for JWKS (JSON Web Key Set) endpoints and improving the handling of JWT aud claims. The changes include:

Support for JWKS Endpoint:

  • Added functionality to fetch and process public keys in JWKS format (RFC 7517 section 5).
  • Keys are now cached and managed dynamically, allowing for automatic key rotation and expiration.
  • Certificates in JWKS format are converted to public keys for JWT verification.

JWT aud Claim Handling Improvement:

  • Enhanced the verify_aud_claim function to support both string and array-based aud claims.
  • Maintain support for wildcard (*) matching in aud claims.

These changes maintain backward compatibility with existing key cache endpoints and 'aud' claim handling, while enhancing standards compatibility. These changes enable Jitsi Meet to work seamlessly with identity providers that use JWKS for key distribution and improve compliance with OIDC standards.

Changes have been manually tested on Jitsi Debian Setup.

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

@damencho
Copy link
Member

Thanks I will try to look at it next week. You have tested it on a machine where also coturn is installed?

@catFurr
Copy link
Author

catFurr commented Feb 22, 2025

Yes.

$ systemctl status coturn
● coturn.service - coTURN STUN/TURN Server
     Loaded: loaded (/usr/lib/systemd/system/coturn.service; enabled; preset: enabled)
     Active: active (running) since Sat 2025-02-22 06:43:00 +03; 11h ago
       Docs: man:coturn(1)
             man:turnadmin(1)
             man:turnserver(1)
   Main PID: 408211 (turnserver)
      Tasks: 7 (limit: 9486)
     Memory: 9.1M (peak: 9.6M)
        CPU: 14.466s
     CGroup: /system.slice/coturn.service
             └─408211 /usr/bin/turnserver -c /etc/turnserver.conf --pidfile=

@damencho
Copy link
Member

And how did you test it?
What is the algorithm used for the key, is it RSA?

@catFurr
Copy link
Author

catFurr commented Feb 22, 2025

I am testing it with RS256 keys produced by Keycloak.
Decoded Header:

{
  "alg": "RS256",
  "typ": "JWT",
  "kid": "some_string_asLSnLKsd8TPL3K_a8ENW"
}

Since the aud claim can be of two varieties as per RFC 7519, I used two types of keys:
Type A - claim as a string:

{
  "iss": "https://auth.mysite.com/realms/jitsi",
  "sub": "meet.jitsi",
  "aud": "jitsi-web",
  ...
}

Type B - claim as an array:

{
  ...
  "aud": [
    "jitsi-web",
    "account"
  ],
  ...
}

For cacheKeysUrl I set it to https://auth.my-site.com/realms/jitsi/protocol/openid-connect/certs which returns a valid JWKS. Tokens were ensured for validity using jwt.io.

While the changes are made to keep backwards compatibility and require zero extra configuration, I haven't tested to ensure that the current style of cacheKeysUrl (kid to certs map) works. Let me know if you really need this, I can try to test with a custom cert server.

@damencho
Copy link
Member

I will try to test it next week. It was added to accommodate the firebase tokens that meet.jit.si uses.

@damencho
Copy link
Member

damencho commented Feb 23, 2025

@catFurr
Copy link
Author

catFurr commented Feb 23, 2025

Yeah you're right. Let me make a few changes to accommodate that format as well. I can push the commit sometime this week.

How can we have testing for this? I wrote a simple script to test these functions for a few cases, but not sure if I should push the test files to the repo or not. I couldn't find any tests directory here.

@damencho
Copy link
Member

Nope there are no tests, I was planning to test it manually

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

Successfully merging this pull request may close these issues.

3 participants