Merge pull request #5075 from RasmusWL/crypto

Python: Port py/weak-crypto-key to use type-tracking

python/change-notes/2021-02-02-port-weak-crypto-key-query.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* Updated _Use of weak cryptographic key_ (`py/weak-crypto-key`) query to use the new type-tracking approach instead of points-to analysis. You may see differences in the results found by the query, but overall this change should result in a more robust and accurate analysis.
+* Renamed the query file for _Use of weak cryptographic key_ (`py/weak-crypto-key`) from `WeakCrypto.ql` to `WeakCryptoKey.ql` (in the `python/ql/src/Security/CWE-326/` folder). This will affect any custom query suites that include or exclude this query using its path.

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

@@ -0,0 +1,24 @@
+/**
+ * @name Use of weak cryptographic key
+ * @description Use of a cryptographic key that is too small may allow the encryption to be broken.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id py/weak-crypto-key
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+import python
+import semmle.python.Concepts
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.filters.Tests
+
+from Cryptography::PublicKey::KeyGeneration keyGen, int keySize, DataFlow::Node origin
+where
+  keySize = keyGen.getKeySizeWithOrigin(origin) and
+  keySize < keyGen.minimumSecureKeySize() and
+  not origin.getScope().getScope*() instanceof TestScope
+select keyGen,
+  "Creation of an " + keyGen.getName() + " key uses $@ bits, which is below " +
+    keyGen.minimumSecureKeySize() + " and considered breakable.", origin, keySize.toString()

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

@@ -526,3 +526,116 @@ module HTTP {
     }
   }
 }
+
+/** Provides models for cryptographic things. */
+module Cryptography {
+  /** Provides models for public-key cryptography, also called asymmetric cryptography. */
+  module PublicKey {
+    /**
+     * A data-flow node that generates a new key-pair for use with public-key cryptography.
+     *
+     * Extend this class to refine existing API models. If you want to model new APIs,
+     * extend `KeyGeneration::Range` instead.
+     */
+    class KeyGeneration extends DataFlow::Node {
+      KeyGeneration::Range range;
+
+      KeyGeneration() { this = range }
+
+      /** Gets the name of the cryptographic algorithm (for example `"RSA"` or `"AES"`). */
+      string getName() { result = range.getName() }
+
+      /** Gets the argument that specifies the size of the key in bits, if available. */
+      DataFlow::Node getKeySizeArg() { result = range.getKeySizeArg() }
+
+      /**
+       * Gets the size of the key generated (in bits), as well as the `origin` that
+       * explains how we obtained this specific key size.
+       */
+      int getKeySizeWithOrigin(DataFlow::Node origin) {
+        result = range.getKeySizeWithOrigin(origin)
+      }
+
+      /** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
+      int minimumSecureKeySize() { result = range.minimumSecureKeySize() }
+    }
+
+    /** Provides classes for modeling new key-pair generation APIs. */
+    module KeyGeneration {
+      /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
+      DataFlow::LocalSourceNode keysizeBacktracker(DataFlow::TypeBackTracker t, DataFlow::Node arg) {
+        t.start() and
+        arg = any(KeyGeneration::Range r).getKeySizeArg() and
+        result = arg.getALocalSource()
+        or
+        // Due to bad performance when using normal setup with we have inlined that code and forced a join
+        exists(DataFlow::TypeBackTracker t2 |
+          exists(DataFlow::StepSummary summary |
+            keysizeBacktracker_first_join(t2, arg, result, summary) and
+            t = t2.prepend(summary)
+          )
+        )
+      }
+
+      pragma[nomagic]
+      private predicate keysizeBacktracker_first_join(
+        DataFlow::TypeBackTracker t2, DataFlow::Node arg, DataFlow::Node res,
+        DataFlow::StepSummary summary
+      ) {
+        DataFlow::StepSummary::step(res, keysizeBacktracker(t2, arg), summary)
+      }
+
+      /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
+      DataFlow::LocalSourceNode keysizeBacktracker(DataFlow::Node arg) {
+        result = keysizeBacktracker(DataFlow::TypeBackTracker::end(), arg)
+      }
+
+      /**
+       * A data-flow node that generates a new key-pair for use with public-key cryptography.
+       *
+       * Extend this class to model new APIs. If you want to refine existing API models,
+       * extend `KeyGeneration` instead.
+       */
+      abstract class Range extends DataFlow::Node {
+        /** Gets the name of the cryptographic algorithm (for example `"RSA"`). */
+        abstract string getName();
+
+        /** Gets the argument that specifies the size of the key in bits, if available. */
+        abstract DataFlow::Node getKeySizeArg();
+
+        /**
+         * Gets the size of the key generated (in bits), as well as the `origin` that
+         * explains how we obtained this specific key size.
+         */
+        int getKeySizeWithOrigin(DataFlow::Node origin) {
+          origin = keysizeBacktracker(this.getKeySizeArg()) and
+          result = origin.asExpr().(IntegerLiteral).getValue()
+        }
+
+        /** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
+        abstract int minimumSecureKeySize();
+      }
+
+      /** A data-flow node that generates a new RSA key-pair. */
+      abstract class RsaRange extends Range {
+        final override string getName() { result = "RSA" }
+
+        final override int minimumSecureKeySize() { result = 2048 }
+      }
+
+      /** A data-flow node that generates a new DSA key-pair. */
+      abstract class DsaRange extends Range {
+        final override string getName() { result = "DSA" }
+
+        final override int minimumSecureKeySize() { result = 2048 }
+      }
+
+      /** A data-flow node that generates a new ECC key-pair. */
+      abstract class EccRange extends Range {
+        final override string getName() { result = "ECC" }
+
+        final override int minimumSecureKeySize() { result = 224 }
+      }
+    }
+  }
+}

python/ql/src/semmle/python/Frameworks.qll

@@ -2,6 +2,8 @@
  * Helper file that imports all framework modeling.
  */
 
+private import semmle.python.frameworks.Cryptodome
+private import semmle.python.frameworks.Cryptography
 private import semmle.python.frameworks.Dill
 private import semmle.python.frameworks.Django
 private import semmle.python.frameworks.Fabric

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

@@ -0,0 +1,104 @@
+/**
+ * Provides classes modeling security-relevant aspects of
+ * - the `pycryptodome` PyPI package (imported as `Crypto`)
+ * - the `pycryptodomex` PyPI package (imported as `Cryptodome`)
+ * See https://pycryptodome.readthedocs.io/en/latest/.
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.Concepts
+private import semmle.python.ApiGraphs
+
+/**
+ * Provides models for
+ * - the `pycryptodome` PyPI package (imported as `Crypto`)
+ * - the `pycryptodomex` PyPI package (imported as `Cryptodome`)
+ * See https://pycryptodome.readthedocs.io/en/latest/
+ */
+private module CryptodomeModel {
+  // ---------------------------------------------------------------------------
+  /**
+   * A call to `Cryptodome.PublicKey.RSA.generate`/`Crypto.PublicKey.RSA.generate`
+   *
+   * See https://pycryptodome.readthedocs.io/en/latest/src/public_key/rsa.html#Crypto.PublicKey.RSA.generate
+   */
+  class CryptodomePublicKeyRsaGenerateCall extends Cryptography::PublicKey::KeyGeneration::RsaRange,
+    DataFlow::CallCfgNode {
+    CryptodomePublicKeyRsaGenerateCall() {
+      this =
+        API::moduleImport(["Crypto", "Cryptodome"])
+            .getMember("PublicKey")
+            .getMember("RSA")
+            .getMember("generate")
+            .getACall()
+    }
+
+    override DataFlow::Node getKeySizeArg() {
+      result in [this.getArg(0), this.getArgByName("bits")]
+    }
+  }
+
+  /**
+   * A call to `Cryptodome.PublicKey.DSA.generate`/`Crypto.PublicKey.DSA.generate`
+   *
+   * See https://pycryptodome.readthedocs.io/en/latest/src/public_key/dsa.html#Crypto.PublicKey.DSA.generate
+   */
+  class CryptodomePublicKeyDsaGenerateCall extends Cryptography::PublicKey::KeyGeneration::DsaRange,
+    DataFlow::CallCfgNode {
+    CryptodomePublicKeyDsaGenerateCall() {
+      this =
+        API::moduleImport(["Crypto", "Cryptodome"])
+            .getMember("PublicKey")
+            .getMember("DSA")
+            .getMember("generate")
+            .getACall()
+    }
+
+    override DataFlow::Node getKeySizeArg() {
+      result in [this.getArg(0), this.getArgByName("bits")]
+    }
+  }
+
+  /**
+   * A call to `Cryptodome.PublicKey.ECC.generate`/`Crypto.PublicKey.ECC.generate`
+   *
+   * See https://pycryptodome.readthedocs.io/en/latest/src/public_key/ecc.html#Crypto.PublicKey.ECC.generate
+   */
+  class CryptodomePublicKeyEccGenerateCall extends Cryptography::PublicKey::KeyGeneration::EccRange,
+    DataFlow::CallCfgNode {
+    CryptodomePublicKeyEccGenerateCall() {
+      this =
+        API::moduleImport(["Crypto", "Cryptodome"])
+            .getMember("PublicKey")
+            .getMember("ECC")
+            .getMember("generate")
+            .getACall()
+    }
+
+    /** Gets the argument that specifies the curve to use (a string). */
+    DataFlow::Node getCurveArg() { result = this.getArgByName("curve") }
+
+    /** Gets the name of the curve to use, as well as the origin that explains how we obtained this name. */
+    string getCurveWithOrigin(DataFlow::Node origin) {
+      exists(StrConst str | origin = DataFlow::exprNode(str) |
+        origin = this.getCurveArg().getALocalSource() and
+        result = str.getText()
+      )
+    }
+
+    override int getKeySizeWithOrigin(DataFlow::Node origin) {
+      exists(string curve | curve = this.getCurveWithOrigin(origin) |
+        // using list from https://pycryptodome.readthedocs.io/en/latest/src/public_key/ecc.html
+        curve in ["NIST P-256", "p256", "P-256", "prime256v1", "secp256r1"] and result = 256
+        or
+        curve in ["NIST P-384", "p384", "P-384", "prime384v1", "secp384r1"] and result = 384
+        or
+        curve in ["NIST P-521", "p521", "P-521", "prime521v1", "secp521r1"] and result = 521
+      )
+    }
+
+    // Note: There is not really a key-size argument, since it's always specified by the curve.
+    override DataFlow::Node getKeySizeArg() { none() }
+  }
+}

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

@@ -0,0 +1,184 @@
+/**
+ * Provides classes modeling security-relevant aspects of the `cryptography` PyPI package.
+ * See https://cryptography.io/en/latest/.
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.Concepts
+private import semmle.python.ApiGraphs
+
+/**
+ * Provides models for the `cryptography` PyPI package.
+ * See https://cryptography.io/en/latest/.
+ */
+private module CryptographyModel {
+  /**
+   * Provides helper predicates for the eliptic curve cryptography parts in
+   * `cryptography.hazmat.primitives.asymmetric.ec`.
+   */
+  module Ecc {
+    /**
+     * Gets a predefined curve class from
+     * `cryptography.hazmat.primitives.asymmetric.ec` with a specific key size (in bits).
+     */
+    private DataFlow::Node curveClassWithKeySize(int keySize) {
+      exists(string curveName |
+        result =
+          API::moduleImport("cryptography")
+              .getMember("hazmat")
+              .getMember("primitives")
+              .getMember("asymmetric")
+              .getMember("ec")
+              .getMember(curveName)
+              .getAUse()
+      |
+        // obtained by manually looking at source code in
+        // https://github.com/pyca/cryptography/blob/cba69f1922803f4f29a3fde01741890d88b8e217/src/cryptography/hazmat/primitives/asymmetric/ec.py#L208-L300
+        curveName = "SECT571R1" and keySize = 570 // Indeed the numbers do not match.
+        or
+        curveName = "SECT409R1" and keySize = 409
+        or
+        curveName = "SECT283R1" and keySize = 283
+        or
+        curveName = "SECT233R1" and keySize = 233
+        or
+        curveName = "SECT163R2" and keySize = 163
+        or
+        curveName = "SECT571K1" and keySize = 571
+        or
+        curveName = "SECT409K1" and keySize = 409
+        or
+        curveName = "SECT283K1" and keySize = 283
+        or
+        curveName = "SECT233K1" and keySize = 233
+        or
+        curveName = "SECT163K1" and keySize = 163
+        or
+        curveName = "SECP521R1" and keySize = 521
+        or
+        curveName = "SECP384R1" and keySize = 384
+        or
+        curveName = "SECP256R1" and keySize = 256
+        or
+        curveName = "SECP256K1" and keySize = 256
+        or
+        curveName = "SECP224R1" and keySize = 224
+        or
+        curveName = "SECP192R1" and keySize = 192
+        or
+        curveName = "BrainpoolP256R1" and keySize = 256
+        or
+        curveName = "BrainpoolP384R1" and keySize = 384
+        or
+        curveName = "BrainpoolP512R1" and keySize = 512
+      )
+    }
+
+    /** Gets a reference to a predefined curve class instance with a specific key size (in bits), as well as the origin of the class. */
+    private DataFlow::Node curveClassInstanceWithKeySize(
+      DataFlow::TypeTracker t, int keySize, DataFlow::Node origin
+    ) {
+      t.start() and
+      result.asCfgNode().(CallNode).getFunction() = curveClassWithKeySize(keySize).asCfgNode() and
+      origin = result
+      or
+      // Due to bad performance when using normal setup with we have inlined that code and forced a join
+      exists(DataFlow::TypeTracker t2 |
+        exists(DataFlow::StepSummary summary |
+          curveClassInstanceWithKeySize_first_join(t2, keySize, origin, result, summary) and
+          t = t2.append(summary)
+        )
+      )
+    }
+
+    pragma[nomagic]
+    private predicate curveClassInstanceWithKeySize_first_join(
+      DataFlow::TypeTracker t2, int keySize, DataFlow::Node origin, DataFlow::Node res,
+      DataFlow::StepSummary summary
+    ) {
+      DataFlow::StepSummary::step(curveClassInstanceWithKeySize(t2, keySize, origin), res, summary)
+    }
+
+    /** Gets a reference to a predefined curve class instance with a specific key size (in bits), as well as the origin of the class. */
+    DataFlow::Node curveClassInstanceWithKeySize(int keySize, DataFlow::Node origin) {
+      result = curveClassInstanceWithKeySize(DataFlow::TypeTracker::end(), keySize, origin)
+    }
+  }
+
+  // ---------------------------------------------------------------------------
+  /**
+   * A call to `cryptography.hazmat.primitives.asymmetric.rsa.generate_private_key`
+   *
+   * See https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa.html#cryptography.hazmat.primitives.asymmetric.rsa.generate_private_key
+   */
+  class CryptographyRsaGeneratePrivateKeyCall extends Cryptography::PublicKey::KeyGeneration::RsaRange,
+    DataFlow::CallCfgNode {
+    CryptographyRsaGeneratePrivateKeyCall() {
+      this =
+        API::moduleImport("cryptography")
+            .getMember("hazmat")
+            .getMember("primitives")
+            .getMember("asymmetric")
+            .getMember("rsa")
+            .getMember("generate_private_key")
+            .getACall()
+    }
+
+    override DataFlow::Node getKeySizeArg() {
+      result in [this.getArg(1), this.getArgByName("key_size")]
+    }
+  }
+
+  /**
+   * A call to `cryptography.hazmat.primitives.asymmetric.dsa.generate_private_key`
+   *
+   * See https://cryptography.io/en/latest/hazmat/primitives/asymmetric/dsa.html#cryptography.hazmat.primitives.asymmetric.dsa.generate_private_key
+   */
+  class CryptographyDsaGeneratePrivateKeyCall extends Cryptography::PublicKey::KeyGeneration::DsaRange,
+    DataFlow::CallCfgNode {
+    CryptographyDsaGeneratePrivateKeyCall() {
+      this =
+        API::moduleImport("cryptography")
+            .getMember("hazmat")
+            .getMember("primitives")
+            .getMember("asymmetric")
+            .getMember("dsa")
+            .getMember("generate_private_key")
+            .getACall()
+    }
+
+    override DataFlow::Node getKeySizeArg() {
+      result in [this.getArg(0), this.getArgByName("key_size")]
+    }
+  }
+
+  /**
+   * A call to `cryptography.hazmat.primitives.asymmetric.ec.generate_private_key`
+   *
+   * See https://cryptography.io/en/latest/hazmat/primitives/asymmetric/ec.html#cryptography.hazmat.primitives.asymmetric.ec.generate_private_key
+   */
+  class CryptographyEcGeneratePrivateKeyCall extends Cryptography::PublicKey::KeyGeneration::EccRange,
+    DataFlow::CallCfgNode {
+    CryptographyEcGeneratePrivateKeyCall() {
+      this =
+        API::moduleImport("cryptography")
+            .getMember("hazmat")
+            .getMember("primitives")
+            .getMember("asymmetric")
+            .getMember("ec")
+            .getMember("generate_private_key")
+            .getACall()
+    }
+
+    /** Gets the argument that specifies the curve to use. */
+    DataFlow::Node getCurveArg() { result in [this.getArg(0), this.getArgByName("curve")] }
+
+    override int getKeySizeWithOrigin(DataFlow::Node origin) {
+      this.getCurveArg() = Ecc::curveClassInstanceWithKeySize(result, origin)
+    }
+
+    // Note: There is not really a key-size argument, since it's always specified by the curve.
+    override DataFlow::Node getKeySizeArg() { none() }
+  }
+}

python/ql/test/experimental/library-tests/frameworks/crypto/ConceptsTest.ql

@@ -0,0 +1,2 @@
+import python
+import experimental.meta.ConceptsTest

python/ql/test/experimental/library-tests/frameworks/crypto/test_dsa.py

@@ -0,0 +1,41 @@
+# DSA is a public-key algorithm for signing messages.
+# Following example at https://pycryptodome.readthedocs.io/en/latest/src/signature/dsa.html
+
+from Crypto.PublicKey import DSA
+from Crypto.Signature import DSS
+from Crypto.Hash import SHA256
+
+
+private_key = DSA.generate(2048) # $ PublicKeyGeneration keySize=2048
+public_key = private_key.publickey()
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+
+message = b"message"
+
+signer = DSS.new(private_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+signature = signer.sign(hasher)
+
+print("signature={}".format(signature))
+
+print()
+
+verifier = DSS.new(public_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+verifier.verify(hasher, signature)
+print("Signature verified (as expected)")
+
+try:
+    hasher = SHA256.new(b"other message")
+    verifier.verify(hasher, signature)
+    raise Exception("Signature verified (unexpected)")
+except ValueError:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/crypto/test_ec.py

@@ -0,0 +1,38 @@
+from Crypto.PublicKey import ECC
+from Crypto.Signature import DSS
+from Crypto.Hash import SHA256
+
+
+private_key = ECC.generate(curve="P-256") # $ PublicKeyGeneration keySize=256
+public_key = private_key.public_key()
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+
+message = b"message"
+
+signer = DSS.new(private_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+signature = signer.sign(hasher)
+
+print("signature={}".format(signature))
+
+print()
+
+verifier = DSS.new(public_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+verifier.verify(hasher, signature)
+print("Signature verified (as expected)")
+
+try:
+    hasher = SHA256.new(b"other message")
+    verifier.verify(hasher, signature)
+    raise Exception("Signature verified (unexpected)")
+except ValueError:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/crypto/test_rsa.py

@@ -0,0 +1,73 @@
+# RSA is a public-key algorithm for encrypting and signing messages.
+
+from Crypto.PublicKey import RSA
+from Crypto.Cipher import PKCS1_OAEP
+from Crypto.Signature import pss
+from Crypto.Hash import SHA256
+
+private_key = RSA.generate(2048) # $ PublicKeyGeneration keySize=2048
+
+# These 2 methods do the same
+public_key = private_key.publickey()
+public_key = private_key.public_key()
+
+# ------------------------------------------------------------------------------
+# encrypt/decrypt
+# ------------------------------------------------------------------------------
+
+print("encrypt/decrypt")
+
+secret_message = b"secret message"
+
+# Following example at https://pycryptodome.readthedocs.io/en/latest/src/examples.html#encrypt-data-with-rsa
+
+encrypt_cipher = PKCS1_OAEP.new(public_key)
+
+encrypted = encrypt_cipher.encrypt(secret_message)
+
+print("encrypted={}".format(encrypted))
+
+print()
+
+decrypt_cipher = PKCS1_OAEP.new(private_key)
+
+decrypted = decrypt_cipher.decrypt(
+    encrypted,
+)
+
+print("decrypted={}".format(decrypted))
+assert decrypted == secret_message
+
+print("\n---\n")
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+
+message = b"message"
+
+signer = pss.new(private_key)
+
+hasher = SHA256.new(message)
+signature = signer.sign(hasher)
+
+print("signature={}".format(signature))
+
+print()
+
+
+verifier = pss.new(public_key)
+hasher = SHA256.new(message)
+verifier.verify(hasher, signature)
+print("Signature verified (as expected)")
+
+try:
+    verifier = pss.new(public_key)
+    hasher = SHA256.new(b"other message")
+    verifier.verify(hasher, signature)
+    raise Exception("Signature verified (unexpected)")
+except ValueError:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/cryptodome/ConceptsTest.ql

@@ -0,0 +1,2 @@
+import python
+import experimental.meta.ConceptsTest

python/ql/test/experimental/library-tests/frameworks/cryptodome/test_dsa.py

@@ -0,0 +1,41 @@
+# DSA is a public-key algorithm for signing messages.
+# Following example at https://pycryptodome.readthedocs.io/en/latest/src/signature/dsa.html
+
+from Cryptodome.PublicKey import DSA
+from Cryptodome.Signature import DSS
+from Cryptodome.Hash import SHA256
+
+
+private_key = DSA.generate(2048) # $ PublicKeyGeneration keySize=2048
+public_key = private_key.publickey()
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+
+message = b"message"
+
+signer = DSS.new(private_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+signature = signer.sign(hasher)
+
+print("signature={}".format(signature))
+
+print()
+
+verifier = DSS.new(public_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+verifier.verify(hasher, signature)
+print("Signature verified (as expected)")
+
+try:
+    hasher = SHA256.new(b"other message")
+    verifier.verify(hasher, signature)
+    raise Exception("Signature verified (unexpected)")
+except ValueError:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/cryptodome/test_ec.py

@@ -0,0 +1,38 @@
+from Cryptodome.PublicKey import ECC
+from Cryptodome.Signature import DSS
+from Cryptodome.Hash import SHA256
+
+
+private_key = ECC.generate(curve="P-256") # $ PublicKeyGeneration keySize=256
+public_key = private_key.public_key()
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+
+message = b"message"
+
+signer = DSS.new(private_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+signature = signer.sign(hasher)
+
+print("signature={}".format(signature))
+
+print()
+
+verifier = DSS.new(public_key, mode='fips-186-3')
+
+hasher = SHA256.new(message)
+verifier.verify(hasher, signature)
+print("Signature verified (as expected)")
+
+try:
+    hasher = SHA256.new(b"other message")
+    verifier.verify(hasher, signature)
+    raise Exception("Signature verified (unexpected)")
+except ValueError:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/cryptodome/test_rsa.py

@@ -0,0 +1,73 @@
+# RSA is a public-key algorithm for encrypting and signing messages.
+
+from Cryptodome.PublicKey import RSA
+from Cryptodome.Cipher import PKCS1_OAEP
+from Cryptodome.Signature import pss
+from Cryptodome.Hash import SHA256
+
+private_key = RSA.generate(2048) # $ PublicKeyGeneration keySize=2048
+
+# These 2 methods do the same
+public_key = private_key.publickey()
+public_key = private_key.public_key()
+
+# ------------------------------------------------------------------------------
+# encrypt/decrypt
+# ------------------------------------------------------------------------------
+
+print("encrypt/decrypt")
+
+secret_message = b"secret message"
+
+# Following example at https://pycryptodome.readthedocs.io/en/latest/src/examples.html#encrypt-data-with-rsa
+
+encrypt_cipher = PKCS1_OAEP.new(public_key)
+
+encrypted = encrypt_cipher.encrypt(secret_message)
+
+print("encrypted={}".format(encrypted))
+
+print()
+
+decrypt_cipher = PKCS1_OAEP.new(private_key)
+
+decrypted = decrypt_cipher.decrypt(
+    encrypted,
+)
+
+print("decrypted={}".format(decrypted))
+assert decrypted == secret_message
+
+print("\n---\n")
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+
+message = b"message"
+
+signer = pss.new(private_key)
+
+hasher = SHA256.new(message)
+signature = signer.sign(hasher)
+
+print("signature={}".format(signature))
+
+print()
+
+verifier = pss.new(public_key)
+
+hasher = SHA256.new(message)
+verifier.verify(hasher, signature)
+print("Signature verified (as expected)")
+
+try:
+    verifier = pss.new(public_key)
+    hasher = SHA256.new(b"other message")
+    verifier.verify(hasher, signature)
+    raise Exception("Signature verified (unexpected)")
+except ValueError:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/cryptography/ConceptsTest.ql

@@ -0,0 +1,2 @@
+import python
+import experimental.meta.ConceptsTest

python/ql/test/experimental/library-tests/frameworks/cryptography/test_dsa.py

@@ -0,0 +1,37 @@
+# DSA is a public-key algorithm for signing messages.
+# see https://cryptography.io/en/latest/hazmat/primitives/asymmetric/dsa.html
+
+from cryptography.hazmat.primitives.asymmetric import dsa
+from cryptography.hazmat.primitives import hashes
+from cryptography.exceptions import InvalidSignature
+
+HASH_ALGORITHM = hashes.SHA256()
+
+private_key = dsa.generate_private_key(key_size=2048) # $ PublicKeyGeneration keySize=2048
+public_key = private_key.public_key()
+
+message = b"message"
+
+# Following example at https://cryptography.io/en/latest/hazmat/primitives/asymmetric/dsa.html#signing
+
+signature = private_key.sign(
+    message,
+    algorithm=HASH_ALGORITHM,
+)
+
+print("signature={}".format(signature))
+
+print()
+
+public_key.verify(
+    signature, message, algorithm=HASH_ALGORITHM
+)
+print("Signature verified (as expected)")
+
+try:
+    public_key.verify(
+        signature, b"other message", algorithm=HASH_ALGORITHM
+    )
+    raise Exception("Signature verified (unexpected)")
+except InvalidSignature:
+    print("Signature mismatch (as expected)")

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

@@ -0,0 +1,43 @@
+# see https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa.html
+
+from cryptography.hazmat.primitives.asymmetric import ec
+from cryptography.hazmat.primitives import hashes
+from cryptography.exceptions import InvalidSignature
+
+
+private_key = ec.generate_private_key(curve=ec.SECP384R1()) # $ PublicKeyGeneration keySize=384
+public_key = private_key.public_key()
+
+HASH_ALGORITHM = hashes.SHA256()
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+SIGNATURE_ALGORITHM = ec.ECDSA(HASH_ALGORITHM)
+
+message = b"message"
+
+signature = private_key.sign(
+    message,
+    signature_algorithm=SIGNATURE_ALGORITHM,
+)
+
+print("signature={}".format(signature))
+
+print()
+
+public_key.verify(
+    signature, message, signature_algorithm=SIGNATURE_ALGORITHM
+)
+print("Signature verified (as expected)")
+
+try:
+    public_key.verify(
+        signature, b"other message", signature_algorithm=SIGNATURE_ALGORITHM
+    )
+    raise Exception("Signature verified (unexpected)")
+except InvalidSignature:
+    print("Signature mismatch (as expected)")

python/ql/test/experimental/library-tests/frameworks/cryptography/test_rsa.py

@@ -0,0 +1,80 @@
+# RSA is a public-key algorithm for encrypting and signing messages.
+# see https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa.html
+
+from cryptography.hazmat.primitives.asymmetric import rsa, padding
+from cryptography.hazmat.primitives import hashes
+from cryptography.exceptions import InvalidSignature
+
+
+private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) # $ PublicKeyGeneration keySize=2048
+public_key = private_key.public_key()
+
+HASH_ALGORITHM = hashes.SHA256()
+
+# ------------------------------------------------------------------------------
+# encrypt/decrypt
+# ------------------------------------------------------------------------------
+
+print("encrypt/decrypt")
+
+ENCRYPT_PADDING = padding.OAEP(
+    mgf=padding.MGF1(algorithm=HASH_ALGORITHM),
+    algorithm=HASH_ALGORITHM,
+    label=None,
+)
+
+
+secret_message = b"secret message"
+
+# Following example at https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa.html#encryption
+encrypted = public_key.encrypt(secret_message, padding=ENCRYPT_PADDING)
+
+print("encrypted={}".format(encrypted))
+
+print()
+
+decrypted = private_key.decrypt(
+    encrypted,
+    padding=ENCRYPT_PADDING
+)
+
+print("decrypted={}".format(decrypted))
+assert decrypted == secret_message
+
+print("\n---\n")
+
+# ------------------------------------------------------------------------------
+# sign/verify
+# ------------------------------------------------------------------------------
+
+print("sign/verify")
+
+SIGN_PADDING = padding.PSS(
+    mgf=padding.MGF1(HASH_ALGORITHM),
+    salt_length=padding.PSS.MAX_LENGTH
+)
+
+message = b"message"
+
+signature = private_key.sign(
+    message,
+    padding=SIGN_PADDING,
+    algorithm=HASH_ALGORITHM,
+)
+
+print("signature={}".format(signature))
+
+print()
+
+public_key.verify(
+    signature, message, padding=SIGN_PADDING, algorithm=HASH_ALGORITHM
+)
+print("Signature verified (as expected)")
+
+try:
+    public_key.verify(
+        signature, b"other message", padding=SIGN_PADDING, algorithm=HASH_ALGORITHM
+    )
+    raise Exception("Signature verified (unexpected)")
+except InvalidSignature:
+    print("Signature mismatch (as expected)")

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

@@ -319,3 +319,25 @@ class SafeAccessCheckTest extends InlineExpectationsTest {
     )
   }
 }
+
+class PublicKeyGenerationTest extends InlineExpectationsTest {
+  PublicKeyGenerationTest() { this = "PublicKeyGenerationTest" }
+
+  override string getARelevantTag() { result in ["PublicKeyGeneration", "keySize"] }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(location.getFile().getRelativePath()) and
+    exists(Cryptography::PublicKey::KeyGeneration keyGen |
+      location = keyGen.getLocation() and
+      (
+        element = keyGen.toString() and
+        value = "" and
+        tag = "PublicKeyGeneration"
+        or
+        element = keyGen.toString() and
+        value = keyGen.getKeySizeWithOrigin(_).toString() and
+        tag = "keySize"
+      )
+    )
+  }
+}

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

@@ -1,8 +0,0 @@
-| weak_crypto.py:67:1:67:30 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
-| weak_crypto.py:68:1:68:28 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:21:11:21:33 | ControlFlowNode for FakeWeakEllipticCurve() | 160 |
-| weak_crypto.py:69:1:69: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 |
-| weak_crypto.py:71:1:71:39 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
-| weak_crypto.py:72:1:72:34 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:21:11:21:33 | ControlFlowNode for FakeWeakEllipticCurve() | 160 |
-| weak_crypto.py:73:1:73:46 | 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 |
-| weak_crypto.py:75:1:75:22 | ControlFlowNode for Attribute() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
-| weak_crypto.py:76:1:76: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/WeakCrypto.qlref

@@ -1 +0,0 @@
-Security/CWE-326/WeakCrypto.ql

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

@@ -0,0 +1,9 @@
+| 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 |
+| 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:24 | ControlFlowNode for Attribute() | 163 |
+| 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 |
+| 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 |
+| 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:24 | ControlFlowNode for Attribute() | 163 |
+| 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 |
+| 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 |
+| 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 |
+| weak_crypto.py:84:12:84:29 | 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.qlref

@@ -0,0 +1 @@
+Security/CWE-326/WeakCryptoKey.ql

python/ql/test/query-tests/Security/CWE-326/options

@@ -1 +0,0 @@
-semmle-extractor-options: -p ../lib/ --max-import-depth=3

python/ql/test/query-tests/Security/CWE-326/test_example.py

@@ -0,0 +1,9 @@
+from Cryptodome.PublicKey import RSA
+
+from weak_crypto import only_used_by_test
+
+def test_example():
+    # This is technically not ok, but since it's in a test, we don't want to alert on it
+    RSA.generate(1024)
+
+    only_used_by_test(1024)

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

@@ -1,7 +1,7 @@
 from cryptography.hazmat import backends
 from cryptography.hazmat.primitives.asymmetric import ec, dsa, rsa
 
-#Crypto and Cryptodome have same API
+# Crypto and Cryptodome have same API
 if random():
     from Crypto.PublicKey import DSA
     from Crypto.PublicKey import RSA
@@ -12,13 +12,14 @@ else:
 RSA_WEAK = 1024
 RSA_OK = 2048
 RSA_STRONG = 3076
+
+DSA_WEAK = 1024
+DSA_OK = 2048
+DSA_STRONG = 3076
+
 BIG = 10000
 
-class FakeWeakEllipticCurve:
-    name = "fake"
-    key_size = 160
-
-EC_WEAK = FakeWeakEllipticCurve()
+EC_WEAK = ec.SECT163K1()  # has key size of 163
 EC_OK = ec.SECP224R1()
 EC_STRONG = ec.SECP384R1()
 EC_BIG = ec.SECT571R1()
@@ -27,50 +28,68 @@ dsa_gen_key = dsa.generate_private_key
 ec_gen_key = ec.generate_private_key
 rsa_gen_key = rsa.generate_private_key
 
-default = backends.default_backend()
 
-#Strong and OK keys.
 
-dsa_gen_key(key_size=RSA_OK, backend=default)
-dsa_gen_key(key_size=RSA_STRONG, backend=default)
-dsa_gen_key(key_size=BIG, backend=default)
-ec_gen_key(curve=EC_OK, backend=default)
-ec_gen_key(curve=EC_STRONG, backend=default)
-ec_gen_key(curve=EC_BIG, backend=default)
-rsa_gen_key(public_exponent=65537, key_size=RSA_OK, backend=default)
-rsa_gen_key(public_exponent=65537, key_size=RSA_STRONG, backend=default)
-rsa_gen_key(public_exponent=65537, key_size=BIG, backend=default)
+# Strong and OK keys.
+
+dsa_gen_key(key_size=DSA_OK)
+dsa_gen_key(key_size=DSA_STRONG)
+dsa_gen_key(key_size=BIG)
+ec_gen_key(curve=EC_OK)
+ec_gen_key(curve=EC_STRONG)
+ec_gen_key(curve=EC_BIG)
+rsa_gen_key(public_exponent=65537, key_size=RSA_OK)
+rsa_gen_key(public_exponent=65537, key_size=RSA_STRONG)
+rsa_gen_key(public_exponent=65537, key_size=BIG)
 
 DSA.generate(bits=RSA_OK)
 DSA.generate(bits=RSA_STRONG)
 RSA.generate(bits=RSA_OK)
 RSA.generate(bits=RSA_STRONG)
 
-dsa_gen_key(RSA_OK, default)
-dsa_gen_key(RSA_STRONG, default)
-dsa_gen_key(BIG, default)
-ec_gen_key(EC_OK, default)
-ec_gen_key(EC_STRONG, default)
-ec_gen_key(EC_BIG, default)
-rsa_gen_key(65537, RSA_OK, default)
-rsa_gen_key(65537, RSA_STRONG, default)
-rsa_gen_key(65537, BIG, default)
+dsa_gen_key(DSA_OK)
+dsa_gen_key(DSA_STRONG)
+dsa_gen_key(BIG)
+ec_gen_key(EC_OK)
+ec_gen_key(EC_STRONG)
+ec_gen_key(EC_BIG)
+rsa_gen_key(65537, RSA_OK)
+rsa_gen_key(65537, RSA_STRONG)
+rsa_gen_key(65537, BIG)
 
-DSA.generate(RSA_OK)
-DSA.generate(RSA_STRONG)
+DSA.generate(DSA_OK)
+DSA.generate(DSA_STRONG)
 RSA.generate(RSA_OK)
 RSA.generate(RSA_STRONG)
 
 
 # Weak keys
 
-dsa_gen_key(RSA_WEAK, default)
-ec_gen_key(EC_WEAK, default)
-rsa_gen_key(65537, RSA_WEAK, default)
+dsa_gen_key(DSA_WEAK)
+ec_gen_key(EC_WEAK)
+rsa_gen_key(65537, RSA_WEAK)
 
-dsa_gen_key(key_size=RSA_WEAK, default)
-ec_gen_key(curve=EC_WEAK, default)
-rsa_gen_key(65537, key_size=RSA_WEAK, default)
+dsa_gen_key(key_size=DSA_WEAK)
+ec_gen_key(curve=EC_WEAK)
+rsa_gen_key(65537, key_size=RSA_WEAK)
 
-DSA.generate(RSA_WEAK)
+DSA.generate(DSA_WEAK)
 RSA.generate(RSA_WEAK)
+
+# ------------------------------------------------------------------------------
+
+# Through function calls
+
+def make_new_rsa_key_weak(bits):
+    return RSA.generate(bits) # NOT OK
+make_new_rsa_key_weak(RSA_WEAK)
+
+
+def make_new_rsa_key_strong(bits):
+    return RSA.generate(bits) # OK
+make_new_rsa_key_strong(RSA_STRONG)
+
+
+def only_used_by_test(bits):
+    # Although this call will technically not be ok, since it's only used in a test, we don't want to alert on it.
+    return RSA.generate(bits)