[
https://issues.apache.org/jira/browse/YARN-11738?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17900221#comment-17900221
]
ASF GitHub Bot commented on YARN-11738:
---------------------------------------
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);
```
> Modernize SecretManager config
> ------------------------------
>
> Key: YARN-11738
> URL: https://issues.apache.org/jira/browse/YARN-11738
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: yarn
> Affects Versions: 3.4.1
> Reporter: Bence Kosztolnik
> Assignee: Bence Kosztolnik
> Priority: Major
> Labels: pull-request-available
>
> FIPS-compliant HMAC-SHA1 algorithms require secret keys to be at least 112
> bits long.
> https://github.com/apache/hadoop/blob/98c2bc87b1445c533268c58d382ea4e4297303fd/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/SecretManager.java#L144
> Should be set to 128 to be FIPS compatible.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
