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 eea3a0b710adef027d8a9d23bc35ad8b35180acd Author: Siyao Meng <[email protected]> AuthorDate: Thu Aug 20 14:33:07 2026 -0700 Enable OM S3 gRPC auth. (cherry picked from commit 1911e03d11e0ccdfb0588ae0a508db5b6f8a2d7a) --- .../org/apache/hadoop/ozone/om/OMConfigKeys.java | 16 +++ .../hadoop/ozone/TestOzoneConfigurationFields.java | 2 + .../hadoop/ozone/om/GrpcOzoneManagerServer.java | 40 +++++- .../hadoop/ozone/om/OzoneManagerServiceGrpc.java | 34 ++++- .../hadoop/ozone/om/request/OMClientRequest.java | 7 +- .../ozone/om/TestGrpcOzoneManagerServer.java | 152 +++++++++++++++++++++ .../request/TestOMClientRequestWithUserInfo.java | 67 +++++---- .../ozone/s3secret/S3SecretEndpointBase.java | 32 +++++ .../ozone/s3secret/TestS3SecretEndpointBase.java | 69 ++++++++++ 9 files changed, 380 insertions(+), 39 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 3ce10ec1d14..770b033d469 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -467,6 +467,22 @@ public final class OMConfigKeys { public static final boolean OZONE_OM_S3_GRPC_SERVER_ENABLED_DEFAULT = true; + /** + * When {@code ozone.security.enabled} is true, reject OMRequests received on + * the OM S3 gateway gRPC endpoint that do not carry S3 authentication. The S3 + * Gateway attaches S3 authentication to the requests it forwards on behalf of + * its clients; requests without it would otherwise be processed with a + * client-asserted identity. Identity-free bootstrap reads the S3 Gateway must + * issue before any per-request S3 authentication exists (ServiceList) are + * exempt. Set to false only for compatibility with deployments that rely on + * unauthenticated (anonymous) access through this endpoint. Has no effect when + * {@code ozone.security.enabled} is false. + */ + public static final String OZONE_OM_S3_GRPC_AUTH_REQUIRED = + "ozone.om.s3.grpc.auth.required"; + public static final boolean OZONE_OM_S3_GRPC_AUTH_REQUIRED_DEFAULT = + true; + public static final String OZONE_OM_NAMESPACE_STRICT_S3 = "ozone.om.namespace.s3.strict"; public static final boolean OZONE_OM_NAMESPACE_STRICT_S3_DEFAULT = diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOzoneConfigurationFields.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOzoneConfigurationFields.java index 98ccd8fac8b..311c2b9e89a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOzoneConfigurationFields.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOzoneConfigurationFields.java @@ -116,6 +116,8 @@ private void addPropertiesNotInXml() { OMConfigKeys.OZONE_OM_RATIS_SNAPSHOT_AUTO_TRIGGER_THRESHOLD_KEY, OMConfigKeys.OZONE_OM_HA_PREFIX, OMConfigKeys.OZONE_OM_GRPC_PORT_KEY, + // Security hardening toggle; documented on the OMConfigKeys constant. + OMConfigKeys.OZONE_OM_S3_GRPC_AUTH_REQUIRED, // TODO HDDS-2856 OMConfigKeys.OZONE_RANGER_OM_IGNORE_SERVER_CERT, OMConfigKeys.OZONE_RANGER_OM_CONNECTION_TIMEOUT, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java index faf6d623f3f..ffeeed45065 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/GrpcOzoneManagerServer.java @@ -28,6 +28,7 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_GRPC_WORKERGROUP_SIZE_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_GRPC_WORKERGROUP_SIZE_KEY; +import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; import io.grpc.Server; import io.grpc.ServerInterceptors; @@ -102,10 +103,30 @@ public GrpcOzoneManagerServer(OzoneConfiguration config, this.threadNamePrefix = threadPrefix; this.omS3gGrpcMetrics = GrpcMetrics.create(config); - init(omTranslator, - delegationTokenMgr, - config, - caClient); + try { + init(omTranslator, + delegationTokenMgr, + config, + caClient); + } catch (RuntimeException e) { + // GrpcMetrics was registered above; if init() fails the server (for + // example failing closed on TLS setup) stop() never runs, so drop the + // registration here to avoid leaking the metrics source. + omS3gGrpcMetrics.unRegister(); + throw e; + } + } + + /** + * Whether the OM S3 gateway gRPC endpoint must reject OMRequests that carry + * no S3 authentication. Only enforced when security is enabled; controlled by + * {@link OMConfigKeys#OZONE_OM_S3_GRPC_AUTH_REQUIRED} (default true). + */ + @VisibleForTesting + static boolean isS3AuthRequired(OzoneConfiguration conf) { + return new SecurityConfig(conf).isSecurityEnabled() + && conf.getBoolean(OMConfigKeys.OZONE_OM_S3_GRPC_AUTH_REQUIRED, + OMConfigKeys.OZONE_OM_S3_GRPC_AUTH_REQUIRED_DEFAULT); } public void init(OzoneManagerProtocolServerSideTranslatorPB omTranslator, @@ -141,6 +162,9 @@ public void init(OzoneManagerProtocolServerSideTranslatorPB omTranslator, workerEventLoopGroup = new NioEventLoopGroup(workerGroupSize, workerFactory); + SecurityConfig secConf = new SecurityConfig(omServerConfig); + boolean s3AuthRequired = isS3AuthRequired(omServerConfig); + NettyServerBuilder nettyServerBuilder = NettyServerBuilder.forPort(port) .maxInboundMessageSize(maxSize) .bossEventLoopGroup(bossEventLoopGroup) @@ -148,14 +172,13 @@ public void init(OzoneManagerProtocolServerSideTranslatorPB omTranslator, .channelType(NioServerSocketChannel.class) .executor(readExecutors) .addService(ServerInterceptors.intercept( - new OzoneManagerServiceGrpc(omTranslator), + new OzoneManagerServiceGrpc(omTranslator, s3AuthRequired), new ClientAddressServerInterceptor(), new GrpcMetricsServerResponseInterceptor(omS3gGrpcMetrics), new GrpcMetricsServerRequestInterceptor(omS3gGrpcMetrics))) .addTransportFilter( new GrpcMetricsServerTransportFilter(omS3gGrpcMetrics)); - SecurityConfig secConf = new SecurityConfig(omServerConfig); if (secConf.isSecurityEnabled() && secConf.isGrpcTlsEnabled()) { try { SslContextBuilder sslClientContextBuilder = @@ -166,7 +189,10 @@ public void init(OzoneManagerProtocolServerSideTranslatorPB omTranslator, HDDS_GRPC_TLS_PROVIDER_DEFAULT))); nettyServerBuilder.sslContext(sslContextBuilder.build()); } catch (Exception ex) { - LOG.error("Unable to setup TLS for secure Om S3g GRPC channel.", ex); + // Fail closed: silently starting this server in plaintext when TLS + // was requested would expose an unauthenticated OM endpoint. + throw new IllegalStateException( + "Unable to setup TLS for secure Om S3g GRPC channel.", ex); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java index 50e43047169..8c8175abfd7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java @@ -20,11 +20,14 @@ import com.google.protobuf.RpcController; import io.grpc.Status; import java.io.IOException; +import java.util.EnumSet; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.apache.hadoop.ipc.RPC; import org.apache.hadoop.ipc.Server; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerServiceGrpc.OzoneManagerServiceImplBase; import org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB; import org.apache.hadoop.ozone.util.UUIDUtil; @@ -41,11 +44,24 @@ public class OzoneManagerServiceGrpc extends OzoneManagerServiceImplBase { * RpcController is not used and hence is set to null. */ private static final RpcController NULL_RPC_CONTROLLER = null; + /** + * OMRequests the S3 gateway client must issue before any per-request S3 + * identity exists: RpcClient bootstrap (OzoneClientCache.initialize -> + * getServiceInfo), the background CA-cert refresher, and server-defaults + * lookups all fetch the ServiceList before EndpointBase has set any + * thread-local S3Auth. These identity-free, read-only cmdTypes stay + * callable without S3 authentication in secure mode. + */ + private static final Set<Type> S3_AUTH_EXEMPT_CMD_TYPES = + EnumSet.of(Type.ServiceList); private final OzoneManagerProtocolServerSideTranslatorPB omTranslator; + private final boolean s3AuthRequired; OzoneManagerServiceGrpc( - OzoneManagerProtocolServerSideTranslatorPB omTranslator) { + OzoneManagerProtocolServerSideTranslatorPB omTranslator, + boolean s3AuthRequired) { this.omTranslator = omTranslator; + this.s3AuthRequired = s3AuthRequired; } @Override @@ -55,6 +71,22 @@ public void submitRequest(OMRequest request, LOG.debug("OzoneManagerServiceGrpc: OzoneManagerServiceImplBase " + "processing s3g client submit request - for command {}", request.getCmdType().name()); + + // This endpoint carries no channel-level client authentication, so in + // secure mode a request acting on behalf of a user must present S3 + // authentication (validated downstream against the caller's S3 secret); + // without it the request identity would be client-asserted. Only the + // identity-free bootstrap reads in S3_AUTH_EXEMPT_CMD_TYPES are exempt. + if (s3AuthRequired && !request.hasS3Authentication() + && !S3_AUTH_EXEMPT_CMD_TYPES.contains(request.getCmdType())) { + LOG.warn("Rejecting {} OMRequest without S3 authentication received" + + " on the S3G gRPC endpoint", request.getCmdType().name()); + responseObserver.onError(Status.UNAUTHENTICATED + .withDescription("S3 authentication is required for requests to" + + " this endpoint in secure mode (ozone.om.s3.grpc.auth.required)") + .asRuntimeException()); + return; + } AtomicInteger callCount = new AtomicInteger(0); org.apache.hadoop.ipc.Server.getCurCall().set(new Server.Call(1, 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 e7689a90b81..c552fe59285 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 @@ -178,8 +178,11 @@ public OzoneManagerProtocolProtos.UserInfo getUserInfo() throws IOException { userInfo.setUserName(user.getUserName()); } - // for gRPC s3g omRequests that contain user name - if (user == null && omRequest.hasUserInfo()) { + // for gRPC s3g omRequests that contain user name. Only adopt the + // client-supplied user name when no identity was established above: + // it is unauthenticated data and must never override the identity + // derived from S3 authentication or from the RPC user. + if (user == null && !userInfo.hasUserName() && omRequest.hasUserInfo()) { userInfo.setUserName(omRequest.getUserInfo().getUserName()); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestGrpcOzoneManagerServer.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestGrpcOzoneManagerServer.java index 106efd73d68..d23da421fb1 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestGrpcOzoneManagerServer.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestGrpcOzoneManagerServer.java @@ -17,9 +17,31 @@ package org.apache.hadoop.ozone.om; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_GRPC_TLS_ENABLED; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY; +import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.createRequestWithS3Credentials; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import com.google.protobuf.ServiceException; +import io.grpc.stub.StreamObserver; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; +import org.apache.hadoop.hdds.security.x509.exception.CertificateException; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; +import org.apache.hadoop.ozone.grpc.metrics.GrpcMetrics; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB; import org.junit.jupiter.api.Test; @@ -47,4 +69,134 @@ public void testStartStop() throws Exception { } } + @Test + public void testRequestWithoutS3AuthRejectedWhenAuthRequired() { + OzoneManagerProtocolServerSideTranslatorPB omTranslator = + mock(OzoneManagerProtocolServerSideTranslatorPB.class); + OzoneManagerServiceGrpc service = + new OzoneManagerServiceGrpc(omTranslator, true); + OMRequest request = OMRequest.newBuilder() + .setCmdType(Type.GetS3VolumeContext) + .setClientId("test-client") + .build(); + @SuppressWarnings("unchecked") + StreamObserver<OMResponse> observer = mock(StreamObserver.class); + + service.submitRequest(request, observer); + + verify(observer).onError(any()); + verify(observer, never()).onNext(any()); + verifyNoInteractions(omTranslator); + } + + @Test + public void testServiceListWithoutS3AuthAllowedForS3gBootstrap() + throws ServiceException { + // ServiceList is the identity-free bootstrap read the S3 gateway issues + // before any S3Auth exists (see S3_AUTH_EXEMPT_CMD_TYPES); it must pass + // even when enforcement is on, or the gateway cannot create its OM client. + OMRequest request = OMRequest.newBuilder() + .setCmdType(Type.ServiceList) + .setClientId("test-client") + .build(); + + assertRequestPassedThrough(request, true); + } + + @Test + public void testRequestWithS3AuthAllowedWhenAuthRequired() + throws ServiceException { + // A request that carries S3 authentication is passed through to the + // translator, which validates the credential; the gate only rejects + // requests that carry no S3 authentication at all. + OMRequest request = createRequestWithS3Credentials("accessId", "signature", + "stringToSign").toBuilder() + .setCmdType(Type.GetS3VolumeContext) + .build(); + + assertRequestPassedThrough(request, true); + } + + @Test + public void testRequestWithoutS3AuthAllowedWhenAuthNotRequired() + throws ServiceException { + // With enforcement off (non-secure mode, or the knob disabled) a request + // without S3 authentication is passed through unchanged. + OMRequest request = OMRequest.newBuilder() + .setCmdType(Type.GetS3VolumeContext) + .setClientId("test-client") + .build(); + + assertRequestPassedThrough(request, false); + } + + /** + * Asserts the request is forwarded to the translator and its response is + * relayed back to the client (onNext + onCompleted, no onError). + */ + private static void assertRequestPassedThrough(OMRequest request, + boolean s3AuthRequired) throws ServiceException { + OzoneManagerProtocolServerSideTranslatorPB omTranslator = + mock(OzoneManagerProtocolServerSideTranslatorPB.class); + OMResponse response = OMResponse.newBuilder() + .setCmdType(request.getCmdType()) + .setStatus(Status.OK) + .build(); + when(omTranslator.submitRequest(any(), any())).thenReturn(response); + OzoneManagerServiceGrpc service = + new OzoneManagerServiceGrpc(omTranslator, s3AuthRequired); + @SuppressWarnings("unchecked") + StreamObserver<OMResponse> observer = mock(StreamObserver.class); + + service.submitRequest(request, observer); + + verify(omTranslator).submitRequest(any(), any()); + verify(observer).onNext(response); + verify(observer, never()).onError(any()); + verify(observer).onCompleted(); + } + + @Test + public void testTlsSetupFailureFailsClosed() throws Exception { + // In secure mode with gRPC TLS enabled, a failure setting up TLS must fail + // OM startup rather than silently exposing a plaintext, unauthenticated + // endpoint, and must not leak the GrpcMetrics registration on the way out. + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true); + conf.setBoolean(HDDS_GRPC_TLS_ENABLED, true); + OzoneManager ozoneManager = mock(OzoneManager.class); + CertificateClient caClient = mock(CertificateClient.class); + when(caClient.getKeyManager()) + .thenThrow(new CertificateException("injected TLS setup failure")); + + assertThrows(IllegalStateException.class, () -> + new GrpcOzoneManagerServer(conf, ozoneManager.getOmServerProtocol(), + ozoneManager.getDelegationTokenMgr(), caClient, "")); + assertNull(DefaultMetricsSystem.instance() + .getSource(GrpcMetrics.class.getSimpleName())); + } + + @Test + public void testS3AuthNotRequiredWhenSecurityDisabled() { + // The gate is off in non-secure mode even if the knob is left on. + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setBoolean(OMConfigKeys.OZONE_OM_S3_GRPC_AUTH_REQUIRED, true); + assertFalse(GrpcOzoneManagerServer.isS3AuthRequired(conf)); + } + + @Test + public void testS3AuthRequiredByDefaultInSecureMode() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true); + assertTrue(GrpcOzoneManagerServer.isS3AuthRequired(conf)); + } + + @Test + public void testS3AuthCanBeDisabledInSecureMode() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true); + conf.setBoolean(OMConfigKeys.OZONE_OM_S3_GRPC_AUTH_REQUIRED, false); + assertFalse(GrpcOzoneManagerServer.isS3AuthRequired(conf)); + } + } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestWithUserInfo.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestWithUserInfo.java index 9fda60374c1..193fd7bee4c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestWithUserInfo.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMClientRequestWithUserInfo.java @@ -41,6 +41,7 @@ import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.protocolPB.grpc.GrpcClientConstants; import org.apache.hadoop.ozone.om.request.bucket.OMBucketCreateRequest; import org.apache.hadoop.ozone.om.request.key.OMKeyCommitRequest; import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager; @@ -132,35 +133,43 @@ public void testUserInfoInCaseOfHadoopTransport() throws Exception { } @Test - public void testUserInfoInCaseOfGrpcTransport() throws IOException { - try (MockedStatic<Context> mockedGrpcRequestContextKey = - mockStatic(Context.class)) { - // given - Context.Key<String> hostnameKey = mock(Context.Key.class); - when(hostnameKey.get()).thenReturn("hostname"); - - Context.Key<String> ipAddress = mock(Context.Key.class); - when(ipAddress.get()).thenReturn("172.5.3.5"); - - mockedGrpcRequestContextKey.when(() -> Context.key("CLIENT_HOSTNAME")) - .thenReturn(hostnameKey); - mockedGrpcRequestContextKey.when(() -> Context.key("CLIENT_IP_ADDRESS")) - .thenReturn(ipAddress); - - OMRequest s3SignedOMRequest = createRequestWithS3Credentials("AccessId", - "Signature", "StringToSign"); - OMClientRequest omClientRequest = - new OMKeyCommitRequest(s3SignedOMRequest, mock(BucketLayout.class)); - - // when - OzoneManagerProtocolProtos.UserInfo userInfo = - omClientRequest.getUserInfo(); - - // then - assertEquals("hostname", userInfo.getHostName()); - assertEquals("172.5.3.5", userInfo.getRemoteAddress()); - assertEquals("AccessId", userInfo.getUserName()); - } + public void testUserInfoInCaseOfGrpcTransport() throws Exception { + OMRequest s3SignedOMRequest = createRequestWithS3Credentials("AccessId", + "Signature", "StringToSign"); + OMClientRequest omClientRequest = + new OMKeyCommitRequest(s3SignedOMRequest, mock(BucketLayout.class)); + + // The gRPC transport propagates the client host/IP through the request + // Context rather than the Hadoop RPC Server thread-local, so attach a real + // Context carrying those values and resolve the UserInfo within it. + Context grpcContext = Context.current() + .withValue(GrpcClientConstants.CLIENT_HOSTNAME_CTX_KEY, "hostname") + .withValue(GrpcClientConstants.CLIENT_IP_ADDRESS_CTX_KEY, "172.5.3.5"); + OzoneManagerProtocolProtos.UserInfo userInfo = + grpcContext.call(omClientRequest::getUserInfo); + + assertEquals("hostname", userInfo.getHostName()); + assertEquals("172.5.3.5", userInfo.getRemoteAddress()); + assertEquals("AccessId", userInfo.getUserName()); + } + + @Test + public void testClientSuppliedUserNameDoesNotOverrideS3AuthIdentity() + throws IOException { + // A gRPC S3G request carries validated S3 authentication (accessId + // "AccessId") but also a client-supplied userInfo.userName. The + // client-supplied name is unauthenticated and must never override the + // identity derived from S3 authentication; getUserInfo() must resolve the + // acting user from the accessId, not the forged name. + OMRequest request = createRequestWithS3Credentials("AccessId", "Signature", + "StringToSign").toBuilder() + .setUserInfo(OzoneManagerProtocolProtos.UserInfo.newBuilder() + .setUserName("forged-admin")) + .build(); + OMClientRequest omClientRequest = + new OMKeyCommitRequest(request, mock(BucketLayout.class)); + + assertEquals("AccessId", omClientRequest.getUserInfo().getUserName()); } } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java index 9a1a6af21ea..c63751c8308 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretEndpointBase.java @@ -17,6 +17,9 @@ package org.apache.hadoop.ozone.s3secret; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_TRANSPORT_CLASS; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_TRANSPORT_CLASS_DEFAULT; + import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.util.Map; @@ -25,6 +28,7 @@ import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.core.Context; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.OzoneSecurityUtil; import org.apache.hadoop.ozone.audit.AuditAction; import org.apache.hadoop.ozone.audit.AuditEventStatus; import org.apache.hadoop.ozone.audit.AuditLogger; @@ -35,12 +39,17 @@ import org.apache.hadoop.ozone.om.protocol.S3Auth; import org.apache.hadoop.ozone.s3.OzoneClientCache; import org.apache.hadoop.ozone.s3.util.AuditUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Base implementation of endpoint for working with S3 secret. */ public class S3SecretEndpointBase implements Auditor { + private static final Logger LOG = + LoggerFactory.getLogger(S3SecretEndpointBase.class); + private final OzoneConfiguration conf; private OzoneClient client; @@ -54,6 +63,24 @@ public class S3SecretEndpointBase implements Auditor { S3SecretEndpointBase(OzoneConfiguration conf) { this.conf = new OzoneConfiguration(conf); this.conf.setBoolean(S3Auth.S3_AUTH_CHECK, false); + // S3 secret generate/revoke carry no per-request S3 signature, and the + // S3G -> OM gRPC endpoint has no client authentication. In secure mode, + // route these operations over the Kerberos-authenticated OM RPC transport + // so OM authorizes the real caller (the S3 Gateway principal, which must + // be an S3 administrator) instead of a client-asserted identity. + if (OzoneSecurityUtil.isSecurityEnabled(this.conf)) { + String configured = this.conf.get(OZONE_OM_TRANSPORT_CLASS); + if (configured != null + && !configured.equals(OZONE_OM_TRANSPORT_CLASS_DEFAULT)) { + // The gateway data path commonly runs on gRPC, which is never safe for + // secret ops; override it for this endpoint only and leave the rest of + // the S3 Gateway on the configured transport. + LOG.warn("Overriding OM transport from {} to {} for S3 secret " + + "operations in secure mode; other S3 Gateway clients are " + + "unaffected.", configured, OZONE_OM_TRANSPORT_CLASS_DEFAULT); + } + this.conf.set(OZONE_OM_TRANSPORT_CLASS, OZONE_OM_TRANSPORT_CLASS_DEFAULT); + } } @PostConstruct @@ -107,6 +134,11 @@ public void setContext(ContainerRequestContext context) { this.context = context; } + @VisibleForTesting + OzoneConfiguration getConf() { + return conf; + } + protected Map<String, String> getAuditParameters() { return AuditUtils.getAuditParameters(context); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestS3SecretEndpointBase.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestS3SecretEndpointBase.java new file mode 100644 index 00000000000..4a1628a0cb6 --- /dev/null +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestS3SecretEndpointBase.java @@ -0,0 +1,69 @@ +/* + * 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.s3secret; + +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_TRANSPORT_CLASS; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_TRANSPORT_CLASS_DEFAULT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.om.protocolPB.GrpcOmTransportFactory; +import org.apache.ozone.test.GenericTestUtils.LogCapturer; +import org.junit.jupiter.api.Test; + +/** + * Tests OM transport selection for the S3 secret endpoints. + */ +class TestS3SecretEndpointBase { + + @Test + void testSecretOpsUseRpcTransportInSecureMode() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true); + // Simulate the S3 Gateway pinning the gRPC transport for its data client. + conf.set(OZONE_OM_TRANSPORT_CLASS, GrpcOmTransportFactory.class.getName()); + + LogCapturer logs = LogCapturer.captureLogs(S3SecretEndpointBase.class); + S3SecretManagementEndpoint endpoint = + new S3SecretManagementEndpoint(conf); + + // The gRPC S3 endpoint has no client authentication, so in secure mode + // secret ops must fall back to the Kerberos-authenticated RPC transport + // where OM can authorize the real caller. + assertEquals(OZONE_OM_TRANSPORT_CLASS_DEFAULT, + endpoint.getConf().get(OZONE_OM_TRANSPORT_CLASS)); + // The override is not silent: it warns that only this endpoint changed. + assertThat(logs.getOutput()).contains("Overriding OM transport"); + } + + @Test + void testTransportUnchangedWhenSecurityDisabled() { + OzoneConfiguration conf = new OzoneConfiguration(); + String grpcTransport = GrpcOmTransportFactory.class.getName(); + conf.set(OZONE_OM_TRANSPORT_CLASS, grpcTransport); + + S3SecretManagementEndpoint endpoint = + new S3SecretManagementEndpoint(conf); + + // Non-secure mode is outside the security model; leave the transport as-is. + assertEquals(grpcTransport, + endpoint.getConf().get(OZONE_OM_TRANSPORT_CLASS)); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
