[
https://issues.apache.org/jira/browse/KNOX-3306?focusedWorklogId=1016982&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1016982
]
ASF GitHub Bot logged work on KNOX-3306:
----------------------------------------
Author: ASF GitHub Bot
Created on: 23/Apr/26 07:47
Start Date: 23/Apr/26 07:47
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1212:
URL: https://github.com/apache/knox/pull/1212#discussion_r3129080779
##########
gateway-spi/src/main/java/org/apache/knox/gateway/fips/FipsUtils.java:
##########
@@ -17,11 +17,43 @@
*/
package org.apache.knox.gateway.fips;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+
public class FipsUtils {
- private static final String FIPS_SYSTEM_PROPERTY =
"com.safelogic.cryptocomply.fips.approved_only";
+ public static final String FIPS_SYSTEM_PROPERTY =
"com.safelogic.cryptocomply.fips.approved_only";
+
+ private static final List<String> FORBIDDEN_ALGORITHMS =
Arrays.asList("MD5", "RC4", "ARC4", "ARCFOUR", "SHA-1", "SHA1");
public static boolean isFipsEnabledWithBCProvider() {
return Boolean.parseBoolean(System.getProperty(FIPS_SYSTEM_PROPERTY));
}
+
+ /**
+ * Validates if the given algorithm is allowed in a FIPS environment.
+ *
+ * @param algorithm The algorithm to validate.
+ * @return true if the algorithm is allowed or FIPS is not enabled, false
otherwise.
+ */
+ private static boolean isAlgorithmAllowed(String algorithm) {
+ if (!isFipsEnabledWithBCProvider() || algorithm == null ||
algorithm.isEmpty()) {
+ return true;
+ }
+
+ String upperCaseAlg = algorithm.toUpperCase(Locale.ROOT);
+ for (String forbidden : FORBIDDEN_ALGORITHMS) {
+ if (upperCaseAlg.contains(forbidden)) {
+ return false;
+ }
+ }
Review Comment:
Why not a simple ` return !isFipsEnabledWithBCProvider() || algorithm ==
null || algorithm.isEmpty() ||
!FORBIDDEN_ALGORITHMS.contains(algorithm.toUpperCase(Locale.ROOT))`?
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/DefaultCryptoService.java:
##########
@@ -57,11 +58,18 @@ public void setAliasService(AliasService as) {
@Override
public void init(GatewayConfig config, Map<String, String> options)
- throws ServiceLifecycleException {
+ throws ServiceLifecycleException {
this.config = config;
- if (aliasService == null) {
+ if (aliasService == null) {
throw new ServiceLifecycleException("Alias service is not set");
}
+ if (FipsUtils.isFipsEnabledWithBCProvider()) {
+ //invoking the following getters will throw IllegalArgumentException in
case a forbidden algorithm is set
+ //so we can use them as a validation at service initialization time
+ config.getCredentialStoreAlgorithm();
+ config.getAlgorithm();
+ config.getPBEAlgorithm();
Review Comment:
We may want to add a new GatewayConfig method called
`validateFipsAlgorithms` and have the `GatewayConfigImpl` self-contain what
does it mean to be valid in FIPS envs?
##########
gateway-spi/src/main/java/org/apache/knox/gateway/fips/FipsUtils.java:
##########
@@ -17,11 +17,43 @@
*/
package org.apache.knox.gateway.fips;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+
public class FipsUtils {
- private static final String FIPS_SYSTEM_PROPERTY =
"com.safelogic.cryptocomply.fips.approved_only";
+ public static final String FIPS_SYSTEM_PROPERTY =
"com.safelogic.cryptocomply.fips.approved_only";
+
+ private static final List<String> FORBIDDEN_ALGORITHMS =
Arrays.asList("MD5", "RC4", "ARC4", "ARCFOUR", "SHA-1", "SHA1");
public static boolean isFipsEnabledWithBCProvider() {
return Boolean.parseBoolean(System.getProperty(FIPS_SYSTEM_PROPERTY));
}
+
+ /**
+ * Validates if the given algorithm is allowed in a FIPS environment.
+ *
+ * @param algorithm The algorithm to validate.
+ * @return true if the algorithm is allowed or FIPS is not enabled, false
otherwise.
+ */
+ private static boolean isAlgorithmAllowed(String algorithm) {
+ if (!isFipsEnabledWithBCProvider() || algorithm == null ||
algorithm.isEmpty()) {
+ return true;
+ }
+
+ String upperCaseAlg = algorithm.toUpperCase(Locale.ROOT);
+ for (String forbidden : FORBIDDEN_ALGORITHMS) {
+ if (upperCaseAlg.contains(forbidden)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ public static void validateAlgorithm(String algorithm, String paramName) {
+ if (!isAlgorithmAllowed(algorithm)) {
+ throw new IllegalArgumentException("In a FIPS environment, you are
not allowed to use " + algorithm + " as " + paramName);
+ }
+ }
Review Comment:
Extracting the error message out to a constant template and re-use it in the
tests?
Issue Time Tracking
-------------------
Worklog Id: (was: 1016982)
Time Spent: 0.5h (was: 20m)
> Make server startup fail if a forbidden security algorithm is configured for
> Knox in a FIPS environment
> -------------------------------------------------------------------------------------------------------
>
> Key: KNOX-3306
> URL: https://issues.apache.org/jira/browse/KNOX-3306
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Affects Versions: 2.1.0
> Reporter: Tamás Hanicz
> Assignee: Tamás Hanicz
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
