python: fixes and additions for experimental key derivation function query library

javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll

@@ -770,7 +770,7 @@ private module Bcrypt {
       super.getArgument(0) = input
     }
 
-    override DataFlow::Node getInitialization() { result = init.asSource() }
+    override DataFlow::Node getInitialization() { result = this }
 
     override DataFlow::Node getAnInput() { result = input }
 

python/ql/lib/experimental/cryptography/Concepts.qll

@@ -4,3 +4,5 @@ import semmle.python.ApiGraphs
 import experimental.cryptography.modules.stdlib.HashlibModule as HashLibModule
 import experimental.cryptography.modules.stdlib.HmacModule as HMacModule
 import experimental.cryptography.modules.CryptographyModule as CryptographyModule
+// contents are wrapped in a module, so 'as' not needed
+import experimental.cryptography.modules.Argon2CffiModule 

python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll

@@ -104,9 +104,11 @@ predicate isSymmetricEncryptionAlgorithm(string name) {
 predicate isKeyDerivationAlgorithm(string name) {
   name =
     [
-      "ARGON2", "CONCATKDF", "CONCATKDFHASH", "CONCATKDFHMAC", "KBKDFCMAC", "BCRYPT", "HKDF",
-      "HKDFEXPAND", "KBKDF", "KBKDFHMAC", "PBKDF1", "PBKDF2", "PBKDF2HMAC", "PKCS5", "SCRYPT",
-      "X963KDF", "EVPKDF"
+      // 'ARGON2' should only be used in cases where the specific variant in use cannot be discerned reliably
+      "ARGON2", "ARGON2D", "ARGON2I", "ARGON2ID",
+      "CONCATKDF", "CONCATKDFHASH", "CONCATKDFHMAC",
+      "KBKDFCMAC", "BCRYPT", "HKDF", "HKDFEXPAND", "KBKDF", "KBKDFHMAC", "PBKDF1", "PBKDF2",
+      "PBKDF2HMAC", "PKCS5", "SCRYPT", "X963KDF", "EVPKDF"
     ]
 }
 

python/ql/lib/experimental/cryptography/CryptoArtifact.qll

@@ -20,7 +20,7 @@ abstract class CryptographicArtifact extends DataFlow::Node { }
 abstract class SymmetricCipher extends CryptographicArtifact {
   abstract SymmetricEncryptionAlgorithm getEncryptionAlgorithm();
 
-  abstract BlockMode getBlockMode();
+  abstract BlockModeInstance getBlockMode();
 
   final predicate hasBlockMode() { exists(this.getBlockMode()) }
 }
@@ -55,9 +55,14 @@ abstract class CryptographicOperation extends CryptographicArtifact, API::CallNo
     not this.hasAlgorithm()
   }
 
+  /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */
+  abstract DataFlow::Node getInitialisation();
   // TODO: this might have to be parameterized by a configuration source for
   //       situations where an operation is passed an algorithm
+  /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
   abstract CryptographicAlgorithm getAlgorithm();
+  /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
+  abstract DataFlow::Node getAnInput();
 }
 
 /** A key generation operation for asymmetric keys */
@@ -129,12 +134,17 @@ abstract class KeyDerivationAlgorithm extends CryptographicAlgorithm {
 }
 
 abstract class KeyDerivationOperation extends CryptographicOperation {
-  DataFlow::Node getIterationSizeSrc() { none() }
-
+  DataFlow::Node getSaltConfigSink() { none() }
   DataFlow::Node getSaltConfigSrc() { none() }
 
+  DataFlow::Node getIterationSizeSrc() { none() }
+
   DataFlow::Node getHashConfigSrc() { none() }
 
+  DataFlow::Node getLanesConfigSrc() { none() }
+
+  DataFlow::Node getMemoryCostConfigSrc() { none() }
+
   // TODO: get encryption algorithm for CBC-based KDF?
   DataFlow::Node getDerivedKeySizeSrc() { none() }
 
@@ -147,6 +157,10 @@ abstract class KeyDerivationOperation extends CryptographicOperation {
 
   abstract predicate requiresHash();
 
+  abstract predicate requiresLanes();
+
+  abstract predicate requiresMemoryCost();
+
   //abstract predicate requiresKeySize(); // Going to assume all requires a size
   abstract predicate requiresMode();
 }
@@ -167,6 +181,8 @@ abstract class EncryptionAlgorithm extends CryptographicAlgorithm {
   //       class does not have this common predicate.
 }
 
+abstract class EncryptionOperation extends CryptographicOperation { }
+
 /**
  * Algorithms directly or indirectly related to asymmetric encryption,
  * e.g., RSA, DSA, but also RSA padding algorithms
@@ -192,6 +208,8 @@ abstract class SymmetricEncryptionAlgorithm extends EncryptionAlgorithm {
   // TODO: add a stream cipher predicate?
 }
 
+abstract class SymmetricEncryptionOperation extends EncryptionOperation { }
+
 // Used only to categorize all padding into a single object,
 // DO_NOT add predicates here. Only for categorization purposes.
 abstract class PaddingAlgorithm extends CryptographicAlgorithm { }
@@ -222,7 +240,7 @@ abstract class EllipticCurveAlgorithm extends AsymmetricAlgorithm {
   final int getCurveBitSize() { isEllipticCurveAlgorithm(this.getCurveName(), result) }
 }
 
-abstract class BlockMode extends CryptographicAlgorithm {
+abstract class BlockModeInstance extends CryptographicAlgorithm {
   final string getBlockModeName() {
     if exists(string n | n = this.getName() and isCipherBlockModeAlgorithm(n))
     then isCipherBlockModeAlgorithm(result) and result = this.getName()
@@ -232,21 +250,38 @@ abstract class BlockMode extends CryptographicAlgorithm {
   /**
    * Gets the source of the IV configuration.
    */
-  abstract DataFlow::Node getIVorNonce();
+  abstract DataFlow::Node getIVOrNonceSrc();
 
-  final predicate hasIVorNonce() { exists(this.getIVorNonce()) }
+  /**
+   * Gets the sink of the IV configuration.
+   */
+  abstract DataFlow::Node getIVOrNonceSink();
+
+  final predicate hasIVorNonce() { exists(this.getIVOrNonceSrc()) }
 }
 
 abstract class KeyWrapOperation extends CryptographicOperation { }
 
 abstract class AuthenticatedEncryptionAlgorithm extends SymmetricEncryptionAlgorithm {
-  final string getAuthticatedEncryptionName() {
+  final string getAuthenticatedEncryptionName() {
     if exists(string n | n = this.getName() and isSymmetricEncryptionAlgorithm(n))
     then isSymmetricEncryptionAlgorithm(result) and result = this.getName()
     else result = unknownAlgorithm()
   }
 }
 
+abstract class AuthenticatedEncryptionOperation extends SymmetricEncryptionOperation {
+  /**
+   * Gets the source of the IV configuration.
+   */
+  abstract DataFlow::Node getIVOrNonceSrc();
+
+  /**
+   * Gets the sink of the IV configuration.
+   */
+  abstract DataFlow::Node getIVOrNonceSink();
+}
+
 abstract class KeyExchangeAlgorithm extends AsymmetricAlgorithm {
   final string getKeyExchangeName() {
     if exists(string n | n = this.getName() and isKeyExchangeAlgorithm(n))

python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll

@@ -0,0 +1,137 @@
+import python
+import semmle.python.ApiGraphs
+import experimental.cryptography.CryptoArtifact
+import experimental.cryptography.CryptoAlgorithmNames
+import experimental.cryptography.utils.CallCfgNodeWithTarget
+private import experimental.cryptography.utils.Utils as Utils
+
+/**
+ * Provides models for the `argon2-cffi` PyPI package.
+ * See https://argon2-cffi.readthedocs.io/en/stable/.
+ */
+module Argon2Cffi {
+    API::CallNode getAPasswordHasher() {
+        result = API::moduleImport("argon2")
+            .getMember("PasswordHasher")
+            .getACall()
+    }
+
+    DataFlow::LocalSourceNode getAnArgonType(string fullName, string member) {
+        result = API::moduleImport("argon2")
+            .getMember("low_level")
+            .getMember("Type")
+            .getMember(member)
+            .asSource()
+        and (
+            (fullName = "ARGON2D" and member = "D")
+            or
+            (fullName = "ARGON2I" and member = "I")
+            or
+            (fullName = "ARGON2ID" and member = "ID")
+        )
+    }
+
+    class ArgonType extends DataFlow::LocalSourceNode {
+        ArgonType() { this = getAnArgonType(_, _) }
+        string getName() { this = getAnArgonType(result, _) }
+    }
+
+    // note: password hashers instantiated using `from_parameters` are not modelled (yet)
+    class PasswordHasher extends KeyDerivationAlgorithm {
+        PasswordHasher() { this = getAPasswordHasher() }
+
+        API::Node getTypeParameter() {
+            result = this.(API::CallNode).getParameter(6, "type")
+        }
+
+        override string getName() {
+            if exists(this.getTypeParameter()) then exists(
+                ArgonType type | type.flowsTo(this.getTypeParameter().asSink()) | result = type.getName()
+            )
+            else result = "ARGON2ID"
+        }
+    }
+
+    abstract class PasswordHasherOperation extends KeyDerivationOperation {
+        PasswordHasher instance;
+
+        PasswordHasherOperation() { 
+            this = instance.(API::CallNode).getAMethodCall(["hash", "verify"])
+        }
+
+        override PasswordHasher getAlgorithm() { result = instance }
+
+        override DataFlow::Node getInitialisation() { result = instance }
+
+        override predicate requiresHash() { none() }
+
+        override predicate requiresMode() { none() }
+
+        // although Argon is salted, the salt parameter is optional as the library generates one for you
+        override predicate requiresSalt() { none() }
+
+        override predicate requiresIteration() { none() }
+
+        override predicate requiresLanes() { none() }
+
+        override predicate requiresMemoryCost() { none() }
+
+        override DataFlow::Node getSaltConfigSink() {
+            result = this.getSaltConfigSink(_)
+        }
+
+        private DataFlow::Node getSaltConfigSink(API::Node apiNode) {
+            result = apiNode.asSink() and 
+            apiNode = this.(API::CallNode).getKeywordParameter("salt")
+        }
+
+        override DataFlow::Node getSaltConfigSrc() {
+            result = this.getSaltConfigSrc(_)
+        }
+
+        private DataFlow::Node getSaltConfigSrc(API::Node apiNode) {
+            exists(this.getSaltConfigSink(apiNode)) and
+            result = Utils::getUltimateSrcFromApiNode(apiNode)
+        }
+
+        override DataFlow::Node getHashConfigSrc() { none() }
+
+        override DataFlow::Node getIterationSizeSrc() {
+            result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(0, "time_cost"))
+        }
+
+        override DataFlow::Node getMemoryCostConfigSrc() {
+            result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(1, "memory_cost"))
+        }
+
+        override DataFlow::Node getLanesConfigSrc() {
+            result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(2, "parallelism"))
+        }
+
+        override DataFlow::Node getDerivedKeySizeSrc() {
+            result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(3, "hash_len"))
+        }
+
+        override DataFlow::Node getModeSrc() { none() }
+    }
+
+    class HashOperation extends PasswordHasherOperation {
+        HashOperation() { 
+            this = instance.(API::CallNode).getAMethodCall("hash")
+        }
+
+        override DataFlow::Node getAnInput() {
+            result = this.(API::CallNode).getArg(0) or result = this.(API::CallNode).getArgByName("password")
+        }
+    }
+
+    class VerifyOperation extends PasswordHasherOperation {
+        VerifyOperation() { 
+            this = instance.(API::CallNode).getAMethodCall("verify")
+        }
+
+        override DataFlow::Node getAnInput() {
+            result = this.(API::CallNode).getArg(1) or result = this.(API::CallNode).getArgByName("password")
+        }
+    }
+}