Qualify gryt_user_id by its certificate issuer (GRYT-267) - #47
Merged
Conversation
`verifyCertificate` already returned the issuer it verified against, and `join.ts` threw it away: `grytUserId = cert.sub`, the bare Keycloak sub. That is safe only while `GRYT_TRUSTED_CERT_ISSUERS` has one entry. With two, a hostile CA mints a certificate whose sub is a UUID that already exists here and inherits that user's roles, ownership, ban state and message attribution — every one of those keys on `gryt_user_id`. The sub is not a secret either: it is a claim in the certificate handed to every server on every join, so an operator who has seen a user already has it. Nothing has to be stolen. The local tier has never had this problem, because its subs are derived from the key and carry a `key:` prefix that a CA is refused. The account tier had no equivalent between two CAs. It does now. The first configured issuer owns the unqualified namespace and everything else is prefixed with the issuer that vouched for it. That leaves existing rows untouched — the alternative rewrites `gryt_user_id` across `users`, `bans`, `server_config.owner_gryt_user_id` and `refresh_tokens` on live servers that have no off-box backups, which is not a trade worth making for a hole that is still latent. The issuer used is the one the signature was checked against, never the one the certificate claimed, so a CA cannot reach outside its own name by writing a different one down. What it costs is an ordering hazard: moving the first entry in `GRYT_TRUSTED_CERT_ISSUERS` changes who owns the unqualified namespace and would silently re-point every account identity. Said as loudly as a comment can say it. `sub` and `grytUserId` are kept as separate fields rather than one. They answer different questions — what the client signed, and what this server files people under — and a single field would have to be right for both at once, which is the confusion that allowed this in the first place. Certificates now dispatch on the issuer they name, matched against the trusted list, instead of being tried against each in turn. One issuer made the difference invisible; more than one makes the loop a liability. A rejected certificate used to cost a signature check per trusted issuer on a path anybody can reach before joining, and one unreachable issuer early in the list added its JWKS fetch timeout to every join behind it. Tested against real JWKS endpoints on loopback rather than a closed port, because the property only exists on the far side of a successful CA verification. Six cases, including two trusted CAs issuing the same sub and ending up as different users. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the hole you worked out in the Carlo thread: if two CAs are ever trusted, the second one can be anybody the first one vouched for.
verifyCertificatealready returned the issuer it verified against.join.tsthrew it away and stored the bare Keycloaksub. Sincegryt_user_idis what roles, ownership, bans and message attribution all key on, a second trusted CA minting a certificate for asubthat already exists here would inherit all of it. Thesubisn't a secret — it's a claim in the certificate handed to every server on every join — so any operator who has seen a user already has what's needed.Latent today: the trusted list has one entry by default. This is the prerequisite for it ever having two.
What to look at
The primary-issuer trick, which is the debatable part. The first configured issuer owns the unqualified namespace; every other issuer gets its name written into the id. That means no migration — existing rows keep the ids they have.
The alternative is qualifying everything and rewriting
gryt_user_idacrossusers,bans,server_config.owner_gryt_user_idandrefresh_tokenson live servers with no off-box backups. I didn't think that was worth doing for a hole that can't be reached yet, but it is the cleaner end state and I'd rather you agreed than assumed.What it trades for: an ordering hazard. Moving the first entry of
GRYT_TRUSTED_CERT_ISSUERSchanges who owns the unqualified namespace and would silently re-point every account identity on the server. There's a loud comment ongetPrimaryCertificateIssuer, but nothing enforces it. Worth a line in the self-hosting docs whenever that env var gets documented at all — it currently isn't, anywhere.subvsgrytUserIdas separate fields. They answer different questions: what the client signed its assertion with, and what this server files people under. One field would have to be right for both at once, which is exactly the conflation that caused this.join.tsusesgrytUserIdfor storage and keepssubfor the assertion check and the identity-link proof, since those are values the client knows.Two rejections that look paranoid and aren't. A CA sub starting with
key:was already refused. A CA sub containing|is refused now — otherwise the primary issuer, whose users are stored bare, could mint a sub that lands exactly on another issuer's user. Keycloak mints UUIDs, so nothing legitimate sends either.Dispatch instead of the loop. Certificates now match the issuer they name against the trusted list and verify once. Previously every trusted issuer was tried in turn, so a junk certificate cost a signature check per issuer on an unauthenticated pre-join path, and one unreachable issuer early in the list added its JWKS fetch timeout to every join behind it.
This changes an error message, and the existing test asserting
/trusted issuers/iis updated to match. The behaviour is stricter in a good way — an untrusted issuer is now refused without any network contact at all.Verified
yarn test— 63 pass, 0 fail. Six new cases, run against real JWKS endpoints on loopback rather than a closed port, because the property only exists on the far side of a successful CA verification:subproduce different users — the actual attacknpx tsc --noEmitclean.Not in this PR
Nothing about federation itself. This only makes it possible to trust a second CA without that CA being able to impersonate everyone — the trust policy, the UI for showing which issuer somebody came from, and whether the official server should accept foreign identities at all are all still open. Worth remembering that a federated identity from an uncurated issuer list is exactly as strong as a guest identity for moderation purposes, since whoever runs the CA can mint unlimited ones.
🤖 Generated with Claude Code