Skip to content

Commit

Permalink
chimera: Let SRM respect setgid on upload
Browse files Browse the repository at this point in the history
Motivation:

SRM instructs the pnfs manager to create temporary upload directories for each upload.
The pnfs manager fails to check the setgid bit on the target directory and thus the
group is not inherited when setting the setgid bit.

Modification:

Check if the setgid bit is set and if so copy both that bit and the group of
the target directory.

Also remove the authorization check on commit and cancel: Indepedently of the setgid
problem, this is necessary as the owner and group of the target directory may be
different from the uid and primary gid and thus the check is erroneous. The setgid
behaviour just happens to run into this flaw as it is one way in which the group may
become different than the primary gid.

Result:

Fixes #1747.

Target: trunk
Request: 2.13
Request: 2.12
Request: 2.11
Request: 2.10
Require-notes: yes
Require-book: no
Acked-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Patch: https://rb.dcache.org/r/8388/
  • Loading branch information
gbehrmann committed Jul 29, 2015
1 parent e9355db commit 40a9516
Showing 1 changed file with 12 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1185,22 +1185,25 @@ public FsPath createUploadPath(Subject subject, FsPath path, FsPath rootPath,

/* Attributes are inherited from real parent directory.
*/
if (uid == DEFAULT) {
if (Subjects.isNobody(subject) || _inheritFileOwnership) {
uid = parentOfPath.statCache().getUid();
} else {
uid = (int) Subjects.getUid(subject);
}
if (mode == DEFAULT) {
mode = parentOfPath.statCache().getMode() & UnixPermission.S_PERMS;
}
if (gid == DEFAULT) {
if ((parentOfPath.statCache().getMode() & UnixPermission.S_ISGID) != 0) {
gid = parentOfPath.statCache().getGid();
mode |= UnixPermission.S_ISGID;
} else if (gid == DEFAULT) {
if (Subjects.isNobody(subject) || _inheritFileOwnership) {
gid = parentOfPath.statCache().getGid();
} else {
gid = (int) Subjects.getPrimaryGid(subject);
}
}
if (mode == DEFAULT) {
mode = parentOfPath.statCache().getMode() & UMASK_DIR;
if (uid == DEFAULT) {
if (Subjects.isNobody(subject) || _inheritFileOwnership) {
uid = parentOfPath.statCache().getUid();
} else {
uid = (int) Subjects.getUid(subject);
}
}

/* ACLs are copied from real parent to the temporary upload directory
Expand Down Expand Up @@ -1287,10 +1290,6 @@ public PnfsId commitUpload(Subject subject, FsPath temporaryPath, FsPath finalPa
throw new FileNotFoundCacheException("No such file or directory: " + temporaryPath, e);
}

/* Subject must be authorized to complete the upload.
*/
checkUploadAuthorization(subject, temporaryDirInode);

/* Target directory must exist.
*/
FsInode finalDirInode;
Expand Down Expand Up @@ -1347,18 +1346,12 @@ public void cancelUpload(Subject subject, FsPath temporaryPath, FsPath finalPath
/* Temporary upload directory must exist.
*/
FsInode uploadDirInode;
FsInode temporaryDirInode;
try {
uploadDirInode = _fs.path2inode(temporaryDir.getParent().toString());
temporaryDirInode = uploadDirInode.inodeOf(temporaryPath.getParent().getName());
} catch (FileNotFoundHimeraFsException e) {
throw new FileNotFoundCacheException("No such file or directory: " + temporaryDir, e);
}

/* Subject must be authorized to cancel the upload.
*/
checkUploadAuthorization(subject, temporaryDirInode);

/* Delete temporary upload directory and any files in it.
*/
removeRecursively(uploadDirInode, temporaryPath.getParent().getName());
Expand All @@ -1383,17 +1376,6 @@ private void removeRecursively(FsInode parent, String name) throws ChimeraFsExce
}
}

private void checkUploadAuthorization(Subject subject, FsInode temporaryDir)
throws ChimeraFsException, PermissionDeniedCacheException
{
/* Subject must be owner of upload directory, i.e. subject must have initiated the download.
*/
if (!Subjects.hasUid(subject, temporaryDir.statCache().getUid()) ||
!Subjects.hasGid(subject, temporaryDir.statCache().getGid())) {
throw new PermissionDeniedCacheException("File must be committed by owner.");
}
}

private FsPath getParentOfFile(FsPath path) throws NotFileCacheException
{
try {
Expand Down

0 comments on commit 40a9516

Please sign in to comment.