This is an automated email from the ASF dual-hosted git repository. jojochuang pushed a commit to branch ozone-2.1 in repository https://gitbox.apache.org/repos/asf/ozone.git
commit 5343cca6d6683d4a0f7ab3b5e4f17145d5aca441 Author: Wei-Chiu Chuang <[email protected]> AuthorDate: Tue Aug 18 10:18:53 2026 -0700 Improve OmMetadataReader Change-Id: Ib21fb5a145d8732733de1a5ca8c5fd727ec20ee5 (cherry picked from commit d5ce83cbd0d4f412d106d87a8800b3dfbd97cf0d) --- .../ozone/om/TestKeyLookupAclNormalization.java | 206 +++++++++++++++++++++ .../apache/hadoop/ozone/om/OmMetadataReader.java | 42 ++++- 2 files changed, 241 insertions(+), 7 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyLookupAclNormalization.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyLookupAclNormalization.java new file mode 100644 index 00000000000..14428d201d7 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyLookupAclNormalization.java @@ -0,0 +1,206 @@ +/* + * 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.om; + +import static org.apache.hadoop.hdds.security.SecurityConfig.OZONE_TEST_AUTHORIZATION_ENABLED; +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.OzoneConfigKeys.OZONE_ACL_ENABLED; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.security.PrivilegedExceptionAction; +import java.util.Collections; +import java.util.Map; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.ozone.OzoneAcl; +import org.apache.hadoop.ozone.client.OzoneBucket; +import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.client.OzoneClientFactory; +import org.apache.hadoop.ozone.client.OzoneKeyDetails; +import org.apache.hadoop.ozone.client.OzoneVolume; +import org.apache.hadoop.ozone.client.io.OzoneOutputStream; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType; +import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType; +import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; +import org.apache.hadoop.security.UserGroupInformation; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Verifies that the key READ ACL check and the key read resolve the SAME + * normalized key name in a FILE_SYSTEM_OPTIMIZED bucket. Because the read path + * normalizes '.'/'..' path segments before the lookup, the ACL check must operate + * on the same normalized name; otherwise a per-key ACL could be evaluated against a + * different (raw, literal) path than the one actually served. These tests assert + * that a user denied by a per-key ACL is denied whether they request the key + * directly or via an equivalent un-normalized path, and that an authorized user can + * still read the key through either form. The same is checked for the read paths + * that share this resolve-then-read shape (key lookup and object tagging). + */ +public class TestKeyLookupAclNormalization { + + private static MiniOzoneCluster cluster; + private static OzoneConfiguration conf; + + private static final String VOL = "vol1"; + private static final String BUCKET = "buck1"; // FSO (default) layout + private static final String KEY = "k1"; + private static final String EQUIVALENT_KEY = "a/../k1"; // normalizes to KEY + private static final byte[] DATA = "some-key-bytes".getBytes( + java.nio.charset.StandardCharsets.UTF_8); + private static final Map<String, String> TAGS = + Collections.singletonMap("t1", "v1"); + + private static final UserGroupInformation ADMIN = + UserGroupInformation.createUserForTesting("admin", new String[] {"admins"}); + private static final UserGroupInformation ALICE = + UserGroupInformation.createUserForTesting("alice", new String[] {"users"}); + private static final UserGroupInformation BOB = + UserGroupInformation.createUserForTesting("bob", new String[] {"users"}); + + @BeforeAll + static void init() throws Exception { + conf = new OzoneConfiguration(); + conf.setBoolean(OZONE_ACL_ENABLED, true); + conf.set(OZONE_ACL_AUTHORIZER_CLASS, OZONE_ACL_AUTHORIZER_CLASS_NATIVE); + conf.set(OZONE_ADMINISTRATORS, "admin"); + // Make authorization (admin + ACL checks) effective without a KDC. + conf.setBoolean(OZONE_TEST_AUTHORIZATION_ENABLED, true); + cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build(); + cluster.waitForClusterToBeReady(); + + ADMIN.doAs((PrivilegedExceptionAction<Void>) () -> { + try (OzoneClient c = OzoneClientFactory.getRpcClient(conf)) { + c.getObjectStore().createVolume(VOL); + OzoneVolume vol = c.getObjectStore().getVolume(VOL); + vol.createBucket(BUCKET); + OzoneBucket bucket = vol.getBucket(BUCKET); + assertEquals(BucketLayout.FILE_SYSTEM_OPTIMIZED, bucket.getBucketLayout()); + + // alice and bob are ordinary users with READ/LIST on the volume and + // the bucket. A native KEY read is only granted when the whole + // volume -> bucket -> key chain grants READ, so both principals need + // volume- and bucket-level READ before the per-key ACL is decisive. + vol.addAcl(OzoneAcl.of(ACLIdentityType.USER, "alice", + OzoneAcl.AclScope.ACCESS, ACLType.READ, ACLType.LIST)); + vol.addAcl(OzoneAcl.of(ACLIdentityType.USER, "bob", + OzoneAcl.AclScope.ACCESS, ACLType.READ, ACLType.LIST)); + bucket.addAcl(OzoneAcl.of(ACLIdentityType.USER, "alice", + OzoneAcl.AclScope.ACCESS, ACLType.READ, ACLType.LIST)); + bucket.addAcl(OzoneAcl.of(ACLIdentityType.USER, "bob", + OzoneAcl.AclScope.ACCESS, ACLType.READ, ACLType.LIST)); + + try (OzoneOutputStream os = bucket.createKey(KEY, DATA.length)) { + os.write(DATA); + } + bucket.putObjectTagging(KEY, TAGS); + + // Tighten the per-key ACL: only alice may READ the key; bob is excluded. + OzoneObj keyObj = OzoneObjInfo.Builder.newBuilder() + .setResType(OzoneObj.ResourceType.KEY) + .setStoreType(OzoneObj.StoreType.OZONE) + .setVolumeName(VOL).setBucketName(BUCKET).setKeyName(KEY).build(); + c.getObjectStore().setAcl(keyObj, Collections.singletonList( + OzoneAcl.of(ACLIdentityType.USER, "alice", OzoneAcl.AclScope.ACCESS, + ACLType.READ, ACLType.ALL))); + } + return null; + }); + } + + @AfterAll + static void shutdown() { + if (cluster != null) { + cluster.shutdown(); + } + } + + private OzoneKeyDetails lookupAs(UserGroupInformation ugi, String keyName) + throws Exception { + return ugi.doAs((PrivilegedExceptionAction<OzoneKeyDetails>) () -> { + try (OzoneClient c = OzoneClientFactory.getRpcClient(conf)) { + return c.getObjectStore().getVolume(VOL).getBucket(BUCKET).getKey(keyName); + } + }); + } + + private Map<String, String> getTagsAs(UserGroupInformation ugi, String keyName) + throws Exception { + return ugi.doAs((PrivilegedExceptionAction<Map<String, String>>) () -> { + try (OzoneClient c = OzoneClientFactory.getRpcClient(conf)) { + return c.getObjectStore().getVolume(VOL).getBucket(BUCKET) + .getObjectTagging(keyName); + } + }); + } + + /** Control: the tighter per-key ACL denies bob the direct read. */ + @Test + void deniedUserCannotReadKeyDirectly() { + OMException ex = assertThrows(OMException.class, () -> lookupAs(BOB, KEY)); + assertEquals(OMException.ResultCodes.PERMISSION_DENIED, ex.getResult()); + } + + /** A denied user is still denied when requesting an equivalent un-normalized path. */ + @Test + void deniedUserCannotReadKeyViaEquivalentPath() { + OMException ex = + assertThrows(OMException.class, () -> lookupAs(BOB, EQUIVALENT_KEY)); + assertEquals(OMException.ResultCodes.PERMISSION_DENIED, ex.getResult()); + } + + /** No regression: an authorized user reads the key directly. */ + @Test + void authorizedUserReadsKeyDirectly() throws Exception { + OzoneKeyDetails details = lookupAs(ALICE, KEY); + assertEquals(KEY, details.getName()); + assertEquals(DATA.length, details.getDataSize()); + } + + /** No regression: an authorized user reads the key via an equivalent path. */ + @Test + void authorizedUserReadsKeyViaEquivalentPath() throws Exception { + OzoneKeyDetails details = lookupAs(ALICE, EQUIVALENT_KEY); + assertEquals(KEY, details.getName()); + assertEquals(DATA.length, details.getDataSize()); + } + + /** + * getObjectTagging resolves the same normalized key as its ACL check: a denied + * user cannot read the tags via an equivalent un-normalized path. + */ + @Test + void deniedUserCannotReadTagsViaEquivalentPath() { + OMException ex = + assertThrows(OMException.class, () -> getTagsAs(BOB, EQUIVALENT_KEY)); + assertEquals(OMException.ResultCodes.PERMISSION_DENIED, ex.getResult()); + } + + /** No regression: an authorized user reads the tags via an equivalent path. */ + @Test + void authorizedUserReadsTagsViaEquivalentPath() throws Exception { + assertEquals(TAGS, getTagsAs(ALICE, EQUIVALENT_KEY)); + } +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java index cbcb7e2dc06..0690dda7c9b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java @@ -54,6 +54,7 @@ import org.apache.hadoop.ozone.om.helpers.OzoneFileStatusLight; import org.apache.hadoop.ozone.om.helpers.S3VolumeContext; import org.apache.hadoop.ozone.om.protocolPB.grpc.GrpcClientConstants; +import org.apache.hadoop.ozone.om.request.OMClientRequest; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType; @@ -122,14 +123,14 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { boolean auditSuccess = true; Map<String, String> auditMap = bucket.audit(args.toAuditMap()); - OmKeyArgs resolvedArgs = bucket.update(args); + OmKeyArgs resolvedArgs = normalizeKeyArgs(bucket.update(args), bucket); try { if (isAclEnabled) { captureLatencyNs(perfMetrics.getLookupAclCheckLatencyNs(), () -> checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, bucket, - args.getKeyName()) + resolvedArgs.getKeyName()) ); } metrics.incNumKeyLookups(); @@ -175,14 +176,15 @@ public KeyInfoWithVolumeContext getKeyInfo(final OmKeyArgs args, () -> ozoneManager.resolveBucketLink(resolvedVolumeArgs)); boolean auditSuccess = true; - OmKeyArgs resolvedArgs = bucket.update(resolvedVolumeArgs); + OmKeyArgs resolvedArgs = + normalizeKeyArgs(bucket.update(resolvedVolumeArgs), bucket); try { if (isAclEnabled) { captureLatencyNs(perfMetrics.getGetKeyInfoAclCheckLatencyNs(), () -> checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, - bucket, args.getKeyName()) + bucket, resolvedArgs.getKeyName()) ); } @@ -298,7 +300,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException { boolean auditSuccess = true; Map<String, String> auditMap = bucket.audit(args.toAuditMap()); - args = bucket.update(args); + args = normalizeKeyArgs(bucket.update(args), bucket); try { if (isAclEnabled) { @@ -440,14 +442,14 @@ public Map<String, String> getObjectTagging(OmKeyArgs args) throws IOException { boolean auditSuccess = true; Map<String, String> auditMap = bucket.audit(args.toAuditMap()); - OmKeyArgs resolvedArgs = bucket.update(args); + OmKeyArgs resolvedArgs = normalizeKeyArgs(bucket.update(args), bucket); try { if (isAclEnabled) { captureLatencyNs(perfMetrics.getGetObjectTaggingAclCheckLatencyNs(), () -> checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ, bucket, - args.getKeyName()) + resolvedArgs.getKeyName()) ); } metrics.incNumGetObjectTagging(); @@ -468,6 +470,32 @@ public Map<String, String> getObjectTagging(OmKeyArgs args) throws IOException { } } + /** + * Normalize the key name on the read args up front, so the ACL check and the + * key read resolve the SAME key. The read path + * ({@link KeyManagerImpl#readKeyInfo}) normalizes '.'/'..' path segments before + * the DB lookup for layouts that use filesystem semantics (FSO, and LEGACY when + * {@code ozone.om.enable.filesystem.paths} is set). If the ACL check instead ran + * on the raw name, a request such as {@code a/../k1} would be evaluated against a + * different (literal) path than the normalized key {@code k1} that the read + * actually serves. Normalizing here keeps the check and the read in agreement for + * every authorizer (native and Ranger). Layouts that do not normalize (OBJECT_STORE) + * are returned unchanged. + */ + private OmKeyArgs normalizeKeyArgs(OmKeyArgs args, ResolvedBucket bucket) + throws OMException { + String keyName = args.getKeyName(); + if (keyName == null || keyName.isEmpty() || bucket.bucketLayout() == null) { + return args; + } + String normalized = OMClientRequest.validateAndNormalizeKey( + ozoneManager.getEnableFileSystemPaths(), keyName, bucket.bucketLayout()); + if (normalized.equals(keyName)) { + return args; + } + return args.toBuilder().setKeyName(normalized).build(); + } + /** * Checks if current caller has acl permissions. * --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
