mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
Add validation query for SSL Engine/Socket and com.rabbitmq.client.ConnectionFactory
This commit is contained in:
@@ -65,4 +65,37 @@ public static void main(String[] args) {
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
{
|
||||
SSLContext sslContext = SSLContext.getInstance("TLS");
|
||||
SSLEngine sslEngine = sslContext.createSSLEngine();
|
||||
SSLParameters sslParameters = sslEngine.getSSLParameters();
|
||||
sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); //GOOD: Set a valid endpointIdentificationAlgorithm for SSL engine to trigger hostname verification
|
||||
sslEngine.setSSLParameters(sslParameters);
|
||||
}
|
||||
|
||||
{
|
||||
SSLContext sslContext = SSLContext.getInstance("TLS");
|
||||
SSLEngine sslEngine = sslContext.createSSLEngine(); //BAD: No endpointIdentificationAlgorithm set
|
||||
}
|
||||
|
||||
{
|
||||
SSLContext sslContext = SSLContext.getInstance("TLS");
|
||||
final SSLSocketFactory socketFactory = sslContext.getSocketFactory();
|
||||
SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
|
||||
SSLParameters sslParameters = sslEngine.getSSLParameters();
|
||||
sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); //GOOD: Set a valid endpointIdentificationAlgorithm for SSL socket to trigger hostname verification
|
||||
socket.setSSLParameters(sslParameters);
|
||||
}
|
||||
|
||||
{
|
||||
com.rabbitmq.client.ConnectionFactory connectionFactory = new com.rabbitmq.client.ConnectionFactory();
|
||||
connectionFactory.useSslProtocol();
|
||||
connectionFactory.enableHostnameVerification(); //GOOD: Enable hostname verification for rabbitmq ConnectionFactory
|
||||
}
|
||||
|
||||
{
|
||||
com.rabbitmq.client.ConnectionFactory connectionFactory = new com.rabbitmq.client.ConnectionFactory();
|
||||
connectionFactory.useSslProtocol(); //BAD: Hostname verification for rabbitmq ConnectionFactory is not enabled
|
||||
}
|
||||
}
|
||||
@@ -5,8 +5,9 @@
|
||||
|
||||
<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>Unsafe implementation of the interface X509TrustManager and HostnameVerifier 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 the hostname verifier is turned off.</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>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
@@ -29,5 +30,17 @@ no validation is performed thus any certificate is trusted. In the 'GOOD' case,
|
||||
<li>
|
||||
<a href="https://github.com/OWASP/owasp-mstg/blob/master/Document/0x05g-Testing-Network-Communication.md">Testing Endpoint Identify Verification (MSTG-NETWORK-3)</a>
|
||||
</li>
|
||||
<li>
|
||||
<a href="https://github.com/advisories/GHSA-xvch-r4wf-h8w9">CVE-2018-17187: Apache Qpid Proton-J transport issue with hostname verification</a>
|
||||
</li>
|
||||
<li>
|
||||
<a href="https://github.com/advisories/GHSA-46j3-r4pj-4835">CVE-2018-8034: Apache Tomcat - host name verification when using TLS with the WebSocket client</a>
|
||||
</li>
|
||||
<li>
|
||||
<a href="https://github.com/advisories/GHSA-w4g2-9hj6-5472">CVE-2018-11087: Pivotal Spring AMQP vulnerability due to lack of hostname validation</a>
|
||||
</li>
|
||||
<li>
|
||||
<a href="https://github.com/advisories/GHSA-m9w8-v359-9ffr">CVE-2018-11775: TLS hostname verification issue when using the Apache ActiveMQ Client</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* @id java/unsafe-cert-trust
|
||||
* @name Unsafe implementation of trusting any certificate in SSL configuration
|
||||
* @description Unsafe implementation of the interface X509TrustManager and HostnameVerifier ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.
|
||||
* @name Unsafe implementation of trusting any certificate or missing hostname verification in SSL configuration
|
||||
* @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.
|
||||
* @kind problem
|
||||
* @tags security
|
||||
* external/cwe-273
|
||||
@@ -9,8 +9,6 @@
|
||||
|
||||
import java
|
||||
import semmle.code.java.security.Encryption
|
||||
import semmle.code.java.dataflow.DataFlow
|
||||
import DataFlow
|
||||
|
||||
/**
|
||||
* X509TrustManager class that blindly trusts all certificates in server SSL authentication
|
||||
@@ -79,7 +77,7 @@ class TrustAllHostnameVerify extends MethodAccess {
|
||||
(
|
||||
exists(NestedClass nc |
|
||||
nc.getASupertype*() instanceof TrustAllHostnameVerifier and
|
||||
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
|
||||
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
|
||||
)
|
||||
or
|
||||
exists(Variable v |
|
||||
@@ -90,6 +88,141 @@ class TrustAllHostnameVerify extends MethodAccess {
|
||||
}
|
||||
}
|
||||
|
||||
class SSLEngine extends RefType {
|
||||
SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
|
||||
}
|
||||
|
||||
class Socket extends RefType {
|
||||
Socket() { this.hasQualifiedName("java.net", "Socket") }
|
||||
}
|
||||
|
||||
class SSLSocket extends RefType {
|
||||
SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") }
|
||||
}
|
||||
|
||||
/**
|
||||
* has setEndpointIdentificationAlgorithm set correctly
|
||||
*/
|
||||
predicate setEndpointIdentificationAlgorithm(MethodAccess createSSL) {
|
||||
exists(
|
||||
Variable sslo, MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
|
||||
|
|
||||
createSSL = sslo.getAnAssignedValue() and
|
||||
ma.getQualifier() = sslo.getAnAccess() and
|
||||
ma.getMethod().hasName("setSSLParameters") and
|
||||
ma.getArgument(0).(VarAccess) = sslparams.getAnAccess() and
|
||||
exists(MethodAccess setepa |
|
||||
setepa.getQualifier().(VarAccess) = sslparams.getAnAccess() and
|
||||
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
|
||||
not setepa.getArgument(0) instanceof NullLiteral
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* has setEndpointIdentificationAlgorithm set correctly
|
||||
*/
|
||||
predicate hasEndpointIdentificationAlgorithm(Variable ssl) {
|
||||
exists(
|
||||
MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
|
||||
|
|
||||
ma.getQualifier() = ssl.getAnAccess() and
|
||||
ma.getMethod().hasName("setSSLParameters") and
|
||||
ma.getArgument(0).(VarAccess) = sslparams.getAnAccess() and
|
||||
exists(MethodAccess setepa |
|
||||
setepa.getQualifier().(VarAccess) = sslparams.getAnAccess() and
|
||||
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
|
||||
not setepa.getArgument(0) instanceof NullLiteral
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* SSL object is created in a separate method call or in the same method
|
||||
*/
|
||||
predicate hasFlowPath(MethodAccess createSSL, Variable ssl) {
|
||||
(
|
||||
createSSL = ssl.getAnAssignedValue()
|
||||
or
|
||||
exists(CastExpr ce |
|
||||
ce.getExpr().(MethodAccess) = createSSL and
|
||||
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl //With a type cast like SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
|
||||
)
|
||||
)
|
||||
or
|
||||
exists(MethodAccess tranm |
|
||||
createSSL.getEnclosingCallable().(Method) = tranm.getMethod() and
|
||||
tranm.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
|
||||
not setEndpointIdentificationAlgorithm(createSSL) //Check the scenario of invocation before used in the current method
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Not have the SSLParameter set
|
||||
*/
|
||||
predicate hasNoEndpointIdentificationSet(MethodAccess createSSL, Variable ssl) {
|
||||
//No setSSLParameters set
|
||||
hasFlowPath(createSSL, ssl) and
|
||||
not exists(MethodAccess ma |
|
||||
ma.getQualifier() = ssl.getAnAccess() and
|
||||
ma.getMethod().hasName("setSSLParameters")
|
||||
)
|
||||
or
|
||||
//No endpointIdentificationAlgorithm set with setSSLParameters
|
||||
hasFlowPath(createSSL, ssl) and
|
||||
not setEndpointIdentificationAlgorithm(createSSL)
|
||||
}
|
||||
|
||||
/**
|
||||
* The setEndpointIdentificationAlgorithm method of SSLParameters with the ssl engine or socket
|
||||
*/
|
||||
class SSLEndpointIdentificationNotSet extends MethodAccess {
|
||||
SSLEndpointIdentificationNotSet() {
|
||||
(
|
||||
this.getMethod().hasName("createSSLEngine") and
|
||||
this.getMethod().getDeclaringType() instanceof SSLContext //createEngine method of SSLContext
|
||||
or
|
||||
this.getMethod().hasName("createSocket") and
|
||||
this.getMethod().getReturnType() instanceof Socket //createSocket method of SSLSocketFactory
|
||||
) and
|
||||
exists(Variable ssl |
|
||||
hasNoEndpointIdentificationSet(this, ssl) and //Not set in itself
|
||||
not exists(VariableAssign ar, Variable newSsl |
|
||||
ar.getSource() = this.getCaller().getAReference() and
|
||||
ar.getDestVar() = newSsl and
|
||||
hasEndpointIdentificationAlgorithm(newSsl) //Not set in its caller either
|
||||
)
|
||||
) and
|
||||
not exists(MethodAccess ma | ma.getMethod() instanceof HostnameVerifierVerify) //Reduce false positives since this method access set default hostname verifier
|
||||
}
|
||||
}
|
||||
|
||||
class RabbitMQConnectionFactory extends RefType {
|
||||
RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") }
|
||||
}
|
||||
|
||||
/**
|
||||
* The com.rabbitmq.client.ConnectionFactory useSslProtocol method access without enableHostnameVerification
|
||||
*/
|
||||
class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
|
||||
RabbitMQEnableHostnameVerificationNotSet() {
|
||||
this.getMethod().hasName("useSslProtocol") and
|
||||
this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and
|
||||
exists(VarAccess va |
|
||||
va.getVariable().getType() instanceof RabbitMQConnectionFactory and
|
||||
this.getQualifier() = va.getVariable().getAnAccess() and
|
||||
not exists(MethodAccess ma |
|
||||
ma.getMethod().hasName("enableHostnameVerification") and
|
||||
ma.getQualifier() = va.getVariable().getAnAccess()
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
from MethodAccess aa
|
||||
where aa instanceof TrustAllHostnameVerify or aa instanceof X509TrustAllManagerInit
|
||||
where
|
||||
aa instanceof TrustAllHostnameVerify or
|
||||
aa instanceof X509TrustAllManagerInit or
|
||||
aa instanceof SSLEndpointIdentificationNotSet or
|
||||
aa instanceof RabbitMQEnableHostnameVerificationNotSet
|
||||
select aa, "Unsafe configuration of trusted certificates"
|
||||
@@ -1,4 +1,7 @@
|
||||
| UnsafeCertTrustTest.java:19:4:19:74 | init(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:34:4:34:38 | init(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:47:3:52:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:65:3:65:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:26:4:26:74 | init(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:41:4:41:38 | init(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:54:3:59:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:72:3:72:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:123:25:123:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:134:25:134:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
|
||||
| UnsafeCertTrustTest.java:143:34:143:83 | createSocket(...) | Unsafe configuration of trusted certificates |
|
||||
|
||||
@@ -1,13 +1,20 @@
|
||||
import javax.net.ssl.HostnameVerifier;
|
||||
import javax.net.ssl.HttpsURLConnection;
|
||||
import javax.net.ssl.SSLSession;
|
||||
import javax.net.ssl.SSLSocket;
|
||||
import javax.net.ssl.SSLContext;
|
||||
import javax.net.ssl.SSLEngine;
|
||||
import javax.net.ssl.SSLParameters;
|
||||
import javax.net.ssl.SSLSocketFactory;
|
||||
import javax.net.ssl.TrustManager;
|
||||
import javax.net.ssl.X509TrustManager;
|
||||
|
||||
import java.net.Socket;
|
||||
import java.security.cert.CertificateException;
|
||||
import java.security.cert.X509Certificate;
|
||||
|
||||
//import com.rabbitmq.client.ConnectionFactory;
|
||||
|
||||
public class UnsafeCertTrustTest {
|
||||
|
||||
/**
|
||||
@@ -15,7 +22,7 @@ public class UnsafeCertTrustTest {
|
||||
*/
|
||||
public SSLSocketFactory testTrustAllCertManager() {
|
||||
try {
|
||||
final SSLContext context = SSLContext.getInstance("SSL");
|
||||
final SSLContext context = SSLContext.getInstance("TLS");
|
||||
context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null);
|
||||
final SSLSocketFactory socketFactory = context.getSocketFactory();
|
||||
return socketFactory;
|
||||
@@ -29,7 +36,7 @@ public class UnsafeCertTrustTest {
|
||||
*/
|
||||
public SSLSocketFactory testTrustAllCertManagerOfVariable() {
|
||||
try {
|
||||
SSLContext context = SSLContext.getInstance("SSL");
|
||||
SSLContext context = SSLContext.getInstance("TLS");
|
||||
TrustManager[] serverTMs = new TrustManager[] { new X509TrustAllManager() };
|
||||
context.init(null, serverTMs, null);
|
||||
|
||||
@@ -107,4 +114,40 @@ public class UnsafeCertTrustTest {
|
||||
return true; // Noncompliant
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Test the endpoint identification of SSL engine is set to null
|
||||
*/
|
||||
public void testSSLEngineEndpointIdSetNull() {
|
||||
SSLContext sslContext = SSLContext.getInstance("TLS");
|
||||
SSLEngine sslEngine = sslContext.createSSLEngine();
|
||||
SSLParameters sslParameters = sslEngine.getSSLParameters();
|
||||
sslParameters.setEndpointIdentificationAlgorithm(null);
|
||||
sslEngine.setSSLParameters(sslParameters);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test the endpoint identification of SSL engine is not set
|
||||
*/
|
||||
public void testSSLEngineEndpointIdNotSet() {
|
||||
SSLContext sslContext = SSLContext.getInstance("TLS");
|
||||
SSLEngine sslEngine = sslContext.createSSLEngine();
|
||||
}
|
||||
|
||||
/**
|
||||
* Test the endpoint identification of SSL socket is not set
|
||||
*/
|
||||
public void testSSLSocketEndpointIdNotSet() {
|
||||
SSLContext sslContext = SSLContext.getInstance("TLS");
|
||||
final SSLSocketFactory socketFactory = sslContext.getSocketFactory();
|
||||
SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
|
||||
}
|
||||
|
||||
// /**
|
||||
// * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set
|
||||
// */
|
||||
// public void testEnableHostnameVerificationOfRabbitMQFactoryNotSet() {
|
||||
// ConnectionFactory connectionFactory = new ConnectionFactory();
|
||||
// connectionFactory.useSslProtocol();
|
||||
// }
|
||||
}
|
||||
Reference in New Issue
Block a user