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

4.2.4 - Originating component permissions #2061

Closed
EnigmaRosa opened this issue Sep 4, 2024 · 17 comments
Closed

4.2.4 - Originating component permissions #2061

EnigmaRosa opened this issue Sep 4, 2024 · 17 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR V4 Temporary label for grouping authorization related issues _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@EnigmaRosa
Copy link
Contributor

Proposing a requirement that cannot be pen-tested but is important for sensitive applications, and thus is L3. This is specific to application infrastructures where a component may be the subject trying to access an object, but it is doing so on behalf of some other subject (like a user) which has its own permissions. An example is where an application makes an API call to execute some function. When access is checked for whatever function, it is not the user's permissions that are checked, but rather those of the service.

# Description L1 L2 L3 CWE
4.2.4 [ADDED] Verify that access to an object is based on the originating subject's (e.g. user's) permissions, not on the permissions of any intermediary or service acting on their behalf. 441
@elarlang
Copy link
Collaborator

elarlang commented Sep 5, 2024

Comment from Elar (#2033 (comment)):

Aat the end, if user can see the data or access functionality he/she has no permission, it is authorization problem on the application side. The description is just for verifying a potential cause for the problem.

For me it seems testing-guide or cheat sheet material. From pen-testing perspective it does not give anything extra for 4.1.3+4.2.1.

Comment from Josh (#2033 (comment)):

I agree that this is not really pentestable and I think that it therefore makes sense as an L3 control.
Anecdotally, this is certainly an issue in real apps and ignoring this consideration in design makes it very hard to correctly enforce authorization.

@elarlang elarlang added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V4 Temporary label for grouping authorization related issues labels Sep 5, 2024
@tghosth tghosth added the _5.0 - prep This needs to be addressed to prepare 5.0 label Sep 5, 2024
@tghosth
Copy link
Collaborator

tghosth commented Sep 8, 2024

# Description L1 L2 L3 CWE
4.2.4 [ADDED] Verify that access to an object is based on the originating subject's (e.g. user's) permissions, not on the permissions of any intermediary or service acting on their behalf. 441

I agree with this. Any final comments @elarlang @jmanico ?

@jmanico
Copy link
Member

jmanico commented Sep 8, 2024 via email

@elarlang
Copy link
Collaborator

elarlang commented Sep 9, 2024

I still think it is more cheat sheet topic to provide such details.

From pen-testing perspective - it is just one edge-case for the problem 4.1.3 / 4.2.1, it means, for me it is a clear duplicate and I recommend to not add this requirement.

The question "how it is different from 4.1.3+4.2.1" is not answered.

@jmanico
Copy link
Member

jmanico commented Sep 9, 2024 via email

@EnigmaRosa
Copy link
Contributor Author

Genuine question: would the case of differently-permissioned components acting as intermediaries when completing a function not be a specific use case for this?

@tghosth
Copy link
Collaborator

tghosth commented Sep 10, 2024

@elarlang let's discuss later in the week

@jmanico I think we are talking about a slightly different case here.

Look at the following example, (you might need to view in GitHub)

sequenceDiagram
    actor U as Fred
    participant W as Web Front End (Browser)
    participant WB as Web Back End
    participant UP as User Profile Service
    U->>W: Working in App
    W->>WB: Get My Profile (Fred's session token)
    WB->>UP: Pull User Profile for Fred
    Note over WB,UP: Is Fred's token used for this interaction?
    UP-->>WB: Return profile for fred
    WB-->>W: Render profile on page
    W-->>U: Sees profile
Loading

We can see a multi-layer application here and the big question is what permissions does the "Web Back End" (the intermediary) use to access the "User Profile Service".

In a less well designed app, a "service to service" token which is not user specific and is assigned to "Web Back End" (the intermediary) will be used which is less secure and increases the risk of inappropriate data being pulled from the "User Profile Service". If Fred's token (the original subject) is passed on to "User Profile Service" and "User Profile Service" checks that Fred is allowed to access the data then this makes it less likely that inappropriate data will be pulled in.

This is based on real world examples I have seen.

@jmanico
Copy link
Member

jmanico commented Sep 10, 2024

This is very helpful Josh. I see this all the time too. I get it now. This is requirement is basically asked developers to "pass the JWT" and instead of relying on service accounts downstream, to continue to use the active users session and identity for permissions, and no some other component or service.

This is a super critical requirement, even more so now that I at least somewhat get it.

@EnigmaRosa
Copy link
Contributor Author

Thank you Josh, this was exactly the type of scenario I was trying to elicit.

@EnigmaRosa
Copy link
Contributor Author

@tghosth @elarlang Can I submit a PR for this, or is it still under discussion?

@tghosth
Copy link
Collaborator

tghosth commented Oct 8, 2024

Please submit a PR @EnigmaRosa, I think this is an important requirement

@EnigmaRosa
Copy link
Contributor Author

PR submitted, closing issue

@elarlang
Copy link
Collaborator

elarlang commented Oct 9, 2024

Probably not important this time, but in general it does not make sense to close the issue before the PR is merged.

Is this requirement clear enough for everyone or it should contain an example in the requirement text?

@elarlang elarlang reopened this Oct 9, 2024
@tghosth
Copy link
Collaborator

tghosth commented Oct 9, 2024

Thanks for the PR @EnigmaRosa :)

FYI note this guidance about getting a commit/PR to automatically close an issue: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

I considered adding explanation to the section introduction but I think that will be tricky and a little disjointed.

I would instead propose adding an example to the requirement:

For example, if a user calls a web service using a signed token for authentication, which then requests data from a different service, the second service should use the user's signed token to make permissions decisions rather than the first service using a machine to machine token.

Do we think that makes things clearer @elarlang ?

@elarlang
Copy link
Collaborator

elarlang commented Oct 9, 2024

Do we think that makes things clearer @elarlang ?

Yes. And in general, for all new requirements or updates, I try to validate it, is it clear from the requirement text, why it exists. So if it is not something really obvious, it should contain an example or name the attack vector or weakness it mitigates.

@tghosth
Copy link
Collaborator

tghosth commented Oct 9, 2024

Ok so @EnigmaRosa if you are comfortable with this wording, add it (with any changes you see fit) to the requirement in the PR please :)

For example, if a user calls a web service using a signed token for authentication, which then requests data from a different service, the second service should use the user's signed token to make permissions decisions rather than the first service using a machine to machine token.

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Oct 12, 2024
@EnigmaRosa
Copy link
Contributor Author

Updated PR accordingly

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 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR V4 Temporary label for grouping authorization related issues _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

4 participants