-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Java: Query to detect weak encryption: insufficient key size #4926
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.
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
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
cfc950f
Query for weak encryption: Insufficient key size
luchua-bc cbaee93
Optimize the query
luchua-bc 058f3af
Refactor the hasShortSymmetricKey method
luchua-bc 2ac7b4b
Update qldoc
luchua-bc ab7d257
Add more cases and change EC to 256 bits
luchua-bc 50be543
Update qldoc
luchua-bc 5e3b6fa
Update qldoc
luchua-bc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
37 changes: 37 additions & 0 deletions
37
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
public class InsufficientKeySize { | ||
public void CryptoMethod() { | ||
KeyGenerator keyGen1 = KeyGenerator.getInstance("AES"); | ||
// BAD: Key size is less than 128 | ||
keyGen1.init(64); | ||
|
||
KeyGenerator keyGen2 = KeyGenerator.getInstance("AES"); | ||
// GOOD: Key size is no less than 128 | ||
keyGen2.init(128); | ||
|
||
KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA"); | ||
// BAD: Key size is less than 2048 | ||
keyPairGen1.initialize(1024); | ||
|
||
KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA"); | ||
// GOOD: Key size is no less than 2048 | ||
keyPairGen2.initialize(2048); | ||
|
||
KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA"); | ||
// BAD: Key size is less than 2048 | ||
keyPairGen3.initialize(1024); | ||
|
||
KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); | ||
// GOOD: Key size is no less than 2048 | ||
keyPairGen4.initialize(2048); | ||
|
||
KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); | ||
keyPairGen5.initialize(ecSpec1); | ||
|
||
KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); | ||
// GOOD: Key size is no less than 256 | ||
ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); | ||
keyPairGen6.initialize(ecSpec2); | ||
} | ||
} |
29 changes: 29 additions & 0 deletions
29
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
<qhelp> | ||
<overview> | ||
<p>This rule finds uses of encryption algorithms with too small a key size. Encryption algorithms | ||
are vulnerable to brute force attack when too small a key size is used.</p> | ||
</overview> | ||
|
||
<recommendation> | ||
<p>The key should be at least 2048 bits long when using RSA and DSA encryption, 256 bits long when using EC encryption, and 128 bits long when using | ||
symmetric encryption.</p> | ||
</recommendation> | ||
|
||
<references> | ||
|
||
<li> | ||
Wikipedia. | ||
<a href="http://en.wikipedia.org/wiki/Key_size">Key size</a> | ||
</li> | ||
<li> | ||
SonarSource. | ||
<a href="https://rules.sonarsource.com/java/type/Vulnerability/RSPEC-4426">Cryptographic keys should be robust</a> | ||
</li> | ||
<li> | ||
CWE. | ||
<a href="https://cwe.mitre.org/data/definitions/326.html">CWE-326: Inadequate Encryption Strength</a> | ||
</li> | ||
|
||
</references> | ||
</qhelp> |
151 changes: 151 additions & 0 deletions
151
java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
/** | ||
* @name Weak encryption: Insufficient key size | ||
* @description Finds uses of encryption algorithms with too small a key size | ||
* @kind problem | ||
* @id java/insufficient-key-size | ||
* @tags security | ||
* external/cwe/cwe-326 | ||
*/ | ||
|
||
import java | ||
import semmle.code.java.security.Encryption | ||
import semmle.code.java.dataflow.TaintTracking | ||
|
||
/** The Java class `java.security.spec.ECGenParameterSpec`. */ | ||
class ECGenParameterSpec extends RefType { | ||
ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") } | ||
} | ||
|
||
/** The `init` method declared in `javax.crypto.KeyGenerator`. */ | ||
class KeyGeneratorInitMethod extends Method { | ||
KeyGeneratorInitMethod() { | ||
getDeclaringType() instanceof KeyGenerator and | ||
hasName("init") | ||
} | ||
} | ||
|
||
/** The `initialize` method declared in `java.security.KeyPairGenerator`. */ | ||
class KeyPairGeneratorInitMethod extends Method { | ||
KeyPairGeneratorInitMethod() { | ||
getDeclaringType() instanceof KeyPairGenerator and | ||
hasName("initialize") | ||
} | ||
} | ||
|
||
/** Returns the key size in the EC algorithm string */ | ||
bindingset[algorithm] | ||
int getECKeySize(string algorithm) { | ||
algorithm.matches("sec%") and // specification such as "secp256r1" | ||
result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt() | ||
or | ||
algorithm.matches("X9.62%") and //specification such as "X9.62 prime192v2" | ||
result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt() | ||
or | ||
(algorithm.matches("prime%") or algorithm.matches("c2tnb%")) and //specification such as "prime192v2" | ||
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt() | ||
} | ||
|
||
/** Taint configuration tracking flow from a key generator to a `init` method call. */ | ||
class KeyGeneratorInitConfiguration extends TaintTracking::Configuration { | ||
KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" } | ||
|
||
override predicate isSource(DataFlow::Node source) { | ||
exists(JavaxCryptoKeyGenerator jcg | jcg = source.asExpr()) | ||
} | ||
|
||
override predicate isSink(DataFlow::Node sink) { | ||
exists(MethodAccess ma | | ||
ma.getMethod() instanceof KeyGeneratorInitMethod and | ||
sink.asExpr() = ma.getQualifier() | ||
) | ||
} | ||
} | ||
|
||
/** Taint configuration tracking flow from a keypair generator to a `initialize` method call. */ | ||
class KeyPairGeneratorInitConfiguration extends TaintTracking::Configuration { | ||
KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" } | ||
|
||
override predicate isSource(DataFlow::Node source) { | ||
exists(JavaSecurityKeyPairGenerator jkg | jkg = source.asExpr()) | ||
} | ||
|
||
override predicate isSink(DataFlow::Node sink) { | ||
exists(MethodAccess ma | | ||
ma.getMethod() instanceof KeyPairGeneratorInitMethod and | ||
sink.asExpr() = ma.getQualifier() | ||
) | ||
} | ||
} | ||
|
||
/** Holds if a symmetric `KeyGenerator` implementing encryption algorithm `type` and initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */ | ||
bindingset[type] | ||
predicate hasShortSymmetricKey(MethodAccess ma, string msg, string type) { | ||
ma.getMethod() instanceof KeyGeneratorInitMethod and | ||
exists( | ||
JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration cc, DataFlow::PathNode source, | ||
DataFlow::PathNode dest | ||
| | ||
jcg.getAlgoSpec().(StringLiteral).getValue() = type and | ||
source.getNode().asExpr() = jcg and | ||
dest.getNode().asExpr() = ma.getQualifier() and | ||
cc.hasFlowPath(source, dest) | ||
) and | ||
ma.getArgument(0).(IntegerLiteral).getIntValue() < 128 and | ||
msg = "Key size should be at least 128 bits for " + type + " encryption." | ||
} | ||
|
||
/** Holds if an AES `KeyGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */ | ||
predicate hasShortAESKey(MethodAccess ma, string msg) { hasShortSymmetricKey(ma, msg, "AES") } | ||
|
||
/** Holds if an asymmetric `KeyPairGenerator` implementing encryption algorithm `type` and initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */ | ||
bindingset[type] | ||
predicate hasShortAsymmetricKeyPair(MethodAccess ma, string msg, string type) { | ||
ma.getMethod() instanceof KeyPairGeneratorInitMethod and | ||
exists( | ||
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kc, | ||
DataFlow::PathNode source, DataFlow::PathNode dest | ||
| | ||
jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = type and | ||
source.getNode().asExpr() = jpg and | ||
dest.getNode().asExpr() = ma.getQualifier() and | ||
kc.hasFlowPath(source, dest) | ||
) and | ||
ma.getArgument(0).(IntegerLiteral).getIntValue() < 2048 and | ||
msg = "Key size should be at least 2048 bits for " + type + " encryption." | ||
} | ||
|
||
/** Holds if a DSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */ | ||
predicate hasShortDSAKeyPair(MethodAccess ma, string msg) { | ||
hasShortAsymmetricKeyPair(ma, msg, "DSA") or hasShortAsymmetricKeyPair(ma, msg, "DH") | ||
} | ||
|
||
/** Holds if a RSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */ | ||
predicate hasShortRSAKeyPair(MethodAccess ma, string msg) { | ||
hasShortAsymmetricKeyPair(ma, msg, "RSA") | ||
} | ||
|
||
/** Holds if an EC `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */ | ||
predicate hasShortECKeyPair(MethodAccess ma, string msg) { | ||
ma.getMethod() instanceof KeyPairGeneratorInitMethod and | ||
exists( | ||
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kc, | ||
DataFlow::PathNode source, DataFlow::PathNode dest, ClassInstanceExpr cie | ||
| | ||
jpg.getAlgoSpec().(StringLiteral).getValue().matches("EC%") and // ECC variants such as ECDH and ECDSA | ||
source.getNode().asExpr() = jpg and | ||
dest.getNode().asExpr() = ma.getQualifier() and | ||
kc.hasFlowPath(source, dest) and | ||
DataFlow::localExprFlow(cie, ma.getArgument(0)) and | ||
ma.getArgument(0).getType() instanceof ECGenParameterSpec and | ||
getECKeySize(cie.getArgument(0).(StringLiteral).getRepresentedString()) < 256 | ||
) and | ||
msg = "Key size should be at least 256 bits for EC encryption." | ||
} | ||
|
||
from Expr e, string msg | ||
where | ||
hasShortAESKey(e, msg) or | ||
hasShortDSAKeyPair(e, msg) or | ||
hasShortRSAKeyPair(e, msg) or | ||
hasShortECKeyPair(e, msg) | ||
select e, msg |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
| InsufficientKeySize.java:9:9:9:24 | init(...) | Key size should be at least 128 bits for AES encryption. | | ||
| InsufficientKeySize.java:17:9:17:36 | initialize(...) | Key size should be at least 2048 bits for RSA encryption. | | ||
| InsufficientKeySize.java:25:9:25:36 | initialize(...) | Key size should be at least 2048 bits for DSA encryption. | | ||
| InsufficientKeySize.java:34:9:34:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:38:9:38:67 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:48:9:48:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:53:9:53:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:58:9:58:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:68:9:68:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:78:9:78:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. | | ||
| InsufficientKeySize.java:87:9:87:37 | initialize(...) | Key size should be at least 2048 bits for DH encryption. | |
93 changes: 93 additions & 0 deletions
93
java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
import java.security.KeyPairGenerator; | ||
import java.security.spec.ECGenParameterSpec; | ||
import javax.crypto.KeyGenerator; | ||
|
||
public class InsufficientKeySize { | ||
public void CryptoMethod() { | ||
KeyGenerator keyGen1 = KeyGenerator.getInstance("AES"); | ||
// BAD: Key size is less than 128 | ||
keyGen1.init(64); | ||
|
||
KeyGenerator keyGen2 = KeyGenerator.getInstance("AES"); | ||
// GOOD: Key size is no less than 128 | ||
keyGen2.init(128); | ||
|
||
KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA"); | ||
// BAD: Key size is less than 2048 | ||
keyPairGen1.initialize(1024); | ||
|
||
KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA"); | ||
// GOOD: Key size is no less than 2048 | ||
keyPairGen2.initialize(2048); | ||
|
||
KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA"); | ||
// BAD: Key size is less than 2048 | ||
keyPairGen3.initialize(1024); | ||
|
||
KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); | ||
// GOOD: Key size is no less than 2048 | ||
keyPairGen4.initialize(2048); | ||
|
||
KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); | ||
keyPairGen5.initialize(ecSpec1); | ||
|
||
KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
keyPairGen6.initialize(new ECGenParameterSpec("secp112r1")); | ||
|
||
KeyPairGenerator keyPairGen7 = KeyPairGenerator.getInstance("EC"); | ||
// GOOD: Key size is no less than 256 | ||
ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); | ||
keyPairGen7.initialize(ecSpec2); | ||
|
||
KeyPairGenerator keyPairGen8 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec3 = new ECGenParameterSpec("X9.62 prime192v2"); | ||
keyPairGen8.initialize(ecSpec3); | ||
|
||
KeyPairGenerator keyPairGen9 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec4 = new ECGenParameterSpec("X9.62 c2tnb191v3"); | ||
keyPairGen9.initialize(ecSpec4); | ||
|
||
KeyPairGenerator keyPairGen10 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec5 = new ECGenParameterSpec("sect163k1"); | ||
keyPairGen10.initialize(ecSpec5); | ||
|
||
KeyPairGenerator keyPairGen11 = KeyPairGenerator.getInstance("EC"); | ||
// GOOD: Key size is no less than 256 | ||
ECGenParameterSpec ecSpec6 = new ECGenParameterSpec("X9.62 c2tnb359v1"); | ||
keyPairGen11.initialize(ecSpec6); | ||
|
||
KeyPairGenerator keyPairGen12 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec7 = new ECGenParameterSpec("prime192v2"); | ||
keyPairGen12.initialize(ecSpec7); | ||
|
||
KeyPairGenerator keyPairGen13 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is no less than 256 | ||
ECGenParameterSpec ecSpec8 = new ECGenParameterSpec("prime256v1"); | ||
keyPairGen13.initialize(ecSpec8); | ||
|
||
KeyPairGenerator keyPairGen14 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is less than 256 | ||
ECGenParameterSpec ecSpec9 = new ECGenParameterSpec("c2tnb191v1"); | ||
keyPairGen14.initialize(ecSpec9); | ||
|
||
KeyPairGenerator keyPairGen15 = KeyPairGenerator.getInstance("EC"); | ||
// BAD: Key size is no less than 256 | ||
ECGenParameterSpec ecSpec10 = new ECGenParameterSpec("c2tnb431r1"); | ||
keyPairGen15.initialize(ecSpec10); | ||
|
||
KeyPairGenerator keyPairGen16 = KeyPairGenerator.getInstance("dh"); | ||
// BAD: Key size is less than 2048 | ||
keyPairGen16.initialize(1024); | ||
|
||
KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH"); | ||
// GOOD: Key size is no less than 2048 | ||
keyPairGen17.initialize(2048); | ||
} | ||
} |
1 change: 1 addition & 0 deletions
1
java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.qlref
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
experimental/Security/CWE/CWE-326/InsufficientKeySize.ql |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.