knox-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kris...@apache.org
Subject [knox] branch master updated: KNOX-2133 - Ensure that Knox always validates TLS (#203)
Date Fri, 22 Nov 2019 00:13:05 GMT
This is an automated email from the ASF dual-hosted git repository.

krisden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 16dd645  KNOX-2133 - Ensure that Knox always validates TLS (#203)
16dd645 is described below

commit 16dd645efeb6a51f840858339430c438490593df
Author: Kevin Risden <risdenk@users.noreply.github.com>
AuthorDate: Thu Nov 21 19:12:56 2019 -0500

    KNOX-2133 - Ensure that Knox always validates TLS (#203)
    
    Signed-off-by: Kevin Risden <krisden@apache.org>
---
 .../build-tools/forbiddenapis/signatures.txt       | 90 ++++++++++++++++++++++
 .../java/org/apache/knox/gateway/util/KnoxCLI.java | 20 +----
 .../gateway/service/test/ServiceTestResource.java  | 17 +---
 .../org/apache/knox/gateway/shell/KnoxSession.java |  1 +
 gateway-spi/pom.xml                                | 14 +++-
 .../gateway/dispatch/DefaultHttpClientFactory.java |  5 +-
 .../services/security/impl/CMFMasterService.java   |  2 +
 pom.xml                                            |  9 +++
 8 files changed, 117 insertions(+), 41 deletions(-)

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.hadoop.util.ToolRunner;
 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.eclipse.persistence.oxm.MediaType;
 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 @@ public class KnoxCLI extends Configured implements Tool {
     @Override
     public void execute() {
       attempts++;
-      SSLContext ctx = null;
       CloseableHttpClient client;
       String http = "http://";
       String https = "https://";
@@ -1682,7 +1678,6 @@ public class KnoxCLI extends Configured implements Tool {
       String gatewayPort;
       String host;
 
-
       if(cluster == null) {
         printKnoxShellUsage();
         out.println("A --cluster argument is required.");
@@ -1693,7 +1688,7 @@ public class KnoxCLI extends Configured implements Tool {
         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 @@ public class KnoxCLI extends Configured implements Tool {
         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 @@ package org.apache.knox.gateway.service.test;
 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.services.topology.TopologyService;
 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 @@ public class ServiceTestResource {
     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 @@ public class ServiceTestResource {
       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 @@ public class KnoxSession implements Closeable {
         .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.client.methods.HttpUriRequest;
 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 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
         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 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
         }
 
         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 @@ public class CMFMasterService {
       }
   }
 
+  @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>


Mime
View raw message