KNOX-3306: Make server startup fail if a forbidden security algorithm…

gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java

@@ -49,6 +49,7 @@
 import org.apache.knox.gateway.GatewayMessages;
 import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.dto.HomePageProfile;
+import org.apache.knox.gateway.fips.FipsUtils;
 import org.apache.knox.gateway.i18n.messages.MessagesFactory;
 import org.apache.knox.gateway.services.security.impl.ZookeeperRemoteAliasService;
 import org.eclipse.jetty.server.handler.ContextHandler;
@@ -236,8 +237,8 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
 
   public static final String COOKIE_SCOPING_ENABLED = GATEWAY_CONFIG_FILE_PREFIX + ".scope.cookies.feature.enabled";
   public static final boolean DEFAULT_COOKIE_SCOPING_FEATURE_ENABLED = false;
-  private static final String CRYPTO_ALGORITHM = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.algorithm";
-  private static final String CRYPTO_PBE_ALGORITHM = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.pbe.algorithm";
+  public static final String CRYPTO_ALGORITHM = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.algorithm";
+  public static final String CRYPTO_PBE_ALGORITHM = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.pbe.algorithm";
   private static final String CRYPTO_TRANSFORMATION = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.transformation";
   private static final String CRYPTO_SALTSIZE = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.salt.size";
   private static final String CRYPTO_ITERATION_COUNT = GATEWAY_CONFIG_FILE_PREFIX + ".crypto.iteration.count";
@@ -813,7 +814,9 @@ public String getHttpClientTruststorePasswordAlias() {
 
   @Override
   public String getCredentialStoreAlgorithm() {
-    return get(CREDENTIAL_STORE_ALG, DEFAULT_CREDENTIAL_STORE_ALG);
+    final String alg = get(CREDENTIAL_STORE_ALG, DEFAULT_CREDENTIAL_STORE_ALG);
+    FipsUtils.validateAlgorithm(alg, CREDENTIAL_STORE_ALG);
+    return alg;
   }
 
   @Override
@@ -1116,12 +1119,16 @@ public String getHeaderNameForRemoteAddress() {
 
   @Override
   public String getAlgorithm() {
-  return getVar(CRYPTO_ALGORITHM, null);
+    final String alg = getVar(CRYPTO_ALGORITHM, null);
+    FipsUtils.validateAlgorithm(alg, CRYPTO_ALGORITHM);
+    return alg;
   }
 
   @Override
   public String getPBEAlgorithm() {
-  return getVar(CRYPTO_PBE_ALGORITHM, null);
+    final String alg = getVar(CRYPTO_PBE_ALGORITHM, null);
+    FipsUtils.validateAlgorithm(alg, CRYPTO_PBE_ALGORITHM);
+    return alg;
   }
 
   @Override

gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/DefaultCryptoService.java

@@ -29,6 +29,7 @@
 
 import org.apache.knox.gateway.GatewayMessages;
 import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.fips.FipsUtils;
 import org.apache.knox.gateway.i18n.messages.MessagesFactory;
 import org.apache.knox.gateway.services.security.AliasService;
 import org.apache.knox.gateway.services.security.AliasServiceException;
@@ -57,11 +58,14 @@ 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()) {
+      FipsUtils.validateAlgorithms(config);
+    }
   }
 
   @Override

gateway-server/src/test/java/org/apache/knox/gateway/services/security/CryptoServiceTest.java

@@ -18,6 +18,8 @@
 package org.apache.knox.gateway.services.security;
 
 import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.config.impl.GatewayConfigImpl;
+import org.apache.knox.gateway.fips.FipsUtils;
 import org.apache.knox.gateway.services.ServiceLifecycleException;
 import org.apache.knox.gateway.services.security.impl.ConfigurableEncryptor;
 import org.apache.knox.gateway.services.security.impl.DefaultCryptoService;
@@ -32,10 +34,12 @@
 import java.security.cert.Certificate;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 
 @Category( { ManualTests.class, MediumTests.class } )
 public class CryptoServiceTest {
@@ -227,7 +231,6 @@ public void testConfigurableEncryptor() throws Exception {
   }
 
   @Test
-  //@Ignore
   public void testEncryptionOfQueryStrings() throws Exception {
     String alias = "encrypt-url";
     String queryString = "url=http://localhost:50070/api/v1/blahblah";
@@ -237,4 +240,38 @@ public void testEncryptionOfQueryStrings() throws Exception {
     byte[] decryptedQueryString = cs.decryptForCluster("Test", alias, result.cipher, result.iv, result.salt);
     assertEquals(queryString.getBytes(StandardCharsets.UTF_8).length, decryptedQueryString.length);
   }
+
+  @Test
+  public void shouldFailIfForbiddenAlgorithmIsSetInFIPSEnvironment() {
+    try {
+      System.setProperty(FipsUtils.FIPS_SYSTEM_PROPERTY, "true");
+      final GatewayConfigImpl config = new GatewayConfigImpl();
+      final String[] forbiddenAlgorithms = {"MD5", "RC4", "ARC4", "ARCFOUR", "SHA1", "SHA-1"};
+      final String[] params = {GatewayConfigImpl.CRYPTO_ALGORITHM, GatewayConfigImpl.CRYPTO_PBE_ALGORITHM, GatewayConfig.CREDENTIAL_STORE_ALG};
+
+      for (String param : params) {
+        for (String algorithm : forbiddenAlgorithms) {
+          testForbiddenAlgorithm(config, param, algorithm);
+        }
+      }
+    } finally {
+      System.clearProperty(FipsUtils.FIPS_SYSTEM_PROPERTY);
+    }
+  }
+
+  private void testForbiddenAlgorithm(GatewayConfigImpl config, String paramName, String algorithm) {
+    config.set(paramName, algorithm);
+    try {
+      final DefaultCryptoService cryptoService = new DefaultCryptoService();
+      cryptoService.setAliasService(as);
+      IllegalArgumentException e = assertThrows(
+              "Should have thrown IllegalArgumentException for " + algorithm + " in " + paramName,
+              IllegalArgumentException.class,
+              () -> cryptoService.init(config, null)
+      );
+      assertEquals(String.format(Locale.ROOT, FipsUtils.PROHIBITED_ALGORITHM_TEMPLATE, algorithm, paramName), e.getMessage());
+    } finally {
+      config.clear();
+    }
+  }
 }

gateway-spi/src/main/java/org/apache/knox/gateway/fips/FipsUtils.java

@@ -17,11 +17,43 @@
  */
 package org.apache.knox.gateway.fips;
 
+import org.apache.knox.gateway.config.GatewayConfig;
+
+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 final String PROHIBITED_ALGORITHM_TEMPLATE = "In a FIPS environment, you are not allowed to use %s as %s";
 
     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) {
+        return !isFipsEnabledWithBCProvider() || algorithm == null
+                || algorithm.isEmpty() || !FORBIDDEN_ALGORITHMS.contains(algorithm.toUpperCase(Locale.ROOT));
+    }
+
+    public static void validateAlgorithm(String algorithm, String paramName) {
+        if (!isAlgorithmAllowed(algorithm)) {
+            throw new IllegalArgumentException(String.format(Locale.ROOT, PROHIBITED_ALGORITHM_TEMPLATE, algorithm, paramName));
+        }
+    }
+
+    public static void validateAlgorithms(GatewayConfig config) {
+        config.getCredentialStoreAlgorithm();
+        config.getAlgorithm();
+        config.getPBEAlgorithm();
+    }
 }

gateway-spi/src/test/java/org/apache/knox/gateway/fips/FipsUtilsTest.java

@@ -20,25 +20,55 @@
 import org.junit.Test;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 public class FipsUtilsTest {
 
     @Test
     public void testIsFipsEnabledWithBCProviderEmpty() {
-        System.clearProperty("com.safelogic.cryptocomply.fips.approved_only");
+        System.clearProperty(FipsUtils.FIPS_SYSTEM_PROPERTY);
         assertFalse(FipsUtils.isFipsEnabledWithBCProvider());
     }
 
     @Test
     public void testIsFipsEnabledWithBCProviderSetToTrue() {
-        System.setProperty("com.safelogic.cryptocomply.fips.approved_only", "true");
-        assertTrue(FipsUtils.isFipsEnabledWithBCProvider());
+        try {
+            System.setProperty(FipsUtils.FIPS_SYSTEM_PROPERTY, "true");
+            assertTrue(FipsUtils.isFipsEnabledWithBCProvider());
+        } finally {
+            System.clearProperty(FipsUtils.FIPS_SYSTEM_PROPERTY);
+        }
     }
 
     @Test
     public void testIsFipsEnabledWithBCProviderSetToFalse() {
-        System.setProperty("com.safelogic.cryptocomply.fips.approved_only", "false");
-        assertFalse(FipsUtils.isFipsEnabledWithBCProvider());
+        try {
+            System.setProperty(FipsUtils.FIPS_SYSTEM_PROPERTY, "false");
+            assertFalse(FipsUtils.isFipsEnabledWithBCProvider());
+        } finally {
+            System.clearProperty(FipsUtils.FIPS_SYSTEM_PROPERTY);
+        }
+    }
+
+    @Test
+    public void testValidateAlgorithm() {
+        try {
+            System.setProperty(FipsUtils.FIPS_SYSTEM_PROPERTY, "true");
+            final String[] forbiddenAlgorithms = {"MD5", "RC4", "ARC4", "ARCFOUR", "SHA1", "SHA-1"};
+            for (String algorithm : forbiddenAlgorithms) {
+                testForbiddenAlgorithm(algorithm);
+            }
+        } finally {
+            System.clearProperty(FipsUtils.FIPS_SYSTEM_PROPERTY);
+        }
+    }
+
+    private void testForbiddenAlgorithm(String algorithm) {
+        assertThrows(
+                "Should have thrown IllegalArgumentException for " + algorithm,
+                IllegalArgumentException.class,
+                () -> FipsUtils.validateAlgorithm(algorithm, null)
+        );
     }
 }