This is an automated email from the ASF dual-hosted git repository.
swagle pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 355096b HDDS-5236. Require block token for more operations (#2254)
355096b is described below
commit 355096b9c218a90d01307b7b8782c469300b94bf
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Mon May 24 22:49:25 2021 +0200
HDDS-5236. Require block token for more operations (#2254)
* Require block token for all chunk/block operations
* Addendum: implement getBlockID; add tests
* No block token for ListBlock (unsupported operation anyway), as it has no
block ID
* Test for more block/chunk operations
* Require container token for ListBlock, but not for ListContainer
* Do not require...Token for unsupported operations
---
.../java/org/apache/hadoop/hdds/HddsUtils.java | 82 +++++++++++------
.../ozone/container/ContainerTestHelper.java | 101 ++++++++++++++++-----
.../hdds/security/token/BlockTokenVerifier.java | 39 ++++----
.../security/token/CompositeTokenVerifier.java | 4 +-
.../security/token/ContainerTokenVerifier.java | 4 +-
.../hdds/security/token/NoopTokenVerifier.java | 6 +-
.../security/token/ShortLivedTokenVerifier.java | 9 +-
.../hadoop/hdds/security/token/TokenVerifier.java | 11 ++-
.../server/TestSecureContainerServer.java | 71 +++++++++++----
9 files changed, 219 insertions(+), 108 deletions(-)
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 e03e928..6e701d5 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.client.BlockID;
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 final class HddsUtils {
* @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 final class HddsUtils {
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 final class HddsUtils {
case CloseContainer:
case CreateContainer:
case DeleteContainer:
- case ListContainer:
case ReadContainer:
case UpdateContainer:
return true;
@@ -426,44 +428,66 @@ public final class HddsUtils {
* @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 4c344ab..5e4a8dc 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.ChecksumTy
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 final class ContainerTestHelper {
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 final class ContainerTestHelper {
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 final class ContainerTestHelper {
* 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 final class ContainerTestHelper {
newRequest.setContainerID(readRequest.getBlockID().getContainerID());
newRequest.setReadChunk(readRequest);
newRequest.setDatanodeUuid(pipeline.getFirstNode().getUuidString());
- return newRequest.build();
+ return newRequest;
}
/**
@@ -259,6 +274,12 @@ public final class ContainerTestHelper {
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 final class ContainerTestHelper {
request.setContainerID(writeRequest.getBlockID().getContainerID());
request.setDeleteChunk(deleteRequest);
request.setDatanodeUuid(pipeline.getFirstNode().getUuidString());
- return request.build();
+ return request;
}
/**
@@ -404,8 +425,18 @@ public final class ContainerTestHelper {
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 final class ContainerTestHelper {
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 final class ContainerTestHelper {
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 final class ContainerTestHelper {
request.setContainerID(blockID.getContainerID());
request.setGetBlock(getRequest);
request.setDatanodeUuid(pipeline.getFirstNode().getUuidString());
- return request.build();
+ return request;
}
/**
@@ -478,8 +510,13 @@ public final class ContainerTestHelper {
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 final class ContainerTestHelper {
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 a43cd2c..092a059 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.HddsUtils;
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 @@ public class BlockTokenVerifier extends
}
@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 @@ public class BlockTokenVerifier extends
@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 ce10c25..750bb76 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 class CompositeTokenVerifier implements TokenVerifier {
@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 806a9ae..941160a 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 @@ package org.apache.hadoop.hdds.security.token;
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 @@ public class ContainerTokenVerifier extends
}
@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 b86b906..084b42e 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 dc89d0b..92b643e 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 @@ public abstract class
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 b2a5ae5..dbf79d5 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 @@ package org.apache.hadoop.hdds.security.token;
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 4db4050..92df53f 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.security.token.Token;
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.assertEquals;
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 @@ public class TestSecureContainerServer {
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<OzoneBlockTokenIdentifier> 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)));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
