Merge pull request #7136 from atorralba/atorralba/promote-insecure-trustmanager

Java: Promote Insecure TrustManager from experimental

java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java

@@ -0,0 +1,52 @@
+public static void main(String[] args) throws Exception {
+    {
+        class InsecureTrustManager implements X509TrustManager {
+            @Override
+            public X509Certificate[] getAcceptedIssuers() {
+                return null;
+            }
+
+            @Override
+            public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
+                // BAD: Does not verify the certificate chain, allowing any certificate.
+            }
+
+            @Override
+            public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
+
+            }
+        }
+        SSLContext context = SSLContext.getInstance("TLS");
+        TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
+        context.init(null, trustManager, null);
+    }
+    {
+        SSLContext context = SSLContext.getInstance("TLS");
+        File certificateFile = new File("path/to/self-signed-certificate");
+        // Create a `KeyStore` with default type
+        KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
+        // `keyStore` is initially empty
+        keyStore.load(null, null);
+        X509Certificate generatedCertificate;
+        try (InputStream cert = new FileInputStream(certificateFile)) {
+            generatedCertificate = (X509Certificate) CertificateFactory.getInstance("X509")
+                    .generateCertificate(cert);
+        }
+        // Add the self-signed certificate to the key store
+        keyStore.setCertificateEntry(certificateFile.getName(), generatedCertificate);
+        // Get default `TrustManagerFactory`
+        TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+        // Use it with our key store that trusts our self-signed certificate
+        tmf.init(keyStore);
+        TrustManager[] trustManagers = tmf.getTrustManagers();
+        context.init(null, trustManagers, null);
+        // GOOD, we are not using a custom `TrustManager` but instead have
+        // added the self-signed certificate we want to trust to the key
+        // store. Note, the `trustManagers` will **only** trust this one
+        // certificate.
+        
+        URL url = new URL("https://self-signed.badssl.com/");
+        HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
+        conn.setSSLSocketFactory(context.getSocketFactory());
+    }
+}

java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp

@@ -0,0 +1,47 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+If the <code>checkServerTrusted</code> method of a <code>TrustManager</code> never throws a <code>CertificateException</code>, it trusts every certificate.
+This allows an attacker to perform a machine-in-the-middle attack against the application, therefore breaking any security Transport Layer Security (TLS) gives.
+</p>
+
+<p>
+An attack might look like this:
+</p>
+
+<ol>
+  <li>The vulnerable program connects to <code>https://example.com</code>.</li>
+  <li>The attacker intercepts this connection and presents a valid, self-signed certificate for <code>https://example.com</code>.</li>
+  <li>The vulnerable program calls the <code>checkServerTrusted</code> method to check whether it should trust the certificate.</li>
+  <li>The <code>checkServerTrusted</code> method of your <code>TrustManager</code> does not throw a <code>CertificateException</code>.</li>
+  <li>The vulnerable program accepts the certificate and proceeds with the connection since your <code>TrustManager</code> implicitly trusted it by not throwing an exception.</li>
+  <li>The attacker can now read the data your program sends to <code>https://example.com</code> and/or alter its replies while the program thinks the connection is secure.</li>
+</ol>
+</overview>
+
+<recommendation>
+<p>
+Do not use a custom <code>TrustManager</code> that trusts any certificate.
+If you have to use a self-signed certificate, don't trust every certificate, but instead only trust this specific certificate.
+See below for an example of how to do this.
+</p>
+
+</recommendation>
+
+<example>
+<p>
+In the first (bad) example, the <code>TrustManager</code> never throws a <code>CertificateException</code> and therefore implicitly trusts any certificate.
+This allows an attacker to perform a machine-in-the-middle attack.
+In the second (good) example, the self-signed certificate that should be trusted
+is loaded into a <code>KeyStore</code>. This explicitly defines the certificate as trusted and there is no need to create a custom <code>TrustManager</code>.
+</p>
+<sample src="InsecureTrustManager.java" />
+</example>
+
+<references>
+<li>Android Developers: <a href="https://developer.android.com/training/articles/security-ssl">Security with HTTPS and SSL</a>.</li>
+</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql

@@ -0,0 +1,21 @@
+/**
+ * @name `TrustManager` that accepts all certificates
+ * @description Trusting all certificates allows an attacker to perform a machine-in-the-middle attack.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id java/insecure-trustmanager
+ * @tags security
+ *       external/cwe/cwe-295
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.security.InsecureTrustManagerQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink
+where any(InsecureTrustManagerConfiguration cfg).hasFlowPath(source, sink)
+select sink, source, sink, "This $@, which is defined $@ and trusts any certificate, is used here.",
+  source, "TrustManager", source.getNode().asExpr().(ClassInstanceExpr).getConstructedType(), "here"