Skip to content

Commit 11cd0db

Browse files
committed
Python: Add concepts for public-key generation
I did spend some time to figure out how to best write `minimumSecureKeySize` predicate. I wanted to write once and for all the recommended sizes for each cryptosystem. I considered making the predicate such as ```codeql int minimumSecureKeySize() { this.getName() = "RSA" and result = 2048 or this.getName() = "DSA" and result = 2048 or this.getName() = "ECC" and result = 244 } ``` but then it would be impossible to add a new model without also being able to modify the body of this predicate -- which seems like a bad way to start off a brand new way of modeling things. So I considered if we could add it to the non-range class, such as ```codeql class RSAKeyGeneration extends KeyGeneration { RSAKeyGeneration() { this.getName() = "RSA" } override int minimumSecureKeySize() { result = 2048 } } ``` This has the major problem that when you're writing the models for a new API (and therefore extending KeyGeneration::Range), there is no way for you to see that you need to take this extra step :| (also problem about how we should define `minimumSecureKeySize` on `KeyGeneration` class then, since if we make it abstract, we effectively disable the ability to refine `KeyGeneration` since any subclass must provide an implementation.) So, therefore I ended up with this solution ;)
1 parent 4ab61bb commit 11cd0db

2 files changed

Lines changed: 109 additions & 0 deletions

File tree

python/ql/src/semmle/python/Concepts.qll

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,3 +526,90 @@ module HTTP {
526526
}
527527
}
528528
}
529+
530+
/** Provides models for cryptographic things. */
531+
module Cryptography {
532+
/** Provides models for public-key cryptography, also called asymmetric cryptography. */
533+
module PublicKey {
534+
/**
535+
* A data-flow node that generates a new key-pair for use with public-key cryptography.
536+
*
537+
* Extend this class to refine existing API models. If you want to model new APIs,
538+
* extend `KeyGeneration::Range` instead.
539+
*/
540+
class KeyGeneration extends DataFlow::Node {
541+
KeyGeneration::Range range;
542+
543+
KeyGeneration() { this = range }
544+
545+
/** Gets the name of the cryptographic algorithm (for example `"RSA"` or `"AES"`). */
546+
string getName() { result = range.getName() }
547+
548+
/** Gets the argument that specifies size of the key in bits, if available. */
549+
DataFlow::Node getKeySizeArg() { result = range.getKeySizeArg() }
550+
551+
/**
552+
* Gets the size of the key generated (in bits), as well as the `origin` that
553+
* explains how we obtained this specific key size.
554+
*/
555+
int getKeySizeWithOrigin(DataFlow::Node origin) {
556+
result = range.getKeySizeWithOrigin(origin)
557+
}
558+
559+
/** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
560+
int minimumSecureKeySize() { result = range.minimumSecureKeySize() }
561+
}
562+
563+
/** Provides classes for modeling new key-pair generation APIs. */
564+
module KeyGeneration {
565+
/**
566+
* A data-flow node that generates a new key-pair for use with public-key cryptography.
567+
*
568+
* Extend this class to model new APIs. If you want to refine existing API models,
569+
* extend `KeyGeneration` instead.
570+
*/
571+
abstract class Range extends DataFlow::Node {
572+
/** Gets the name of the cryptographic algorithm (for example `"RSA"`). */
573+
abstract string getName();
574+
575+
/** Gets the argument that specifies size of the key in bits, if available. */
576+
abstract DataFlow::Node getKeySizeArg();
577+
578+
/**
579+
* Gets the size of the key generated (in bits), as well as the `origin` that
580+
* explains how we obtained this specific key size.
581+
*/
582+
int getKeySizeWithOrigin(DataFlow::Node origin) {
583+
exists(IntegerLiteral size | origin = DataFlow::exprNode(size) |
584+
origin.(DataFlow::LocalSourceNode).flowsTo(this.getKeySizeArg()) and
585+
result = size.getValue()
586+
)
587+
}
588+
589+
/** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
590+
abstract int minimumSecureKeySize();
591+
}
592+
593+
/** A data-flow node that generates a new RSA key-pair. */
594+
abstract class RSARange extends Range {
595+
override string getName() { result = "RSA" }
596+
597+
override int minimumSecureKeySize() { result = 2048 }
598+
}
599+
600+
/** A data-flow node that generates a new DSA key-pair. */
601+
abstract class DSARange extends Range {
602+
override string getName() { result = "DSA" }
603+
604+
override int minimumSecureKeySize() { result = 2048 }
605+
}
606+
607+
/** A data-flow node that generates a new ECC key-pair. */
608+
abstract class ECCRange extends Range {
609+
override string getName() { result = "ECC" }
610+
611+
override int minimumSecureKeySize() { result = 224 }
612+
}
613+
}
614+
}
615+
}

python/ql/test/experimental/meta/ConceptsTest.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,3 +319,25 @@ class SafeAccessCheckTest extends InlineExpectationsTest {
319319
)
320320
}
321321
}
322+
323+
class PublicKeyGenerationTest extends InlineExpectationsTest {
324+
PublicKeyGenerationTest() { this = "PublicKeyGenerationTest" }
325+
326+
override string getARelevantTag() { result in ["PublicKeyGeneration", "keySize"] }
327+
328+
override predicate hasActualResult(Location location, string element, string tag, string value) {
329+
exists(location.getFile().getRelativePath()) and
330+
exists(Cryptography::PublicKey::KeyGeneration keyGen |
331+
location = keyGen.getLocation() and
332+
(
333+
element = keyGen.toString() and
334+
value = "" and
335+
tag = "PublicKeyGeneration"
336+
or
337+
element = keyGen.toString() and
338+
value = keyGen.getKeySizeWithOrigin(_).toString() and
339+
tag = "keySize"
340+
)
341+
)
342+
}
343+
}

0 commit comments

Comments
 (0)