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

proposal/discussion: JWT - 3.5.6 rephrase it to describe the goal and/or split to different requirements based on different goals #1967

Closed
elarlang opened this issue May 21, 2024 · 18 comments · Fixed by #2405
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V3 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@elarlang
Copy link
Collaborator

elarlang commented May 21, 2024

spin-off from #1925 "proposal 4"

From @TobiasAhnoff

4 Verify that only access-tokens are used for authorization by the RS (not id-tokens or other kinds of tokens)

Proposal from me:

I think we should add typ (RFC7519) to this requirement:

# Description L1 L2 L3 CWE NIST §
3.5.6 [ADDED] Verify that other, security-sensitive attributes of a stateless token are being verified. For example, in a JWT this may include issuer, subject, and audience. 287

Response from @TobiasAhnoff

I agree, this could be part of 3.5.6


Proposal/Goal 1: add typ (Type) to the requirement to have special spotlight for it in OAuth context.

The question: should the requirement list issuer, subject, audience, and type - or iss, sub, aud, and typ?

Proposal/Goal 2: get rid of "that other" part and describe why this requirement exists defining the goal to ask those parameters to be verified.

The CWE-287 is "Improper Authentication", which I would say is not correct. Additionally, it points to CWE "Class" or "Category" and should not be used for mapping.

@elarlang elarlang added the V51 Group issues related to OAuth label May 21, 2024
@elarlang
Copy link
Collaborator Author

Note, I labeled it as V51, although it is not OAuth-specific. Also, at the moment it belongs to V3 Session management, but it is not related to sessions. It should belong to V13 - for that discussion, there is a separate issue:#1917

@jmanico
Copy link
Member

jmanico commented May 21, 2024

A few notes:

  1. Even though it's covered elsewhere, it's a bit weird to talk about security-centric attributes that do not include the signature - the most essential security-centric attribute.

  2. Here is a list of other security-centric attributes to consider for the requirement text:

  • Expiration (exp)
  • Issued At (iat)
  • Not Before (nbf)
  • Audience (aud)
  • Issuer (iss)
  • Subject (sub)
  • JWT ID (jti)
  • Scopes/Permissions

And of course:

  • Signature
  • Algorithm (alg)

@elarlang
Copy link
Collaborator Author

Jim, you should take a look at section V3.5 first, and see, which one of them is already covered by other requirements. But this is precisely the reason for proposal 2 - that we should define the clear goal, why you need to verify aud, iss, sub, typ and keep the focus only on those. Also note, that even the current requirement lists them as "For example".

@jmanico
Copy link
Member

jmanico commented May 21, 2024

Sounds good, Elar. I just wanted to provide the complete list here for consideration. I did read the entire thread before I commented. Here is what I see for the main list of token claims:

  • Expiration (exp) - 3.5.4
  • Issued At (iat) - Not Covered yet
  • Not Before (nbf) - Not Covered yet
  • Audience (aud) - 3.5.6
  • Issuer (is) - 3.5.6
  • Subject (sub) - 3.5.6
  • JWT ID (jti) - Not covered yet
  • Scopes/Permissions - Not converted in 3.5.x
  • Signature - Covered in 3.5.3 but I think the text needs to be a bit clearer
  • Algorithm (alg) - Covered in 3.5.5, but the specific claim (alg) is not mentioned

@elarlang
Copy link
Collaborator Author

In my opinion, this requirement/issue focuses only on "Verify that the token is generated by an expected party" and "Verify that the token is meant for this usage". As it is not clearly defined (yet), it can cause confusion.

If something needs to be improved, please open separate issues for those - let's keep the issue here clear and focused on solving this one and precise problem, not covering the entire section or paragraph.

@elarlang
Copy link
Collaborator Author

Scopes/Permissions - Not converted in 3.5.x

I think the scope and permission is not general JWT topic, but it comes from OAuth 2.0 Token Exchange. For that discussion we have #1964

@elarlang elarlang assigned elarlang and csfreak92 and unassigned elarlang May 21, 2024
@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 May 23, 2024
@TobiasAhnoff
Copy link

Given level of detail for other 3.5 verifications a suggestion is to merge 3.5.4 into 3.5.6 (and remove 3.5.4), like this:

3.5.6 - "Verify that any security-sensitive attributes of a stateless token are being verified before processing it further. For example, issuer, expiration and intended usage. Note that the exact list of attributes depends on token format, for JWT see chapter 13.??".

The new 13.?? section could be JWT specific and have a list claims to verify details like alg, iss, aud, exp, nfb, typ etc, maybe also add a reference to a JWT cheat sheet (or JWT RFC on token validation)

@elarlang
Copy link
Collaborator Author

The requirement should be independent without references to other requirements or sections.

@TobiasAhnoff
Copy link

The requirement should be independent without references to other requirements or sections.

Should 3.5 be independent of token format (without e g JWT or SAML detalails)? If so, then 3.5.6 could be

3.5.6 - "Verify that security-sensitive attributes of a stateless token are being verified before processing it further. Regardless of token format, the following must be verified by the service protecting resources (e g an API):

  • Issuer, is it signed by a CSP that the service trusts?
  • Expiration, is it valid at this point in time?
  • Intended usage, is it an access-token (not another kind of token) meant for this particular service and kind of operation?

And then have specific token verification details in chapter 13, for e g JWT and SAML, without reference in 3.5.6.

But, perhaps also add a note in chapter 3 text to make it clear for the reader that specific token format details are found in chapter 13 (as 3.5 is independent of token format)?

@elarlang
Copy link
Collaborator Author

elarlang commented Sep 15, 2024

Original reason to open the issue:

4 Verify that only access-tokens are used for authorization by the RS (not id-tokens or other kinds of tokens)

it is / will be covered by #2005 (comment)

Verify that tokens (such as ID tokens, access tokens and refresh tokens) can only be used according to their intended purpose without allowing cross-usage between them.

(the final wording can be changed/updated)

In the requirement 3.5.6 is not defined, why it exists:

... in a JWT this may include issuer, subject, and audience.

By checking:

  • issuer - who made the token
  • subject - for whom (for example the end-user)
  • aud - for which application or (oidc) client it is made

@elarlang elarlang added V3 and removed V51 Group issues related to OAuth labels Oct 5, 2024
@elarlang elarlang changed the title proposal/discussion: JWT - 3.5.6 add "type", and rephrase it to describe the goal proposal/discussion: JWT - 3.5.6 rephrase it to describe the goal and/or split to different requirements based on different goals Oct 23, 2024
@elarlang
Copy link
Collaborator Author

elarlang commented Nov 5, 2024

#2261

Need to check do we need a dedicated JWT section.

@elarlang
Copy link
Collaborator Author

The requirement as it stands:

# Description L1 L2 L3 CWE
3.5.6 [ADDED] Verify that other, security-sensitive attributes of a stateless token are being verified. For example, in a JWT this may include issuer, subject, and audience. 287

We have now separate requirements to check:

  • is the token correct type
  • audience
  • time window (nfb .. exp)

Do we need to have separate requirement for 'subject' or it should be covered by authorization decisions? It feels like there may be need for that check if the user controls the tokens e. g. "public client" for OAuth.

Do we need a separate requirement to check issuer from the token claim or is it or will be covered by something else?

Any other "security sensitive attribute" to cover with a separate requirement to check from token content?

I think we don't need current requirement 3.5.6 after that...

ping @randomstuff @TobiasAhnoff @ryarmst

@randomstuff
Copy link
Contributor

randomstuff commented Nov 20, 2024

Do we need a separate requirement to check issuer from the token claim or is it or will be covered by something else?

We have the requirement in the sense that the key material used to check the issuers depends on the issuer but explicitly adding a requirement about the verification of the verifier should probably be useful.

@tghosth
Copy link
Collaborator

tghosth commented Nov 22, 2024

I think we don't need current requirement 3.5.6 after that...

At this point I agree, @randomstuff @TobiasAhnoff @ryarmst

Is there something here which we definitely don't cover and if so can you propose an alternatve wording to this requirement which focuses on that.

@TobiasAhnoff
Copy link

TobiasAhnoff commented Nov 23, 2024

As far as I can tell this is all covered by other 3.5 requirements, except for 'sub', which covered by

2.11.1

Verify that, if the application supports multiple identity providers (IDPs), the user's identity cannot be spoofed via another supported identity provider (eg. by using the same user identifier). Usually, the application should register and identify the user using a combination of the IdP ID (serving as a namespace) and the user's ID in the IDP.

51.4.4

Verify that if an access control decision requires identifying a unique user from an access token (JWT or related token introspection response), the resource server identifies the user from claims that can not be reassigned to other users. Typically it means using a combination of 'iss' and 'sub' claims.

Perhaps it would make sense to have a requirement in 3.5 for "sub", perhaps change 3.5.6 to only address "sub"?
(this is from https://www.rfc-editor.org/rfc/rfc8725.html#section-3.8)

Verify that the consumers identity (subject) cannot be spoofed via another trusted issuer (eg. by using the same user identifier). For a JWT, if it contains a 'sub' claim, the application must validate that the subject value corresponds to a valid subject and/or issuer-subject pair at the application.

This is basically the same as 2.11.1, but 2.11.1 is focused on IdP and users (not users and clients, consumers) and 51.4.4 has OAuth details, so maybe all three are needed?

@tghosth
Copy link
Collaborator

tghosth commented Nov 24, 2024

@TobiasAhnoff I am opening a PR to delete 3.5.6 but please open a separate issue if you think there is deduplication to be done here: #1967 (comment)

@tghosth tghosth closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2024
@tghosth tghosth reopened this Nov 24, 2024
@tghosth tghosth linked a pull request Nov 24, 2024 that will close this issue
@tghosth
Copy link
Collaborator

tghosth commented Nov 24, 2024

Opened #2405

I deliberately did not renumber because we have other discussions going on so I did not want to confuse matters at this point.

@elarlang
Copy link
Collaborator Author

@TobiasAhnoff I am opening a PR to delete 3.5.6 but please open a separate issue if you think there is deduplication to be done here: #1967 (comment)

Note that I opened a seperate issue #2406 for that.

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 V3 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants