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

Authorize RecordsRead with globalRole #512

Merged
merged 5 commits into from
Sep 25, 2023
Merged

Authorize RecordsRead with globalRole #512

merged 5 commits into from
Sep 25, 2023

Conversation

diehuxx
Copy link

@diehuxx diehuxx commented Sep 21, 2023

This implements the first step in supporting protocol roles. This PR creates the concept of $globalRole. It allows RecordsRead to be authorized by a role. It does NOT yet implement revocation and retention lists for roles.

Copy link
Contributor

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks legit! One comment.

src/types/records-types.ts Outdated Show resolved Hide resolved
@diehuxx diehuxx force-pushed the diehuxx/global-role branch from f703003 to 300b156 Compare September 25, 2023 05:25
src/core/dwn-error.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏅

@diehuxx diehuxx merged commit 39cf553 into main Sep 25, 2023
3 checks passed
@alanhkarp
Copy link

I'm not clear how this works. Does the submitter of a request provide proof of being in a role with the access decision made based on that information? Or, does the party in a role prove that to get an authorization that is then used when making the request? The former can lead to a confused deputy vulnerability.

@diehuxx
Copy link
Author

diehuxx commented Oct 2, 2023

Hi @alanhkarp --
Let's say Bob is trying to get access to Alice's DWN. Alice writes a friend role record to her DWN with Bob's DID in the recipient field. Let's say that in this protocol configuration friends can read other types of messages. Bob sends Alice a request to read; he includes the field protocolRole: 'friend' in the authorization payload section of his request. Alice's DWN checks to see if Bob has in fact been assigned to the friend role. In other words, Alice checks her own DWN to see a list of role records.

This test demonstrates this example: https://github.com/TBD54566975/dwn-sdk-js/pull/512/files#diff-9fb19363159f93356a17db530c83de231c08f7c853522922fc6bc00d22164306R477-R524

@alanhkarp
Copy link

This test uses Role-Based Access Control (RBAC), and there are no security issues with such a simple example. However, when Bob is acting on a resource specified by somebody else, he is vulnerable to a confused deputy attack.

There is no need to get rid of roles in order to avoid this problem. The test works as before, but Bob first makes a request to Alice's DWN that verifies he's in the role he claims and returns a capability granting access to the chat. Bob can then use that capability to read Alice's chat record. Even if Alice's DWN can't create the capability, Alice can include it in the friendRoleRecord when she tells her DWN that Bob is in the role.

RBAC has a long history, so we know its strengths and weaknesses. While it is conceptually simple, there are issues managing granular access. Every enterprise use of RBAC that I've ever heard of suffered from role explosion as more and more roles get introduced to manage different combinations of permissions. (See From ABAC to ZBAC published in the Journal of Information Warfare, but the Journal's link is broken.) On the plus side, experience with User Managed Access (UMA) shows that individuals can manage simple policies with RBAC. (I haven't seen evidence one way or the other on even moderately complex policies.)

@alanhkarp
Copy link

A few weeks ago, I mentioned that running a service on top of DWN could expose the service to a confused deputy vulnerability when using roles (or any other form of authentication for that matter) to make access decisions. I was asked to provide an example. Forgive the delay, but here is one I came up with.

@LiranCohen runs a service that adds captions to music videos using DWN for backend storage. @andorsk signs up for songs by Adele. When he wants captions added to the "Hello" video, he submits a request specifying the video and the record to hold the output. Liran's service accesses the video, puts the captioned version where Andor said to, and updates the billing record.

There are some questions to be answered.

  1. How does Liran's service get permission to read any video that Andor or any other user might specify? Since it's Liran's service, we can assume the service has permission to read any video in Liran's collection. That fact means Liran's service must make sure Andor's request isn't for a "Cruel Summer" video.

  2. How does Liran's service get permission to create the output record? Andor grants to Liran's service permission to create that record before making the captioning request.

  3. How does Liran's service get permission to update the billing record? We can assume it's in a role that has permission to update all billing records.

  4. What happens if Andor specifies a billing record where Liran's service expects to see the output record designation? The service will overwrite the billing record with the captioned video. This mistake sounds silly, but it actually occurred as described in Norm Hardy's 1988 very short paper.

There are many ways Liran's service can implement checks to prevent this attack. The real question is whether it should have to. Let's look at the same example with capabilities.

Liran's has a capability granting access to every video the service can provide. When Andor signs up for videos of Adele songs, Liran delegates to Andor a capability for the subset of records that apply to Adele videos and a capability to read Andor's billing record. When Andor wants "Hello" captioned, he delegates to Liran's service a capability to read the record containing the video and a capability to write the record for the captioned version. Andor can revoke those capabilities when the request has been completed.

Liran's service now has the least privilege needed to carry out the request. Also, if Andor delegates a billing record capability in place of the one for the output record, the service's update request will fail as it should because Andor's capability only grants him permission to read his billing record.

@thehenrytsai thehenrytsai deleted the diehuxx/global-role branch April 8, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants