Skip to content

Commit f71c648

Browse files
authored
HDDS-14898. Fix Latent S3 API Issue having No Acl Check for ListParts (#9976)
1 parent 83e77c0 commit f71c648

2 files changed

Lines changed: 217 additions & 13 deletions

File tree

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3941,24 +3941,32 @@ public OmMultipartUploadListParts listParts(final String volumeName,
39413941
final String bucketName, String keyName, String uploadID,
39423942
int partNumberMarker, int maxParts) throws IOException {
39433943

3944-
ResolvedBucket bucket = resolveBucketLink(Pair.of(volumeName, bucketName));
3945-
3946-
Map<String, String> auditMap = bucket.audit();
3947-
auditMap.put(OzoneConsts.KEY, keyName);
3948-
auditMap.put(OzoneConsts.UPLOAD_ID, uploadID);
3949-
auditMap.put(OzoneConsts.PART_NUMBER_MARKER,
3950-
Integer.toString(partNumberMarker));
3951-
auditMap.put(OzoneConsts.MAX_PARTS, Integer.toString(maxParts));
3944+
final ResolvedBucket bucket = resolveBucketLink(Pair.of(volumeName, bucketName));
3945+
final String realVolumeName = bucket.realVolume();
3946+
final String realBucketName = bucket.realBucket();
39523947

3953-
metrics.incNumListMultipartUploadParts();
3948+
final Map<String, String> auditMap = bucket.audit();
39543949
try {
3955-
OmMultipartUploadListParts omMultipartUploadListParts =
3956-
keyManager.listParts(bucket.realVolume(), bucket.realBucket(),
3957-
keyName, uploadID, partNumberMarker, maxParts);
3950+
auditMap.put(OzoneConsts.KEY, keyName);
3951+
auditMap.put(OzoneConsts.UPLOAD_ID, uploadID);
3952+
auditMap.put(OzoneConsts.PART_NUMBER_MARKER,
3953+
Integer.toString(partNumberMarker));
3954+
auditMap.put(OzoneConsts.MAX_PARTS, Integer.toString(maxParts));
3955+
3956+
if (getAclsEnabled()) {
3957+
omMetadataReader.checkAcls(
3958+
ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, realVolumeName, realBucketName, null);
3959+
omMetadataReader.checkAcls(
3960+
ResourceType.KEY, StoreType.OZONE, ACLType.READ, realVolumeName, realBucketName, keyName);
3961+
}
3962+
3963+
metrics.incNumListMultipartUploadParts();
3964+
final OmMultipartUploadListParts omMultipartUploadListParts = keyManager.listParts(
3965+
realVolumeName, realBucketName, keyName, uploadID, partNumberMarker, maxParts);
39583966
AUDIT.logReadSuccess(buildAuditMessageForSuccess(OMAction
39593967
.LIST_MULTIPART_UPLOAD_PARTS, auditMap));
39603968
return omMultipartUploadListParts;
3961-
} catch (IOException ex) {
3969+
} catch (Exception ex) {
39623970
metrics.incNumListMultipartUploadPartFails();
39633971
AUDIT.logReadFailure(buildAuditMessageForFailure(OMAction
39643972
.LIST_MULTIPART_UPLOAD_PARTS, auditMap, ex));
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.hadoop.ozone.om;
19+
20+
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ;
21+
import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.BUCKET;
22+
import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.KEY;
23+
import static org.apache.hadoop.ozone.security.acl.OzoneObj.StoreType.OZONE;
24+
import static org.junit.jupiter.api.Assertions.assertThrows;
25+
import static org.mockito.ArgumentMatchers.any;
26+
import static org.mockito.ArgumentMatchers.anyInt;
27+
import static org.mockito.ArgumentMatchers.anyMap;
28+
import static org.mockito.ArgumentMatchers.anyString;
29+
import static org.mockito.ArgumentMatchers.eq;
30+
import static org.mockito.Mockito.doNothing;
31+
import static org.mockito.Mockito.doReturn;
32+
import static org.mockito.Mockito.doThrow;
33+
import static org.mockito.Mockito.inOrder;
34+
import static org.mockito.Mockito.mock;
35+
import static org.mockito.Mockito.never;
36+
import static org.mockito.Mockito.spy;
37+
import static org.mockito.Mockito.verify;
38+
import static org.mockito.Mockito.when;
39+
40+
import java.io.File;
41+
import org.apache.commons.lang3.tuple.Pair;
42+
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
43+
import org.apache.hadoop.hdds.scm.HddsWhiteboxTestUtils;
44+
import org.apache.hadoop.hdds.server.ServerUtils;
45+
import org.apache.hadoop.ozone.audit.AuditMessage;
46+
import org.apache.hadoop.ozone.om.exceptions.OMException;
47+
import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadListParts;
48+
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.S3Authentication;
49+
import org.junit.jupiter.api.AfterAll;
50+
import org.junit.jupiter.api.AfterEach;
51+
import org.junit.jupiter.api.BeforeAll;
52+
import org.junit.jupiter.api.BeforeEach;
53+
import org.junit.jupiter.api.Test;
54+
import org.junit.jupiter.api.TestInstance;
55+
import org.junit.jupiter.api.io.TempDir;
56+
import org.mockito.InOrder;
57+
58+
59+
/**
60+
* Unit tests for ACL checks on {@link OzoneManager#listParts}.
61+
*/
62+
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
63+
public class TestOzoneManagerListPartsAcls {
64+
65+
private OmTestManagers omTestManagers;
66+
private OzoneManager om;
67+
68+
private OzoneManager omSpy;
69+
private OmMetadataReader omMetadataReader;
70+
private KeyManager keyManager;
71+
private OMMetrics metrics;
72+
73+
private static final String REQUESTED_VOLUME = "requestedVolume";
74+
private static final String REQUESTED_BUCKET = "requestedBucket";
75+
private static final String REAL_VOLUME = "realVolume";
76+
private static final String REAL_BUCKET = "realBucket";
77+
private static final String KEY_NAME = "object/key";
78+
private static final String UPLOAD_ID = "uploadId";
79+
80+
@BeforeAll
81+
void setup(@TempDir File folder) throws Exception {
82+
final OzoneConfiguration conf = new OzoneConfiguration();
83+
ServerUtils.setOzoneMetaDirPath(conf, folder.toString());
84+
omTestManagers = new OmTestManagers(conf);
85+
om = omTestManagers.getOzoneManager();
86+
}
87+
88+
@AfterAll
89+
void cleanup() {
90+
if (omTestManagers != null) {
91+
omTestManagers.stop();
92+
}
93+
}
94+
95+
@BeforeEach
96+
void init() throws Exception {
97+
omSpy = spy(om);
98+
omMetadataReader = mock(OmMetadataReader.class);
99+
keyManager = mock(KeyManager.class);
100+
metrics = mock(OMMetrics.class);
101+
102+
HddsWhiteboxTestUtils.setInternalState(omSpy, "omMetadataReader", omMetadataReader);
103+
HddsWhiteboxTestUtils.setInternalState(omSpy, "keyManager", keyManager);
104+
HddsWhiteboxTestUtils.setInternalState(omSpy, "metrics", metrics);
105+
106+
doReturn(new ResolvedBucket(REQUESTED_VOLUME, REQUESTED_BUCKET, REAL_VOLUME, REAL_BUCKET, "owner", null))
107+
.when(omSpy).resolveBucketLink(Pair.of(REQUESTED_VOLUME, REQUESTED_BUCKET));
108+
109+
final AuditMessage mockAuditMessage = mock(AuditMessage.class);
110+
when(mockAuditMessage.getOp()).thenReturn("LIST_MULTIPART_UPLOAD_PARTS");
111+
doReturn(mockAuditMessage).when(omSpy).buildAuditMessageForSuccess(any(), anyMap());
112+
doReturn(mockAuditMessage).when(omSpy).buildAuditMessageForFailure(any(), anyMap(), any(Throwable.class));
113+
114+
when(keyManager.listParts(anyString(), anyString(), anyString(), anyString(), anyInt(), anyInt()))
115+
.thenReturn(mock(OmMultipartUploadListParts.class));
116+
}
117+
118+
@AfterEach
119+
void tearDown() {
120+
OzoneManager.setS3Auth(null);
121+
}
122+
123+
@Test
124+
void testSkipsAclChecksWhenAclsAreDisabled() throws Exception {
125+
setupS3Request();
126+
when(omSpy.getAclsEnabled()).thenReturn(false);
127+
128+
omSpy.listParts(REQUESTED_VOLUME, REQUESTED_BUCKET, KEY_NAME, UPLOAD_ID, 0, 10);
129+
130+
verify(omMetadataReader, never()).checkAcls(any(), any(), any(), any(), any(), any());
131+
verify(keyManager).listParts(
132+
eq(REAL_VOLUME), eq(REAL_BUCKET), eq(KEY_NAME), eq(UPLOAD_ID), eq(0), eq(10));
133+
verify(metrics).incNumListMultipartUploadParts();
134+
verify(metrics, never()).incNumListMultipartUploadPartFails();
135+
}
136+
137+
@Test
138+
void testAclsEnabledChecksBucketReadThenKeyReadUsingResolvedNames() throws Exception {
139+
setupS3Request();
140+
when(omSpy.getAclsEnabled()).thenReturn(true);
141+
142+
omSpy.listParts(REQUESTED_VOLUME, REQUESTED_BUCKET, KEY_NAME, UPLOAD_ID, 0, 10);
143+
144+
final InOrder inOrder = inOrder(omMetadataReader);
145+
inOrder.verify(omMetadataReader).checkAcls(BUCKET, OZONE, READ, REAL_VOLUME, REAL_BUCKET, null);
146+
inOrder.verify(omMetadataReader).checkAcls(KEY, OZONE, READ, REAL_VOLUME, REAL_BUCKET, KEY_NAME);
147+
verify(keyManager).listParts(
148+
eq(REAL_VOLUME), eq(REAL_BUCKET), eq(KEY_NAME), eq(UPLOAD_ID), eq(0), eq(10));
149+
verify(metrics).incNumListMultipartUploadParts();
150+
verify(metrics, never()).incNumListMultipartUploadPartFails();
151+
}
152+
153+
@Test
154+
void testReadAclAccessDeniedSkipsKeyManagerAndKeyReadAclChecksAndRecordsFailure() throws Exception {
155+
setupS3Request();
156+
when(omSpy.getAclsEnabled()).thenReturn(true);
157+
158+
doThrow(new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED))
159+
.when(omMetadataReader).checkAcls(BUCKET, OZONE, READ, REAL_VOLUME, REAL_BUCKET, null);
160+
161+
assertThrows(
162+
OMException.class, () -> omSpy.listParts(
163+
REQUESTED_VOLUME, REQUESTED_BUCKET, KEY_NAME, UPLOAD_ID, 0, 10));
164+
165+
verify(keyManager, never()).listParts(
166+
anyString(), anyString(), anyString(), anyString(), anyInt(), anyInt());
167+
verify(metrics, never()).incNumListMultipartUploadParts();
168+
verify(metrics).incNumListMultipartUploadPartFails();
169+
verify(omSpy).buildAuditMessageForFailure(any(), anyMap(), any(Throwable.class));
170+
verify(omMetadataReader, never()).checkAcls(KEY, OZONE, READ, REAL_VOLUME, REAL_BUCKET, KEY_NAME);
171+
}
172+
173+
@Test
174+
void testKeyReadAclAccessDeniedSkipsKeyManagerAndRecordsFailure() throws Exception {
175+
setupS3Request();
176+
when(omSpy.getAclsEnabled()).thenReturn(true);
177+
178+
doNothing().when(omMetadataReader).checkAcls(BUCKET, OZONE, READ, REAL_VOLUME, REAL_BUCKET, null);
179+
doThrow(new OMException("denied", OMException.ResultCodes.PERMISSION_DENIED))
180+
.when(omMetadataReader).checkAcls(KEY, OZONE, READ, REAL_VOLUME, REAL_BUCKET, KEY_NAME);
181+
182+
assertThrows(
183+
OMException.class, () -> omSpy.listParts(
184+
REQUESTED_VOLUME, REQUESTED_BUCKET, KEY_NAME, UPLOAD_ID, 0, 10));
185+
186+
verify(keyManager, never()).listParts(
187+
anyString(), anyString(), anyString(), anyString(), anyInt(), anyInt());
188+
verify(metrics, never()).incNumListMultipartUploadParts();
189+
verify(metrics).incNumListMultipartUploadPartFails();
190+
verify(omSpy).buildAuditMessageForFailure(any(), anyMap(), any(Throwable.class));
191+
}
192+
193+
private void setupS3Request() {
194+
OzoneManager.setS3Auth(S3Authentication.newBuilder().setAccessId("AKIAJWFJK62WUTKNFJJA").build());
195+
}
196+
}

0 commit comments

Comments
 (0)