Skip to content

Commit

Permalink
PnfsManager: check restrictions before resolving path to PNFS-ID
Browse files Browse the repository at this point in the history
Motivation:

An agent may have authenticated with dCache such that the login session
includes restrictions on what that agent may do.  The two main examples
of such authentication are macaroons and AuthZ tokens (e.g., SciTokens).

The method `populatePnfsId` discovers a file's PNFS-ID if the value was
not provided by the door.  This method is typically called as one of the
first activities when processing a message.  This is because it updates
the message with the PNFS-ID as a side-effect, making that information
available for all subsequent activity.  Note that `populatePnfsId` will
throw FileNotFoundCacheException if it detects that the path does not
exist.

The problem here is that `populatePnfsId` is called before the
restrictions are checked.

This means dCache will "leak" information about the namespace, as a
different error is returned depending on whether the target is missing
(FileNotFoundCacheException) or the restriction prevents access
(PermissionDeniedCacheException).

As a concrete example, if I have a home directory `/home/paul` in which
the directory `private` contains information I do not wish to share.  If
I then create a macaroon that I share with an agent that allows (or
"should allow") that agent only to interact with the `/home/paul/shared`
folder then that agent can still learn whether files exist in the
`/home/paul/private` directory by making HTTP HEAD requests: if the file
does not exist then dCache replies with a "404 No Found" response, while
existing files trigger a "403 Forbidden" response.

EGI SVG-RAT have evaluated this problem as LOW risk.

Modification:

Be sure to check the restrictions before resolving the path to a
PNFS-ID.

Result:

dCache no longer leaks information about whether or not files exist when
using macaroons or Scitokens / WLCG AuthZ JWT profile tokens.

Target: master
Requires-notes: yes
Requires-book: no
Request: 7.2
Request: 7.1
Request: 7.0
Request: 6.2
Patch: https://rb.dcache.org/r/13354/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed Jan 7, 2022
1 parent a36f469 commit 5b80b3a
Showing 1 changed file with 7 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1314,11 +1314,11 @@ public void clearCacheLocation(PnfsClearCacheLocationMessage pnfsMessage) {
public void getCacheLocations(PnfsGetCacheLocationsMessage pnfsMessage) {
Subject subject = pnfsMessage.getSubject();
try {
checkRestriction(pnfsMessage, READ_METADATA);
PnfsId pnfsId = populatePnfsId(pnfsMessage);
_log.info("get cache locations for {}", pnfsId);

checkMask(pnfsMessage);
checkRestriction(pnfsMessage, READ_METADATA);
pnfsMessage.setCacheLocations(_nameSpaceProvider.getCacheLocation(subject, pnfsId));
pnfsMessage.setSucceeded();
} catch (FileNotFoundCacheException fnf) {
Expand Down Expand Up @@ -2123,10 +2123,10 @@ private void sendTimeout(CellMessage envelope, String error) {

public void getFileAttributes(PnfsGetFileAttributes message) {
try {
checkRestriction(message, READ_METADATA);
Subject subject = message.getSubject();
PnfsId pnfsId = populatePnfsId(message);
checkMask(message);
checkRestriction(message, READ_METADATA);
Set<FileAttribute> requested = message.getRequestedAttributes();
if (message.getUpdateAtime() && _atimeGap >= 0) {
requested.add(ACCESS_TIME);
Expand Down Expand Up @@ -2193,10 +2193,10 @@ public void getFileAttributes(PnfsGetFileAttributes message) {

public void setFileAttributes(PnfsSetFileAttributes message) {
try {
checkRestriction(message, UPDATE_METADATA);
FileAttributes attr = message.getFileAttributes();
PnfsId pnfsId = populatePnfsId(message);
checkMask(message);
checkRestriction(message, UPDATE_METADATA);
if (attr.isDefined(FileAttribute.LOCATIONS)) {
/*
* Save for post-processing.
Expand Down Expand Up @@ -2236,9 +2236,9 @@ private void listExtendedAttributes(PnfsListExtendedAttributesMessage message) {
throw new CacheException("PNFS-ID based xattr listing is not supported");
}

checkRestriction(message, READ_METADATA);
populatePnfsId(message);
checkMask(message);
checkRestriction(message, READ_METADATA);

Set<String> names = _nameSpaceProvider.listExtendedAttributes(message.getSubject(),
message.getFsPath());
Expand All @@ -2256,9 +2256,9 @@ private void readExtendedAttributes(PnfsReadExtendedAttributesMessage message) {
throw new CacheException("PNFS-ID based xattr reading is not supported");
}

checkRestriction(message, READ_METADATA);
populatePnfsId(message);
checkMask(message);
checkRestriction(message, READ_METADATA);

FsPath path = message.getFsPath();
for (String name : message.getAllNames()) {
Expand All @@ -2281,9 +2281,9 @@ private void writeExtendedAttributes(PnfsWriteExtendedAttributesMessage message)
throw new CacheException("PNFS-ID based xattr writing is not supported");
}

checkRestriction(message, UPDATE_METADATA);
populatePnfsId(message);
checkMask(message);
checkRestriction(message, UPDATE_METADATA);

FsPath path = message.getFsPath();

Expand Down Expand Up @@ -2321,9 +2321,9 @@ private void removeExtendedAttributes(PnfsRemoveExtendedAttributesMessage messag
throw new CacheException("PNFS-ID based xattr removal is not supported");
}

checkRestriction(message, UPDATE_METADATA);
populatePnfsId(message);
checkMask(message);
checkRestriction(message, UPDATE_METADATA);

FsPath path = message.getFsPath();

Expand Down

0 comments on commit 5b80b3a

Please sign in to comment.