Java: Remove overlapping code

github · aschackmull · Jan 13, 2021 · Dec 4, 2020 · Dec 4, 2020 · Dec 4, 2020
commit 70b0703952ef4c364990a4a183348ab44e901944
@@ -1,30 +1,4 @@
 public static void main(String[] args) {
-	{
-		HostnameVerifier verifier = new HostnameVerifier() {
-			@Override
-			public boolean verify(String hostname, SSLSession session) {
-				try { //GOOD: verify the certificate
-					Certificate[] certs = session.getPeerCertificates();
-					X509Certificate x509 = (X509Certificate) certs[0];
-					check(new String[]{host}, x509);
-					return true;
-				} catch (SSLException e) {
-					return false;
-				}
-			}
-		};
-		HttpsURLConnection.setDefaultHostnameVerifier(verifier);
-	}
-
-	{
-		HostnameVerifier verifier = new HostnameVerifier() {
-			@Override
-			public boolean verify(String hostname, SSLSession session) {
-				return true; // BAD: accept even if the hostname doesn't match
-			}
-		};
-		HttpsURLConnection.setDefaultHostnameVerifier(verifier);
-	}
 
 	{
 		X509TrustManager trustAllCertManager = new X509TrustManager() {

@@ -4,18 +4,18 @@
 <qhelp>
 
 <overview>
-<p>Java offers two mechanisms for SSL authentication - trust manager and hostname verifier. Trust manager validates the peer's certificate chain while hostname verification establishes that the hostname in the URL matches the hostname in the server's identification.</p>
+<p>Java offers two mechanisms for SSL authentication - trust manager and hostname verifier (not checked by this query). Trust manager validates the peer's certificate chain while hostname verification establishes that the hostname in the URL matches the hostname in the server's identification.</p>
 <p>And when SSLSocket or SSLEngine is created without a valid parameter of setEndpointIdentificationAlgorithm, hostname verification is disabled by default.</p>
-<p>Unsafe implementation of the interface X509TrustManager, HostnameVerifier, and SSLSocket/SSLEngine ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.</p>
-<p>This query checks whether trust manager is set to trust all certificates, the hostname verifier is turned off, or setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.</p>
+<p>Unsafe implementation of the interface X509TrustManager and SSLSocket/SSLEngine ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.</p>
+<p>This query checks whether trust manager is set to trust all certificates or setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.</p>
 </overview>
 
 <recommendation>
 <p>Validate SSL certificate in SSL authentication.</p>
 </recommendation>
 
 <example>
-<p>The following two examples show two ways of configuring X509 trust cert manager and hostname verifier. In the 'BAD' case,
+<p>The following two examples show two ways of configuring X509 trust cert manager. In the 'BAD' case,
 no validation is performed thus any certificate is trusted. In the 'GOOD' case, the proper validation is performed.</p>
 <sample src="UnsafeCertTrust.java" />
 </example>

@@ -1,6 +1,6 @@
 /**
- * @name Unsafe certificate trust and improper hostname verification
- * @description Unsafe implementation of the interface X509TrustManager, HostnameVerifier, and SSLSocket/SSLEngine ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.
+ * @name Unsafe certificate trust
+ * @description Unsafe implementation of the interface X509TrustManager and SSLSocket/SSLEngine ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.
  * @kind problem
  * @id java/unsafe-cert-trust
  * @tags security
@@ -53,42 +53,6 @@ class X509TrustAllManagerInit extends MethodAccess {
   }
 }
 
-/**
- * HostnameVerifier class that allows a certificate whose CN (Common Name) does not match the host name in the URL
- */
-class TrustAllHostnameVerifier extends RefType {
-  TrustAllHostnameVerifier() {
-    this.getASupertype*() instanceof HostnameVerifier and
-    exists(Method m, ReturnStmt rt |
-      m.getDeclaringType() = this and
-      m.hasName("verify") and
-      rt.getEnclosingCallable() = m and
-      rt.getResult().(BooleanLiteral).getBooleanValue() = true
-    )
-  }
-}
-
-/**
- * The setDefaultHostnameVerifier method of HttpsURLConnection with the trust all configuration
- */
-class TrustAllHostnameVerify extends MethodAccess {
-  TrustAllHostnameVerify() {
-    this.getMethod().hasName("setDefaultHostnameVerifier") and
-    this.getMethod().getDeclaringType() instanceof HttpsURLConnection and //httpsURLConnection.setDefaultHostnameVerifier method
-    (
-      exists(NestedClass nc |
-        nc.getASupertype*() instanceof TrustAllHostnameVerifier and
-        this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
-      )
-      or
-      exists(Variable v |
-        this.getArgument(0).(VarAccess).getVariable() = v and
-        v.getInitializer().getType() instanceof TrustAllHostnameVerifier //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(verifier);
-      )
-    )
-  }
-}
-
 class SSLEngine extends RefType {
   SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
 }
@@ -239,7 +203,6 @@ class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
 
 from MethodAccess aa
 where
-  aa instanceof TrustAllHostnameVerify or
   aa instanceof X509TrustAllManagerInit or
   aa instanceof SSLEndpointIdentificationNotSet or
   aa instanceof RabbitMQEnableHostnameVerificationNotSet

@@ -48,31 +48,6 @@ public SSLSocketFactory testTrustAllCertManagerOfVariable() {
 		}
 	}
 
-	/**
-	 * Test the implementation of trusting all hostnames as an anonymous class
-	 */
-	public void testTrustAllHostnameOfAnonymousClass() {
-		HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {
-			@Override
-			public boolean verify(String hostname, SSLSession session) {
-				return true; // Noncompliant
-			}
-		});
-	}
-
-	/**
-	 * Test the implementation of trusting all hostnames as a variable
-	 */
-	public void testTrustAllHostnameOfVariable() {
-		HostnameVerifier verifier = new HostnameVerifier() {
-			@Override
-			public boolean verify(String hostname, SSLSession session) {
-				return true; // Noncompliant
-			}
-		};
-		HttpsURLConnection.setDefaultHostnameVerifier(verifier);
-	}
-
 	private static final X509TrustManager TRUST_ALL_CERTIFICATES = new X509TrustManager() {
 		@Override
 		public void checkClientTrusted(final X509Certificate[] chain, final String authType)
@@ -109,13 +84,6 @@ public X509Certificate[] getAcceptedIssuers() {
 		}
 	};
 
-	public static final HostnameVerifier ALLOW_ALL_HOSTNAME_VERIFIER = new HostnameVerifier() {
-		@Override
-		public boolean verify(String hostname, SSLSession session) {
-			return true; // Noncompliant
-		}
-	};
-
 	/**
 	 * Test the endpoint identification of SSL engine is set to null
 	 */