-
Notifications
You must be signed in to change notification settings - Fork 21
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
jwt isValidIssuer check not strict enough? #309
Comments
Related to clerk#309 Update the `IsValid` method in `clerk/tokens_issuer.go` to strictly check the issuer's domain segment. * Parse the issuer URL and extract the hostname. * Accept only issuers that match the exact domain `clerk.com` or subdomains of `clerk.com`. * Return false if the URL parsing fails. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/clerk/clerk-sdk-go/issues/309?shareId=XXXX-XXXX-XXXX-XXXX).
Hello @bgentry, You are right, the issuer validity check is incomplete. I would say that it's a compromise though. A more robust and complete check would be to verify that the JWT's issuer is the same as the instance's Frontend API. Here's an example. If your domain is github.com then most likely your FAPI lives in clerk.github.com, and that's what the JWT's iss claim should be set to. Unfortunately, the Go SDK currently has no way of knowing your domain without contacting Clerk's servers. The problem could be easily solved if users provided their FAPI (or domain) when setting up the SDK, but we wanted to avoid having to pass too many configuration options. The check (and its rationale) was carried over from v1. It provides a simple check leveraging just enough information, but it's by no means complete. Hope the above explains the rationale behind this function. Now, where do we go from here? I guess at some point we'd want to provide more configuration options to the SDK, especially if we want to support cookie based authentication too. We should definitely revisit the |
Hi, while perusing the code I noticed this function:
clerk-sdk-go/jwt/jwt.go
Lines 127 to 133 in 685118c
It appears to accept any issuer whose first domain segment is
clerk.
. Is this intentional? While it may not lead to a vulnerability on its own, it seems like it would have to be undesirable to accept an issuer likehttps://clerk.example.com
(or substitute any attacker domain suffix).Again I'm not saying this is itself a vulnerability, but if this check has any purpose at all then it would seem to be not doing a good enough job of serving it.
The text was updated successfully, but these errors were encountered: