Skip to content

fix: handle JWKS with unsupported elliptic curves (Telegram OIDC)#4580

Open
sergioperezcheco wants to merge 2 commits into
ory:masterfrom
sergioperezcheco:fix/jwks-unsupported-curve-4572
Open

fix: handle JWKS with unsupported elliptic curves (Telegram OIDC)#4580
sergioperezcheco wants to merge 2 commits into
ory:masterfrom
sergioperezcheco:fix/jwks-unsupported-curve-4572

Conversation

@sergioperezcheco

@sergioperezcheco sergioperezcheco commented Jul 1, 2026

Copy link
Copy Markdown

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

  • Chores
    • Updated a backend identity-related dependency to point to a newer upstream source/version.
    • Removed an unused indirect dependency reference from the project configuration.

@sergioperezcheco
sergioperezcheco requested review from a team and aeneasr as code owners July 1, 2026 15:57
@CLAassistant

CLAassistant commented Jul 1, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The go.mod replace directive for github.com/coreos/go-oidc/v3 now targets a different fork and version, while the indirect github.com/laher/mergefs dependency is removed.

Changes

Dependency changes

Layer / File(s) Summary
Update go-oidc replace target and remove indirect dependency
go.mod
The replace directive targets github.com/sergioperezcheco/go-oidc/v3 at a new pseudo-version, and the indirect github.com/laher/mergefs requirement is removed.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: aeneasr

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the bug and links the issue, but it omits the template's checklist section and further comments. Add the checklist with applicable boxes ticked and include any relevant testing, documentation, or implementation notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing Telegram OIDC JWKS handling for unsupported elliptic curves.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread go.mod

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't switch to your fork here, let's wait until we merged your fix in our fork.

@sergioperezcheco
sergioperezcheco force-pushed the fix/jwks-unsupported-curve-4572 branch from 6f78b3d to 0fc010e Compare July 15, 2026 02:29
@sergioperezcheco

Copy link
Copy Markdown
Author

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 scanners and format CI failures: are these caused by the fork dependency in go.mod, or are there formatting/lint issues I need to fix in my code?

@sergioperezcheco

Copy link
Copy Markdown
Author

Just rebased onto master (was 8 commits behind) — that resolves the format failure, which was an upstream oryx/configx/cors.go formatting issue unrelated to my change.

The scanners failure is a trivy CVE scan over the build image's transitive deps, not something in this PR's diff. It'll need to be tolerated or resolved upstream regardless of this change.

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.

@sergioperezcheco

Copy link
Copy Markdown
Author

Thanks @hperl, understood. I'll leave this as-is until ory/go-oidc #4 lands and the go.mod can point at the upstream module instead of my fork. I'll rebase and remove the replace directive once that's merged.

@sergioperezcheco
sergioperezcheco force-pushed the fix/jwks-unsupported-curve-4572 branch from 33cf485 to 12d35f0 Compare July 20, 2026 01:28
@sergioperezcheco

Copy link
Copy Markdown
Author

Both failing checks look unrelated to this change (which only touches go.mod/go.sum). The format job reports an import-ordering drift in oryx/configx/cors.go — a file not touched by this PR. The scanners (Docker Image Scanners) check is also currently failing on master (see the latest push run), so it's a repo-wide issue rather than something introduced here. Happy to help if there's anything on my side that needs adjusting.

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
@sergioperezcheco
sergioperezcheco force-pushed the fix/jwks-unsupported-curve-4572 branch from 12d35f0 to 48bd2db Compare July 22, 2026 00:41
Separate github.com/ory/x/corsx into the local import group as expected
by goimports -local github.com/ory.
@sergioperezcheco

Copy link
Copy Markdown
Author

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.

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.

JWKS parsing fails entirely when provider includes ES256K (secp256k1 in Telegram OIDC)

3 participants