Skip to content

Java: Promote Unsafe certificate trust query from experimental #6171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
atorralba merged 26 commits into from
Jan 20, 2022
Merged

Java: Promote Unsafe certificate trust query from experimental #6171

Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e0f4c73
Move from experimental
atorralba Jun 21, 2021
4313baf
Big refactor:
atorralba Jun 21, 2021
02d0fa9
Minor changes in QLDocs and a sanitizer's type
atorralba Jun 22, 2021
e43fff2
Use InlineExpectationsTest
atorralba Jun 22, 2021
5d4cd70
Adjusted sources and sanitizer of UnsafeCertTrust taint tracking config
atorralba Jun 22, 2021
e842acf
Improve qhelp
atorralba Jun 23, 2021
4508945
Fix assumption regarding when an SSLSocket does the TLS handhsake
atorralba Jun 23, 2021
64518bf
Handle a specific pass-by-reference flow issue
atorralba Jun 23, 2021
19d1a78
Generalize sanitizer using local flow
atorralba Jun 23, 2021
9e93aec
Add spurious test case
atorralba Jun 28, 2021
5997b87
Add change note
atorralba Jun 28, 2021
c24520c
Adjust qhelp after rebase
atorralba Jun 28, 2021
68fe3dd
Fix conflicts in experimental query
atorralba Jun 28, 2021
698fd64
Adjust test after rebase
atorralba Jun 28, 2021
e9712f0
Add missing QLDoc
atorralba Jun 29, 2021
999acb0
Improve qhelp references
atorralba Jul 1, 2021
4d20710
Fix QLDoc
atorralba Jul 1, 2021
d9e98ce
Consider setSslContextFactory and fix tests
atorralba Jul 1, 2021
1e2a956
Remove unused stub
atorralba Jul 1, 2021
000a544
Decouple UnsafeCertTrust.qll to reuse the taint tracking configuration
atorralba Jul 21, 2021
c16181d
QLDocs
atorralba Jul 21, 2021
9ffc5ab
Update java/ql/src/semmle/code/java/security/UnsafeCertTrustQuery.qll
atorralba Jul 26, 2021
0302058
Apply suggestions from code review
atorralba Jul 29, 2021
101ad77
Move things around after rebase
atorralba Nov 12, 2021
e442e50
Apply suggestions from code review
atorralba Jan 19, 2022
695e77a
Simplify isSslSocket predicate
atorralba Jan 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Improve qhelp
  • Loading branch information
@atorralba
atorralba committed Jan 19, 2022
commit e842acf9e0da32247eb8cb5c3497f634af9207f1
13 changes: 5 additions & 8 deletions java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,23 @@
<qhelp>

<overview>
<p>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 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 setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.</p>
<p>Java offers two mechanisms for SSL authentication - trust manager and hostname verifier (the later is checked by the <code>java/insecure-hostname-verifier</code> 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>
Comment thread
atorralba marked this conversation as resolved.
Outdated
<p>When <code>SSLSocket</code> or <code>SSLEngine</code> are created without a secure <code>setEndpointIdentificationAlgorithm</code>, hostname verification is disabled by default.</p>
<p>This query checks whether a trust manager is set to trust all certificates or <code>setEndpointIdentificationAlgorithm</code> is missing, thereby making the application vulnerable to man-in-the-middle attacks. The query also covers insecure configurations of <code>com.rabbitmq.client.ConnectionFactory</code>.</p>
</overview>

<recommendation>
<p>Validate SSL certificate in SSL authentication.</p>
<p>Validate SSL certificates in SSL authentication.</p>
</recommendation>

<example>
<p>The following two examples show two ways of configuring SSLSocket/SSLEngine. In the 'BAD' case,
setEndpointIdentificationAlgorithm is not called, thus no hostname verification takes place. In the 'GOOD' case, setEndpointIdentificationAlgorithm is called.</p>
<code>setEndpointIdentificationAlgorithm</code> is not called, thus no hostname verification takes place. In the 'GOOD' case, <code>setEndpointIdentificationAlgorithm</code> is called.</p>
<sample src="UnsafeCertTrust.java" />
</example>

<references>
Comment thread
atorralba marked this conversation as resolved.
<li>
<a href="https://cwe.mitre.org/data/definitions/273.html">CWE-273</a>
</li>
<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>
Comment thread
atorralba marked this conversation as resolved.
Outdated
</li>
<li>
Expand Down