Skip to content

Commit 668bfd3

Browse files
committed
Python: Support EC keygen without class-instance for cryptography
I also added a new test to show off how what the origin ends up looking like... I think it looks ok
1 parent 3ceb8bb commit 668bfd3

File tree

5 files changed

+56
-5
lines changed

5 files changed

+56
-5
lines changed

python/ql/src/semmle/python/frameworks/Cryptography.qll

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ private module CryptographyModel {
2222
* Gets a predefined curve class from
2323
* `cryptography.hazmat.primitives.asymmetric.ec` with a specific key size (in bits).
2424
*/
25-
private DataFlow::Node curveClassWithKeySize(int keySize) {
25+
private API::Node predefinedCurveClass(int keySize) {
2626
exists(string curveName |
2727
result =
2828
API::moduleImport("cryptography")
@@ -31,7 +31,6 @@ private module CryptographyModel {
3131
.getMember("asymmetric")
3232
.getMember("ec")
3333
.getMember(curveName)
34-
.getAUse()
3534
|
3635
// obtained by manually looking at source code in
3736
// https://github.com/pyca/cryptography/blob/cba69f1922803f4f29a3fde01741890d88b8e217/src/cryptography/hazmat/primitives/asymmetric/ec.py#L208-L300
@@ -75,13 +74,30 @@ private module CryptographyModel {
7574
)
7675
}
7776

77+
/** Gets a reference to a predefined curve class with a specific key size (in bits), as well as the origin of the class. */
78+
private DataFlow::LocalSourceNode curveClassWithKeySize(
79+
DataFlow::TypeTracker t, int keySize, DataFlow::Node origin
80+
) {
81+
t.start() and
82+
result = predefinedCurveClass(keySize).getAnImmediateUse() and
83+
origin = result
84+
or
85+
exists(DataFlow::TypeTracker t2 |
86+
result = curveClassWithKeySize(t2, keySize, origin).track(t2, t)
87+
)
88+
}
89+
90+
/** Gets a reference to a predefined curve class with a specific key size (in bits), as well as the origin of the class. */
91+
DataFlow::Node curveClassWithKeySize(int keySize, DataFlow::Node origin) {
92+
curveClassWithKeySize(DataFlow::TypeTracker::end(), keySize, origin).flowsTo(result)
93+
}
94+
7895
/** Gets a reference to a predefined curve class instance with a specific key size (in bits), as well as the origin of the class. */
7996
private DataFlow::LocalSourceNode curveClassInstanceWithKeySize(
8097
DataFlow::TypeTracker t, int keySize, DataFlow::Node origin
8198
) {
8299
t.start() and
83-
result.(DataFlow::CallCfgNode).getFunction() = curveClassWithKeySize(keySize) and
84-
origin = result
100+
result.(DataFlow::CallCfgNode).getFunction() = curveClassWithKeySize(keySize, origin)
85101
or
86102
exists(DataFlow::TypeTracker t2 |
87103
result = curveClassInstanceWithKeySize(t2, keySize, origin).track(t2, t)
@@ -164,6 +180,8 @@ private module CryptographyModel {
164180

165181
override int getKeySizeWithOrigin(DataFlow::Node origin) {
166182
this.getCurveArg() = Ecc::curveClassInstanceWithKeySize(result, origin)
183+
or
184+
this.getCurveArg() = Ecc::curveClassWithKeySize(result, origin)
167185
}
168186

169187
// Note: There is not really a key-size argument, since it's always specified by the curve.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| ec_keygen_origin.py:8:1:8:45 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:8:31:8:42 | ControlFlowNode for Attribute |
2+
| ec_keygen_origin.py:9:1:9:43 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:9:31:9:42 | ControlFlowNode for Attribute |
3+
| ec_keygen_origin.py:12:1:12:36 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:11:9:11:20 | ControlFlowNode for Attribute |
4+
| ec_keygen_origin.py:15:1:15:39 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:11:9:11:20 | ControlFlowNode for Attribute |
5+
| ec_keygen_origin.py:20:1:20:32 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:6:58:6:66 | ControlFlowNode for ImportMember |
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import semmle.python.dataflow.new.DataFlow
2+
import semmle.python.Concepts
3+
4+
from Cryptography::PublicKey::KeyGeneration keyGen, int keySize, DataFlow::Node origin
5+
where
6+
keyGen.getLocation().getFile().getShortName() = "ec_keygen_origin.py" and
7+
keySize = keyGen.getKeySizeWithOrigin(origin)
8+
select keyGen, keySize, origin
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Since key-size is not specified explicitly as an integer for the predefined
2+
# classes in the `cryptography.hazmat.primitives.asymmetric.ec` module, we need
3+
# special handling of the origin... this test is simply to show off how we handle this.
4+
5+
from cryptography.hazmat.primitives.asymmetric import ec
6+
from cryptography.hazmat.primitives.asymmetric.ec import SECP384R1
7+
8+
ec.generate_private_key(curve=ec.SECP384R1()) # $ PublicKeyGeneration keySize=384
9+
ec.generate_private_key(curve=ec.SECP384R1) # $ PublicKeyGeneration keySize=384
10+
11+
alias = ec.SECP384R1
12+
ec.generate_private_key(curve=alias) # $ PublicKeyGeneration keySize=384
13+
14+
instance = alias()
15+
ec.generate_private_key(curve=instance) # $ PublicKeyGeneration keySize=384
16+
17+
18+
x = SECP384R1
19+
y = x
20+
ec.generate_private_key(curve=y) # $ PublicKeyGeneration keySize=384

python/ql/test/library-tests/frameworks/cryptography/test_ec.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77

88
private_key = ec.generate_private_key(curve=ec.SECP384R1()) # $ PublicKeyGeneration keySize=384
9-
private_key = ec.generate_private_key(curve=ec.SECP384R1) # $ MISSING: PublicKeyGeneration keySize=384
9+
private_key = ec.generate_private_key(curve=ec.SECP384R1) # $ PublicKeyGeneration keySize=384
1010
public_key = private_key.public_key()
1111

1212
HASH_ALGORITHM = hashes.SHA256()

0 commit comments

Comments
 (0)