diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java index e03e9287ef18..6e701d5a3f54 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java @@ -41,7 +41,7 @@ import org.apache.hadoop.hdds.conf.ConfigurationException; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.ha.SCMHAUtils; import org.apache.hadoop.hdds.scm.ha.SCMNodeInfo; @@ -354,7 +354,7 @@ public static String getHostName(ConfigurationSource conf) * @return True if its readOnly , false otherwise. */ public static boolean isReadOnly( - ContainerProtos.ContainerCommandRequestProto proto) { + ContainerCommandRequestProtoOrBuilder proto) { switch (proto.getCmdType()) { case ReadContainer: case ReadChunk: @@ -394,12 +394,15 @@ public static boolean isReadOnly( public static boolean requireBlockToken( ContainerProtos.Type cmdType) { switch (cmdType) { - case ReadChunk: + case DeleteBlock: + case DeleteChunk: case GetBlock: - case WriteChunk: + case GetCommittedBlockLength: + case GetSmallFile: case PutBlock: case PutSmallFile: - case GetSmallFile: + case ReadChunk: + case WriteChunk: return true; default: return false; @@ -412,7 +415,6 @@ public static boolean requireContainerToken( case CloseContainer: case CreateContainer: case DeleteContainer: - case ListContainer: case ReadContainer: case UpdateContainer: return true; @@ -426,44 +428,66 @@ public static boolean requireContainerToken( * @param msg container command * @return block ID. */ - public static BlockID getBlockID(ContainerCommandRequestProto msg) { + public static BlockID getBlockID(ContainerCommandRequestProtoOrBuilder msg) { + ContainerProtos.DatanodeBlockID blockID = null; switch (msg.getCmdType()) { - case ReadChunk: - if (msg.hasReadChunk()) { - return BlockID.getFromProtobuf(msg.getReadChunk().getBlockID()); + case DeleteBlock: + if (msg.hasDeleteBlock()) { + blockID = msg.getDeleteBlock().getBlockID(); } - return null; + break; + case DeleteChunk: + if (msg.hasDeleteChunk()) { + blockID = msg.getDeleteChunk().getBlockID(); + } + break; case GetBlock: if (msg.hasGetBlock()) { - return BlockID.getFromProtobuf(msg.getGetBlock().getBlockID()); + blockID = msg.getGetBlock().getBlockID(); } - return null; - case WriteChunk: - if (msg.hasWriteChunk()) { - return BlockID.getFromProtobuf(msg.getWriteChunk().getBlockID()); + break; + case GetCommittedBlockLength: + if (msg.hasGetCommittedBlockLength()) { + blockID = msg.getGetCommittedBlockLength().getBlockID(); } - return null; + break; + case GetSmallFile: + if (msg.hasGetSmallFile()) { + blockID = msg.getGetSmallFile().getBlock().getBlockID(); + } + break; + case ListChunk: + if (msg.hasListChunk()) { + blockID = msg.getListChunk().getBlockID(); + } + break; case PutBlock: if (msg.hasPutBlock()) { - return BlockID.getFromProtobuf(msg.getPutBlock().getBlockData() - .getBlockID()); + blockID = msg.getPutBlock().getBlockData().getBlockID(); } - return null; + break; case PutSmallFile: if (msg.hasPutSmallFile()) { - return BlockID.getFromProtobuf(msg.getPutSmallFile().getBlock() - .getBlockData().getBlockID()); + blockID = msg.getPutSmallFile().getBlock().getBlockData().getBlockID(); } - return null; - case GetSmallFile: - if (msg.hasGetSmallFile()) { - return BlockID.getFromProtobuf(msg.getGetSmallFile().getBlock() - .getBlockID()); + break; + case ReadChunk: + if (msg.hasReadChunk()) { + blockID = msg.getReadChunk().getBlockID(); } - return null; + break; + case WriteChunk: + if (msg.hasWriteChunk()) { + blockID = msg.getWriteChunk().getBlockID(); + } + break; default: - return null; + break; } + + return blockID != null + ? BlockID.getFromProtobuf(blockID) + : null; } /** diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java index 4c344ab58033..5e4a8dc46052 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java @@ -33,6 +33,7 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto.Builder; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandResponseProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.DatanodeBlockID; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.KeyValue; import org.apache.hadoop.hdds.scm.pipeline.MockPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; @@ -135,6 +136,17 @@ public static ContainerCommandRequestProto getWriteChunkRequest( public static ContainerCommandRequestProto getWriteChunkRequest( Pipeline pipeline, BlockID blockID, int datalen, int seq, String token) throws IOException { + Builder builder = newWriteChunkRequestBuilder(pipeline, blockID, datalen, + seq); + if (!Strings.isNullOrEmpty(token)) { + builder.setEncodedToken(token); + } + return builder.build(); + } + + public static Builder newWriteChunkRequestBuilder( + Pipeline pipeline, BlockID blockID, int datalen, int seq) + throws IOException { LOG.trace("writeChunk {} (blockID={}) to pipeline={}", datalen, blockID, pipeline); ContainerProtos.WriteChunkRequestProto.Builder writeRequest = @@ -156,11 +168,8 @@ public static ContainerCommandRequestProto getWriteChunkRequest( request.setContainerID(blockID.getContainerID()); request.setWriteChunk(writeRequest); request.setDatanodeUuid(pipeline.getFirstNode().getUuidString()); - if (!Strings.isNullOrEmpty(token)) { - request.setEncodedToken(token); - } - return request.build(); + return request; } /** @@ -225,19 +234,25 @@ public static ContainerCommandRequestProto getReadSmallFileRequest( * Returns a read Request. * * @param pipeline pipeline. - * @param request writeChunkRequest. + * @param writeChunk writeChunkRequest. * @return Request. */ public static ContainerCommandRequestProto getReadChunkRequest( - Pipeline pipeline, ContainerProtos.WriteChunkRequestProto request) + Pipeline pipeline, ContainerProtos.WriteChunkRequestProto writeChunk) + throws IOException { + return newReadChunkRequestBuilder(pipeline, writeChunk).build(); + } + + public static Builder newReadChunkRequestBuilder(Pipeline pipeline, + ContainerProtos.WriteChunkRequestProtoOrBuilder writeChunk) throws IOException { LOG.trace("readChunk blockID={} from pipeline={}", - request.getBlockID(), pipeline); + writeChunk.getBlockID(), pipeline); ContainerProtos.ReadChunkRequestProto.Builder readRequest = ContainerProtos.ReadChunkRequestProto.newBuilder(); - readRequest.setBlockID(request.getBlockID()); - readRequest.setChunkData(request.getChunkData()); + readRequest.setBlockID(writeChunk.getBlockID()); + readRequest.setChunkData(writeChunk.getChunkData()); readRequest.setReadChunkVersion(ContainerProtos.ReadChunkVersion.V1); Builder newRequest = @@ -246,7 +261,7 @@ public static ContainerCommandRequestProto getReadChunkRequest( newRequest.setContainerID(readRequest.getBlockID().getContainerID()); newRequest.setReadChunk(readRequest); newRequest.setDatanodeUuid(pipeline.getFirstNode().getUuidString()); - return newRequest.build(); + return newRequest; } /** @@ -259,6 +274,12 @@ public static ContainerCommandRequestProto getReadChunkRequest( public static ContainerCommandRequestProto getDeleteChunkRequest( Pipeline pipeline, ContainerProtos.WriteChunkRequestProto writeRequest) throws IOException { + return newDeleteChunkRequestBuilder(pipeline, writeRequest).build(); + } + + public static Builder newDeleteChunkRequestBuilder(Pipeline pipeline, + ContainerProtos.WriteChunkRequestProtoOrBuilder writeRequest) + throws IOException { LOG.trace("deleteChunk blockID={} from pipeline={}", writeRequest.getBlockID(), pipeline); @@ -275,7 +296,7 @@ public static ContainerCommandRequestProto getDeleteChunkRequest( request.setContainerID(writeRequest.getBlockID().getContainerID()); request.setDeleteChunk(deleteRequest); request.setDatanodeUuid(pipeline.getFirstNode().getUuidString()); - return request.build(); + return request; } /** @@ -404,8 +425,18 @@ public static ContainerCommandRequestProto getPutBlockRequest( Pipeline pipeline, String token, ContainerProtos.WriteChunkRequestProto writeRequest) throws IOException { - LOG.trace("putBlock: {} to pipeline={} with token {}", - writeRequest.getBlockID(), pipeline, token); + Builder builder = newPutBlockRequestBuilder(pipeline, writeRequest); + if (!Strings.isNullOrEmpty(token)) { + builder.setEncodedToken(token); + } + return builder.build(); + } + + public static Builder newPutBlockRequestBuilder(Pipeline pipeline, + ContainerProtos.WriteChunkRequestProtoOrBuilder writeRequest) + throws IOException { + LOG.trace("putBlock: {} to pipeline={}", + writeRequest.getBlockID(), pipeline); ContainerProtos.PutBlockRequestProto.Builder putRequest = ContainerProtos.PutBlockRequestProto.newBuilder(); @@ -424,10 +455,7 @@ public static ContainerCommandRequestProto getPutBlockRequest( request.setContainerID(blockData.getContainerID()); request.setPutBlock(putRequest); request.setDatanodeUuid(pipeline.getFirstNode().getUuidString()); - if (!Strings.isNullOrEmpty(token)) { - request.setEncodedToken(token); - } - return request.build(); + return request; } /** @@ -440,9 +468,13 @@ public static ContainerCommandRequestProto getPutBlockRequest( public static ContainerCommandRequestProto getBlockRequest( Pipeline pipeline, ContainerProtos.PutBlockRequestProto putBlockRequest) throws IOException { - ContainerProtos.DatanodeBlockID blockID = - putBlockRequest.getBlockData().getBlockID(); - LOG.trace("getKey: blockID={}", blockID); + return newGetBlockRequestBuilder(pipeline, putBlockRequest).build(); + } + + public static Builder newGetBlockRequestBuilder( + Pipeline pipeline, ContainerProtos.PutBlockRequestProtoOrBuilder putBlock) + throws IOException { + DatanodeBlockID blockID = putBlock.getBlockData().getBlockID(); ContainerProtos.GetBlockRequestProto.Builder getRequest = ContainerProtos.GetBlockRequestProto.newBuilder(); @@ -454,7 +486,7 @@ public static ContainerCommandRequestProto getBlockRequest( request.setContainerID(blockID.getContainerID()); request.setGetBlock(getRequest); request.setDatanodeUuid(pipeline.getFirstNode().getUuidString()); - return request.build(); + return request; } /** @@ -478,8 +510,13 @@ public static void verifyGetBlock(ContainerCommandRequestProto request, public static ContainerCommandRequestProto getDeleteBlockRequest( Pipeline pipeline, ContainerProtos.PutBlockRequestProto putBlockRequest) throws IOException { - ContainerProtos.DatanodeBlockID blockID = putBlockRequest.getBlockData() - .getBlockID(); + return newDeleteBlockRequestBuilder(pipeline, putBlockRequest).build(); + } + + public static Builder newDeleteBlockRequestBuilder(Pipeline pipeline, + ContainerProtos.PutBlockRequestProtoOrBuilder putBlockRequest) + throws IOException { + DatanodeBlockID blockID = putBlockRequest.getBlockData().getBlockID(); LOG.trace("deleteBlock: name={}", blockID); ContainerProtos.DeleteBlockRequestProto.Builder delRequest = ContainerProtos.DeleteBlockRequestProto.newBuilder(); @@ -490,7 +527,23 @@ public static ContainerCommandRequestProto getDeleteBlockRequest( request.setContainerID(blockID.getContainerID()); request.setDeleteBlock(delRequest); request.setDatanodeUuid(pipeline.getFirstNode().getUuidString()); - return request.build(); + return request; + } + + public static Builder newGetCommittedBlockLengthBuilder(Pipeline pipeline, + ContainerProtos.PutBlockRequestProtoOrBuilder putBlock) + throws IOException { + DatanodeBlockID blockID = putBlock.getBlockData().getBlockID(); + + ContainerProtos.GetCommittedBlockLengthRequestProto.Builder req = + ContainerProtos.GetCommittedBlockLengthRequestProto.newBuilder() + .setBlockID(blockID); + + return ContainerCommandRequestProto.newBuilder() + .setCmdType(ContainerProtos.Type.GetCommittedBlockLength) + .setContainerID(blockID.getContainerID()) + .setDatanodeUuid(pipeline.getFirstNode().getUuidString()) + .setGetCommittedBlockLength(req); } /** diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/BlockTokenVerifier.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/BlockTokenVerifier.java index a43cd2c97130..092a05959610 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/BlockTokenVerifier.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/BlockTokenVerifier.java @@ -23,17 +23,15 @@ import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.client.ContainerBlockID; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.GetBlock; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.GetSmallFile; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.PutBlock; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.PutSmallFile; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.ReadChunk; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.WriteChunk; +import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.DeleteBlock; +import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Type.DeleteChunk; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockTokenSecretProto.AccessModeProto.DELETE; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockTokenSecretProto.AccessModeProto.READ; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockTokenSecretProto.AccessModeProto.WRITE; @@ -68,7 +66,7 @@ protected OzoneBlockTokenIdentifier createTokenIdentifier() { } @Override - protected Object getService(ContainerCommandRequestProto cmd) { + protected Object getService(ContainerCommandRequestProtoOrBuilder cmd) { BlockID blockID = HddsUtils.getBlockID(cmd); Preconditions.checkNotNull(blockID, "no blockID in %s command", cmd.getCmdType()); @@ -77,21 +75,20 @@ protected Object getService(ContainerCommandRequestProto cmd) { @Override protected void verify(OzoneBlockTokenIdentifier tokenId, - ContainerCommandRequestProto cmd) throws SCMSecurityException { + ContainerCommandRequestProtoOrBuilder cmd) throws SCMSecurityException { - ContainerProtos.Type type = cmd.getCmdType(); - if (type == ReadChunk || type == GetBlock || type == GetSmallFile) { - if (!tokenId.getAccessModes().contains(READ)) { - throw new BlockTokenException("Block token with " + tokenId.getService() - + " doesn't have READ permission"); - } - } else if (type == WriteChunk || type == PutBlock || type == PutSmallFile) { - if (!tokenId.getAccessModes().contains(WRITE)) { - throw new BlockTokenException("Block token with " + tokenId.getService() - + " doesn't have WRITE permission"); - } + HddsProtos.BlockTokenSecretProto.AccessModeProto accessMode; + if (HddsUtils.isReadOnly(cmd)) { + accessMode = READ; + } else if (cmd.getCmdType() == DeleteBlock || + cmd.getCmdType() == DeleteChunk) { + accessMode = DELETE; } else { - throw new BlockTokenException("Block token does not support " + cmd); + accessMode = WRITE; + } + if (!tokenId.getAccessModes().contains(accessMode)) { + throw new BlockTokenException("Block token with " + tokenId.getService() + + " doesn't have " + accessMode + " permission"); } } } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/CompositeTokenVerifier.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/CompositeTokenVerifier.java index ce10c25fb07f..750bb7613f91 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/CompositeTokenVerifier.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/CompositeTokenVerifier.java @@ -17,7 +17,7 @@ */ package org.apache.hadoop.hdds.security.token; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.security.token.Token; @@ -37,7 +37,7 @@ public CompositeTokenVerifier(List delegates) { @Override public void verify(String user, Token token, - ContainerCommandRequestProto cmd) throws SCMSecurityException { + ContainerCommandRequestProtoOrBuilder cmd) throws SCMSecurityException { for (TokenVerifier verifier : delegates) { verifier.verify(user, token, cmd); diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ContainerTokenVerifier.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ContainerTokenVerifier.java index 806a9ae56d34..941160a042d5 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ContainerTokenVerifier.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ContainerTokenVerifier.java @@ -19,7 +19,7 @@ import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; @@ -45,7 +45,7 @@ protected ContainerTokenIdentifier createTokenIdentifier() { } @Override - protected Object getService(ContainerCommandRequestProto cmd) { + protected Object getService(ContainerCommandRequestProtoOrBuilder cmd) { return ContainerID.valueOf(cmd.getContainerID()); } } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/NoopTokenVerifier.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/NoopTokenVerifier.java index b86b906e9766..084b42efe90b 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/NoopTokenVerifier.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/NoopTokenVerifier.java @@ -17,7 +17,7 @@ */ package org.apache.hadoop.hdds.security.token; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; import org.apache.hadoop.security.token.Token; /** No-op verifier, used when block token is disabled in config. */ @@ -25,12 +25,12 @@ public class NoopTokenVerifier implements TokenVerifier { @Override public void verify(String user, Token token, - ContainerCommandRequestProto cmd) { + ContainerCommandRequestProtoOrBuilder cmd) { // no-op } @Override // to avoid "failed to find token" - public void verify(ContainerCommandRequestProto cmd, String user, + public void verify(ContainerCommandRequestProtoOrBuilder cmd, String user, String encodedToken) { // no-op } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ShortLivedTokenVerifier.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ShortLivedTokenVerifier.java index dc89d0b64fa5..92b643e28592 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ShortLivedTokenVerifier.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/ShortLivedTokenVerifier.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hdds.security.token; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; @@ -58,17 +58,18 @@ protected ShortLivedTokenVerifier(SecurityConfig conf, protected abstract T createTokenIdentifier(); /** Extract info on "service" being accessed by {@code cmd}. */ - protected abstract Object getService(ContainerCommandRequestProto cmd); + protected abstract Object getService( + ContainerCommandRequestProtoOrBuilder cmd); /** Hook for further verification. */ - protected void verify(T tokenId, ContainerCommandRequestProto cmd) + protected void verify(T tokenId, ContainerCommandRequestProtoOrBuilder cmd) throws SCMSecurityException { // NOP } @Override public void verify(String user, Token token, - ContainerCommandRequestProto cmd) throws SCMSecurityException { + ContainerCommandRequestProtoOrBuilder cmd) throws SCMSecurityException { if (!isTokenRequired(cmd.getCmdType())) { return; diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/TokenVerifier.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/TokenVerifier.java index b2a5ae54c339..dbf79d5482c3 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/TokenVerifier.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/token/TokenVerifier.java @@ -21,7 +21,7 @@ import com.google.common.base.Strings; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.annotation.InterfaceStability; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProtoOrBuilder; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; @@ -47,12 +47,13 @@ public interface TokenVerifier { * @param cmd container command * @throws SCMSecurityException if token verification fails. */ - void verify(String user, Token token, ContainerCommandRequestProto cmd) + void verify(String user, Token token, + ContainerCommandRequestProtoOrBuilder cmd) throws SCMSecurityException; - /** Same as {@link #verify(String, Token, ContainerCommandRequestProto)}, but - * with encoded token. */ - default void verify(ContainerCommandRequestProto cmd, String user, + /** Same as {@link #verify(String, Token, + * ContainerCommandRequestProtoOrBuilder)}, but with encoded token. */ + default void verify(ContainerCommandRequestProtoOrBuilder cmd, String user, String encodedToken) throws SCMSecurityException { if (Strings.isNullOrEmpty(encodedToken)) { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java index 4db40500fbbf..92df53f6556f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java @@ -72,22 +72,28 @@ import org.apache.hadoop.test.GenericTestUtils; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED; +import static org.apache.hadoop.hdds.HddsUtils.isReadOnly; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.SUCCESS; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getCreateContainerRequest; -import static org.apache.hadoop.ozone.container.ContainerTestHelper.getPutBlockRequest; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getTestBlockID; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getTestContainerID; -import static org.apache.hadoop.ozone.container.ContainerTestHelper.getWriteChunkRequest; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newDeleteBlockRequestBuilder; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newDeleteChunkRequestBuilder; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newGetBlockRequestBuilder; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newGetCommittedBlockLengthBuilder; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newPutBlockRequestBuilder; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newReadChunkRequestBuilder; +import static org.apache.hadoop.ozone.container.ContainerTestHelper.newWriteChunkRequestBuilder; import com.google.common.collect.Maps; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.RandomUtils; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.apache.hadoop.test.LambdaTestUtils; import org.apache.ratis.rpc.RpcType; + import static org.apache.ratis.rpc.SupportedRpcType.GRPC; import org.apache.ratis.util.function.CheckedBiConsumer; import org.junit.After; @@ -96,6 +102,8 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; + +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; import org.mockito.Mockito; @@ -253,45 +261,72 @@ private static void runTestClientServer( ContainerProtocolCalls.createContainer(client, containerID, getToken(ContainerID.valueOf(containerID))); - // Test 1: Test putBlock failure without block token. - assertFailsTokenVerification(client, getPutBlockRequest(pipeline, null, - getWriteChunkRequest(pipeline, blockID, 1024, null).getWriteChunk())); - - // Test 2: Test putBlock succeeded with valid block token. Token token = blockTokenSecretManager.generateToken(blockID, - EnumSet.allOf(AccessModeProto.class), RandomUtils.nextLong()); + EnumSet.allOf(AccessModeProto.class), RandomUtils.nextLong()); + String encodedToken = token.encodeToUrlString(); + + ContainerCommandRequestProto.Builder writeChunk = + newWriteChunkRequestBuilder(pipeline, blockID, 1024, 0); + assertRequiresToken(client, encodedToken, writeChunk); + + ContainerCommandRequestProto.Builder putBlock = + newPutBlockRequestBuilder(pipeline, writeChunk.getWriteChunk()); + assertRequiresToken(client, encodedToken, putBlock); + + ContainerCommandRequestProto.Builder readChunk = + newReadChunkRequestBuilder(pipeline, writeChunk.getWriteChunk()); + assertRequiresToken(client, encodedToken, readChunk); + + ContainerCommandRequestProto.Builder getBlock = + newGetBlockRequestBuilder(pipeline, putBlock.getPutBlock()); + assertRequiresToken(client, encodedToken, getBlock); - ContainerCommandRequestProto writeChunkRequest = getWriteChunkRequest( - pipeline, blockID, 1024, token.encodeToUrlString()); - assertSucceeds(client, writeChunkRequest); + ContainerCommandRequestProto.Builder getCommittedBlockLength = + newGetCommittedBlockLengthBuilder(pipeline, putBlock.getPutBlock()); + assertRequiresToken(client, encodedToken, getCommittedBlockLength); - assertSucceeds(client, getPutBlockRequest(pipeline, - token.encodeToUrlString(), writeChunkRequest.getWriteChunk())); + ContainerCommandRequestProto.Builder deleteChunk = + newDeleteChunkRequestBuilder(pipeline, writeChunk.getWriteChunk()); + assertRequiresToken(client, encodedToken, deleteChunk); + + ContainerCommandRequestProto.Builder deleteBlock = + newDeleteBlockRequestBuilder(pipeline, putBlock.getPutBlock()); + assertRequiresToken(client, encodedToken, deleteBlock); } finally { stopServer.accept(pipeline); servers.forEach(XceiverServerSpi::stop); } } - private static ContainerCommandRequestProto assertSucceeds( + private static void assertRequiresToken(XceiverClientSpi client, + String encodedToken, ContainerCommandRequestProto.Builder requestBuilder) + throws Exception { + + requestBuilder.setEncodedToken(""); + assertFailsTokenVerification(client, requestBuilder.build()); + + requestBuilder.setEncodedToken(encodedToken); + assertSucceeds(client, requestBuilder.build()); + } + + private static void assertSucceeds( XceiverClientSpi client, ContainerCommandRequestProto req) throws IOException { ContainerCommandResponseProto response = client.sendCommand(req); assertEquals(SUCCESS, response.getResult()); - return req; } private static void assertFailsTokenVerification(XceiverClientSpi client, ContainerCommandRequestProto request) throws Exception { - if (client instanceof XceiverClientGrpc) { + if (client instanceof XceiverClientGrpc || isReadOnly(request)) { ContainerCommandResponseProto response = client.sendCommand(request); assertNotEquals(response.getResult(), ContainerProtos.Result.SUCCESS); String msg = response.getMessage(); assertTrue(msg, msg.contains("token verification failed")); } else { assertRootCauseMessage("token verification failed", - LambdaTestUtils.intercept(IOException.class, () -> + Assert.assertThrows(IOException.class, () -> client.sendCommand(request))); } }