From 148b27c9732868480761e01d093e1478cbc49774 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Sat, 3 Oct 2020 19:38:54 -0700 Subject: [PATCH 1/4] HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights. --- .../ozone/security/acl/OzoneObjInfo.java | 10 + .../ozone/security/acl/RequestContext.java | 48 ++- .../apache/hadoop/ozone/om/TestOmAcls.java | 9 +- .../ozone/om/TestOzoneManagerListVolumes.java | 36 +-- .../apache/hadoop/ozone/om/OzoneManager.java | 130 +++++--- .../ozone/om/request/OMClientRequest.java | 8 +- .../request/bucket/OMBucketCreateRequest.java | 2 +- .../request/bucket/OMBucketDeleteRequest.java | 2 +- .../bucket/OMBucketSetPropertyRequest.java | 2 +- .../bucket/acl/OMBucketAclRequest.java | 2 +- .../ozone/om/request/key/OMKeyRequest.java | 4 +- .../om/request/key/acl/OMKeyAclRequest.java | 2 +- .../key/acl/prefix/OMPrefixAclRequest.java | 9 +- .../ozone/om/request/util/ObjectParser.java | 6 +- .../request/volume/OMVolumeCreateRequest.java | 2 +- .../request/volume/OMVolumeDeleteRequest.java | 2 +- .../volume/OMVolumeSetOwnerRequest.java | 2 +- .../volume/OMVolumeSetQuotaRequest.java | 2 +- .../volume/acl/OMVolumeAclRequest.java | 2 +- .../security/acl/OzoneNativeAuthorizer.java | 44 ++- .../ozone/security/acl/TestVolumeOwner.java | 298 ++++++++++++++++++ 21 files changed, 530 insertions(+), 92 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java index cbae18cb784c..42ddbb9a2112 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java @@ -159,6 +159,16 @@ public static Builder newBuilder() { return new Builder(); } + public static Builder getBuilder(ResourceType resType, + StoreType storeType, String vol, String bucket, String key) { + return OzoneObjInfo.Builder.newBuilder() + .setResType(resType) + .setStoreType(storeType) + .setVolumeName(vol) + .setBucketName(bucket) + .setKeyName(key); + } + public static Builder fromKeyArgs(OmKeyArgs args) { return new Builder() .setVolumeName(args.getVolumeName()) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java index 329582721e0b..043cd558987a 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java @@ -16,6 +16,7 @@ */ package org.apache.hadoop.ozone.security.acl; +import org.apache.hadoop.ipc.ProtobufRpcEngine; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType; @@ -32,16 +33,20 @@ public class RequestContext { private final String serviceId; private final ACLIdentityType aclType; private final ACLType aclRights; + private final String ownerName; + @SuppressWarnings("parameternumber") public RequestContext(String host, InetAddress ip, UserGroupInformation clientUgi, String serviceId, - ACLIdentityType aclType, ACLType aclRights) { + ACLIdentityType aclType, ACLType aclRights, + String ownerName) { this.host = host; this.ip = ip; this.clientUgi = clientUgi; this.serviceId = serviceId; this.aclType = aclType; this.aclRights = aclRights; + this.ownerName = ownerName; } /** @@ -55,6 +60,12 @@ public static class Builder { private IAccessAuthorizer.ACLIdentityType aclType; private IAccessAuthorizer.ACLType aclRights; + /** + * ownerName is specially added to allow + * authorizer to honor owner privilege. + */ + private String ownerName; + public Builder setHost(String bHost) { this.host = bHost; return this; @@ -80,14 +91,23 @@ public Builder setAclType(ACLIdentityType acl) { return this; } + public ACLType getAclRights() { + return this.aclRights; + } + public Builder setAclRights(ACLType aclRight) { this.aclRights = aclRight; return this; } + public Builder setOwnerName(String owner) { + this.ownerName = owner; + return this; + } + public RequestContext build() { return new RequestContext(host, ip, clientUgi, serviceId, aclType, - aclRights); + aclRights, ownerName); } } @@ -95,6 +115,27 @@ public static Builder newBuilder() { return new Builder(); } + public static RequestContext.Builder getBuilder( + UserGroupInformation ugi, InetAddress remoteAddress, String hostName, + ACLType aclType, String ownerName) { + RequestContext.Builder contextBuilder = RequestContext.newBuilder() + .setClientUgi(ugi) + .setIp(remoteAddress) + .setHost(hostName) + .setAclType(ACLIdentityType.USER) + .setAclRights(aclType) + .setOwnerName(ownerName); + return contextBuilder; + } + + public static RequestContext.Builder getBuilder(UserGroupInformation ugi, + ACLType aclType, String ownerName) { + return getBuilder(ugi, + ProtobufRpcEngine.Server.getRemoteIp(), + ProtobufRpcEngine.Server.getRemoteIp().getHostName(), + aclType, ownerName); + } + public String getHost() { return host; } @@ -119,4 +160,7 @@ public ACLType getAclRights() { return aclRights; } + public String getOwnerName() { + return ownerName; + } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java index 34303ff9eedb..271109f0b3c7 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java @@ -116,7 +116,14 @@ public void testBucketCreationPermissionDenied() throws Exception { String volumeName = RandomStringUtils.randomAlphabetic(5).toLowerCase(); String bucketName = RandomStringUtils.randomAlphabetic(5).toLowerCase(); - cluster.getClient().getObjectStore().createVolume(volumeName); + + VolumeArgs createVolumeArgs = VolumeArgs.newBuilder() + .setOwner("user" + RandomStringUtils.randomNumeric(5)) + .setAdmin("admin" + RandomStringUtils.randomNumeric(5)) + .build(); + + cluster.getClient().getObjectStore().createVolume(volumeName, + createVolumeArgs); OzoneVolume volume = cluster.getClient().getObjectStore().getVolume(volumeName); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java index d7aaf37fd637..a47aa08f2452 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java @@ -201,16 +201,16 @@ public void testListVolumeWithOtherUsersListAllAllowed() throws Exception { // Login as user1, list other users' volumes UserGroupInformation.setLoginUser(user1); - checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume5"), - true); + checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume4", + "volume5"), true); // Add "s3v" created default by OM. checkUser(cluster, adminUser, Arrays.asList("volume1", "volume2", "volume3", "volume4", "volume5", "s3v"), true); UserGroupInformation.setLoginUser(user2); - checkUser(cluster, user1, Arrays.asList("volume1", "volume4", "volume5"), - true); + checkUser(cluster, user1, Arrays.asList("volume1", "volume3", "volume4", + "volume5"), true); checkUser(cluster, adminUser, Arrays.asList("volume1", "volume2", "volume3", "volume4", "volume5", "s3v"), true); @@ -229,18 +229,18 @@ public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception { // Login as user1, list other users' volumes, expect failure UserGroupInformation.setLoginUser(user1); - checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume5"), - false); + checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume4", + "volume5"), false); // Add "s3v" created default by OM. checkUser(cluster, adminUser, Arrays.asList("volume1", "volume2", "volume3", "volume4", "volume5", "s3v"), false); // While admin should be able to list volumes just fine. UserGroupInformation.setLoginUser(adminUser); - checkUser(cluster, user1, Arrays.asList("volume1", "volume4", "volume5"), - true); - checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume5"), - true); + checkUser(cluster, user1, Arrays.asList("volume1", "volume3", "volume4", + "volume5"), true); + checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume4", + "volume5"), true); stopCluster(cluster); } @@ -249,10 +249,10 @@ public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception { public void testAclEnabledListAllAllowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = true MiniOzoneCluster cluster = startCluster(true, true); - checkUser(cluster, user1, Arrays.asList("volume1", "volume4", "volume5"), - true); - checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume5"), - true); + checkUser(cluster, user1, Arrays.asList("volume1", "volume3", "volume4", + "volume5"), true); + checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume4", + "volume5"), true); // Add "s3v" created default by OM. checkUser(cluster, adminUser, Arrays.asList("volume1", "volume2", "volume3", @@ -267,11 +267,11 @@ public void testAclEnabledListAllDisallowed() throws Exception { // The default user is adminUser as set in init(), // listall always succeeds if we use that UGI, we should use non-admin here UserGroupInformation.setLoginUser(user1); - checkUser(cluster, user1, Arrays.asList("volume1", "volume4", "volume5"), - false); + checkUser(cluster, user1, Arrays.asList("volume1", "volume3", "volume4", + "volume5"), false); UserGroupInformation.setLoginUser(user2); - checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume5"), - false); + checkUser(cluster, user2, Arrays.asList("volume2", "volume3", "volume4", + "volume5"), false); UserGroupInformation.setLoginUser(adminUser); // Add "s3v" created default by OM. checkUser(cluster, adminUser, Arrays.asList("volume1", "volume2", diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index e58af8b49abb..23f8ddf5cf03 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -226,6 +226,7 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TOKEN_ERROR_OTHER; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneManagerService.newReflectiveBlockingService; import org.apache.hadoop.util.Time; @@ -528,6 +529,7 @@ private void instantiateServices() throws IOException { authorizer.setKeyManager(keyManager); authorizer.setPrefixManager(prefixManager); authorizer.setOzoneAdmins(getOzoneAdmins(configuration)); + authorizer.setAllowListAllVolumes(allowListAllVolumes); } } else { accessAuthorizer = null; @@ -1593,7 +1595,7 @@ public void createVolume(OmVolumeArgs args) throws IOException { metrics.incNumVolumeCreates(); if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.CREATE, - args.getVolume(), null, null); + args.getVolume(), null, null, args.getOwnerName()); } volumeManager.createVolume(args); AUDIT.logWriteSuccess(buildAuditMessageForSuccess(OMAction.CREATE_VOLUME, @@ -1618,16 +1620,17 @@ public void createVolume(OmVolumeArgs args) throws IOException { * @param vol - name of volume * @param bucket - bucket name * @param key - key + * @param ownerName - volume owner name which should have full permission. * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied. */ private void checkAcls(ResourceType resType, StoreType store, - ACLType acl, String vol, String bucket, String key) - throws OMException { + ACLType acl, String vol, String bucket, String key, + String ownerName) throws OMException { checkAcls(resType, store, acl, vol, bucket, key, ProtobufRpcEngine.Server.getRemoteUser(), ProtobufRpcEngine.Server.getRemoteIp(), ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - true); + true, ownerName); } /** @@ -1642,25 +1645,34 @@ private boolean hasAcls(String userName, ResourceType resType, UserGroupInformation.createRemoteUser(userName), ProtobufRpcEngine.Server.getRemoteIp(), ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - false); + false, null); } catch (OMException ex) { - // Should not trigger exception here at all return false; } } - /** - * CheckAcls for the ozone object. - * - * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied. - */ - @SuppressWarnings("parameternumber") - public void checkAcls(ResourceType resType, StoreType storeType, - ACLType aclType, String vol, String bucket, String key, - UserGroupInformation ugi, InetAddress remoteAddress, String hostName) - throws OMException { - checkAcls(resType, storeType, aclType, vol, bucket, key, - ugi, remoteAddress, hostName, true); + private String getVolumeOwner(OMMetadataManager metadataMgr, + String volume) throws OMException { + Boolean lockAcquired = metadataMgr.getLock().acquireReadLock( + VOLUME_LOCK, volume); + String dbVolumeKey = metadataMgr.getVolumeKey(volume); + OmVolumeArgs volumeArgs = null; + try { + volumeArgs = metadataMgr.getVolumeTable().get(dbVolumeKey); + } catch (IOException ioe) { + throw new OMException("Volume " + volume + " is not found", + OMException.ResultCodes.VOLUME_NOT_FOUND); + } finally { + if (lockAcquired) { + metadataMgr.getLock().releaseReadLock(VOLUME_LOCK, volume); + } + } + if (volumeArgs != null) { + return volumeArgs.getOwnerName(); + } else { + throw new OMException("Volume " + volume + " is not found", + OMException.ResultCodes.VOLUME_NOT_FOUND); + } } /** @@ -1671,11 +1683,18 @@ public void checkAcls(ResourceType resType, StoreType storeType, * and throwOnPermissionDenied set to true. */ @SuppressWarnings("parameternumber") - private boolean checkAcls(ResourceType resType, StoreType storeType, + public boolean checkAcls(ResourceType resType, StoreType storeType, ACLType aclType, String vol, String bucket, String key, UserGroupInformation ugi, InetAddress remoteAddress, String hostName, - boolean throwIfPermissionDenied) + boolean throwIfPermissionDenied, String ownerName) throws OMException { + String volOwnerName = ownerName; + // In the case of ROOT "/", we send null as owner name. + // OzoneNativeAuthorizer has as special handling to avoid NPE + // TODO: send the OM login user as the owner of non-exist "/" volume. + if ((ownerName == null && !vol.equals(OzoneConsts.OZONE_ROOT))) { + volOwnerName = getVolumeOwner(metadataManager, vol); + } OzoneObj obj = OzoneObjInfo.Builder.newBuilder() .setResType(resType) .setStoreType(storeType) @@ -1688,13 +1707,17 @@ private boolean checkAcls(ResourceType resType, StoreType storeType, .setHost(hostName) .setAclType(ACLIdentityType.USER) .setAclRights(aclType) + .setOwnerName(volOwnerName) .build(); if (!accessAuthorizer.checkAccess(obj, context)) { if (throwIfPermissionDenied) { LOG.warn("User {} doesn't have {} permission to access {} /{}/{}/{}", - ugi.getUserName(), aclType, resType, vol, bucket, key); - throw new OMException("User " + ugi.getUserName() + " doesn't have " + - aclType + " permission to access " + resType, + context.getClientUgi().getUserName(), context.getAclRights(), + obj.getResourceType(), obj.getVolumeName(), obj.getBucketName(), + obj.getKeyName()); + throw new OMException("User " + context.getClientUgi().getUserName() + + " doesn't have " + context.getAclRights() + + " permission to access " + obj.getResourceType(), ResultCodes.PERMISSION_DENIED); } return false; @@ -1719,7 +1742,7 @@ public boolean getAclsEnabled() { public boolean setOwner(String volume, String owner) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.WRITE_ACL, volume, - null, null); + null, null, null); } Map auditMap = buildAuditMap(volume); auditMap.put(OzoneConsts.OWNER, owner); @@ -1767,7 +1790,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, - ACLType.READ, volume, null, null); + ACLType.READ, volume, null, null, null); } boolean auditSuccess = true; Map auditMap = buildAuditMap(volume); @@ -1801,7 +1824,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) public OmVolumeArgs getVolumeInfo(String volume) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.READ, volume, - null, null); + null, null, null); } boolean auditSuccess = true; @@ -1834,7 +1857,7 @@ public void deleteVolume(String volume) throws IOException { try { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.DELETE, volume, - null, null); + null, null, null); } metrics.incNumVolumeDeletes(); volumeManager.deleteVolume(volume); @@ -1936,7 +1959,7 @@ public List listAllVolumes(String prefix, String prevKey, int // Only admin can list all volumes when disallowed in config if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.LIST, - OzoneConsts.OZONE_ROOT, null, null); + OzoneConsts.OZONE_ROOT, null, null, null); } } return volumeManager.listVolumes(null, prefix, prevKey, maxKeys); @@ -1965,7 +1988,7 @@ public void createBucket(OmBucketInfo bucketInfo) throws IOException { try { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.CREATE, - bucketInfo.getVolumeName(), bucketInfo.getBucketName(), null); + bucketInfo.getVolumeName(), bucketInfo.getBucketName(), null, null); } metrics.incNumBucketCreates(); bucketManager.createBucket(bucketInfo); @@ -1989,7 +2012,7 @@ public List listBuckets(String volumeName, throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.LIST, volumeName, - null, null); + null, null, null); } boolean auditSuccess = true; Map auditMap = buildAuditMap(volumeName); @@ -2028,7 +2051,7 @@ public OmBucketInfo getBucketInfo(String volume, String bucket) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, volume, - bucket, null); + bucket, null, null); } boolean auditSuccess = true; Map auditMap = buildAuditMap(volume); @@ -2065,13 +2088,14 @@ public OpenKeySession openKey(OmKeyArgs args) throws IOException { if (isAclEnabled) { try { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } catch (OMException ex) { // For new keys key checkAccess call will fail as key doesn't exist. // Check user access for bucket. if (ex.getResult().equals(KEY_NOT_FOUND)) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), + null); } else { throw ex; } @@ -2108,13 +2132,14 @@ public void commitKey(OmKeyArgs args, long clientID) if (isAclEnabled) { try { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } catch (OMException ex) { // For new keys key checkAccess call will fail as key doesn't exist. // Check user access for bucket. if (ex.getResult().equals(KEY_NOT_FOUND)) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), + null); } else { throw ex; } @@ -2157,13 +2182,15 @@ public OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID, if (isAclEnabled) { try { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), + null); } catch (OMException ex) { // For new keys key checkAccess call will fail as key doesn't exist. // Check user access for bucket. if (ex.getResult().equals(KEY_NOT_FOUND)) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), + null); } else { throw ex; } @@ -2206,7 +2233,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } boolean auditSuccess = true; @@ -2247,7 +2274,7 @@ public void renameKey(OmKeyArgs args, String toKeyName) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } Map auditMap = bucket.audit(args.toAuditMap()); @@ -2283,7 +2310,7 @@ public void deleteKey(OmKeyArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.DELETE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } metrics.incNumKeyDeletes(); @@ -2319,7 +2346,7 @@ public List listKeys(String volumeName, String bucketName, if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.LIST, - bucket.realVolume(), bucket.realBucket(), keyPrefix); + bucket.realVolume(), bucket.realBucket(), keyPrefix, null); } boolean auditSuccess = true; @@ -2355,7 +2382,7 @@ public List listTrash(String volumeName, if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.LIST, - volumeName, bucketName, keyPrefix); + volumeName, bucketName, keyPrefix, null); } boolean auditSuccess = true; @@ -2394,7 +2421,7 @@ public void setBucketProperty(OmBucketArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - args.getVolumeName(), args.getBucketName(), null); + args.getVolumeName(), args.getBucketName(), null, null); } try { metrics.incNumBucketUpdates(); @@ -2420,7 +2447,7 @@ public void setBucketProperty(OmBucketArgs args) public void deleteBucket(String volume, String bucket) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, volume, - bucket, null); + bucket, null, null); } Map auditMap = buildAuditMap(volume); auditMap.put(OzoneConsts.BUCKET, bucket); @@ -2865,7 +2892,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } boolean auditSuccess = true; @@ -2898,7 +2925,7 @@ public List listStatus(OmKeyArgs args, boolean recursive, if (isAclEnabled) { checkAcls(getResourceType(args), StoreType.OZONE, ACLType.READ, - bucket.realVolume(), bucket.realBucket(), args.getKeyName()); + bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); } boolean auditSuccess = true; @@ -2955,7 +2982,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); } switch (obj.getResourceType()) { case VOLUME: @@ -2996,7 +3023,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); } switch (obj.getResourceType()) { case VOLUME: @@ -3038,7 +3065,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); } switch (obj.getResourceType()) { case VOLUME: @@ -3077,7 +3104,7 @@ public List getAcl(OzoneObj obj) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.READ_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); } switch (obj.getResourceType()) { case VOLUME: @@ -3497,7 +3524,6 @@ public ResolvedBucket resolveBucketLink(Pair requested, return new ResolvedBucket(requested, resolved); } - public ResolvedBucket resolveBucketLink(Pair requested) throws IOException { @@ -3550,7 +3576,7 @@ private Pair resolveBucketLink( if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, volumeName, bucketName, null, userGroupInformation, - remoteAddress, hostName); + remoteAddress, hostName, true, null); } return resolveBucketLink( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 728a62402cbd..a8cbfc34c055 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -146,14 +146,18 @@ public OzoneManagerProtocolProtos.UserInfo getUserInfo() { * @param vol * @param bucket * @param key + * @param ownerName - only used for createVolume, null otherwise. * @throws IOException */ + @SuppressWarnings("parameternumber") public void checkAcls(OzoneManager ozoneManager, OzoneObj.ResourceType resType, OzoneObj.StoreType storeType, IAccessAuthorizer.ACLType aclType, - String vol, String bucket, String key) throws IOException { + String vol, String bucket, String key, + String ownerName) throws IOException { ozoneManager.checkAcls(resType, storeType, aclType, vol, bucket, key, - createUGI(), getRemoteAddress(), getHostName()); + createUGI(), getRemoteAddress(), getHostName(), true, + ownerName); } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java index fd303e7f09ac..3121af1f9cc1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java @@ -171,7 +171,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.CREATE, - volumeName, bucketName, null); + volumeName, bucketName, null, null); } acquiredVolumeLock = diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java index 91aef6a2d44a..4d321070a0a2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java @@ -99,7 +99,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, - volumeName, bucketName, null); + volumeName, bucketName, null, null); } // acquire lock diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index 583facbc0fca..0dc5fee5b95b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -107,7 +107,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE, - volumeName, bucketName, null); + volumeName, bucketName, null, null); } // acquire lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java index a493f9fa1472..099b17e815e0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java @@ -93,7 +93,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null); + volume, null, null, null); } lockAcquired = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index a048533001e1..696a9f59afe4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -411,7 +411,7 @@ protected void checkBucketAcls(OzoneManager ozoneManager, String volume, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, aclType, - volume, bucket, key); + volume, bucket, key, null); } } @@ -432,7 +432,7 @@ protected void checkKeyAcls(OzoneManager ozoneManager, String volume, throws IOException { if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, resourceType, OzoneObj.StoreType.OZONE, aclType, - volume, bucket, key); + volume, bucket, key, null); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java index 9fae4988fa68..686700c1f9dc 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java @@ -79,7 +79,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, key); + volume, bucket, key, null); } lockAcquired = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java index e928402643ef..c24ee69a7311 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java @@ -29,7 +29,9 @@ import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.om.request.OMClientRequest; +import org.apache.hadoop.ozone.om.request.util.ObjectParser; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; @@ -71,12 +73,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, (PrefixManagerImpl) ozoneManager.getPrefixManager(); try { String prefixPath = getOzoneObj().getPath(); + ObjectParser objectParser = new ObjectParser(prefixPath, + OzoneManagerProtocolProtos.OzoneObj.ObjectType.PREFIX); + volume = objectParser.getVolume(); + bucket = objectParser.getBucket(); + key = objectParser.getKey(); // check Acl if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.PREFIX, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, key); + volume, bucket, key, null); } lockAcquired = diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/ObjectParser.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/ObjectParser.java index c12cdac6c862..9b8270205a8f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/ObjectParser.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/util/ObjectParser.java @@ -43,7 +43,6 @@ public class ObjectParser { public ObjectParser(String path, ObjectType objectType) throws OMException { Preconditions.checkNotNull(path); String[] tokens = StringUtils.split(path, OZONE_URI_DELIMITER, 3); - if (objectType == ObjectType.VOLUME && tokens.length == 1) { volume = tokens[0]; } else if (objectType == ObjectType.BUCKET && tokens.length == 2) { @@ -53,6 +52,11 @@ public ObjectParser(String path, ObjectType objectType) throws OMException { volume = tokens[0]; bucket = tokens[1]; key = tokens[2]; + } else if (objectType == ObjectType.PREFIX && tokens.length >= 1) { + volume = tokens[0]; + if (tokens.length >= 2) { + bucket = tokens[1]; + } } else { throw new OMException("Illegal path " + path, OMException.ResultCodes.INVALID_PATH_IN_ACL_REQUEST); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java index 9c81c36eaf22..771c5fab7816 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java @@ -136,7 +136,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.CREATE, volume, - null, null); + null, null, owner); } // acquire lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java index ce93e269e250..fdc0ad81a052 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java @@ -91,7 +91,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, volume, - null, null); + null, null, null); } acquiredVolumeLock = omMetadataManager.getLock().acquireWriteLock( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java index 6873086750a4..1740fdc23650 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java @@ -114,7 +114,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null); + volume, null, null, null); } long maxUserVolumeCount = ozoneManager.getMaxUserVolumeCount(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java index fc54c8819101..826f426f04b4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java @@ -121,7 +121,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE, volume, - null, null); + null, null, null); } acquireVolumeLock = omMetadataManager.getLock().acquireWriteLock( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java index de7f0c0a36d0..dd07f3ad1fce 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java @@ -78,7 +78,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null); + volume, null, null, null); } lockAcquired = omMetadataManager.getLock().acquireWriteLock( VOLUME_LOCK, volume); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java index df98e2049601..aebefdceeb9c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java @@ -50,6 +50,7 @@ public class OzoneNativeAuthorizer implements IAccessAuthorizer { private KeyManager keyManager; private PrefixManager prefixManager; private Collection ozAdmins; + private boolean allowListAllVolumes; public OzoneNativeAuthorizer() { } @@ -87,14 +88,18 @@ public boolean checkAccess(IOzoneObj ozObject, RequestContext context) "configured to work with OzoneObjInfo type only.", INVALID_REQUEST); } - // by pass all checks for admin + // bypass all checks for admin boolean isAdmin = isAdmin(context.getClientUgi()); if (isAdmin) { return true; } + boolean isOwner = isOwner(context.getClientUgi(), context.getOwnerName()); boolean isListAllVolume = ((context.getAclRights() == ACLType.LIST) && objInfo.getVolumeName().equals(OzoneConsts.OZONE_ROOT)); + if (isListAllVolume) { + return getAllowListAllVolumes(); + } // For CREATE and DELETE acl requests, the parents need to be checked // for WRITE acl. If Key create request is received, then we need to @@ -114,13 +119,19 @@ public boolean checkAccess(IOzoneObj ozObject, RequestContext context) switch (objInfo.getResourceType()) { case VOLUME: LOG.trace("Checking access for volume: {}", objInfo); - if (isACLTypeCreate || isListAllVolume) { + if (isACLTypeCreate) { // only admin is allowed to create volume and list all volumes return false; } - return volumeManager.checkAccess(objInfo, context); + boolean volumeAccess = isOwner || + volumeManager.checkAccess(objInfo, context); + return volumeAccess; case BUCKET: LOG.trace("Checking access for bucket: {}", objInfo); + // Skip check for volume owner + if (isOwner) { + return true; + } // Skip bucket access check for CREATE acl since // bucket will not exist at the time of creation boolean bucketAccess = isACLTypeCreate @@ -129,6 +140,10 @@ public boolean checkAccess(IOzoneObj ozObject, RequestContext context) && volumeManager.checkAccess(objInfo, parentContext)); case KEY: LOG.trace("Checking access for Key: {}", objInfo); + // Skip check for volume owner + if (isOwner) { + return true; + } // Skip key access check for CREATE acl since // key will not exist at the time of creation boolean keyAccess = isACLTypeCreate @@ -139,6 +154,10 @@ public boolean checkAccess(IOzoneObj ozObject, RequestContext context) && volumeManager.checkAccess(objInfo, parentContext)); case PREFIX: LOG.trace("Checking access for Prefix: {}", objInfo); + // Skip check for volume owner + if (isOwner) { + return true; + } // Skip prefix access check for CREATE acl since // prefix will not exist at the time of creation boolean prefixAccess = isACLTypeCreate @@ -176,6 +195,25 @@ public Collection getOzoneAdmins() { return Collections.unmodifiableCollection(this.ozAdmins); } + public void setAllowListAllVolumes(boolean allowListAllVolumes) { + this.allowListAllVolumes = allowListAllVolumes; + } + + public boolean getAllowListAllVolumes() { + return allowListAllVolumes; + } + + private boolean isOwner(UserGroupInformation callerUgi, String ownerName) { + if (ownerName == null) { + return false; + } + if (callerUgi.getUserName().equals(ownerName) || + callerUgi.getShortUserName().equals(ownerName)) { + return true; + } + return false; + } + private boolean isAdmin(UserGroupInformation callerUgi) { if (ozAdmins == null) { return false; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java new file mode 100644 index 000000000000..cb7471df8674 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java @@ -0,0 +1,298 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.security.acl; + +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; +import org.apache.hadoop.ozone.om.BucketManagerImpl; +import org.apache.hadoop.ozone.om.KeyManagerImpl; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; +import org.apache.hadoop.ozone.om.PrefixManager; +import org.apache.hadoop.ozone.om.PrefixManagerImpl; +import org.apache.hadoop.ozone.om.VolumeManagerImpl; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; +import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; +import org.apache.hadoop.ozone.om.helpers.OpenKeySession; +import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil; +import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; +import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.test.GenericTestUtils; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.CREATE; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.NONE; +import static org.mockito.Mockito.mock; + + +/** + * Test Ozone owner check from OzoneNativeAuthorizer. + */ +public class TestVolumeOwner { + + private static OzoneConfiguration ozoneConfig; + private static OzoneNativeAuthorizer nativeAuthorizer; + private static KeyManagerImpl keyManager; + private static VolumeManagerImpl volumeManager; + private static BucketManagerImpl bucketManager; + private static PrefixManager prefixManager; + private static OMMetadataManager metadataManager; + private static UserGroupInformation testUgi; + + @BeforeClass + public static void setup() throws IOException { + ozoneConfig = new OzoneConfiguration(); + ozoneConfig.set(OZONE_ACL_AUTHORIZER_CLASS, + OZONE_ACL_AUTHORIZER_CLASS_NATIVE); + File dir = GenericTestUtils.getRandomizedTestDir(); + ozoneConfig.set(OZONE_METADATA_DIRS, dir.toString()); + + metadataManager = new OmMetadataManagerImpl(ozoneConfig); + volumeManager = new VolumeManagerImpl(metadataManager, ozoneConfig); + bucketManager = new BucketManagerImpl(metadataManager); + keyManager = new KeyManagerImpl(mock(ScmBlockLocationProtocol.class), + metadataManager, ozoneConfig, "om1", null); + prefixManager = new PrefixManagerImpl(metadataManager, false); + + nativeAuthorizer = new OzoneNativeAuthorizer(volumeManager, bucketManager, + keyManager, prefixManager, + Collections.singletonList("om")); + + testUgi = UserGroupInformation.createUserForTesting("testuser", + new String[]{"test"}); + + prepareTestVols(); + prepareTestBuckets(); + prepareTestKeys(); + } + + // create 2 volumes + private static void prepareTestVols() throws IOException { + for (int i = 0; i < 2; i++) { + OmVolumeArgs volumeArgs = OmVolumeArgs.newBuilder() + .setVolume(getTestVolumeName(i)) + .setAdminName("om") + .setOwnerName(getTestVolOwnerName(i)) + .build(); + TestOMRequestUtils.addVolumeToOM(metadataManager, volumeArgs); + } + } + + // create 2 buckets under each volume + private static void prepareTestBuckets() throws IOException { + for (int i = 0; i < 2; i++) { + for (int j = 0; j < 2; j++) { + OmBucketInfo bucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(getTestVolumeName(i)) + .setBucketName(getTestBucketName(j)) + .build(); + TestOMRequestUtils.addBucketToOM(metadataManager, bucketInfo); + } + } + } + + // create 2 keys under each test buckets + private static void prepareTestKeys() throws IOException { + for (int i = 0; i < 2; i++) { + for (int j = 0; j < 2; j++) { + for (int k = 0; k < 2; k++) { + OmKeyArgs.Builder keyArgsBuilder = new OmKeyArgs.Builder() + .setVolumeName(getTestVolumeName(i)) + .setBucketName(getTestBucketName(j)) + .setKeyName(getTestKeyName(k)) + .setFactor(HddsProtos.ReplicationFactor.ONE) + .setDataSize(0) + .setType(HddsProtos.ReplicationType.STAND_ALONE); + if (k == 0) { + keyArgsBuilder.setAcls(OzoneAclUtil.getAclList( + testUgi.getUserName(), testUgi.getGroupNames(), ALL, ALL)); + } else { + keyArgsBuilder.setAcls(OzoneAclUtil.getAclList( + testUgi.getUserName(), testUgi.getGroupNames(), NONE, NONE)); + } + OmKeyArgs keyArgs = keyArgsBuilder.build(); + OpenKeySession keySession = keyManager.createFile(keyArgs, true, + false); + keyArgs.setLocationInfoList( + keySession.getKeyInfo().getLatestVersionLocations() + .getLocationList()); + keyManager.commitKey(keyArgs, keySession.getId()); + } + } + } + } + + @Test + public void testVolumeOps() throws Exception { + OzoneObj vol0 = getTestVolumeobj(0); + + // admin = true, owner = false, ownerName = testvolumeOwner + RequestContext nonOwnerContext = getUserRequestContext("om", + IAccessAuthorizer.ACLType.CREATE, false, getTestVolOwnerName(0)); + Assert.assertTrue("matching admins are allowed to perform admin " + + "operations", nativeAuthorizer.checkAccess(vol0, nonOwnerContext)); + + // admin = true, owner = false, ownerName = null + Assert.assertTrue("matching admins are allowed to perform admin " + + "operations", nativeAuthorizer.checkAccess(vol0, nonOwnerContext)); + + // admin = false, owner = false, ownerName = testvolumeOwner + RequestContext nonAdminNonOwnerContext = getUserRequestContext("testuser", + IAccessAuthorizer.ACLType.CREATE, false, getTestVolOwnerName(0)); + Assert.assertFalse("mismatching admins are not allowed to perform admin " + + "operations", nativeAuthorizer.checkAccess(vol0, + nonAdminNonOwnerContext)); + + // admin = false, owner = true + RequestContext nonAdminOwnerContext = getUserRequestContext( + getTestVolOwnerName(0), IAccessAuthorizer.ACLType.CREATE, + true, getTestVolOwnerName(0)); + Assert.assertFalse("mismatching admins are not allowed to perform admin " + + "operations even for owner", nativeAuthorizer.checkAccess(vol0, + nonAdminOwnerContext)); + + List aclsToTest = + Arrays.stream(IAccessAuthorizer.ACLType.values()).filter( + (type)-> type != NONE && type != CREATE) + .collect(Collectors.toList()); + for (IAccessAuthorizer.ACLType type: aclsToTest) { + nonAdminOwnerContext = getUserRequestContext(getTestVolOwnerName(0), + type, true, getTestVolOwnerName(0)); + Assert.assertTrue("Owner is allowed to perform all non-admin " + + "operations", nativeAuthorizer.checkAccess(vol0, + nonAdminOwnerContext)); + } + } + + @Test + public void testBucketOps() throws Exception { + OzoneObj obj = getTestBucketobj(1, 1); + List aclsToTest = getAclsToTest(); + + // admin = false, owner = true + for (IAccessAuthorizer.ACLType type: aclsToTest) { + RequestContext nonAdminOwnerContext = getUserRequestContext( + getTestVolOwnerName(1), type, true, getTestVolOwnerName(1)); + Assert.assertTrue("non admin volume owner without acls are allowed" + + " to do " + type + " on bucket", + nativeAuthorizer.checkAccess(obj, nonAdminOwnerContext)); + } + + // admin = false, owner = false + for (IAccessAuthorizer.ACLType type: aclsToTest) { + RequestContext nonAdminOwnerContext = getUserRequestContext( + getTestVolOwnerName(1), type, false, getTestVolOwnerName(0)); + Assert.assertFalse("non admin non volume owner without acls" + + " are not allowed to do " + type + " on bucket", + nativeAuthorizer.checkAccess(obj, nonAdminOwnerContext)); + } + } + + @Test + public void testKeyOps() throws Exception { + OzoneObj obj = getTestKeyobj(0, 0, 1); + List aclsToTest = getAclsToTest(); + + // admin = false, owner = true + for (IAccessAuthorizer.ACLType type: aclsToTest) { + RequestContext nonAdminOwnerContext = getUserRequestContext( + getTestVolOwnerName(0), type, true, getTestVolOwnerName(0)); + Assert.assertTrue("non admin volume owner without acls are allowed to " + + "access key", + nativeAuthorizer.checkAccess(obj, nonAdminOwnerContext)); + } + + // admin = false, owner = false + for (IAccessAuthorizer.ACLType type: aclsToTest) { + RequestContext nonAdminOwnerContext = getUserRequestContext( + getTestVolOwnerName(0), type, false, getTestVolOwnerName(1)); + Assert.assertFalse("non admin volume owner without acls are" + + " not allowed to access key", + nativeAuthorizer.checkAccess(obj, nonAdminOwnerContext)); + } + } + + private RequestContext getUserRequestContext(String username, + IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName) { + return RequestContext.getBuilder( + UserGroupInformation.createRemoteUser(username), null, null, + type, ownerName).build(); + } + + private static String getTestVolumeName(int index) { + return "vol" + index; + } + + private static String getTestVolOwnerName(int index) { + return "owner" + index; + } + + private static String getTestBucketName(int index) { + return "bucket" + index; + } + + private static String getTestKeyName(int index) { + return "key" + index; + } + + private OzoneObj getTestVolumeobj(int index) { + return OzoneObjInfo.Builder.getBuilder(OzoneObj.ResourceType.VOLUME, + OzoneObj.StoreType.OZONE, + getTestVolumeName(index), null, null).build(); + } + + private OzoneObj getTestBucketobj(int volIndex, int bucketIndex) { + return OzoneObjInfo.Builder.newBuilder() + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE) + .setVolumeName(getTestVolumeName(volIndex)) + .setBucketName(getTestBucketName(bucketIndex)).build(); + } + + private OzoneObj getTestKeyobj(int volIndex, int bucketIndex, + int keyIndex) { + return OzoneObjInfo.Builder.newBuilder() + .setResType(OzoneObj.ResourceType.KEY) + .setStoreType(OzoneObj.StoreType.OZONE) + .setVolumeName(getTestVolumeName(volIndex)) + .setBucketName(getTestBucketName(bucketIndex)) + .setKeyName(getTestKeyName(keyIndex)) + .build(); + } + + List getAclsToTest() { + return Arrays.stream(IAccessAuthorizer.ACLType.values()).filter( + (type)-> type != NONE).collect(Collectors.toList()); + } +} From 0148aa4260b14f44b5b642f1914d497da24d5fce Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Wed, 7 Oct 2020 22:07:44 -0700 Subject: [PATCH 2/4] refactor --- .../apache/hadoop/ozone/om/OzoneManager.java | 82 +++++++++---------- .../ozone/om/request/OMClientRequest.java | 7 +- .../request/bucket/OMBucketCreateRequest.java | 2 +- .../request/bucket/OMBucketDeleteRequest.java | 2 +- .../bucket/OMBucketSetPropertyRequest.java | 2 +- .../bucket/acl/OMBucketAclRequest.java | 2 +- .../ozone/om/request/key/OMKeyRequest.java | 4 +- .../om/request/key/acl/OMKeyAclRequest.java | 2 +- .../key/acl/prefix/OMPrefixAclRequest.java | 2 +- .../request/volume/OMVolumeCreateRequest.java | 2 +- .../request/volume/OMVolumeDeleteRequest.java | 2 +- .../volume/OMVolumeSetOwnerRequest.java | 2 +- .../volume/OMVolumeSetQuotaRequest.java | 2 +- .../volume/acl/OMVolumeAclRequest.java | 2 +- 14 files changed, 56 insertions(+), 59 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 23f8ddf5cf03..f1b664a3f70f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1595,7 +1595,7 @@ public void createVolume(OmVolumeArgs args) throws IOException { metrics.incNumVolumeCreates(); if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.CREATE, - args.getVolume(), null, null, args.getOwnerName()); + args.getVolume(), null, null); } volumeManager.createVolume(args); AUDIT.logWriteSuccess(buildAuditMessageForSuccess(OMAction.CREATE_VOLUME, @@ -1620,17 +1620,16 @@ public void createVolume(OmVolumeArgs args) throws IOException { * @param vol - name of volume * @param bucket - bucket name * @param key - key - * @param ownerName - volume owner name which should have full permission. * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied. */ private void checkAcls(ResourceType resType, StoreType store, - ACLType acl, String vol, String bucket, String key, - String ownerName) throws OMException { + ACLType acl, String vol, String bucket, String key) + throws OMException { checkAcls(resType, store, acl, vol, bucket, key, ProtobufRpcEngine.Server.getRemoteUser(), ProtobufRpcEngine.Server.getRemoteIp(), ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - true, ownerName); + true); } /** @@ -1645,8 +1644,9 @@ private boolean hasAcls(String userName, ResourceType resType, UserGroupInformation.createRemoteUser(userName), ProtobufRpcEngine.Server.getRemoteIp(), ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - false, null); + false); } catch (OMException ex) { + // Should not trigger exception here at all return false; } } @@ -1686,13 +1686,17 @@ private String getVolumeOwner(OMMetadataManager metadataMgr, public boolean checkAcls(ResourceType resType, StoreType storeType, ACLType aclType, String vol, String bucket, String key, UserGroupInformation ugi, InetAddress remoteAddress, String hostName, - boolean throwIfPermissionDenied, String ownerName) + boolean throwIfPermissionDenied) throws OMException { - String volOwnerName = ownerName; + String volOwnerName = null; + boolean isVolumeCreate = false; + if (resType == ResourceType.VOLUME && aclType == ACLType.CREATE) { + isVolumeCreate = true; + } // In the case of ROOT "/", we send null as owner name. // OzoneNativeAuthorizer has as special handling to avoid NPE // TODO: send the OM login user as the owner of non-exist "/" volume. - if ((ownerName == null && !vol.equals(OzoneConsts.OZONE_ROOT))) { + if (!vol.equals(OzoneConsts.OZONE_ROOT) && !isVolumeCreate) { volOwnerName = getVolumeOwner(metadataManager, vol); } OzoneObj obj = OzoneObjInfo.Builder.newBuilder() @@ -1742,7 +1746,7 @@ public boolean getAclsEnabled() { public boolean setOwner(String volume, String owner) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.WRITE_ACL, volume, - null, null, null); + null, null); } Map auditMap = buildAuditMap(volume); auditMap.put(OzoneConsts.OWNER, owner); @@ -1790,7 +1794,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, - ACLType.READ, volume, null, null, null); + ACLType.READ, volume, null, null); } boolean auditSuccess = true; Map auditMap = buildAuditMap(volume); @@ -1824,7 +1828,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) public OmVolumeArgs getVolumeInfo(String volume) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.READ, volume, - null, null, null); + null, null); } boolean auditSuccess = true; @@ -1857,7 +1861,7 @@ public void deleteVolume(String volume) throws IOException { try { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.DELETE, volume, - null, null, null); + null, null); } metrics.incNumVolumeDeletes(); volumeManager.deleteVolume(volume); @@ -1959,7 +1963,7 @@ public List listAllVolumes(String prefix, String prevKey, int // Only admin can list all volumes when disallowed in config if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.LIST, - OzoneConsts.OZONE_ROOT, null, null, null); + OzoneConsts.OZONE_ROOT, null, null); } } return volumeManager.listVolumes(null, prefix, prevKey, maxKeys); @@ -1988,7 +1992,7 @@ public void createBucket(OmBucketInfo bucketInfo) throws IOException { try { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.CREATE, - bucketInfo.getVolumeName(), bucketInfo.getBucketName(), null, null); + bucketInfo.getVolumeName(), bucketInfo.getBucketName(), null); } metrics.incNumBucketCreates(); bucketManager.createBucket(bucketInfo); @@ -2012,7 +2016,7 @@ public List listBuckets(String volumeName, throws IOException { if (isAclEnabled) { checkAcls(ResourceType.VOLUME, StoreType.OZONE, ACLType.LIST, volumeName, - null, null, null); + null, null); } boolean auditSuccess = true; Map auditMap = buildAuditMap(volumeName); @@ -2051,7 +2055,7 @@ public OmBucketInfo getBucketInfo(String volume, String bucket) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, volume, - bucket, null, null); + bucket, null); } boolean auditSuccess = true; Map auditMap = buildAuditMap(volume); @@ -2088,14 +2092,13 @@ public OpenKeySession openKey(OmKeyArgs args) throws IOException { if (isAclEnabled) { try { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } catch (OMException ex) { // For new keys key checkAccess call will fail as key doesn't exist. // Check user access for bucket. if (ex.getResult().equals(KEY_NOT_FOUND)) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), - null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } else { throw ex; } @@ -2132,14 +2135,13 @@ public void commitKey(OmKeyArgs args, long clientID) if (isAclEnabled) { try { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } catch (OMException ex) { // For new keys key checkAccess call will fail as key doesn't exist. // Check user access for bucket. if (ex.getResult().equals(KEY_NOT_FOUND)) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), - null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } else { throw ex; } @@ -2182,15 +2184,13 @@ public OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID, if (isAclEnabled) { try { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), - null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } catch (OMException ex) { // For new keys key checkAccess call will fail as key doesn't exist. // Check user access for bucket. if (ex.getResult().equals(KEY_NOT_FOUND)) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), - null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } else { throw ex; } @@ -2233,7 +2233,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } boolean auditSuccess = true; @@ -2274,7 +2274,7 @@ public void renameKey(OmKeyArgs args, String toKeyName) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.WRITE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } Map auditMap = bucket.audit(args.toAuditMap()); @@ -2310,7 +2310,7 @@ public void deleteKey(OmKeyArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.DELETE, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } metrics.incNumKeyDeletes(); @@ -2346,7 +2346,7 @@ public List listKeys(String volumeName, String bucketName, if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.LIST, - bucket.realVolume(), bucket.realBucket(), keyPrefix, null); + bucket.realVolume(), bucket.realBucket(), keyPrefix); } boolean auditSuccess = true; @@ -2382,7 +2382,7 @@ public List listTrash(String volumeName, if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.LIST, - volumeName, bucketName, keyPrefix, null); + volumeName, bucketName, keyPrefix); } boolean auditSuccess = true; @@ -2421,7 +2421,7 @@ public void setBucketProperty(OmBucketArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, - args.getVolumeName(), args.getBucketName(), null, null); + args.getVolumeName(), args.getBucketName(), null); } try { metrics.incNumBucketUpdates(); @@ -2447,7 +2447,7 @@ public void setBucketProperty(OmBucketArgs args) public void deleteBucket(String volume, String bucket) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.WRITE, volume, - bucket, null, null); + bucket, null); } Map auditMap = buildAuditMap(volume); auditMap.put(OzoneConsts.BUCKET, bucket); @@ -2892,7 +2892,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException { if (isAclEnabled) { checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } boolean auditSuccess = true; @@ -2925,7 +2925,7 @@ public List listStatus(OmKeyArgs args, boolean recursive, if (isAclEnabled) { checkAcls(getResourceType(args), StoreType.OZONE, ACLType.READ, - bucket.realVolume(), bucket.realBucket(), args.getKeyName(), null); + bucket.realVolume(), bucket.realBucket(), args.getKeyName()); } boolean auditSuccess = true; @@ -2982,7 +2982,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); } switch (obj.getResourceType()) { case VOLUME: @@ -3023,7 +3023,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); } switch (obj.getResourceType()) { case VOLUME: @@ -3065,7 +3065,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.WRITE_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); } switch (obj.getResourceType()) { case VOLUME: @@ -3104,7 +3104,7 @@ public List getAcl(OzoneObj obj) throws IOException { try { if (isAclEnabled) { checkAcls(obj.getResourceType(), obj.getStoreType(), ACLType.READ_ACL, - obj.getVolumeName(), obj.getBucketName(), obj.getKeyName(), null); + obj.getVolumeName(), obj.getBucketName(), obj.getKeyName()); } switch (obj.getResourceType()) { case VOLUME: @@ -3576,7 +3576,7 @@ private Pair resolveBucketLink( if (isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, volumeName, bucketName, null, userGroupInformation, - remoteAddress, hostName, true, null); + remoteAddress, hostName, true); } return resolveBucketLink( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index a8cbfc34c055..78cf6a300537 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -146,18 +146,15 @@ public OzoneManagerProtocolProtos.UserInfo getUserInfo() { * @param vol * @param bucket * @param key - * @param ownerName - only used for createVolume, null otherwise. * @throws IOException */ @SuppressWarnings("parameternumber") public void checkAcls(OzoneManager ozoneManager, OzoneObj.ResourceType resType, OzoneObj.StoreType storeType, IAccessAuthorizer.ACLType aclType, - String vol, String bucket, String key, - String ownerName) throws IOException { + String vol, String bucket, String key) throws IOException { ozoneManager.checkAcls(resType, storeType, aclType, vol, bucket, key, - createUGI(), getRemoteAddress(), getHostName(), true, - ownerName); + createUGI(), getRemoteAddress(), getHostName(), true); } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java index 3121af1f9cc1..fd303e7f09ac 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java @@ -171,7 +171,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.CREATE, - volumeName, bucketName, null, null); + volumeName, bucketName, null); } acquiredVolumeLock = diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java index 4d321070a0a2..91aef6a2d44a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java @@ -99,7 +99,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, - volumeName, bucketName, null, null); + volumeName, bucketName, null); } // acquire lock diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index 0dc5fee5b95b..583facbc0fca 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -107,7 +107,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE, - volumeName, bucketName, null, null); + volumeName, bucketName, null); } // acquire lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java index 099b17e815e0..a493f9fa1472 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java @@ -93,7 +93,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null, null); + volume, null, null); } lockAcquired = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index 696a9f59afe4..a048533001e1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -411,7 +411,7 @@ protected void checkBucketAcls(OzoneManager ozoneManager, String volume, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET, OzoneObj.StoreType.OZONE, aclType, - volume, bucket, key, null); + volume, bucket, key); } } @@ -432,7 +432,7 @@ protected void checkKeyAcls(OzoneManager ozoneManager, String volume, throws IOException { if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, resourceType, OzoneObj.StoreType.OZONE, aclType, - volume, bucket, key, null); + volume, bucket, key); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java index 686700c1f9dc..9fae4988fa68 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java @@ -79,7 +79,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, key, null); + volume, bucket, key); } lockAcquired = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java index c24ee69a7311..6fbd7d22bbdb 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/prefix/OMPrefixAclRequest.java @@ -83,7 +83,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.PREFIX, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, bucket, key, null); + volume, bucket, key); } lockAcquired = diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java index 771c5fab7816..9c81c36eaf22 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java @@ -136,7 +136,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.CREATE, volume, - null, null, owner); + null, null); } // acquire lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java index fdc0ad81a052..ce93e269e250 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java @@ -91,7 +91,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE, volume, - null, null, null); + null, null); } acquiredVolumeLock = omMetadataManager.getLock().acquireWriteLock( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java index 1740fdc23650..6873086750a4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java @@ -114,7 +114,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null, null); + volume, null, null); } long maxUserVolumeCount = ozoneManager.getMaxUserVolumeCount(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java index 826f426f04b4..fc54c8819101 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java @@ -121,7 +121,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE, volume, - null, null, null); + null, null); } acquireVolumeLock = omMetadataManager.getLock().acquireWriteLock( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java index dd07f3ad1fce..de7f0c0a36d0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java @@ -78,7 +78,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (ozoneManager.getAclsEnabled()) { checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME, OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL, - volume, null, null, null); + volume, null, null); } lockAcquired = omMetadataManager.getLock().acquireWriteLock( VOLUME_LOCK, volume); From a8b7c31d76a777534f63d82f9c770aec233b862b Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Mon, 12 Oct 2020 13:14:38 -0700 Subject: [PATCH 3/4] fix locking heriarchy --- .../apache/hadoop/ozone/om/OzoneManager.java | 44 +++++++++---------- .../ozone/om/request/OMClientRequest.java | 25 ++++++++++- .../ozone/om/request/key/OMKeyRequest.java | 34 ++++++++++++++ .../om/request/key/OMKeysDeleteRequest.java | 4 +- .../om/request/key/OMKeysRenameRequest.java | 9 +++- 5 files changed, 89 insertions(+), 27 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index f1b664a3f70f..7a704e71cf2c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1629,7 +1629,7 @@ private void checkAcls(ResourceType resType, StoreType store, ProtobufRpcEngine.Server.getRemoteUser(), ProtobufRpcEngine.Server.getRemoteIp(), ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - true); + true, getVolumeOwner(vol, acl)); } /** @@ -1644,27 +1644,34 @@ private boolean hasAcls(String userName, ResourceType resType, UserGroupInformation.createRemoteUser(userName), ProtobufRpcEngine.Server.getRemoteIp(), ProtobufRpcEngine.Server.getRemoteIp().getHostName(), - false); + false, getVolumeOwner(vol, acl)); } catch (OMException ex) { // Should not trigger exception here at all return false; } } - private String getVolumeOwner(OMMetadataManager metadataMgr, - String volume) throws OMException { - Boolean lockAcquired = metadataMgr.getLock().acquireReadLock( + public String getVolumeOwner(String vol, ACLType type) throws OMException { + String volOwnerName = null; + if (!vol.equals(OzoneConsts.OZONE_ROOT) && (type != ACLType.CREATE)) { + volOwnerName = getVolumeOwner(vol); + } + return volOwnerName; + } + + private String getVolumeOwner(String volume) throws OMException { + Boolean lockAcquired = metadataManager.getLock().acquireReadLock( VOLUME_LOCK, volume); - String dbVolumeKey = metadataMgr.getVolumeKey(volume); + String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = null; try { - volumeArgs = metadataMgr.getVolumeTable().get(dbVolumeKey); + volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey); } catch (IOException ioe) { throw new OMException("Volume " + volume + " is not found", OMException.ResultCodes.VOLUME_NOT_FOUND); } finally { if (lockAcquired) { - metadataMgr.getLock().releaseReadLock(VOLUME_LOCK, volume); + metadataManager.getLock().releaseReadLock(VOLUME_LOCK, volume); } } if (volumeArgs != null) { @@ -1686,19 +1693,8 @@ private String getVolumeOwner(OMMetadataManager metadataMgr, public boolean checkAcls(ResourceType resType, StoreType storeType, ACLType aclType, String vol, String bucket, String key, UserGroupInformation ugi, InetAddress remoteAddress, String hostName, - boolean throwIfPermissionDenied) + boolean throwIfPermissionDenied, String volumeOwner) throws OMException { - String volOwnerName = null; - boolean isVolumeCreate = false; - if (resType == ResourceType.VOLUME && aclType == ACLType.CREATE) { - isVolumeCreate = true; - } - // In the case of ROOT "/", we send null as owner name. - // OzoneNativeAuthorizer has as special handling to avoid NPE - // TODO: send the OM login user as the owner of non-exist "/" volume. - if (!vol.equals(OzoneConsts.OZONE_ROOT) && !isVolumeCreate) { - volOwnerName = getVolumeOwner(metadataManager, vol); - } OzoneObj obj = OzoneObjInfo.Builder.newBuilder() .setResType(resType) .setStoreType(storeType) @@ -1711,7 +1707,7 @@ public boolean checkAcls(ResourceType resType, StoreType storeType, .setHost(hostName) .setAclType(ACLIdentityType.USER) .setAclRights(aclType) - .setOwnerName(volOwnerName) + .setOwnerName(volumeOwner) .build(); if (!accessAuthorizer.checkAccess(obj, context)) { if (throwIfPermissionDenied) { @@ -3574,9 +3570,11 @@ private Pair resolveBucketLink( } if (isAclEnabled) { - checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, + final ACLType type = ACLType.READ; + checkAcls(ResourceType.BUCKET, StoreType.OZONE, type, volumeName, bucketName, null, userGroupInformation, - remoteAddress, hostName, true); + remoteAddress, hostName, true, + getVolumeOwner(volumeName, type)); } return resolveBucketLink( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 78cf6a300537..674ee089f48f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -153,8 +153,31 @@ public void checkAcls(OzoneManager ozoneManager, OzoneObj.ResourceType resType, OzoneObj.StoreType storeType, IAccessAuthorizer.ACLType aclType, String vol, String bucket, String key) throws IOException { + checkAcls(ozoneManager, resType, storeType, aclType, vol, bucket, key, + ozoneManager.getVolumeOwner(vol, aclType)); + } + + /** + * Check Acls of ozone object with volOwner given. + * @param ozoneManager + * @param resType + * @param storeType + * @param aclType + * @param vol + * @param bucket + * @param key + * @param volOwner + * @throws IOException + */ + @SuppressWarnings("parameternumber") + public void checkAcls(OzoneManager ozoneManager, + OzoneObj.ResourceType resType, + OzoneObj.StoreType storeType, IAccessAuthorizer.ACLType aclType, + String vol, String bucket, String key, String volOwner) + throws IOException { ozoneManager.checkAcls(resType, storeType, aclType, vol, bucket, key, - createUGI(), getRemoteAddress(), getHostName(), true); + createUGI(), getRemoteAddress(), getHostName(), true, + volOwner); } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index a048533001e1..ee48f9b3da21 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -198,6 +198,19 @@ public void validateBucketAndVolume(OMMetadataManager omMetadataManager, } } + // For keys batch delete and rename only + protected String getVolumeOwner(OMMetadataManager omMetadataManager, + String volumeName) throws IOException { + String dbVolumeKey = omMetadataManager.getVolumeKey(volumeName); + OmVolumeArgs volumeArgs = + omMetadataManager.getVolumeTable().get(dbVolumeKey); + if (volumeArgs == null) { + throw new OMException("Volume not found " + volumeName, + VOLUME_NOT_FOUND); + } + return volumeArgs.getOwnerName(); + } + protected static Optional getFileEncryptionInfo( OzoneManager ozoneManager, OmBucketInfo bucketInfo) throws IOException { Optional encInfo = Optional.absent(); @@ -436,6 +449,27 @@ protected void checkKeyAcls(OzoneManager ozoneManager, String volume, } } + /** + * Check Acls for the ozone key with volumeOwner. + * @param ozoneManager + * @param volume + * @param bucket + * @param key + * @param aclType + * @param resourceType + * @throws IOException + */ + @SuppressWarnings("parameternumber") + protected void checkKeyAcls(OzoneManager ozoneManager, String volume, + String bucket, String key, IAccessAuthorizer.ACLType aclType, + OzoneObj.ResourceType resourceType, String volumeOwner) + throws IOException { + if (ozoneManager.getAclsEnabled()) { + checkAcls(ozoneManager, resourceType, OzoneObj.StoreType.OZONE, aclType, + volume, bucket, key, volumeOwner); + } + } + /** * Check ACLs for Ozone Key in OpenKey table * if ozone native authorizer is enabled. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index c6e7b9bafa37..71e15f541819 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -124,6 +124,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, volumeName, bucketName); // Validate bucket and volume exists or not. validateBucketAndVolume(omMetadataManager, volumeName, bucketName); + String volumeOwner = getVolumeOwner(omMetadataManager, volumeName); for (indexFailed = 0; indexFailed < length; indexFailed++) { String keyName = deleteKeyArgs.getKeys(indexFailed); @@ -143,7 +144,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, try { // check Acl checkKeyAcls(ozoneManager, volumeName, bucketName, keyName, - IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY, + volumeOwner); omKeyInfoList.add(omKeyInfo); } catch (Exception ex) { deleteStatus = false; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java index abaa4ae3c074..556c6f59db53 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java @@ -117,6 +117,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); + // Validate bucket and volume exists or not. + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); + String volumeOwner = getVolumeOwner(omMetadataManager, volumeName); for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) { fromKeyName = renameKey.getFromKeyName(); @@ -137,9 +140,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // check Acls to see if user has access to perform delete operation // on old key and create operation on new key checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName, - IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY, + volumeOwner); checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, - IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); + IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY, + volumeOwner); } catch (Exception ex) { renameStatus = false; unRenamedKeys.add( From d29621850894c1bc4ab38a87f385df1f66de9371 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Thu, 22 Oct 2020 12:36:54 -0700 Subject: [PATCH 4/4] address code review feedback --- .../java/org/apache/hadoop/ozone/om/OzoneManager.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 7a704e71cf2c..258564c1b077 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1667,8 +1667,12 @@ private String getVolumeOwner(String volume) throws OMException { try { volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey); } catch (IOException ioe) { - throw new OMException("Volume " + volume + " is not found", - OMException.ResultCodes.VOLUME_NOT_FOUND); + if (ioe instanceof OMException) { + throw (OMException)ioe; + } else { + throw new OMException("getVolumeOwner for Volume " + volume + " failed", + ResultCodes.INTERNAL_ERROR); + } } finally { if (lockAcquired) { metadataManager.getLock().releaseReadLock(VOLUME_LOCK, volume);