From a8de5478b5745905b462d437e5eef16ad445c108 Mon Sep 17 00:00:00 2001 From: Albert Louis Rossi Date: Fri, 8 Sep 2023 08:56:32 -0500 Subject: [PATCH] dcache-frontend,dcache-bulk,dcache-qos: regularize observance of the admin role Motivation: Frontend currently does some special things in terms of admin permissions by changing the Subject to ROOT. But this is actually an improper subject to pass along to the Bulk or QoS services, particularly because it may result in root ownership of the request. Now that we have an admin role Principal, the most consistent idea would be to allow each service in the message chain to make its own decision as to whether to promote the subject to ROOT capabilities or not based on the RolePrincipal. Modification: Make the necessary changes to regularize the Admin role semantics. Result: Correct behavior in Bulk and QoS with respect to admin privileges. Target: master Patch: https://rb.dcache.org/r/14090/ Requires-notes: no Acked-by: Tigran --- .../main/java/org/dcache/auth/Subjects.java | 10 +++++ .../bulk/activity/BulkActivityFactory.java | 19 +++++---- .../plugin/delete/DeleteActivity.java | 1 + .../jdbc/request/JdbcBulkRequestStore.java | 9 +++-- .../restful/resources/bulk/BulkResources.java | 39 +++++++------------ .../resources/namespace/FileResources.java | 2 +- .../restful/util/HttpServletRequests.java | 6 +-- .../clients/RemoteQoSVerificationClient.java | 9 ++++- .../services/engine/util/QoSPolicyCache.java | 7 +++- .../DefaultAuthorizationPolicy.java | 2 +- 10 files changed, 59 insertions(+), 45 deletions(-) diff --git a/modules/common/src/main/java/org/dcache/auth/Subjects.java b/modules/common/src/main/java/org/dcache/auth/Subjects.java index 404db60a62b..91f444a4f87 100644 --- a/modules/common/src/main/java/org/dcache/auth/Subjects.java +++ b/modules/common/src/main/java/org/dcache/auth/Subjects.java @@ -23,6 +23,7 @@ import javax.annotation.Nullable; import javax.security.auth.Subject; import javax.security.auth.kerberos.KerberosPrincipal; +import org.dcache.auth.RolePrincipal.Role; import org.dcache.util.PrincipalSetMaker; import org.globus.gsi.gssapi.jaas.GlobusPrincipal; import org.slf4j.Logger; @@ -70,6 +71,15 @@ public static boolean isRoot(Subject subject) { return hasUid(subject, 0); } + public static boolean hasAdminRole(Subject subject) { + return hasRole(subject, Role.ADMIN); + } + + public static boolean hasRole(Subject subject, Role role) { + return subject.getPrincipals().stream().filter(p -> p instanceof RolePrincipal) + .anyMatch(p -> ((RolePrincipal) p).hasRole(role)); + } + /** * Return true if the subject is root or has the special ExemptFromNamespaceChecks principal. * diff --git a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/BulkActivityFactory.java b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/BulkActivityFactory.java index 560acda6a43..e37840934bb 100644 --- a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/BulkActivityFactory.java +++ b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/BulkActivityFactory.java @@ -112,7 +112,6 @@ public final class BulkActivityFactory implements CellMessageSender, Environment private CellStub poolManager; private CellStub qosEngine; private PoolMonitor poolMonitor; - private PnfsHandler pnfsHandler; private QoSResponseReceiver qoSResponseReceiver; private CellEndpoint endpoint; @@ -134,11 +133,13 @@ public BulkActivity createActivity(BulkRequest request, Subject subject, "cannot create " + activity + "; no such activity."); } - LOGGER.debug("creating instance of activity {} for request {}.", activity, request.getUid()); + LOGGER.debug("creating instance of activity {} for request {}.", activity, + request.getUid()); BulkActivity bulkActivity = provider.createActivity(); bulkActivity.setSubject(subject); bulkActivity.setRestriction(restriction); + bulkActivity.setActivityExecutor(activityExecutors.get(activity)); bulkActivity.setCallbackExecutor(callbackExecutors.get(activity)); BulkTargetRetryPolicy retryPolicy = retryPolicies.get(activity); @@ -164,9 +165,6 @@ public void initialize() { provider.configure(environment); providers.put(provider.getActivity(), provider); } - pnfsHandler = new PnfsHandler(pnfsManager); - pnfsHandler.setRestriction(Restrictions.none()); - pnfsHandler.setSubject(Subjects.ROOT); } /** @@ -240,8 +238,15 @@ public void setEnvironment(Map environment) { private void configureEndpoints(BulkActivity activity) { if (activity instanceof NamespaceHandlerAware) { PnfsHandler pnfsHandler = new PnfsHandler(pnfsManager); - pnfsHandler.setRestriction(activity.getRestriction()); - pnfsHandler.setSubject(activity.getSubject()); + Subject subject = activity.getSubject(); + Restriction restriction = activity.getRestriction(); + if (Subjects.hasAdminRole(subject)) { + pnfsHandler.setSubject(Subjects.ROOT); + pnfsHandler.setRestriction(Restrictions.none()); + } else { + pnfsHandler.setSubject(subject); + pnfsHandler.setRestriction(restriction); + } ((NamespaceHandlerAware) activity).setNamespaceHandler(pnfsHandler); } diff --git a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/plugin/delete/DeleteActivity.java b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/plugin/delete/DeleteActivity.java index 8234e2c52be..5a3ce9ab66d 100644 --- a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/plugin/delete/DeleteActivity.java +++ b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/plugin/delete/DeleteActivity.java @@ -91,6 +91,7 @@ public DeleteActivity(String name, TargetType targetType) { public ListenableFuture perform(String rid, long tid, FsPath path, FileAttributes attributes) { PnfsDeleteEntryMessage msg = new PnfsDeleteEntryMessage(path.toString()); + msg.setSubject(subject); if (attributes != null && attributes.getFileType() == FileType.DIR && skipDirs) { msg.setSucceeded(); return Futures.immediateFuture(msg); diff --git a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java index 43f11aadd42..f36708d5633 100644 --- a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java +++ b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java @@ -325,8 +325,8 @@ public BulkArchivedRequestInfo getArchivedInfo(Subject subject, String rid) BulkArchivedRequestInfo info = list.get(0); - if (!Subjects.isRoot(subject) && !BulkRequestStore.uidGidKey(subject) - .equals(info.getOwner())) { + if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject) && + !BulkRequestStore.uidGidKey(subject).equals(info.getOwner())) { throw new BulkPermissionDeniedException("Subject does not have " + "permission to read archived request " + rid); } @@ -468,7 +468,7 @@ public boolean isRequestSubject(Subject subject, String uid) if (requestSubject.isEmpty()) { return false; } - return Subjects.isRoot(subject) || + return Subjects.isRoot(subject) || Subjects.hasAdminRole(subject) || BulkRequestStore.uidGidKey(subject) .equals(BulkRequestStore.uidGidKey(requestSubject.get())); } @@ -773,7 +773,8 @@ private String checkRequestPermissions(Subject subject, String uid) + "request has no subject.", uid); } - if (!Subjects.isRoot(subject) && !key.equals(BulkRequestStore.uidGidKey(requestSubject))) { + if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject) && + !key.equals(BulkRequestStore.uidGidKey(requestSubject))) { throw new BulkPermissionDeniedException(uid); } diff --git a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java index c3dce077e95..ecdad401731 100644 --- a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java +++ b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java @@ -59,11 +59,15 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING */ package org.dcache.restful.resources.bulk; +import static org.dcache.restful.util.HttpServletRequests.getUserRootAwareTargetPrefix; +import static org.dcache.restful.util.JSONUtils.newBadRequestException; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.gson.Gson; import com.google.gson.JsonParseException; +import diskCacheV111.util.PnfsHandler; import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; @@ -72,10 +76,14 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING import io.swagger.annotations.Authorization; import io.swagger.annotations.Example; import io.swagger.annotations.ExampleProperty; -import org.json.JSONArray; -import org.json.JSONObject; -import org.springframework.stereotype.Component; - +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Named; import javax.security.auth.Subject; @@ -95,19 +103,6 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; - -import diskCacheV111.util.PnfsHandler; - -import org.dcache.auth.Subjects; import org.dcache.auth.attributes.Restriction; import org.dcache.auth.attributes.Restrictions; import org.dcache.cells.CellStub; @@ -129,9 +124,9 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING import org.dcache.services.bulk.BulkRequestStatus; import org.dcache.services.bulk.BulkRequestStatusMessage; import org.dcache.services.bulk.BulkRequestSummary; - -import static org.dcache.restful.util.HttpServletRequests.getUserRootAwareTargetPrefix; -import static org.dcache.restful.util.JSONUtils.newBadRequestException; +import org.json.JSONArray; +import org.json.JSONObject; +import org.springframework.stereotype.Component; /** *

RESTful API to the BulkService.

@@ -483,10 +478,6 @@ public static Subject getSubject() { throw new NotAuthorizedException("User cannot be anonymous."); } - if (RequestUser.isAdmin()) { - return Subjects.ROOT; - } - return RequestUser.getSubject(); } diff --git a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/namespace/FileResources.java b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/namespace/FileResources.java index 09f4e3469cb..edc442f1eaf 100644 --- a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/namespace/FileResources.java +++ b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/namespace/FileResources.java @@ -446,7 +446,7 @@ public Response cmrResources( String targetQos = reqPayload.getString("target"); String qosPolicy = reqPayload.getString("policy"); Integer qosState = (Integer)reqPayload.get("state"); - Subject subject = RequestUser.isAdmin() ? Subjects.ROOT : RequestUser.getSubject(); + Subject subject = RequestUser.getSubject(); if (!useQosService) { new QoSTransitionEngine(poolmanager, poolMonitor, diff --git a/modules/dcache-frontend/src/main/java/org/dcache/restful/util/HttpServletRequests.java b/modules/dcache-frontend/src/main/java/org/dcache/restful/util/HttpServletRequests.java index e3433958905..099cb01709b 100644 --- a/modules/dcache-frontend/src/main/java/org/dcache/restful/util/HttpServletRequests.java +++ b/modules/dcache-frontend/src/main/java/org/dcache/restful/util/HttpServletRequests.java @@ -27,8 +27,6 @@ import javax.security.auth.Subject; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.BadRequestException; -import org.dcache.auth.RolePrincipal; -import org.dcache.auth.RolePrincipal.Role; import org.dcache.auth.Subjects; import org.dcache.auth.attributes.LoginAttribute; import org.dcache.auth.attributes.Restriction; @@ -56,9 +54,7 @@ public static Set getLoginAttributes(HttpServletRequest request) } public static boolean isAdmin(HttpServletRequest request) { - return RequestUser.getSubject().getPrincipals().stream() - .filter(p -> p instanceof RolePrincipal) - .anyMatch(p -> ((RolePrincipal) p).hasRole(Role.ADMIN)); + return Subjects.hasAdminRole(RequestUser.getSubject()); } public static Subject roleAwareSubject(HttpServletRequest request) { diff --git a/modules/dcache-qos/src/main/java/org/dcache/qos/remote/clients/RemoteQoSVerificationClient.java b/modules/dcache-qos/src/main/java/org/dcache/qos/remote/clients/RemoteQoSVerificationClient.java index a422d67db93..8655bd88a82 100644 --- a/modules/dcache-qos/src/main/java/org/dcache/qos/remote/clients/RemoteQoSVerificationClient.java +++ b/modules/dcache-qos/src/main/java/org/dcache/qos/remote/clients/RemoteQoSVerificationClient.java @@ -100,7 +100,14 @@ public void fileQoSAdjustmentCompleted(QoSAdjustmentResponse adjustmentResponse) @Override public void fileQoSVerificationCancelled(PnfsId pnfsId, Subject subject) throws QoSException { QoSVerificationCancelledMessage msg = new QoSVerificationCancelledMessage(pnfsId); - msg.setSubject(subject); + /* + * The only cancellation that can currently take place is either + * through the Bulk service or the admin interface. + * In the former case, a pre-authorization check of the subject is done; + * in the latter, the subject has the admin role. + * Hence no further authorization is necessary here and no futher checks of + * the subject are done. + */ verificationService.send(msg); } diff --git a/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/util/QoSPolicyCache.java b/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/util/QoSPolicyCache.java index b4a224efc01..f06fa8d0768 100644 --- a/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/util/QoSPolicyCache.java +++ b/modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/util/QoSPolicyCache.java @@ -70,6 +70,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import javax.security.auth.Subject; import org.dcache.auth.Subjects; import org.dcache.cells.CellStub; import org.dcache.qos.QoSPolicy; @@ -155,7 +156,8 @@ public PnfsManagerGetQoSPolicyMessage messageArrived(PnfsManagerGetQoSPolicyMess } public PnfsManagerRmQoSPolicyMessage messageArrived(PnfsManagerRmQoSPolicyMessage message) throws CacheException { - if (!Subjects.isRoot(message.getSubject())) { + Subject subject = message.getSubject(); + if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject)) { throw new PermissionDeniedCacheException("not authorized to remove policies."); } removePolicy(message.getPolicyName()); @@ -169,7 +171,8 @@ public PnfsManagerRmQoSPolicyMessage messageArrived(PnfsManagerRmQoSPolicyMessag */ public PnfsManagerAddQoSPolicyMessage messageArrived(PnfsManagerAddQoSPolicyMessage message) throws CacheException { - if (!Subjects.isRoot(message.getSubject())) { + Subject subject = message.getSubject(); + if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject)) { throw new PermissionDeniedCacheException("not authorized to update policies."); } diff --git a/modules/dcache/src/main/java/org/dcache/pinmanager/DefaultAuthorizationPolicy.java b/modules/dcache/src/main/java/org/dcache/pinmanager/DefaultAuthorizationPolicy.java index 41959e2fd16..d66e60ad9e8 100644 --- a/modules/dcache/src/main/java/org/dcache/pinmanager/DefaultAuthorizationPolicy.java +++ b/modules/dcache/src/main/java/org/dcache/pinmanager/DefaultAuthorizationPolicy.java @@ -7,7 +7,7 @@ public class DefaultAuthorizationPolicy implements AuthorizationPolicy { private boolean isAuthorized(Subject subject, Pin pin) { - return (Subjects.isRoot(subject) || + return (Subjects.isRoot(subject) || Subjects.hasAdminRole(subject) || Subjects.hasUid(subject, pin.getUid()) || Subjects.hasGid(subject, pin.getGid())); }