Skip to content

HDDS-8361. Useless object in TestOzoneBlockTokenIdentifier #4517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChenSammi merged 7 commits into from
Apr 4, 2023
Merged

HDDS-8361. Useless object in TestOzoneBlockTokenIdentifier #4517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d79198
HDDS-8361. Useless object in TestOzoneBlockTokenIdentifier
adoroszlai Apr 3, 2023
332e05c
Delete unused method
adoroszlai Apr 3, 2023
00fbb44
Raw use of parameterized class
adoroszlai Apr 3, 2023
895da62
Inline variable
adoroszlai Apr 3, 2023
012d3e0
Reduce method visibility
adoroszlai Apr 3, 2023
5da9471
Move TestOzoneBlockTokenIdentifier to hdds-common
adoroszlai Apr 3, 2023
62cf4e1
Do not ignore return value
adoroszlai Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 18 additions & 24 deletions .../token/TestOzoneBlockTokenIdentifier.java → .../token/TestOzoneBlockTokenIdentifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.apache.hadoop.util.Time;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
Expand All @@ -60,7 +61,7 @@
/**
* Test class for {@link OzoneBlockTokenIdentifier}.
*/
public class TestOzoneBlockTokenIdentifier {
class TestOzoneBlockTokenIdentifier {

private static final Logger LOG = LoggerFactory
.getLogger(TestOzoneBlockTokenIdentifier.class);
Expand All @@ -73,10 +74,10 @@ public class TestOzoneBlockTokenIdentifier {
private static X509Certificate cert;

@BeforeAll
public static void setUp() throws Exception {
static void setUp() throws Exception {
File base = new File(BASEDIR);
FileUtil.fullyDelete(base);
base.mkdirs();
Assumptions.assumeTrue(base.mkdirs());
expiryTime = Time.monotonicNow() + 60 * 60 * 24;

// Create Ozone Master key pair.
Expand All @@ -87,12 +88,12 @@ public static void setUp() throws Exception {
}

@AfterEach
public void cleanUp() throws Exception {
void cleanUp() {
// KeyStoreTestUtil.cleanupSSLConfig(KEYSTORES_DIR, sslConfsDir);
}

@Test
public void testSignToken() throws GeneralSecurityException, IOException {
void testSignToken() throws GeneralSecurityException, IOException {
String keystore = new File(KEYSTORES_DIR, "keystore.jks")
.getAbsolutePath();
String truststore = new File(KEYSTORES_DIR, "truststore.jks")
Expand Down Expand Up @@ -134,7 +135,7 @@ public void testSignToken() throws GeneralSecurityException, IOException {
}

@Test
public void testTokenSerialization() throws GeneralSecurityException,
void testTokenSerialization() throws GeneralSecurityException,
IOException {
String keystore = new File(KEYSTORES_DIR, "keystore.jks")
.getAbsolutePath();
Expand Down Expand Up @@ -162,12 +163,12 @@ public void testTokenSerialization() throws GeneralSecurityException,
byte[] signedToken = signTokenAsymmetric(tokenId, privateKey);


Token<OzoneBlockTokenIdentifier> token = new Token(tokenId.getBytes(),
Token<OzoneBlockTokenIdentifier> token = new Token<>(tokenId.getBytes(),
signedToken, tokenId.getKind(), new Text("host:port"));

String encodeToUrlString = token.encodeToUrlString();

Token<OzoneBlockTokenIdentifier>decodedToken = new Token();
Token<OzoneBlockTokenIdentifier>decodedToken = new Token<>();
decodedToken.decodeFromUrlString(encodeToUrlString);

OzoneBlockTokenIdentifier decodedTokenId = new OzoneBlockTokenIdentifier();
Expand All @@ -184,24 +185,22 @@ public void testTokenSerialization() throws GeneralSecurityException,
}


public byte[] signTokenAsymmetric(OzoneBlockTokenIdentifier tokenId,
private byte[] signTokenAsymmetric(OzoneBlockTokenIdentifier tokenId,
PrivateKey privateKey) throws NoSuchAlgorithmException,
InvalidKeyException, SignatureException {
Signature rsaSignature = Signature.getInstance("SHA256withRSA");
rsaSignature.initSign(privateKey);
rsaSignature.update(tokenId.getBytes());
byte[] signature = rsaSignature.sign();
return signature;
return rsaSignature.sign();
}

public boolean verifyTokenAsymmetric(OzoneBlockTokenIdentifier tokenId,
private boolean verifyTokenAsymmetric(OzoneBlockTokenIdentifier tokenId,
byte[] signature, Certificate certificate) throws InvalidKeyException,
NoSuchAlgorithmException, SignatureException {
Signature rsaSignature = Signature.getInstance("SHA256withRSA");
rsaSignature.initVerify(certificate);
rsaSignature.update(tokenId.getBytes());
boolean isValid = rsaSignature.verify(signature);
return isValid;
return rsaSignature.verify(signature);
}

private byte[] signTokenSymmetric(OzoneBlockTokenIdentifier identifier,
Expand All @@ -215,15 +214,15 @@ private byte[] signTokenSymmetric(OzoneBlockTokenIdentifier identifier,
return mac.doFinal(identifier.getBytes());
}

OzoneBlockTokenIdentifier generateTestToken() {
private OzoneBlockTokenIdentifier generateTestToken() {
return new OzoneBlockTokenIdentifier(RandomStringUtils.randomAlphabetic(6),
RandomStringUtils.randomAlphabetic(5),
EnumSet.allOf(HddsProtos.BlockTokenSecretProto.AccessModeProto.class),
expiryTime, cert.getSerialNumber().toString(), 1024768L);
}

@Test
public void testAsymmetricTokenPerf() throws NoSuchAlgorithmException,
void testAsymmetricTokenPerf() throws NoSuchAlgorithmException,
CertificateEncodingException, NoSuchProviderException,
InvalidKeyException, SignatureException {
final int testTokenCount = 1000;
Expand Down Expand Up @@ -260,15 +259,15 @@ public void testAsymmetricTokenPerf() throws NoSuchAlgorithmException,
}

@Test
public void testSymmetricTokenPerf() {
void testSymmetricTokenPerf() {
String hmacSHA1 = "HmacSHA1";
String hmacSHA256 = "HmacSHA256";

testSymmetricTokenPerfHelper(hmacSHA1, 64);
testSymmetricTokenPerfHelper(hmacSHA256, 1024);
}

public void testSymmetricTokenPerfHelper(String hmacAlgorithm, int keyLen) {
private void testSymmetricTokenPerfHelper(String hmacAlgorithm, int keyLen) {
Copy link
Contributor

@ChenSammi ChenSammi Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adoroszlai , thanks for fix the issue.
Does the scope change from public to private solve the problem?

Copy link
Contributor Author

@adoroszlai adoroszlai Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the problem is fixed by actually using the "useless object" here:

    LOG.info("Average token sign time with {}({} symmetric key) is {} ns",
        hmacAlgorithm, keyLen, duration / tokenPasswordSym.size());

final int testTokenCount = 1000;
List<OzoneBlockTokenIdentifier> tokenIds = new ArrayList<>();
List<byte[]> tokenPasswordSym = new ArrayList<>();
Expand Down Expand Up @@ -302,11 +301,6 @@ public void testSymmetricTokenPerfHelper(String hmacAlgorithm, int keyLen) {
}
long duration = Time.monotonicNowNanos() - startTime;
LOG.info("Average token sign time with {}({} symmetric key) is {} ns",
hmacAlgorithm, keyLen, duration / testTokenCount);
}

// TODO: verify certificate with a trust store
public boolean verifyCert(Certificate certificate) {
return true;
hmacAlgorithm, keyLen, duration / tokenPasswordSym.size());
}
}
4 changes: 0 additions & 4 deletions hadoop-hdds/framework/dev-support/findbugsExcludeFile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,4 @@
<Class name="org.apache.hadoop.hdds.utils.BufferedMetricsCollector$BufferedMetricsRecordBuilderImpl"></Class>
<Bug pattern="URF_UNREAD_FIELD" />
</Match>
<Match>
<Class name="org.apache.hadoop.hdds.security.token.TestOzoneBlockTokenIdentifier"></Class>
<Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
</Match>
</FindBugsFilter>
4 changes: 4 additions & 0 deletions hadoop-hdds/hadoop-dependency-test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</dependency>
Copy link
Contributor

@ChenSammi ChenSammi Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change related?

Copy link
Contributor Author

@adoroszlai adoroszlai Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OzoneBlockTokenIdentifier is in hadoop-hdds/common, so I'm moving the test to the same submodule. This dependency is required at runtime for Hadoop code invoked by the test.

[ERROR] Tests run: 4, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.506 s <<< FAILURE! - in org.apache.hadoop.hdds.security.token.TestOzoneBlockTokenIdentifier
[ERROR] org.apache.hadoop.hdds.security.token.TestOzoneBlockTokenIdentifier.testTokenSerialization  Time elapsed: 0.04 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/apache/commons/codec/binary/Base64
	at org.apache.hadoop.security.token.Token.encodeWritable(Token.java:342)
	at org.apache.hadoop.security.token.Token.encodeToUrlString(Token.java:373)
	at org.apache.hadoop.hdds.security.token.TestOzoneBlockTokenIdentifier.testTokenSerialization(TestOzoneBlockTokenIdentifier.java:169)

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
Expand Down