fix: handle JWKS with unsupported elliptic curves (Telegram OIDC)#4580
fix: handle JWKS with unsupported elliptic curves (Telegram OIDC)#4580sergioperezcheco wants to merge 2 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe ChangesDependency changes
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| replace ( | ||
| github.com/coreos/go-oidc/v3 => github.com/ory/go-oidc/v3 v3.0.0-20250124100243-69986dfaf891 | ||
| github.com/coreos/go-oidc/v3 => github.com/sergioperezcheco/go-oidc/v3 v3.0.0-20260701155222-10b406941ed7 |
There was a problem hiding this comment.
We can't switch to your fork here, let's wait until we merged your fix in our fork.
6f78b3d to
0fc010e
Compare
|
Understood — I'll remove the fork replace directive once #4 is merged upstream. The go-oidc PR is ready for review whenever you have time to look. Regarding the |
0fc010e to
33cf485
Compare
|
Just rebased onto master (was 8 commits behind) — that resolves the The Still waiting on ory/go-oidc#4 to merge so I can drop the fork replace directive as you requested — I'll update this PR the same day it lands. |
|
Thanks @hperl, understood. I'll leave this as-is until |
33cf485 to
12d35f0
Compare
|
Both failing checks look unrelated to this change (which only touches go.mod/go.sum). The |
Telegram recently added an ES256K (secp256k1) key to their JWKS endpoint. go-jose doesn't support secp256k1, causing the entire JWKS set to fail parsing. This breaks all Telegram OIDC logins. Update go-oidc fork to include a fix that gracefully skips keys with unsupported curves when parsing JWKS, falling back to per-key parsing. See ory/go-oidc#4 for the upstream fix. Closes ory#4572
12d35f0 to
48bd2db
Compare
Separate github.com/ory/x/corsx into the local import group as expected by goimports -local github.com/ory.
|
Quick note on the two failing checks — both are unrelated to the go-oidc dependency bump in this PR. The format failure is a pre-existing goimports grouping issue in oryx/configx/cors.go (committed on master July 8, before this PR branched). I included the one-line fix in the latest push so format passes. The scanners failure is a high-severity CVE in the Debian libc6 base image (CVE-2026-5450), not in our code — any PR hitting that Docker scan would fail the same way. |
Telegram recently added an ES256K key (secp256k1 curve) to their JWKS at oauth.telegram.org/.well-known/jwks.json. go-jose does not support secp256k1, so when it tries to unmarshal the entire JWKS set, it fails on that one key and the whole set gets rejected. This completely breaks all Telegram OIDC logins — even though the actual id_token is signed with RS256 which works fine.
The error people see is: "failed to verify signature: fetching keys oidc: failed to decode keys: unsupported elliptic curve secp256k1"
I submitted a fix to the go-oidc fork (ory/go-oidc#4) that adds a fallback: when the full JWKS unmarshal fails, it parses each key individually and skips any that cannot be decoded (like the secp256k1 one). If ALL keys are unsupported, it returns a proper error. This way providers that mix supported and unsupported key types still work.
This PR updates the go-oidc dependency to include that fix. The go-oidc PR should be merged first, then this go.mod can be updated to point to the official ory/go-oidc commit instead of my fork.
Closes #4572
Summary by CodeRabbit