Skip to content

Commit 06e3f10

Browse files
Java: Added a query for disabled certificate revocation checking
- Added experimental/Security/CWE/CWE-299/DisabledRevocationChecking.ql The query looks for PKIXParameters.setRevocationEnabled(false) calls. - Added RevocationCheckingLib.qll - Added a qhelp file with examples - Added tests in java/ql/test/experimental/Security/CWE/CWE-299
1 parent e00a8f7 commit 06e3f10

9 files changed

+373
-0
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
public void validate(KeyStore cacerts, CertPath certPath) throws Exception {
2+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
3+
PKIXParameters params = new PKIXParameters(cacerts);
4+
params.setRevocationEnabled(false);
5+
PKIXRevocationChecker checker = (PKIXRevocationChecker) validator.getRevocationChecker();
6+
checker.setOcspResponder(OCSP_RESPONDER_URL);
7+
checker.setOcspResponderCert(OCSP_RESPONDER_CERT);
8+
params.addCertPathChecker(checker);
9+
validator.validate(certPath, params);
10+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
public void validate(KeyStore cacerts, CertPath chain) throws Exception {
2+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
3+
PKIXParameters params = new PKIXParameters(cacerts);
4+
validator.validate(chain, params);
5+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>Validating a certificate chain includes multiple steps. One of them is checking whether or not
6+
certificates in the chain have been revoked. A certificate may be revoked due to multiple reasons.
7+
One of the reasons why the certificate authority (CA) may revoke a certificate is that its private key
8+
has been compromised. For example, the private key might have been stolen by an adversary.
9+
In this case, the adversary may be able to impersonate the owner of the private key.
10+
Therefore, trusting a revoked certificate may be dangerous.</p>
11+
12+
<p>The Java Certification Path API provides a revocation checking mechanism
13+
that supports both CRL and OCSP.
14+
Revocation checking happens while building and validating certificate chains.
15+
If at least one of the certificates is revoked, then an exception is thrown.
16+
This mechanism is enabled by default. However, it may be disabled
17+
by passing <code>false</code> to the <code>PKIXParameters.setRevocationEnabled()</code> method.
18+
If an application doesn't set a custom <code>PKIXRevocationChecker</code>
19+
via <code>PKIXParameters.addCertPathChecker()</code>
20+
or <code>PKIXParameters.setCertPathCheckers()</code> methods,
21+
then revocation checking is not going to happen.</p>
22+
23+
</overview>
24+
<recommendation>
25+
26+
<p>An application should not disable the default revocationg checking mechanism
27+
unless it provides a custom revocation checker.</p>
28+
29+
</recommendation>
30+
<example>
31+
32+
<p>The following example turns off revocation checking for validating a certificate chain.
33+
That should be avoided.</p>
34+
35+
<sample src="NoRevocationChecking.java" />
36+
37+
<p>The next example uses the default revocation checking mechanism.</p>
38+
39+
<sample src="DefaultRevocationChecking.java" />
40+
41+
<p>The third example turns off the default revocation mechanism. However, it registers another
42+
revocation checker that uses OCSP to obtain revocation status of certificates.</p>
43+
44+
<sample src="CustomRevocationChecking.java" />
45+
46+
</example>
47+
<references>
48+
49+
<li>
50+
Wikipedia:
51+
<a href="https://en.wikipedia.org/wiki/Public_key_certificate">Public key certificate</a>
52+
</li>
53+
<li>
54+
Java SE Documentation:
55+
<a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/certpath/CertPathProgGuide.html">Java PKI Programmer's Guide</a>
56+
</li>
57+
<li>
58+
Java SE API Specification:
59+
<a href="https://docs.oracle.com/javase/8/docs/api/index.html?java/security/cert/CertPathValidator.html">CertPathValidator</a>
60+
</li>
61+
62+
</references>
63+
</qhelp>
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Disabled ceritificate revocation checking
3+
* @description Using revoked certificates is dangerous.
4+
* Therefore, revocation status of ceritifcates in a chain should be checked.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/disabled-certificate-revocation-checking
9+
* @tags security
10+
* external/cwe/cwe-299
11+
*/
12+
13+
import java
14+
import RevocationCheckingLib
15+
import DataFlow::PathGraph
16+
17+
from DataFlow::PathNode source, DataFlow::PathNode sink, DisabledRevocationCheckingConfig config
18+
where config.hasFlowPath(source, sink)
19+
select source.getNode(), source, sink, "Revocation checking is disabled $@.", source.getNode(),
20+
"here"
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
public void validateUnsafe(KeyStore cacerts, CertPath chain) throws Exception {
2+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
3+
PKIXParameters params = new PKIXParameters(cacerts);
4+
params.setRevocationEnabled(false);
5+
validator.validate(chain, params);
6+
}
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
import semmle.code.java.dataflow.TaintTracking2
4+
import DataFlow
5+
6+
/**
7+
* A taint-tracking configuration for disabling revocation checking.
8+
*/
9+
class DisabledRevocationCheckingConfig extends TaintTracking::Configuration {
10+
DisabledRevocationCheckingConfig() { this = "DisabledRevocationCheckingConfig" }
11+
12+
override predicate isSource(DataFlow::Node source) {
13+
exists(BooleanLiteral b | b.getBooleanValue() = false | source.asExpr() = b)
14+
}
15+
16+
override predicate isSink(DataFlow::Node sink) { sink instanceof SetRevocationEnabledSink }
17+
}
18+
19+
/**
20+
* A sink that disables revocation checking,
21+
* i.e. calling `PKIXParameters.setRevocationEnabled(false)`
22+
* without setting a custom revocation checker in `PKIXParameters`.
23+
*/
24+
class SetRevocationEnabledSink extends DataFlow::ExprNode {
25+
SetRevocationEnabledSink() {
26+
exists(MethodAccess setRevocationEnabledCall |
27+
setRevocationEnabledCall.getMethod() instanceof SetRevocationEnabledMethod and
28+
setRevocationEnabledCall.getArgument(0) = getExpr() and
29+
not exists(
30+
SettingRevocationCheckerConfig config, DataFlow2::PathNode source, DataFlow2::PathNode sink
31+
|
32+
config.hasFlowPath(source, sink) and
33+
sink.getNode().(SettingRevocationCheckerSink).getVariable() =
34+
setRevocationEnabledCall.getQualifier().(VarAccess).getVariable()
35+
)
36+
)
37+
}
38+
}
39+
40+
/**
41+
* A dataflow config for tracking a custom revocation checker.
42+
*/
43+
class SettingRevocationCheckerConfig extends DataFlow2::Configuration {
44+
SettingRevocationCheckerConfig() {
45+
this = "DisabledRevocationChecking::SettingRevocationCheckerConfig"
46+
}
47+
48+
override predicate isSource(DataFlow::Node source) {
49+
source instanceof GetRevocationCheckerSource
50+
}
51+
52+
override predicate isSink(DataFlow::Node sink) { sink instanceof SettingRevocationCheckerSink }
53+
54+
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
55+
createSingletonListStep(node1, node2) or
56+
convertArrayToListStep(node1, node2) or
57+
addToListStep(node1, node2)
58+
}
59+
60+
override int fieldFlowBranchLimit() { result = 0 }
61+
}
62+
63+
/**
64+
* A source that creates a custom revocation checker,
65+
* i.e. `CertPathValidator.getRevocationChecker()`.
66+
*/
67+
class GetRevocationCheckerSource extends DataFlow::ExprNode {
68+
GetRevocationCheckerSource() {
69+
exists(MethodAccess ma | ma.getMethod() instanceof GetRevocationCheckerMethod |
70+
ma = asExpr() or ma.getQualifier() = asExpr()
71+
)
72+
}
73+
}
74+
75+
/**
76+
* A sink that sets a custom revocation checker in `PKIXParameters`,
77+
* i.e. `PKIXParameters.addCertPathChecker()` or `PKIXParameters.setCertPathCheckers()`.
78+
*/
79+
class SettingRevocationCheckerSink extends DataFlow::ExprNode {
80+
MethodAccess ma;
81+
82+
SettingRevocationCheckerSink() {
83+
(
84+
ma.getMethod() instanceof AddCertPathCheckerMethod or
85+
ma.getMethod() instanceof SetCertPathCheckersMethod
86+
) and
87+
ma.getArgument(0) = asExpr()
88+
}
89+
90+
Variable getVariable() { result = ma.getQualifier().(VarAccess).getVariable() }
91+
}
92+
93+
/**
94+
* Holds if `node1` to `node2` is a dataflow step that creates a singleton list,
95+
* i.e. `Collections.singletonList(element)`.
96+
*/
97+
predicate createSingletonListStep(DataFlow::Node node1, DataFlow::Node node2) {
98+
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
99+
m.getDeclaringType() instanceof Collections and
100+
m.hasName("singletonList") and
101+
ma.getArgument(0) = node1.asExpr() and
102+
(ma = node2.asExpr() or ma.getQualifier() = node2.asExpr())
103+
)
104+
}
105+
106+
/**
107+
* Holds if `node1` to `node2` is a dataflow step that converts an array to a list,class
108+
* i.e. `Arrays.asList(element)`.
109+
*/
110+
predicate convertArrayToListStep(DataFlow::Node node1, DataFlow::Node node2) {
111+
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
112+
m.getDeclaringType() instanceof Arrays and
113+
m.hasName("asList") and
114+
ma.getArgument(0) = node1.asExpr() and
115+
(ma = node2.asExpr() or ma.getQualifier() = node2.asExpr())
116+
)
117+
}
118+
119+
/**
120+
* Holds if `node1` to `node2` is a dataflow step that adds an element to a list,
121+
* i.e. `list.add(element)` or `list.addAll(elements)`.
122+
*/
123+
predicate addToListStep(DataFlow::Node node1, DataFlow::Node node2) {
124+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
125+
m.getDeclaringType() instanceof List and
126+
(
127+
m.hasName("add") or
128+
m.hasName("addAll")
129+
) and
130+
ma.getArgument(0) = node1.asExpr() and
131+
(ma = node2.asExpr() or ma.getQualifier() = node2.asExpr())
132+
)
133+
}
134+
135+
class SetRevocationEnabledMethod extends Method {
136+
SetRevocationEnabledMethod() {
137+
getDeclaringType() instanceof PKIXParameters and
138+
hasName("setRevocationEnabled")
139+
}
140+
}
141+
142+
class GetRevocationCheckerMethod extends Method {
143+
GetRevocationCheckerMethod() {
144+
getDeclaringType() instanceof CertPathValidator and
145+
hasName("getRevocationChecker")
146+
}
147+
}
148+
149+
class AddCertPathCheckerMethod extends Method {
150+
AddCertPathCheckerMethod() {
151+
getDeclaringType() instanceof PKIXParameters and
152+
hasName("addCertPathChecker")
153+
}
154+
}
155+
156+
class SetCertPathCheckersMethod extends Method {
157+
SetCertPathCheckersMethod() {
158+
getDeclaringType() instanceof PKIXParameters and
159+
hasName("setCertPathCheckers")
160+
}
161+
}
162+
163+
class PKIXParameters extends RefType {
164+
PKIXParameters() { hasQualifiedName("java.security.cert", "PKIXParameters") }
165+
}
166+
167+
class CertPathValidator extends RefType {
168+
CertPathValidator() { hasQualifiedName("java.security.cert", "CertPathValidator") }
169+
}
170+
171+
class Collections extends RefType {
172+
Collections() { hasQualifiedName("java.util", "Collections") }
173+
}
174+
175+
class Arrays extends RefType {
176+
Arrays() { hasQualifiedName("java.util", "Arrays") }
177+
}
178+
179+
class List extends ParameterizedInterface {
180+
List() { getGenericType().hasQualifiedName("java.util", "List") }
181+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
edges
2+
| DisabledRevocationChecking.java:17:5:17:8 | this <.field> [post update] [flag] : Boolean | DisabledRevocationChecking.java:21:5:21:31 | this <.method> [post update] [flag] : Boolean |
3+
| DisabledRevocationChecking.java:17:12:17:16 | false : Boolean | DisabledRevocationChecking.java:17:5:17:8 | this <.field> [post update] [flag] : Boolean |
4+
| DisabledRevocationChecking.java:21:5:21:31 | this <.method> [post update] [flag] : Boolean | DisabledRevocationChecking.java:22:5:22:31 | this <.method> [flag] : Boolean |
5+
| DisabledRevocationChecking.java:22:5:22:31 | this <.method> [flag] : Boolean | DisabledRevocationChecking.java:25:15:25:22 | parameter this [flag] : Boolean |
6+
| DisabledRevocationChecking.java:25:15:25:22 | parameter this [flag] : Boolean | DisabledRevocationChecking.java:28:33:28:36 | this <.field> [flag] : Boolean |
7+
| DisabledRevocationChecking.java:28:33:28:36 | this <.field> [flag] : Boolean | DisabledRevocationChecking.java:28:33:28:36 | flag |
8+
nodes
9+
| DisabledRevocationChecking.java:17:5:17:8 | this <.field> [post update] [flag] : Boolean | semmle.label | this <.field> [post update] [flag] : Boolean |
10+
| DisabledRevocationChecking.java:17:12:17:16 | false : Boolean | semmle.label | false : Boolean |
11+
| DisabledRevocationChecking.java:21:5:21:31 | this <.method> [post update] [flag] : Boolean | semmle.label | this <.method> [post update] [flag] : Boolean |
12+
| DisabledRevocationChecking.java:22:5:22:31 | this <.method> [flag] : Boolean | semmle.label | this <.method> [flag] : Boolean |
13+
| DisabledRevocationChecking.java:25:15:25:22 | parameter this [flag] : Boolean | semmle.label | parameter this [flag] : Boolean |
14+
| DisabledRevocationChecking.java:28:33:28:36 | flag | semmle.label | flag |
15+
| DisabledRevocationChecking.java:28:33:28:36 | this <.field> [flag] : Boolean | semmle.label | this <.field> [flag] : Boolean |
16+
#select
17+
| DisabledRevocationChecking.java:17:12:17:16 | false | DisabledRevocationChecking.java:17:12:17:16 | false : Boolean | DisabledRevocationChecking.java:28:33:28:36 | flag | Revocation checking is disabled $@. | DisabledRevocationChecking.java:17:12:17:16 | false | here |
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import java.security.KeyStore;
2+
import java.security.cert.CertPath;
3+
import java.security.cert.CertPathValidator;
4+
import java.security.cert.PKIXCertPathChecker;
5+
import java.security.cert.PKIXParameters;
6+
import java.security.cert.PKIXRevocationChecker;
7+
import java.util.ArrayList;
8+
import java.util.Arrays;
9+
import java.util.Collections;
10+
import java.util.List;
11+
12+
public class DisabledRevocationChecking {
13+
14+
private boolean flag = true;
15+
16+
public void disableRevocationChecking() {
17+
flag = false;
18+
}
19+
20+
public void testDisabledRevocationChecking(KeyStore cacerts, CertPath certPath) throws Exception {
21+
disableRevocationChecking();
22+
validate(cacerts, certPath);
23+
}
24+
25+
public void validate(KeyStore cacerts, CertPath certPath) throws Exception {
26+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
27+
PKIXParameters params = new PKIXParameters(cacerts);
28+
params.setRevocationEnabled(flag);
29+
validator.validate(certPath, params);
30+
}
31+
32+
public void testSettingRevocationCheckerWithCollectionsSingletonList(KeyStore cacerts, CertPath certPath) throws Exception {
33+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
34+
PKIXParameters params = new PKIXParameters(cacerts);
35+
params.setRevocationEnabled(false);
36+
PKIXRevocationChecker checker = (PKIXRevocationChecker) validator.getRevocationChecker();
37+
params.setCertPathCheckers(Collections.singletonList(checker));
38+
validator.validate(certPath, params);
39+
}
40+
41+
public void testSettingRevocationCheckerWithArraysAsList(KeyStore cacerts, CertPath certPath) throws Exception {
42+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
43+
PKIXParameters params = new PKIXParameters(cacerts);
44+
params.setRevocationEnabled(false);
45+
PKIXRevocationChecker checker = (PKIXRevocationChecker) validator.getRevocationChecker();
46+
params.setCertPathCheckers(Arrays.asList(checker));
47+
validator.validate(certPath, params);
48+
}
49+
50+
public void testSettingRevocationCheckerWithList(KeyStore cacerts, CertPath certPath) throws Exception {
51+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
52+
PKIXParameters params = new PKIXParameters(cacerts);
53+
params.setRevocationEnabled(false);
54+
PKIXRevocationChecker checker = (PKIXRevocationChecker) validator.getRevocationChecker();
55+
List<PKIXCertPathChecker> checkers = new ArrayList<>();
56+
checkers.add(checker);
57+
params.setCertPathCheckers(checkers);
58+
validator.validate(certPath, params);
59+
}
60+
61+
public void testAddingRevocationChecker(KeyStore cacerts, CertPath certPath) throws Exception {
62+
CertPathValidator validator = CertPathValidator.getInstance("PKIX");
63+
PKIXParameters params = new PKIXParameters(cacerts);
64+
params.setRevocationEnabled(false);
65+
PKIXRevocationChecker checker = (PKIXRevocationChecker) validator.getRevocationChecker();
66+
params.addCertPathChecker(checker);
67+
validator.validate(certPath, params);
68+
}
69+
70+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-299/DisabledRevocationChecking.ql

0 commit comments

Comments
 (0)