szetszwo commented on code in PR #7144:
URL: https://github.com/apache/hadoop/pull/7144#discussion_r1853159801
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java:
##########
@@ -1005,6 +1005,22 @@ public class CommonConfigurationKeysPublic {
public static final String HADOOP_SECURITY_CREDENTIAL_PASSWORD_FILE_KEY =
"hadoop.security.credstore.java-keystore-provider.password-file";
+ /**
+ * @see
+ * <a
href="{@docRoot}/../hadoop-project-dist/hadoop-common/core-default.xml">
+ * core-default.xml</a>
+ */
+ public static final String HMAC_ALGORITHM = "hadoop.security.hmac-algorithm";
+ public static final String DEFAULT_HMAC_ALGORITHM = "HmacSHA1";
Review Comment:
Let's add "secret-manager.key-generator.algorithm" (we may use a non-hmac
algorithm).
- Also, we should use the existing naming convention.
- The javadoc is not useful. Let's skip it.
```java
public static final String
HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_KEY
= "hadoop.security.secret-manager.key-generator.algorithm";
public static final String
HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_DEFAULT
= "HmacSHA1";
```
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/SecretManager.java:
##########
@@ -140,11 +153,10 @@ protected Mac initialValue() {
private final KeyGenerator keyGen;
{
try {
- keyGen = KeyGenerator.getInstance(DEFAULT_HMAC_ALGORITHM);
- keyGen.init(KEY_LENGTH);
+ keyGen = KeyGenerator.getInstance(SELECTED_ALGORITHM);
+ keyGen.init(SELECTED_LENGTH);
} catch (NoSuchAlgorithmException nsa) {
- throw new IllegalArgumentException("Can't find " +
DEFAULT_HMAC_ALGORITHM +
- " algorithm.");
+ throw new IllegalArgumentException("Can't find " + SELECTED_ALGORITHM +
" algorithm.");
Review Comment:
Let's include the cause:
```java
throw new IllegalArgumentException("Failed to get " + HMAC_ALGORITHM,
nsa);
```
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/SecretManager.java:
##########
@@ -107,16 +114,23 @@ public byte[] retriableRetrievePassword(T identifier)
public void checkAvailableForRead() throws StandbyException {
// Default to being available for read.
}
-
- /**
- * The name of the hashing algorithm.
- */
- private static final String DEFAULT_HMAC_ALGORITHM = "HmacSHA1";
- /**
- * The length of the random keys to use.
- */
- private static final int KEY_LENGTH = 64;
+ private static final String SELECTED_ALGORITHM;
+ private static final int SELECTED_LENGTH;
Review Comment:
Let use `HMAC_ALGORITHM` and `KEY_LENGTH`.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java:
##########
@@ -1005,6 +1005,22 @@ public class CommonConfigurationKeysPublic {
public static final String HADOOP_SECURITY_CREDENTIAL_PASSWORD_FILE_KEY =
"hadoop.security.credstore.java-keystore-provider.password-file";
+ /**
+ * @see
+ * <a
href="{@docRoot}/../hadoop-project-dist/hadoop-common/core-default.xml">
+ * core-default.xml</a>
+ */
+ public static final String HMAC_ALGORITHM = "hadoop.security.hmac-algorithm";
+ public static final String DEFAULT_HMAC_ALGORITHM = "HmacSHA1";
+
+ /**
+ * @see
+ * <a
href="{@docRoot}/../hadoop-project-dist/hadoop-common/core-default.xml">
+ * core-default.xml</a>
+ */
+ public static final String HMAC_LENGTH = "hadoop.security.hmac-length";
+ public static final int DEFAULT_HMAC_LENGTH = 64;
Review Comment:
This should be "key-length". It has nothing to do with hmac.
```java
public static final String HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_KEY
= "hadoop.security.secret-manager.key-length";
public static final int HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_DEFAULT
= 64;
```
##########
hadoop-common-project/hadoop-common/src/main/resources/core-default.xml:
##########
@@ -1046,6 +1046,32 @@
</description>
</property>
+<property>
+ <name>hadoop.security.hmac-algorithm</name>
+ <value>HmacSHA1</value>
+ <description>The configuration key specifying the hashing algorithm used for
+ HMAC (Hash-based Message Authentication Code) operations.
+
+ The HMAC algorithm is used in token management to compute secure
+ message digests. This configuration allows users to specify the
+ algorithm to be used for HMAC operations. The algorithm must be a
+ valid cryptographic hash algorithm supported by the Java Cryptography
+ Architecture (JCA). Common examples include "HmacSHA1", "HmacSHA256",
+ and "HmacSHA512".</description>
+</property>
+
+<property>
+ <name>hadoop.security.hmac-length</name>
+ <value>64</value>
+ <description>The configuration key specifying the key length for HMAC
(Hash-based
+ Message Authentication Code) operations.
+
+ This property determines the size of the secret keys generated
+ for HMAC computations. The key length must be appropriate for the
+ selected HMAC algorithm. For example, longer keys are generally
+ more secure but may not be supported by all algorithms.</description>
Review Comment:
Let's change this to:
> The configuration key specifying the key length of the generated secret
keys
> in SecretManager. The key length must be appropriate for the algorithm.
> For example, longer keys are generally more secure but may not be supported
> by all algorithms.</description>
##########
hadoop-common-project/hadoop-common/src/main/resources/core-default.xml:
##########
@@ -1046,6 +1046,32 @@
</description>
</property>
+<property>
+ <name>hadoop.security.hmac-algorithm</name>
+ <value>HmacSHA1</value>
+ <description>The configuration key specifying the hashing algorithm used for
+ HMAC (Hash-based Message Authentication Code) operations.
+
+ The HMAC algorithm is used in token management to compute secure
+ message digests. This configuration allows users to specify the
+ algorithm to be used for HMAC operations. The algorithm must be a
+ valid cryptographic hash algorithm supported by the Java Cryptography
+ Architecture (JCA). Common examples include "HmacSHA1", "HmacSHA256",
+ and "HmacSHA512".</description>
Review Comment:
Let's update the description as below:
> The configuration key specifying the KeyGenerator algorithm used in
SecretManager
> for generating secret keys. The algorithm must be a KeyGenerator
algorithm supported by
> the Java Cryptography Architecture (JCA). Common examples include
"HmacSHA1",
> "HmacSHA256", and "HmacSHA512".
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/SecretManager.java:
##########
@@ -126,10 +140,9 @@ public void checkAvailableForRead() throws
StandbyException {
@Override
protected Mac initialValue() {
try {
- return Mac.getInstance(DEFAULT_HMAC_ALGORITHM);
+ return Mac.getInstance(SELECTED_ALGORITHM);
} catch (NoSuchAlgorithmException nsa) {
- throw new IllegalArgumentException("Can't find " +
DEFAULT_HMAC_ALGORITHM +
- " algorithm.");
+ throw new IllegalArgumentException("Can't find " + SELECTED_ALGORITHM
+ " algorithm.");
Review Comment:
Let's include the cause:
```java
throw new IllegalArgumentException("Failed to get " + HMAC_ALGORITHM,
nsa);
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
