KNOX-2133 - Ensure that Knox always validates TLS (#203)
Signed-off-by: Kevin Risden <krisden@apache.org>
diff --git a/build-tools/src/main/resources/build-tools/forbiddenapis/signatures.txt b/build-tools/src/main/resources/build-tools/forbiddenapis/signatures.txt
new file mode 100644
index 0000000..4c0a57e
--- /dev/null
+++ b/build-tools/src/main/resources/build-tools/forbiddenapis/signatures.txt
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to you under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Signatures of APIs to avoid.
+# Cribbed from Elasticsearch and Apache Calcite
+
+java.lang.Character#codePointBefore(char[],int) @ Implicit start offset is error-prone when the char[] is a buffer and the first chars are random chars
+java.lang.Character#codePointAt(char[],int) @ Implicit end offset is error-prone when the char[] is a buffer and the last chars are random chars
+
+#@defaultMessage Only use wait / notify when really needed. Try to use concurrency primitives, latches or callbacks instead.
+#java.lang.Object#wait()
+#java.lang.Object#wait(long)
+#java.lang.Object#wait(long,int)
+#java.lang.Object#notify()
+#java.lang.Object#notifyAll()
+
+#@defaultMessage Use StringBuilder; it is more efficient
+#java.lang.StringBuffer
+
+@defaultMessage Please do not try to stop the world
+java.lang.System#gc()
+
+#@defaultMessage Please do not try to kill the world
+#java.lang.System#exit(int)
+#java.lang.Runtime#exit(int)
+
+@defaultMessage Don't interrupt threads use FutureUtils#cancel(Future<T>) instead
+java.util.concurrent.Future#cancel(boolean)
+
+@defaultMessage Spawning processes is a potential security issue
+java.lang.ProcessBuilder
+java.lang.Runtime#exec(java.lang.String)
+java.lang.Runtime#exec(java.lang.String[])
+java.lang.Runtime#exec(java.lang.String, java.lang.String[])
+java.lang.Runtime#exec(java.lang.String, java.lang.String[], java.io.File)
+java.lang.Runtime#exec(java.lang.String[], java.lang.String[])
+java.lang.Runtime#exec(java.lang.String[], java.lang.String[], java.io.File)
+
+#@defaultMessage For an enum, use == rather than equals
+#java.lang.Enum#equals(java.lang.Object)
+
+# Preconditions.checkArgument,
+# Preconditions.checkPositionIndex, and
+# Preconditions.checkState are still OK
+#@defaultMessage Use Objects.requireNonNull
+#com.google.common.base.Preconditions#checkNotNull(java.lang.Object)
+#com.google.common.base.Preconditions#checkNotNull(java.lang.Object, java.lang.Object)
+
+@defaultMessage Use java.util.Objects.equals
+com.google.common.base.Objects#equal(java.lang.Object, java.lang.Object)
+
+#@defaultMessage Use java.util.Objects
+#com.google.common.base.Objects
+
+@defaultMessage Use java.lang.String.join
+com.google.common.base.Joiner
+
+# Remove Guava calls to construct empty collections;
+# Sets.identityHashSet(),
+# Sets.newHashSet(Iterable) are still OK
+
+#@defaultMessage Use "new ArrayList<>()"
+#com.google.common.collect.Lists#newArrayList()
+
+@defaultMessage Use "new HashMap<>()"
+com.google.common.collect.Maps#newHashMap()
+
+@defaultMessage Use "new IdentityHashMap<>()"
+com.google.common.collect.Maps#newIdentityHashMap()
+
+@defaultMessage Use "new TreeMap<>()"
+com.google.common.collect.Maps#newTreeMap()
+
+@defaultMessage Use "new HashSet<>()"
+com.google.common.collect.Sets#newHashSet()
+
+@defaultMessage Fix the truststore instead of working around
+org.apache.http.conn.ssl.TrustSelfSignedStrategy
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java b/gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java
index c4de38d..7b69a15 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java
@@ -26,10 +26,8 @@
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
-import org.apache.http.conn.ssl.TrustSelfSignedStrategy;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
-import org.apache.http.ssl.SSLContexts;
import org.apache.knox.gateway.GatewayCommandLine;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.config.impl.GatewayConfigImpl;
@@ -63,7 +61,6 @@
import org.jboss.shrinkwrap.api.exporter.ExplodedExporter;
import org.jboss.shrinkwrap.api.spec.EnterpriseArchive;
-import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import java.io.BufferedReader;
import java.io.Console;
@@ -1674,7 +1671,6 @@
@Override
public void execute() {
attempts++;
- SSLContext ctx = null;
CloseableHttpClient client;
String http = "http://";
String https = "https://";
@@ -1682,7 +1678,6 @@
String gatewayPort;
String host;
-
if(cluster == null) {
printKnoxShellUsage();
out.println("A --cluster argument is required.");
@@ -1693,7 +1688,7 @@
host = hostname;
} else {
try {
- host = InetAddress.getLocalHost().getHostAddress();
+ host = InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
out.println(e.toString());
out.println("Defaulting address to localhost. Use --hostname option to specify a different hostname");
@@ -1724,19 +1719,8 @@
out.println("Username and/or password not supplied. Expect HTTP 401 Unauthorized responses.");
}
-// Attempt to build SSL context for HTTP client.
- try {
- ctx = SSLContexts.custom().loadTrustMaterial(null, new TrustSelfSignedStrategy()).build();
- } catch (Exception e) {
- out.println(e.toString());
- }
-
// Initialize the HTTP client
- if(ctx == null) {
- client = HttpClients.createDefault();
- } else {
- client = HttpClients.custom().setSSLContext(ctx).build();
- }
+ client = HttpClients.createDefault();
HttpGet request;
if(ssl) {
diff --git a/gateway-service-test/src/main/java/org/apache/knox/gateway/service/test/ServiceTestResource.java b/gateway-service-test/src/main/java/org/apache/knox/gateway/service/test/ServiceTestResource.java
index 7a0a614..a1812a0 100644
--- a/gateway-service-test/src/main/java/org/apache/knox/gateway/service/test/ServiceTestResource.java
+++ b/gateway-service-test/src/main/java/org/apache/knox/gateway/service/test/ServiceTestResource.java
@@ -20,10 +20,8 @@
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.utils.URIBuilder;
-import org.apache.http.conn.ssl.TrustSelfSignedStrategy;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
-import org.apache.http.ssl.SSLContexts;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.services.ServiceType;
import org.apache.knox.gateway.services.GatewayServices;
@@ -31,7 +29,6 @@
import org.apache.knox.gateway.topology.Service;
import org.apache.knox.gateway.topology.Topology;
-import javax.net.ssl.SSLContext;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
@@ -70,7 +67,6 @@
List<String> messages = new ArrayList<>();
String authString;
GatewayConfig config = (GatewayConfig) request.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
- SSLContext ctx = null;
CloseableHttpClient client = null;
String id = getTopologyName();
@@ -87,20 +83,9 @@
authString = null;
}
-// Attempt to build SSL context for HTTP client.
- try {
- ctx = SSLContexts.custom().loadTrustMaterial(null, new TrustSelfSignedStrategy()).build();
- } catch (Exception e) {
- messages.add(e.getMessage());
- }
-
// Initialize the HTTP client
try {
- if (ctx == null) {
- client = HttpClients.createDefault();
- } else {
- client = HttpClients.custom().setSSLContext(ctx).build();
- }
+ client = HttpClients.createDefault();
if (topology != null) {
for (Service s : topology.getServices()) {
diff --git a/gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java b/gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
index 470aa44..5fa26c0 100644
--- a/gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
+++ b/gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
@@ -244,6 +244,7 @@
.connection().secure(false).end());
}
+ @SuppressForbidden
protected CloseableHttpClient createClient(ClientContext clientContext) throws GeneralSecurityException {
// SSL
diff --git a/gateway-spi/pom.xml b/gateway-spi/pom.xml
index 461b8d8..a5e2000 100644
--- a/gateway-spi/pom.xml
+++ b/gateway-spi/pom.xml
@@ -150,6 +150,16 @@
</dependency>
<dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-util</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>de.thetaphi</groupId>
+ <artifactId>forbiddenapis</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.apache.knox</groupId>
<artifactId>gateway-test-utils</artifactId>
<scope>test</scope>
@@ -160,9 +170,5 @@
<artifactId>velocity</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-util</artifactId>
- </dependency>
</dependencies>
</project>
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
index 2db6c12..8e5b34d 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java
@@ -51,7 +51,6 @@
import org.apache.http.config.Registry;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
-import org.apache.http.conn.ssl.TrustSelfSignedStrategy;
import org.apache.http.cookie.Cookie;
import org.apache.http.impl.DefaultConnectionReuseStrategy;
import org.apache.http.impl.client.BasicCredentialsProvider;
@@ -164,7 +163,7 @@
identityKeystore = null;
identityKeyPassphrase = null;
- // The the behavior before KNOX-1812 was to use the HttpClients default SslContext. However,
+ // The behavior before KNOX-1812 was to use the HttpClients default SslContext. However,
// if a truststore was explicitly configured in gateway-site (gateway.truststore.password.alias,
// gateway.truststore.path, gateway.truststore.type) create a custom SslContext and use it.
trustKeystore = ks.getTruststoreForHttpClient();
@@ -180,7 +179,7 @@
}
if (trustKeystore != null) {
- sslContextBuilder.loadTrustMaterial(trustKeystore, new TrustSelfSignedStrategy());
+ sslContextBuilder.loadTrustMaterial(trustKeystore, null);
}
return sslContextBuilder.build();
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/impl/CMFMasterService.java b/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/impl/CMFMasterService.java
index 47f2f18..0aba570 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/impl/CMFMasterService.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/impl/CMFMasterService.java
@@ -17,6 +17,7 @@
*/
package org.apache.knox.gateway.services.security.impl;
+import de.thetaphi.forbiddenapis.SuppressForbidden;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.io.FileUtils;
import org.apache.commons.net.ntp.TimeStamp;
@@ -176,6 +177,7 @@
}
}
+ @SuppressForbidden
private void chmod(String args, File file) throws IOException {
// TODO: move to Java 7 NIO support to add windows as well
// TODO: look into the following for Windows: Runtime.getRuntime().exec("attrib -r myFile");
diff --git a/pom.xml b/pom.xml
index 26ea966..52912a8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -576,6 +576,15 @@
<!-- disallow unsafe commons-io classes -->
<bundledSignature>commons-io-unsafe-${commons-io.version}</bundledSignature>
</bundledSignatures>
+ <signaturesArtifacts>
+ <signaturesArtifact>
+ <groupId>org.apache.knox</groupId>
+ <artifactId>build-tools</artifactId>
+ <version>1.0.0</version>
+ <type>jar</type>
+ <path>build-tools/forbiddenapis/signatures.txt</path>
+ </signaturesArtifact>
+ </signaturesArtifacts>
</configuration>
<executions>
<execution>
