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

Clarify horizontal and vertical access control (4.2.1) #1934

Open
tghosth opened this issue Apr 18, 2024 · 11 comments
Open

Clarify horizontal and vertical access control (4.2.1) #1934

tghosth opened this issue Apr 18, 2024 · 11 comments
Assignees
Labels
4b Major-rework These issues need to be part of a full chapter rework next meeting Filter for leaders V4 Temporary label for grouping authorization related issues _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@tghosth
Copy link
Collaborator

tghosth commented Apr 18, 2024

4.2.1 alludes to horizontal access control but we should decide whether we want to be more specific about access control types, e.g.

Making sure that the user has permission to perform a particular operation, on a particular data value of a particular entity of a particular type.

I have a slide about this somewhere....

@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 V4 Temporary label for grouping authorization related issues 4b Major-rework These issues need to be part of a full chapter rework labels Apr 18, 2024
@tghosth
Copy link
Collaborator Author

tghosth commented Apr 18, 2024

This came from a discussion in #1312

@elarlang
Copy link
Collaborator

issue for authorization reorg #1352

@EnigmaRosa
Copy link

I also feel that 4.2.1 is 1. a bit of a mouthful and 2. a roundabout way of calling for horizontal access controls. What if we replace it with

Verify that a users is authorized to perform a given operation on a given item by checking the user's permissions to both the function and the item.

@elarlang
Copy link
Collaborator

I add here my concern mentioned in V4 regorg issue (#1352 (comment)):

Q8 - what is actual difference between 4.1.3 and 4.2.1?

Current requirements:

# Description L1 L2 L3 CWE
4.1.3 Verify that the principle of least privilege exists - users should only be able to access functions, data files, URLs, controllers, services, and other resources, for which they possess specific authorization. This implies protection against spoofing and elevation of privilege. 285
4.2.1 Verify that sensitive data and APIs are protected against Insecure Direct Object Reference (IDOR) attacks targeting creation, reading, updating and deletion of records, such as creating or updating someone else's record, viewing everyone's records, or deleting all records. 639

CWE:

  • CWE-285: Improper Authorization
    • It's a category / class and should not used for mapping
  • CWE-639: Authorization Bypass Through User-Controlled Key

From @tghosth :

Making sure that the user has permission to perform a particular operation, on a particular data value of a particular entity of a particular type.

The main question here is the level of abstraction - do we want to have a separate requirement for each check (checklist style) or one requirement to list them all (principle style)?

@jmanico
Copy link
Member

jmanico commented May 26, 2024 via email

@tghosth
Copy link
Collaborator Author

tghosth commented Jun 2, 2024

Q8 - what is actual difference between 4.1.3 and 4.2.1?

@elarlang So I think 4.1.3 as currently written is too abstract. I think it should be more actionable, such as saying that new users receive no/low privileges by default and have to have permissions added as they require them. Although maybe that is covered by 4.1.4...?

The main question here is the level of abstraction - do we want to have a separate requirement for each check (checklist style) or one requirement to list them all (principle style)?

@elarlang I think that horizontal access control should be a single requirement. Maybe we want a separate requirement for vertical but I would not want to split to more requirements than that.

@EnigmaRosa suggested:

Verify that a users is authorized to perform a given operation on a given item by checking the user's permissions to both the function and the item.

I think that is in the right direction although it feels a little internally repetitive. Do we want to make the context of the item a little clearer with something like:

Verify that "horizontal" permission enforcement exists which ensures that a user is authorized to perform a given operation for a given item, to prevent IDOR/BOLA style attacks.

What do we think?

@elarlang
Copy link
Collaborator

elarlang commented Jun 2, 2024

Making sure that the user has permission to perform a particular operation, on a particular data value of a particular entity of a particular type.

Probably it makes sense to split the different vectors to requirements first and then finetune the wording for each of them. At the moment is not easy to follow, what idea or vectors is meant to cover in which requirement.

@tghosth
Copy link
Collaborator Author

tghosth commented Jun 2, 2024

Making sure that the user has permission to perform a particular operation, on a particular data value of a particular entity of a particular type.

Probably it makes sense to split the different vectors to requirements first and then finetune the wording for each of them. At the moment is not easy to follow, what idea or vectors is meant to cover in which requirement.

That is what I am saying, I don't think that statement should be multiple requirements, I think it should be a single requirement

@jmanico
Copy link
Member

jmanico commented Jun 2, 2024 via email

@EnigmaRosa
Copy link

EnigmaRosa commented Jun 2, 2024 via email

@tghosth
Copy link
Collaborator Author

tghosth commented Jun 6, 2024

Enjoy your holiday @EnigmaRosa :)

I can get on board with splitting horizontal and vertical but I think we need to be careful about splitting further than that. As we see elsewhere in our work for 5.0, I would rather we had shorter requirements but then referenced more detailed guidance.

@jmanico suggest you hold off until @EnigmaRosa is back and then coordinate with her if that is ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4b Major-rework These issues need to be part of a full chapter rework next meeting Filter for leaders 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