OWASP / ASVS

Application Security Verification Standard
Creative Commons Attribution Share Alike 4.0 International
2.73k stars 666 forks source link

4.2.4 - Originating component permissions #2061

Closed EnigmaRosa closed 2 weeks ago

EnigmaRosa commented 1 month ago

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 commented 1 month ago

Comment from Elar (https://github.com/OWASP/ASVS/issues/2033#issuecomment-2323964821):

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 (https://github.com/OWASP/ASVS/issues/2033#issuecomment-2324157184):

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.

tghosth commented 1 month ago
# 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 commented 1 month ago

Maybe a little language cleanup:

Verify that access to an object is based on the original user's permissions, not on the permissions of any intermediary or service acting for them.

elarlang commented 1 month ago

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 commented 1 month ago

This is a special use case like when a customer service agent is taking over a users account to do something on the users behalf. These can be tricky to code.

EnigmaRosa commented 1 month ago

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 commented 1 month ago

@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

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 commented 1 month ago

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 commented 1 month ago

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

EnigmaRosa commented 1 month ago

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

tghosth commented 3 weeks ago

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

EnigmaRosa commented 3 weeks ago

PR submitted, closing issue

elarlang commented 3 weeks ago

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?

tghosth commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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.

EnigmaRosa commented 2 weeks ago

Updated PR accordingly