Skip to content
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

Add requirement about usage of claims other than subject and issuer as an identifier for OpenID Connect #1826

Open
jsherm-fwdsec opened this issue Jan 17, 2024 · 23 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 4a) Waiting for another This issue is waiting for another issue to be resolved V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@jsherm-fwdsec
Copy link

jsherm-fwdsec commented Jan 17, 2024

Usage of claims other than the subject and issuer identifier to uniquely identify an end user in OpenID Connect is non-compliant with the framework. As per a recent Microsoft report, there is a false identifier anti-pattern being followed and exploited in the wild resulting in account takeover.

To keep it simple, the sub (subject) and iss (issuer) claims, when used together, are considered to be a unique primary identifier for OIDC, as the uniqueness across users is guaranteed. Other claims such as email, username or phone number should not be used, as they can change over time and an attacker can falsify these claims.

I'm interested in contributing and happy to create a PR for this, as well as adding other OIDC requirements. Perhaps this is something I can help with as part of the OAuth2 changes being discussed here ?

@jmanico
Copy link
Member

jmanico commented Jan 17, 2024

Super excited about this. A dedicated OIDC section would be fantastic!

@elarlang
Copy link
Collaborator

Yes, that is correct - we have OAuth PR to be discussed and merged and that is why we don't have sepparate issues per requirement so far on the topic.

The branch for that is located at https://github.com/OWASP/ASVS/blob/master-v51-oauth/5.0/en/0x51-V51-OAuth2.md

If you have in mind some wording for the proposed requirement, please share it that (we can fine-tune and help with that) and matching section from the OAuth category.

@elarlang elarlang added the V51 Group issues related to OAuth label Jan 17, 2024
@ImanSharaf
Copy link
Collaborator

@jsherm-fwdsec great idea!

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels Jan 24, 2024
@tghosth
Copy link
Collaborator

tghosth commented Jan 24, 2024

@csfreak92 did you see this? Can you work with @jsherm-fwdsec on this?

@csfreak92
Copy link
Collaborator

For sure @tghosth, will work with @jsherm-fwdsec on this one! I think some parts of it were covered by OAuth 2.0 requirements that I wrote, but I will double/triple check to ensure we got them nailed to good detail to include OpenID stuff.

@elarlang
Copy link
Collaborator

elarlang commented Feb 8, 2024

I'm interested in contributing and happy to create a PR for this

our usual process is, that we first have agreement on the requirement wording in an issue and then we go for PR. This way it is easier to follow later, why some change was made and what where the arguments behind the change were.

@jsherm-fwdsec - do you have a wording or an idea for the point for additional requirement(s)?

@jmanico
Copy link
Member

jmanico commented Feb 8, 2024 via email

@elarlang
Copy link
Collaborator

elarlang commented Feb 9, 2024

How about… Verify that applications utilizing OpenID Connect identify and utilize at least one additional claim, beyond sub and iss. This could include claims such as email, preferred_username, or a custom claim agreed upon with the identity provider (IdP).

For me it seems it is not correct, see https://openid.net/specs/openid-connect-core-1_0.html#ClaimStability.

Therefore, the only guaranteed unique identifier for a given End-User is the combination of the iss Claim and the sub Claim.

@jsherm-fwdsec
Copy link
Author

Hi @jmanico @elarlang! Agreed I think we need to be more specific about what the unique identifier should be and it should conform to the spec.

How about something such as this:
Verify that the unique identifier in the ID token for an end-user is a combination of the sub Claim and the iss Claim. Avoid usage of other claims such as email, phone_number, preferred_username, or name , as they may be mutable and can be potentially falsified.

@elarlang
Copy link
Collaborator

@jsherm-fwdsec
Copy link
Author

That works here's what I would suggest:
Verify that the unique identifier in the ID/access token for an end-user is a combination of the sub claim and the iss claim, if a unique identifier is needed. Avoid usage of other claims such as email, phone_number, preferred_username, or name , as they may be mutable and can be potentially falsified.

Also agreed this should go under https://github.com/OWASP/ASVS/blob/master-v51-oauth/5.0/en/0x51-V51-OAuth2.md#v513-resource-server.

Since we're touching on ID tokens that are specific to OIDC, we might also want to add and update some terminology under this section for ID token, access token, and "Authorization Server" to include info about the OpenID Provider: https://github.com/OWASP/ASVS/blob/master-v51-oauth/5.0/en/0x51-V51-OAuth2.md#terminology.

Here's my attempt:

  • ID tokens - These are issued by an OpenID Provider (OP) and contain a set of claims (assertions) about the authentication event. These tokens are represented as JSON Web Tokens (JWTs), which means they are compact, URL-safe, and contain a payload of claims encoded in JSON format. The claims in an ID token typically include information about the user (such as the user's identifier), information about the token's validity period, and information about the authentication event (such as the time of authentication and the method used). ID tokens are intended for the client that initiates the authentication request. The client can validate the ID token to ensure that it was issued by the expected OpenID Provider and that the authentication event meets the client's requirements.
  • Access tokens - Access tokens are used to access protected resources. They are issued by an authorization server and are sent to an API or resource server to authorize access to a resource. Unlike ID tokens, access tokens are not necessarily intended to be understood by the client. They can be opaque strings or JWTs that represent the authorization granted to the client by the resource owner. The format and content of access tokens are typically intended for the resource server and are not standardized in the same way as ID tokens. Access tokens have a limited lifetime and scope, which means they grant temporary access to a specific set of resources with defined permissions. The resource server must validate the access token to ensure it is valid, has not expired, and grants the required permissions for the requested resource.
  • Authorization Server (AS) - This refers to the server issuing ID/access tokens to the client after successfully authenticating the resource owner and obtaining authorization. For OpenID Connect (OIDC) flows, this is sometimes also referred to as the OpenID Provider (OP) or Identity Provider (IdP) in certain cases, especially when the service is designed to provide both authentication and authorization functionality as part of a single platform. This combination is common in many implementations of OAuth 2.0 and OIDC.

@elarlang
Copy link
Collaborator

Just for info, we need to solve some other open issues for the OAuth paragraph before moving this one further.

@csfreak92
Copy link
Collaborator

Yeah, I'm working on the OAuth section on my local copy. I will push commits soon.

@csfreak92
Copy link
Collaborator

Question for the leaders: @jmanico, @tghosth, @elarlang, Will OpenID Connect section be under the OAuth 2.0 chapter that we were working on? Or will it be on a separate chapter on its own? If it was the latter part, I would suggest @jsherm-fwdsec to create a PR on a separate branch, but if someone can point him to where to push it so that he won't be waiting too long for me on the OAuth section to be merged.

@elarlang
Copy link
Collaborator

At the moment we put all OAuth and OpenID-related requirements into one chapter.

Based on the current direction, we will not have a separate section for OpenID flow. But let's see, how many OpenID-related requirements we will have, we may reorganize them into separate/different chapters.

For current requirement we agreed to add it to the Resource Server chapter:
#1826 (comment)

category/section choice - does it fit to the "Resource Server" (as "Relying Party") section in our current structure (https://github.com/OWASP/ASVS/blob/master-v51-oauth/5.0/en/0x51-V51-OAuth2.md)

#1826 (comment)

Also agreed this should go under https://github.com/OWASP/ASVS/blob/master-v51-oauth/5.0/en/0x51-V51-OAuth2.md#v513-resource-server.

I can see the procedure, that we get the current OAuth chapter merged to the main branch, and then we go issue-by-issue with the modifications. At the moment it can be a slow-downer, as there are too many "opened issues" as separate comments in different places and it's complicated to get an overview, what is the situation and what must be done.

@elarlang elarlang added the 4a) Waiting for another This issue is waiting for another issue to be resolved label Feb 28, 2024
@jmanico
Copy link
Member

jmanico commented Feb 28, 2024 via email

@tghosth
Copy link
Collaborator

tghosth commented Feb 29, 2024

Are they going to be different enough to merit two chapters @csfreak92 ?

@tghosth
Copy link
Collaborator

tghosth commented Feb 29, 2024

They should probably be in different sections at least

@csfreak92
Copy link
Collaborator

I haven't looked at OpenID Connect as closely with OAuth. Maybe separate sections at least for now. Can you draft something for it @jsherm-fwdsec?

@elarlang
Copy link
Collaborator

Gentlemen, please, re-read my comment: #1826 (comment) :)

Let's get the OAuth chapter in, let's get the changes done, let's get OIDC requirements in and then we can shake everything to correct or better place if needed. One step at a time.

@jsherm-fwdsec
Copy link
Author

Sounds good @elarlang. I'd be happy to help contribute to the OIDC requirements when the time comes.

@randomstuff
Copy link

Other claims such as email, username or phone number should not be used, as they can change over time and an attacker can falsify these claims.

Provision should probably be added to allow for using some other claims if this requirement is overridden by another specification (for a specific issuer). Some issuers could provide custom claims with identifiers which are intended to be stable and reliable for user identification (for this issuer).

For example ProSanteConnect (a French SSO service for medical practitioners) uses a SubjectNameID claim for declaring the user national identifier and its usage is required:

Le Fournisseur de Service DOIT utiliser le champ SubjectNameID pour récupérer l'identifiant unique de l'identité […]

Roughly translated as:

The Service Provider [i.e. the Relying party] MUST use the SubjectNameID field [i.e. claim] in order to fetch the identity unique identifier

AgentConnect, a French SSO service for national agents states:

Il ne faut pas effectuer une réconciliation sur le SUB, car le SUB est calculé en fonction du FI utilisé. Nous vous recommandons l'utilisation de l'email professionnel pour cet usage.

Translated as:

SUB shall not be used for [identity] reconciliation because the SUB is computed depending on the identity provider [i.e. OpenID provider]. We recommend using the professional email [address] for this usage.

@elarlang
Copy link
Collaborator

elarlang commented May 22, 2024

Thank you, Gabriel (@randomstuff) for reaching out!

Proposal from Jordan #1826 (comment)

Verify that the unique identifier in the ID/access token for an end-user is a combination of the sub claim and the iss claim, if a unique identifier is needed. Avoid usage of other claims such as email, phone_number, preferred_username, or name , as they may be mutable and can be potentially falsified.

In general, it would be nice to set the requirement against the attack vector, or defining the principle that must be achieved, not saying directly "use this configuration". Then it's potentially more clear, why such a requirement exists.

I think we can rephrase the proposal from Jordan to achieve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 4a) Waiting for another This issue is waiting for another issue to be resolved V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

7 participants