[ 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: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org