Skip to content

HDDS-13855. Move ACL check in Key and Prefix requests to preExecute #10330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ss77892 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

HDDS-13855. Move ACL check in Key and Prefix requests to preExecute #10330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,7 @@ message RenameKeysArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated RenameKeysMap renameKeysMap = 3;
repeated RenameKeysMap aclDeniedRenameKeys = 4;
}

message RenameKeysMap {
Expand Down Expand Up @@ -1400,6 +1401,7 @@ message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
repeated string aclDeniedKeys = 4;
}

message DeleteKeyError {
Expand Down
91 changes: 71 additions & 20 deletions ...one-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,65 @@ public OMKeysDeleteRequest(OMRequest omRequest, BucketLayout bucketLayout) {
super(omRequest, bucketLayout);
}

@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
super.preExecute(ozoneManager);

DeleteKeysRequest deleteKeysRequest = getOmRequest().getDeleteKeysRequest();
DeleteKeyArgs deleteKeyArgs = deleteKeysRequest.getDeleteKeys();

String volumeName = deleteKeyArgs.getVolumeName();
String bucketName = deleteKeyArgs.getBucketName();
List<String> keys = deleteKeyArgs.getKeysList();

ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink(
Pair.of(volumeName, bucketName));
String resolvedVolume = resolvedBucketObj.realVolume();
String resolvedBucket = resolvedBucketObj.realBucket();

String volumeOwner = getVolumeOwner(ozoneManager.getMetadataManager(), resolvedVolume);

List<String> keysPassingAcl = new ArrayList<>();
List<String> aclDeniedKeys = new ArrayList<>();
if (ozoneManager.getAclsEnabled()) {
for (String keyName : keys) {
try {
checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, keyName,
IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY,
volumeOwner);
keysPassingAcl.add(keyName);
} catch (IOException ex) {
LOG.warn("ACL check failed for key {} during preExecute: {}",
keyName, ex.getMessage());
aclDeniedKeys.add(keyName);
Map<String, String> auditMap = new LinkedHashMap<>();
auditMap.put(VOLUME, resolvedVolume);
auditMap.put(BUCKET, resolvedBucket);
auditMap.put(KEY, keyName);
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(DELETE_KEYS, auditMap, ex,
getOmRequest().getUserInfo()));
}
}
} else {
keysPassingAcl.addAll(keys);
}

DeleteKeyArgs.Builder modifiedArgs = deleteKeyArgs.toBuilder()
.clearKeys()
.addAllKeys(keysPassingAcl)
.clearAclDeniedKeys()
.addAllAclDeniedKeys(aclDeniedKeys);

DeleteKeysRequest.Builder modifiedRequest = deleteKeysRequest.toBuilder()
.setDeleteKeys(modifiedArgs);

return getOmRequest().toBuilder()
.setDeleteKeysRequest(modifiedRequest)
.setUserInfo(getUserIfNotExists(ozoneManager))
.build();
}

@Override @SuppressWarnings("methodlength")
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) {
final long trxnLogIndex = context.getIndex();
Expand Down Expand Up @@ -147,7 +206,13 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
acquiredLock = getOmLockDetails().isLockAcquired();
// Validate bucket and volume exists or not.
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
String volumeOwner = getVolumeOwner(omMetadataManager, volumeName);

for (String deniedKey : deleteKeyArgs.getAclDeniedKeysList()) {
deleteStatus = false;
unDeletedKeys.addKeys(deniedKey);
keyToError.put(deniedKey,
new ErrorInfo(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed"));
}

for (indexFailed = 0; indexFailed < length; indexFailed++) {
String keyName = deleteKeyArgs.getKeys(indexFailed);
Expand All @@ -166,25 +231,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
continue;
}

try {
// check Acl
long startNanosDeleteKeysAclCheckLatency = Time.monotonicNowNanos();
checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY,
volumeOwner);
perfMetrics.setDeleteKeysAclCheckLatencyNs(Time.monotonicNowNanos() - startNanosDeleteKeysAclCheckLatency);
OzoneFileStatus fileStatus = getOzoneKeyStatus(
ozoneManager, omMetadataManager, volumeName, bucketName, keyName);
addKeyToAppropriateList(omKeyInfoList, omKeyInfo, dirList,
fileStatus);
deleteKeysInfo.add(omKeyInfo);
} catch (Exception ex) {
deleteStatus = false;
LOG.error("Acl check failed for Key: {}", objectKey, ex);
deleteKeys.remove(keyName);
unDeletedKeys.addKeys(keyName);
keyToError.put(keyName, new ErrorInfo(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed"));
}
OzoneFileStatus fileStatus = getOzoneKeyStatus(
ozoneManager, omMetadataManager, volumeName, bucketName, keyName);
addKeyToAppropriateList(omKeyInfoList, omKeyInfo, dirList,
fileStatus);
deleteKeysInfo.add(omKeyInfo);
}

OmBucketInfo omBucketInfo =
Expand Down
100 changes: 80 additions & 20 deletions ...one-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@

package org.apache.hadoop.ozone.om.request.key;

import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
import static org.apache.hadoop.ozone.OzoneConsts.DST_KEY;
import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
import static org.apache.hadoop.ozone.OzoneConsts.SRC_KEY;
import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource.BUCKET_LOCK;
import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
Expand Down Expand Up @@ -77,6 +81,76 @@ public OMKeysRenameRequest(OMRequest omRequest, BucketLayout bucketLayout) {
super(omRequest, bucketLayout);
}

@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
super.preExecute(ozoneManager);

RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs();

String volumeName = renameKeysArgs.getVolumeName();
String bucketName = renameKeysArgs.getBucketName();

ResolvedBucket resolvedBucketObj = ozoneManager.resolveBucketLink(
Pair.of(volumeName, bucketName));
String resolvedVolume = resolvedBucketObj.realVolume();
String resolvedBucket = resolvedBucketObj.realBucket();

String volumeOwner = getVolumeOwner(ozoneManager.getMetadataManager(), resolvedVolume);

List<RenameKeysMap> keyPairsPassingAcl = new ArrayList<>();
List<RenameKeysMap> aclDeniedPairs = new ArrayList<>();
if (ozoneManager.getAclsEnabled()) {
for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {
String fromKeyName = renameKey.getFromKeyName();
String toKeyName = renameKey.getToKeyName();

if (fromKeyName.isEmpty() || toKeyName.isEmpty()) {
keyPairsPassingAcl.add(renameKey);
continue;
}

try {
checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, fromKeyName,
IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY,
volumeOwner);
checkKeyAcls(ozoneManager, resolvedVolume, resolvedBucket, toKeyName,
IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY,
volumeOwner);
keyPairsPassingAcl.add(renameKey);
} catch (IOException ex) {
LOG.warn("ACL check failed for rename {} -> {} during preExecute: {}",
fromKeyName, toKeyName, ex.getMessage());
aclDeniedPairs.add(renameKey);
Map<String, String> auditMap = new LinkedHashMap<>();
auditMap.put(VOLUME, resolvedVolume);
auditMap.put(BUCKET, resolvedBucket);
auditMap.put(SRC_KEY, fromKeyName);
auditMap.put(DST_KEY, toKeyName);
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(OMAction.RENAME_KEYS, auditMap, ex,
getOmRequest().getUserInfo()));
}
}
} else {
keyPairsPassingAcl.addAll(renameKeysArgs.getRenameKeysMapList());
}

RenameKeysArgs.Builder modifiedArgs = renameKeysArgs.toBuilder()
.clearRenameKeysMap()
.addAllRenameKeysMap(keyPairsPassingAcl)
.clearAclDeniedRenameKeys()
.addAllAclDeniedRenameKeys(aclDeniedPairs);

RenameKeysRequest.Builder modifiedRequest = renameKeysRequest.toBuilder()
.setRenameKeysArgs(modifiedArgs);

return getOmRequest().toBuilder()
.setRenameKeysRequest(modifiedRequest)
.setUserInfo(getUserIfNotExists(ozoneManager))
.build();
}

@Override
@SuppressWarnings("methodlength")
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) {
Expand Down Expand Up @@ -125,7 +199,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut

// Validate bucket and volume exists or not.
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
String volumeOwner = getVolumeOwner(omMetadataManager, volumeName);

for (RenameKeysMap deniedPair : renameKeysArgs.getAclDeniedRenameKeysList()) {
renameStatus = false;
unRenamedKeys.add(deniedPair);
}

for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {

fromKeyName = renameKey.getFromKeyName();
Expand All @@ -142,25 +221,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
continue;
}

try {
// 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,
volumeOwner);
checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY,
volumeOwner);
} catch (Exception ex) {
renameStatus = false;
unRenamedKeys.add(
unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
.build());
LOG.error("Acl check failed for fromKeyName {} toKeyName {}",
fromKeyName, toKeyName, ex);
continue;
}

// Check if toKey exists
String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
fromKeyName);
Expand Down
48 changes: 42 additions & 6 deletions ...one-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@

import java.io.IOException;
import java.nio.file.InvalidPathException;
import java.util.LinkedHashMap;
import java.util.Map;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
import org.apache.hadoop.ozone.audit.AuditLogger;
import org.apache.hadoop.ozone.audit.OMAction;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.ResolvedBucket;
Expand Down Expand Up @@ -61,6 +63,46 @@ public OMKeyAclRequest(OMRequest omRequest) {
super(omRequest);
}

@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
OMRequest omRequest = super.preExecute(ozoneManager);

// ACL check during preExecute
if (ozoneManager.getAclsEnabled()) {
try {
ObjectParser objectParser = new ObjectParser(getPath(),
ObjectType.KEY);
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(
Pair.of(objectParser.getVolume(), objectParser.getBucket()));
String volume = resolvedBucket.realVolume();
String bucket = resolvedBucket.realBucket();
String key = objectParser.getKey();

checkAcls(ozoneManager, OzoneObj.ResourceType.KEY,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volume, bucket, key);
} catch (IOException ex) {
// Ensure audit log captures preExecute failures
Map<String, String> auditMap = new LinkedHashMap<>();
OzoneObj obj = getObject();
auditMap.putAll(obj.toAuditMap());
// Determine which action based on request type
OMAction action = OMAction.SET_ACL;
if (omRequest.hasAddAclRequest()) {
action = OMAction.ADD_ACL;
} else if (omRequest.hasRemoveAclRequest()) {
action = OMAction.REMOVE_ACL;
}
markForAudit(ozoneManager.getAuditLogger(),
buildAuditMessage(action, auditMap, ex,
omRequest.getUserInfo()));
throw ex;
}
}

return omRequest;
}

@Override
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context) {
final long trxnLogIndex = context.getIndex();
Expand All @@ -87,12 +129,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
bucket = resolvedBucket.realBucket();
key = objectParser.getKey();

// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.KEY,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volume, bucket, key);
}
mergeOmLockDetails(
omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume,
bucket));
Expand Down
Loading