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

Origin capabilities may be bypassed in some cases #1828

Open
jhiemstrawisc opened this issue Dec 13, 2024 · 3 comments · May be fixed by #2075
Open

Origin capabilities may be bypassed in some cases #1828

jhiemstrawisc opened this issue Dec 13, 2024 · 3 comments · May be fixed by #2075
Assignees
Labels
bug Something isn't working critical High priority for next release origin Issue relating to the origin component security
Milestone

Comments

@jhiemstrawisc
Copy link
Member

Origins and namespaces each take their own list of capabilities ("Reads", "PublicReads", "Writes", "Listings", "DirectReads") that indicate what the administrator would like to permit for the resource.

However, our mechanisms to enforce these capabilities at the Origin are only semi-functional, mostly relying on XRootD's Scitokens authorization library. In some cases, certain combinations of capabilities may allow unexpected behavior if the origin receives a broadly permissive, validly-signed token. This happens because we have no way telling XRootD that it should accept or deny tokens based on their prescribed claims -- only that certain issuers are allowed to issue tokens for certain base paths.

For example, if an Origin is configured only with the authorized "Reads" as a capability, an entry is added to the Origin's XRootD Scitokens config that describes permitted issuers and the base paths those issuers are allowed to mint tokens for. If a properly-signed storage.Modify token arrives at the Origin along with a PUT request, we'd expect the Origin to deny the token because it does not have the "Writes" capability. However, the authorization library has no way of knowing this, so the token will be accepted and the XRootD endpoint will attempt to move forward with the write.

Until the XRootD Scitokens auth library has a way to describe which actions/verbs/scopes are allowed from certain issuers for a given path, I don't think there's much we can do about this. On one hand, we do handle some filtering in the Director, and in theory we shouldn't ever send a client to a non writable origin if the client sends the director a PUT request. However, @Saartank has observed some cases where the Director doesn't handle this correctly.

Finally, I'll note that this assumes the issuer is minting broadly-permissive tokens. If the issuer chooses only to issue tokens with capabilities aligning with the Origin's wishes, there's no issue. But in a world where origins and issuers can be two separate entities, I don't think it's great to rely on a policy in one place being correctly translated into a policy for a separate, autonomous entity even if there's an inherent trust relationship between the two.

@jhiemstrawisc jhiemstrawisc added bug Something isn't working critical High priority for next release origin Issue relating to the origin component security labels Dec 13, 2024
@jhiemstrawisc jhiemstrawisc added this to the v7.13.0 milestone Dec 13, 2024
@brianaydemir
Copy link
Contributor

brianaydemir commented Dec 14, 2024

On this same topic, I have observed that Pelican's Listings capability has no effect on the underlying XRootD instance if PublicReads are enabled. Which is to say, HTTP PROPFIND requests directly to an origin succeed regardless of the Listings capability.

And a priori, I would argue that the fact that anyone can read objects doesn't necessarily imply that anyone should be able to determine what objects actually exist.

@Saartank
Copy link
Collaborator

So I have been going through the code and this is what I found:

The director enforces origin capabilities but not all namespace capabilities while matchmaking. The capabilities set using origin.exports are for the namespace, and origin capabilities are set using the following configuration parameters (as per docs):

Config Parameter Default Value
Origin.EnablePublicReads false
Origin.EnableReads true
Origin.EnableWrites true
Origin.EnableListings true
Origin.EnableDirectReads true

While testing my object delete command, I had removed the writes capability only from the namespace and did not look into the origin capabilities default, and now because the director was only taking into account the origin capabilities (with writes enabled by default) and not all namespace capabilities, the client was getting back the origin URL. Because of the SciToken issue, the client was able to delete/write at the origin, with a valid token with modify capability, of course.

Further Comments

  1. The director receives the namespace capabilities in the server-ad from the origin, and I don't see any reason to not enforce them while matchmaking.

  2. Currently, every origin has capabilities, and then each namespace in it has its own origin-specific capabilities. Would it be a good idea to change this and follow one of these:

    • Either namespace capabilities should be federation-wide, and origin should have its separate capabilities (to add further restrictions on the federation-wide namespace capabilities).
    • Or origin capabilities should be removed altogether, and namespaces should have origin-specific capabilities.

@brianaydemir
Copy link
Contributor

The director enforces origin capabilities but not all namespace capabilities while matchmaking.

Why do the Origin.Enable* parameters exist? Why would someone not just set the capabilities on each export?

@turetske turetske assigned turetske and unassigned turetske Jan 9, 2025
@turetske turetske modified the milestones: v7.13.0, v7.14 Jan 23, 2025
@turetske turetske modified the milestones: v7.14, v7.15 Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical High priority for next release origin Issue relating to the origin component security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants