spolavarpau1 commented on code in PR #723:
URL: https://github.com/apache/ranger/pull/723#discussion_r2487391798
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java:
##########
@@ -41,14 +40,14 @@ public class PasswordUtils {
private static final Logger LOG =
LoggerFactory.getLogger(PasswordUtils.class);
public static final String PBE_SHA512_AES_128 =
"PBEWITHHMACSHA512ANDAES_128";
Review Comment:
Do we still need this? Aren't we now going to use
PBEWithHmacSHA512AndAES_128 from RangerSupportedCryptoAlgo
##########
agents-common/src/test/java/org/apache/ranger/plugin/util/PasswordUtilsTest.java:
##########
@@ -72,7 +72,7 @@ public void testEncryptWithSHA1AndDESede() throws IOException
{
@Test
public void testEncryptWithSHA512AndAES128() throws IOException,
NoSuchAlgorithmException {
- String freeTextPasswordMetaData = join("PBEWITHHMACSHA512ANDAES_128",
"ENCRYPT_KEY", "SALTSALT", "4",
PasswordUtils.generateIvIfNeeded("PBEWITHHMACSHA512ANDAES_128"));
+ String freeTextPasswordMetaData = join("PBEWithHmacSHA512AndAES_128",
"ENCRYPT_KEY", "SALTSALT", "4",
PasswordUtils.generateIvIfNeeded("PBEWithHmacSHA512AndAES_128"));
Review Comment:
In case of upgrades, would this case difference in the algorithm name has
any affect?
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java:
##########
@@ -67,12 +66,12 @@ public class PasswordUtils {
if (cryptAlgoArray != null && cryptAlgoArray.length > 4) {
int index = 0;
- cryptAlgo = cryptAlgoArray[index++]; // 0
+ cryptAlgo =
RangerSupportedCryptoAlgo.valueOf(cryptAlgoArray[index++]); // 0
Review Comment:
I think we need to handle the case for crypto algorithm that is not in the
list of enums supported in RangerSupportedCryptoAlgo. Can you please check?
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java:
##########
@@ -130,7 +129,7 @@ public static boolean needsIv(String cryptoAlgo) {
}
return PBE_SHA512_AES_128.equalsIgnoreCase(cryptoAlgo)
Review Comment:
Same as above. Do we need to replace this check with the one from
RangerSupportedCryptoAlgo?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
