Docs should make clear that setting oidc_providers.allow_existing_users: true has security implications

[micolous]

The oidc_providers.allow_existing_users documentation is pretty sparse:

allow_existing_users: set to true to allow a user logging in via OIDC to match a pre-existing account instead of failing. This could be used if switching from password logins to OIDC. Defaults to false.

The way the sample configurations do this makes assumptions contrary to OIDC claim stability guidelines. If a Synapse instance was configured in this way, it could allow account take-overs¹ by an attacker with an unknown-to-Synapse IdP account.

The documentation should describe the security implications of enabling the option.

While there are some weaknesses in Synapse that make this possible at all, the ability to exploit it is mostly configuration dependant – so I feel this is a "security-related docs bug" rather than "docs-related security bug".

Background

From OpenID Connect Basic Client Implementer's Guide 1.0, Section 2.5.3, Claim Stability and Uniqueness, emphasis added:

The sub (subject) and iss (issuer) Claims, used together, are the only Claims that an RP can rely upon as a stable identifier for the End-User, since the sub Claim MUST be locally unique and never reassigned within the Issuer for a particular End-User, as described in Section 2.2. Therefore, the only guaranteed unique identifier for a given End-User is the combination of the iss Claim and the sub Claim.

All other Claims carry no such guarantees across different issuers in terms of stability over time or uniqueness across users, and Issuers are permitted to apply local restrictions and policies. For instance, an Issuer MAY re-use an email Claim Value across different End-Users at different points in time, and the claimed email address for a given End-User MAY change over time. Therefore, other Claims such as email, phone_number, and preferred_username and MUST NOT be used as unique identifiers for the End-User.

By default, Synapse uses the sub claim to uniquely identify a user, which is in-line with OIDC recommendations.

Detail

Setting oidc_providers.allow_existing_users: true triggers an extra code path in grandfather_existing_users() on login if lookup by external user ID fails.

This maps an OIDC user to a Matrix user based on localpart_template:

synapse/synapse/handlers/oidc.py

Lines 1247 to 1255 in 2ce7a1e

	attributes = await oidc_response_to_user_attributes(failures=0)
	if attributes.localpart is None:
	# If no localpart is returned then we will generate one, so
	# there is no need to search for existing users.
	return None
	user_id = UserID(attributes.localpart, self._server_name).to_string()
	users = await self._store.get_users_by_id_case_insensitive(user_id)

If there is an existing Matrix user which maps, the new sub is added to the database:

synapse/synapse/handlers/sso.py

Lines 474 to 479 in 2ce7a1e

	user_id = await grandfather_existing_users()
	if user_id:
	# Future logins should also match this user ID.
	await self._store.record_user_external_id(
	auth_provider_id, remote_user_id, user_id
	)

As there doesn't seem to be any further authentication (such as requesting a user login with a password, or with another configured external identity provider), my working assumption (from static analysis) is that enabling allow_existing_users allows an unknown-to-Synapse OIDC user to permanently link their OIDC identity to an existing Matrix account that matches by localpart_template.

I couldn't find anywhere that branch checks to see if a matched account already has an user_external_id associated with it (only that an external ID can't be re-used), so it looks like this would allow two OIDC users to "claim" the same Matrix account.

One caveat is that if an instance sets enable_registration: true (the default), an attacker would need a new IdP account for every attempt they made to take over an account.

The localpart_template

The impact of the issue has depends on the localpart_template.

There is no default localpart_template, but the example OIDC configs doc have some suggestions, and many of them look insecure:

• Entra ID (Azure AD): Using everything before an @ character in preferred_username.

  Entra ID sets the preferred_username to the account email address, so this would treat [email protected] and [email protected] as the same user.

  Entra ID also has a tenancy ID attribute, but this doesn't seem to be used.

• Django OAuth Toolkit: Using everything before an @ character in email.

  Similar issue to Entra ID.

• Shibboleth: Using everything before an @ character in user.sub, and also sets allow_existing_users: true.

  Shibboleth has custom mapping rules for sub, but this example seems to suggest sub is an email-like field.

  If it is an email field, then this would have a similar issue to Entra ID, with an insecure-by-default config.

• Auth0, Authentik, Keycloak, LemonLDAP: Using preferred_username as-is – this is an unstable claim.

• Twitch: Using preferred_username as-is.

  This is the display name, and can be changed by users, or recycled by Twitch.

• GitHub, GitLab, Gitea, Mastodon, Twitter: Using user.login / user.nickname / user.username as-is.

  Those accounts may be renamed.

• Google: Using user.given_name as-is.

  This may be freely changed by the user.

If an attacker with an unknown-to-Synapse IdP account can discover a Synapse server's localpart_template, and find that it is under user control, they could exploit a server with oidc_providers.allow_existing_users: true:

1. Identify a target user's localpart (this could be learned by federation).
2. Get a new account on the Synapse instance's IdP. If enable_registration: true (the default), an attacker would need a new IdP account for each takeover attempt, though this is a low bar for a public identity provider.
3. Attacker sets the field used in localpart_template to match the target user.
4. Attacker logs in to the target's Synapse instance with OIDC for the first time.
5. Synapse links the attacker's external user ID to the target account.
6. Attacker is now authenticated as the target user.

It's not clear to me (from static analysis) whether a user can discover what external user IDs are associated with their account; though it would appear as another unverified session (as they wouldn't have the E2EE keys).

What could change in the docs?

oidc_config.allow_existing_users should describe how it maps existing users (with localpart_template), and that enabling the option has significant security implications.

It should also note that enabling the option adds extra constraints to any claim used by localpart_template:

• the identifier must be unique among all users, stable and never re-issued
• violating those additional constraints may allow account take-overs
• it's best to set up a custom claim (eg: matrix_username) in the IdP to map users, rather than using other, potentially-unstable identifiers (like email or username)

What could change in Synapse?

When "grandfathering" existing Matrix user accounts, Synapse should also authenticate the user by some other method, even if those methods are not normally available (eg: password backend). This would prevent account take-overs during the migration process.

There should be a way to prevent more than one external user ID being assigned to the same Matrix user. This would prevent account take-overs for accounts which are already migrated to OIDC.

Footnotes

1. If a user has enabled end-to-end encryption and uses it in all channels, I suspect it would prevent an attacker from accessing prior messages or impersonating the user on those channels. ↩

[sandhose]

Thanks for the very detailed report! This is a well-known behaviour and intended to work like this, as the main usage we've seen of this feature is for deployments moving off a password-based auth to an SSO-based one. In those cases, it would be impractical to ask the user to re-enter their original credentials, as the intent is to make it as smooth as possible for end users.

This is what the docs says about the option:

synapse/docs/usage/configuration/config_documentation.md

Lines 3465 to 3467 in 2ce7a1e

	* `allow_existing_users`: set to true to allow a user logging in via OIDC to
	match a pre-existing account instead of failing. This could be used if
	switching from password logins to OIDC. Defaults to false.

Two things to note:

• it defaults to false
• it explicitly talks about the moving off password use case

Which makes me feel like a sensible homeserver administrator would not enable this with a public IDP, but maybe with one they control.

I'm happy with expanding this particular point in the doc to warn about the potential misuse of this option, but would rather avoid changing any of the existing behaviour.
Feel free to open a PR to help improve this

Last but not least, Synapse's auth is being reworked through Matrix Authentication Service, which is quite stricter on this, and currently doesn't have this 'allow existing users' feature.
We plan to add it, with a more granular behaviour, so if you have thoughts on the implementation, please voice them in the following issue: element-hq/matrix-authentication-service#2089

[micolous]

The main usage we've seen of this feature is for deployments moving off a password-based auth to an SSO-based one. In those cases, it would be impractical to ask the user to re-enter their original credentials, as the intent is to make it as smooth as possible for end users.

Yes - automatic account linking makes migration to SSO marginally smoother, but it's not secure unless based on claims which are unique and stable at the IdP, and not under user control.

This is a similar bug in Mastodon, where it incorrectly asserted that email was a stable and always-verified claim: GHSA-vm39-j3vx-pch3

As I understand it, there's no alternative to auto-linkage-on-claims for migrating existing accounts to SSO, aside from perhaps manually editing your Matrix server's user database. This may be an option for some, but isn't a great admin experience.

A secure automated migration strategy for an existing Matrix user could be done a couple of ways:

1. User logs in to a Matrix server with OIDC for the first time
2. The Matrix server cannot find any account matching the iss + sub claims (which would indicate the account is already migrated to OIDC)
3. User is then prompted to either:
   • sign in without OIDC to link an account to their OIDC credential (this could also check that the account is not already associated with another OIDC identity after sign-in)
   • create a new Matrix account associated with the OIDC credential (this option could be made unavailable if the Matrix server uses an OIDC claim to enforce a localpart that is already taken by another Matrix account)

Or:

1. User logs in to a Matrix server without OIDC
2. Matrix server prompts them (could be mandatory, or some option in the client settings) to connect their account with an OIDC provider
3. User logs in to the OIDC provider, and returned to the Matrix server
4. Matrix server associates the returned iss + sub claims with their Matrix account

Both of those migration flows would ensure that the user signing in with OIDC holds an existing Matrix credential for the account before setting up any linkage, while still being fairly smooth.

This would also automatically handle someone who uses a different username/localpart for Matrix and the IdP, or has changed their name during the migration process, as they would provide their Matrix localpart directly.

For an organisation running their own Matrix server, they might only want to offer OIDC first, and only fall back to non-OIDC if a user doesn't already have an account. If the organisation can ensure the claim(s) used for localpart are stable, unique and protected during migration, they could safely allow auto-linkage without a password.

For a public Matrix server, they may wish to offer authentication with one or more public IdPs in addition to local login, but have some fallbacks in place to warn someone if they might already have an account. But in a public server, auto-linkage requires users fully trust all available IdPs – not just the IdP(s) they use.

The complication is doing all of this is providing a protocol for a Matrix server to signal the OIDC migration offer (or demand) to a Matrix client. 😄

Two things to note:

• it defaults to false

• it explicitly talks about the moving off password use case

Yes, false as a default is good, and the use case is understood. 😄

However, the Shibboleth sample configuration sets this to true (in a likely-insecure config), and none of the other examples are particularly safe to use with this set to true.

Which makes me feel like a sensible homeserver administrator would not enable this with a public IDP, but maybe with one they control.

There are several examples of public IdPs in the docs, and all of those set a localpart_template based on unstable claims.

While it's fairly obvious that a name should not be considered a stable claim, it's less obvious that email and preferred_username are also unstable – especially when their IdP can give a false impression that they are somehow stable (eg: by preventing users from changing it).