Skip to content

Commit 5e694b5

Browse files
jcogs33jcogs33
authored
Merge pull request github#11192 from jcogs33/jcogs33/share-key-sizes
Share encryption key sizes between Java and Python
2 parents 2c08b95 + f54480b commit 5e694b5

File tree

9 files changed

+100
-21
lines changed

9 files changed

+100
-21
lines changed

‎config/identical-files.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,5 +580,9 @@
580580
"IncompleteMultiCharacterSanitization JS/Ruby": [
581581
"javascript/ql/lib/semmle/javascript/security/IncompleteMultiCharacterSanitizationQuery.qll",
582582
"ruby/ql/lib/codeql/ruby/security/IncompleteMultiCharacterSanitizationQuery.qll"
583+
],
584+
"EncryptionKeySizes Python/Java": [
585+
"python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll",
586+
"java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll"
583587
]
584588
}

‎java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
private import semmle.code.java.security.Encryption
44
private import semmle.code.java.dataflow.DataFlow
5+
private import semmle.code.java.security.internal.EncryptionKeySizes
56

67
/** A source for an insufficient key size. */
78
abstract class InsufficientKeySizeSource extends DataFlow::Node {
@@ -21,39 +22,67 @@ private module Asymmetric {
2122
private module NonEllipticCurve {
2223
/** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */
2324
private class Source extends InsufficientKeySizeSource {
24-
Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() }
25+
string algoName;
2526

26-
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
27+
Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize(algoName) }
28+
29+
override predicate hasState(DataFlow::FlowState state) {
30+
state = getMinKeySize(algoName).toString()
31+
}
2732
}
2833

2934
/** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */
3035
private class Sink extends InsufficientKeySizeSink {
36+
string algoName;
37+
3138
Sink() {
3239
exists(KeyPairGenInit kpgInit, KeyPairGen kpg |
33-
kpg.getAlgoName().matches(["RSA", "DSA", "DH"]) and
40+
algoName in ["RSA", "DSA", "DH"] and
41+
kpg.getAlgoName() = algoName and
3442
DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and
3543
this.asExpr() = kpgInit.getKeySizeArg()
3644
)
3745
or
38-
exists(Spec spec | this.asExpr() = spec.getKeySizeArg())
46+
exists(Spec spec | this.asExpr() = spec.getKeySizeArg() and algoName = spec.getAlgoName())
3947
}
4048

41-
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
49+
override predicate hasState(DataFlow::FlowState state) {
50+
state = getMinKeySize(algoName).toString()
51+
}
4252
}
4353

4454
/** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */
45-
private int getMinKeySize() { result = 2048 }
55+
private int getMinKeySize(string algoName) {
56+
algoName = "RSA" and
57+
result = minSecureKeySizeRsa()
58+
or
59+
algoName = "DSA" and
60+
result = minSecureKeySizeDsa()
61+
or
62+
algoName = "DH" and
63+
result = minSecureKeySizeDh()
64+
}
4665

4766
/** An instance of an RSA, DSA, or DH algorithm specification. */
4867
private class Spec extends ClassInstanceExpr {
68+
string algoName;
69+
4970
Spec() {
50-
this.getConstructedType() instanceof RsaKeyGenParameterSpec or
51-
this.getConstructedType() instanceof DsaGenParameterSpec or
52-
this.getConstructedType() instanceof DhGenParameterSpec
71+
this.getConstructedType() instanceof RsaKeyGenParameterSpec and
72+
algoName = "RSA"
73+
or
74+
this.getConstructedType() instanceof DsaGenParameterSpec and
75+
algoName = "DSA"
76+
or
77+
this.getConstructedType() instanceof DhGenParameterSpec and
78+
algoName = "DH"
5379
}
5480

5581
/** Gets the `keysize` argument of this instance. */
5682
Argument getKeySizeArg() { result = this.getArgument(0) }
83+
84+
/** Gets the algorithm name of this spec. */
85+
string getAlgoName() { result = algoName }
5786
}
5887
}
5988

@@ -87,7 +116,7 @@ private module Asymmetric {
87116
}
88117

89118
/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */
90-
private int getMinKeySize() { result = 256 }
119+
private int getMinKeySize() { result = minSecureKeySizeEcc() }
91120

92121
/** Returns the key size from an EC algorithm's curve name string */
93122
bindingset[algorithm]
@@ -168,7 +197,7 @@ private module Symmetric {
168197
}
169198

170199
/** Returns the minimum recommended key size for AES algorithms. */
171-
private int getMinKeySize() { result = 128 }
200+
private int getMinKeySize() { result = minSecureKeySizeAes() }
172201

173202
/** A call to the `init` method declared in `javax.crypto.KeyGenerator`. */
174203
private class KeyGenInit extends MethodAccess {

‎java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* INTERNAL: Do not use.
3+
*
4+
* Provides predicates for recommended encryption key sizes.
5+
* Such that we can share this logic across our CodeQL analysis of different languages.
6+
*/
7+
8+
/** Returns the minimum recommended key size for RSA. */
9+
int minSecureKeySizeRsa() { result = 2048 }
10+
11+
/** Returns the minimum recommended key size for DSA. */
12+
int minSecureKeySizeDsa() { result = 2048 }
13+
14+
/** Returns the minimum recommended key size for DH. */
15+
int minSecureKeySizeDh() { result = 2048 }
16+
17+
/** Returns the minimum recommended key size for elliptic curve cryptography. */
18+
int minSecureKeySizeEcc() { result = 256 }
19+
20+
/** Returns the minimum recommended key size for AES. */
21+
int minSecureKeySizeAes() { result = 128 }

‎python/ql/lib/semmle/python/Concepts.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
99
private import semmle.python.dataflow.new.RemoteFlowSources
1010
private import semmle.python.dataflow.new.TaintTracking
1111
private import semmle.python.Frameworks
12+
private import semmle.python.security.internal.EncryptionKeySizes
1213

1314
/**
1415
* A data-flow node that executes an operating system command,
@@ -1141,21 +1142,21 @@ module Cryptography {
11411142
abstract class RsaRange extends Range {
11421143
final override string getName() { result = "RSA" }
11431144

1144-
final override int minimumSecureKeySize() { result = 2048 }
1145+
final override int minimumSecureKeySize() { result = minSecureKeySizeRsa() }
11451146
}
11461147

11471148
/** A data-flow node that generates a new DSA key-pair. */
11481149
abstract class DsaRange extends Range {
11491150
final override string getName() { result = "DSA" }
11501151

1151-
final override int minimumSecureKeySize() { result = 2048 }
1152+
final override int minimumSecureKeySize() { result = minSecureKeySizeDsa() }
11521153
}
11531154

11541155
/** A data-flow node that generates a new ECC key-pair. */
11551156
abstract class EccRange extends Range {
11561157
final override string getName() { result = "ECC" }
11571158

1158-
final override int minimumSecureKeySize() { result = 224 }
1159+
final override int minimumSecureKeySize() { result = minSecureKeySizeEcc() }
11591160
}
11601161
}
11611162
}

‎python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* INTERNAL: Do not use.
3+
*
4+
* Provides predicates for recommended encryption key sizes.
5+
* Such that we can share this logic across our CodeQL analysis of different languages.
6+
*/
7+
8+
/** Returns the minimum recommended key size for RSA. */
9+
int minSecureKeySizeRsa() { result = 2048 }
10+
11+
/** Returns the minimum recommended key size for DSA. */
12+
int minSecureKeySizeDsa() { result = 2048 }
13+
14+
/** Returns the minimum recommended key size for DH. */
15+
int minSecureKeySizeDh() { result = 2048 }
16+
17+
/** Returns the minimum recommended key size for elliptic curve cryptography. */
18+
int minSecureKeySizeEcc() { result = 256 }
19+
20+
/** Returns the minimum recommended key size for AES. */
21+
int minSecureKeySizeAes() { result = 128 }

‎python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ As computational power increases, the ability to break ciphers grows and keys ne
1111
<p>
1212
The three main asymmetric key algorithms currently in use are Rivest–Shamir–Adleman (RSA) cryptography, Digital Signature Algorithm (DSA), and Elliptic-curve cryptography (ECC).
1313
With current technology, key sizes of 2048 bits for RSA and DSA,
14-
or 224 bits for ECC, are regarded as unbreakable.
14+
or 256 bits for ECC, are regarded as unbreakable.
1515
</p>
1616
</overview>
1717

1818
<recommendation>
1919
<p>
20-
Increase the key size to the recommended amount or larger. For RSA or DSA this is at least 2048 bits, for ECC this is at least 224 bits.
20+
Increase the key size to the recommended amount or larger. For RSA or DSA this is at least 2048 bits, for ECC this is at least 256 bits.
2121
</p>
2222
</recommendation>
2323

@@ -45,4 +45,3 @@ Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Len
4545
</li>
4646
</references>
4747
</qhelp>
48-

‎python/ql/src/change-notes/2022-11-29-ecc-min-secure-keysize.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Bumped the minimum keysize we consider secure for elliptic curve cryptography from 224 to 256 bits, following current best practices. This might effect results from the _Use of weak cryptographic key_ (`py/weak-crypto-key`) query.

‎python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/WeakCryptoKey.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
| weak_crypto.py:68:1:68:21 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
2-
| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 |
2+
| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 256 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 224 |
33
| weak_crypto.py:70:1:70:28 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
44
| weak_crypto.py:72:1:72:30 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
5-
| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 |
5+
| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 256 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 224 |
66
| weak_crypto.py:74:1:74:37 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
77
| weak_crypto.py:76:1:76:22 | ControlFlowNode for Attribute() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
88
| weak_crypto.py:77:1:77:22 | ControlFlowNode for Attribute() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |

‎python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/weak_crypto.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919

2020
BIG = 10000
2121

22-
EC_WEAK = ec.SECT163K1() # has key size of 163
23-
EC_OK = ec.SECP224R1()
22+
EC_WEAK = ec.SECP224R1()
23+
EC_OK = ec.SECP256R1()
2424
EC_STRONG = ec.SECP384R1()
2525
EC_BIG = ec.SECT571R1()
2626

0 commit comments

Comments
 (0)