Add model and sample query for constants used in key specs

java/ql/lib/experimental/Quantum/JCA.qll

@@ -532,4 +532,209 @@ module JCAModel {
       this.getMethod().getParameterType(2).hasName("AlgorithmParameterSpec")
     }
   }
+
+  /**
+   * Key Material Concept
+   * any class that implements `java.security.spec.KeySpec`
+   */
+  class KeyMaterialObject extends Class {
+    KeyMaterialObject() {
+      exists(RefType t |
+        this.extendsOrImplements*(t) and
+        t.hasQualifiedName("java.security.spec", "KeySpec")
+      )
+    }
+  }
+
+  /**
+   * KeyMaterial
+   * ie some plain material that gets used to generate a Key
+   */
+  class KeyMaterialInstantiation extends Crypto::KeyMaterialInstance instanceof ClassInstanceExpr {
+    KeyMaterialInstantiation() {
+      this.(ClassInstanceExpr).getConstructedType() instanceof KeyMaterialObject
+    }
+
+    override DataFlow::Node asOutputData() { result.asExpr() = this }
+
+    override DataFlow::Node getInput() { result.asExpr() = this.(ClassInstanceExpr).getArgument(0) }
+  }
+
+  abstract class JCAKeyGenerationCall extends Call { }
+
+  private class JCAKeyGenerationCallWithMaterialArg extends JCAKeyGenerationCall {
+    JCAKeyGenerationCallWithMaterialArg() {
+      exists(string s | s in ["generatePrivate", "generatePublic", "translateKey"] |
+        this.getCallee().hasQualifiedName("java.security", "KeyFactory", s)
+      )
+    }
+
+    DataFlow::Node getInputData() { result.asExpr() = this.getArgument(0) }
+  }
+
+  private class JCAKeyGenerationCallWithoutMaterialArg extends JCAKeyGenerationCall {
+    JCAKeyGenerationCallWithoutMaterialArg() {
+      this.getCallee().hasQualifiedName("javax.crypto", "KeyGenerator", "generateKey")
+    }
+
+    DataFlow::Node getInputData() { result.asExpr() = this.getArgument(0) }
+  }
+
+  abstract class KeyGenerators extends Class {
+    Method getInstance() { result = this.getAMethod() and result.hasName("getInstance") }
+
+    abstract Method keySizeSetter();
+  }
+
+  class KeyGenerator extends KeyGenerators {
+    KeyGenerator() { this.hasQualifiedName("javax.crypto", "KeyGenerator") }
+
+    override Method keySizeSetter() { result = this.getAMethod() and result.hasName("init") }
+  }
+
+  class KeyFactory extends KeyGenerators {
+    KeyFactory() { this.hasQualifiedName("java.security", "KeyFactory") }
+
+    //todo this requires flow from a keyspec
+    override Method keySizeSetter() { none() }
+  }
+
+  class KeyPairGenerator extends KeyGenerators {
+    KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") }
+
+    override Method keySizeSetter() { result = this.getAMethod() and result.hasName("initialize") }
+  }
+
+  /**
+   * Data-flow configuration modelling flow from a string literal to a `KeyGeneratorsGetInstanceCall` argument.
+   */
+  private module KeyGenAlgorithmStringToFetchFlow implements DataFlow::ConfigSig {
+    //TODO narrow this type based on JCA standard names doc for whats possible here, like is done for the cipher algs
+    predicate isSource(DataFlow::Node src) { src.asExpr() instanceof StringLiteral }
+
+    predicate isSink(DataFlow::Node sink) {
+      exists(KeyGenerators gen, Call c |
+        c.getCallee() = gen.getInstance() and
+        c.getArgument(0) = sink.asExpr()
+      )
+    }
+  }
+
+  module KeyGenAlgorithmStringToFetchFlowIns = DataFlow::Global<KeyGenAlgorithmStringToFetchFlow>;
+
+  /**
+   * The key generation algorithm argument to a `KeyGeneratorsGetInstanceCall `.
+   *
+   * For example, in `KeyGenerator.getInstance(algorithm)`, this class represents `algorithm`.
+   *
+   * the algorithm is both what the resulting key is meant to be used for but also informs how it
+   * was created
+   */
+  class KeyDerivationAlgorithmArg extends Crypto::KeyDerivationAlgorithmInstance instanceof Expr {
+    Call getInstanceCall;
+
+    KeyDerivationAlgorithmArg() {
+      exists(KeyGenerators gen |
+        getInstanceCall.getCallee() = gen.getInstance() and this = getInstanceCall.getArgument(0)
+      )
+    }
+
+    /**
+     * Returns the `StringLiteral` from which this argument is derived, if known.
+     */
+    StringLiteral getOrigin() {
+      KeyGenAlgorithmStringToFetchFlowIns::flow(DataFlow::exprNode(result),
+        DataFlow::exprNode(this.(Expr).getAChildExpr*()))
+    }
+
+    Call getCall() { result = getInstanceCall }
+  }
+
+  /**
+   * A data-flow configuration to track flow from a integer value to
+   * the keysize argument of the various `init` methods of the KeyGeneratorsGetInstanceCall types
+   */
+  private module IntToKeyGeneratorInitConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node src) { src.asExpr() instanceof IntegerLiteral }
+
+    predicate isSink(DataFlow::Node sink) {
+      exists(KeyGenerators c, Call call |
+        call.getCallee() = c.keySizeSetter() and call.getArgument(0) = sink.asExpr()
+      )
+    }
+  }
+
+  module IntToKeyGeneratorInitConfigFlow = DataFlow::Global<IntToKeyGeneratorInitConfig>;
+
+  /**
+   * Data-flow configuration modelling flow from a keygenerator type getinstance call
+   * to generation step
+   */
+  module KeyDerivationOperationInstanceConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node src) {
+      exists(Call getInstanceCall, KeyGenerators kg |
+        kg.getInstance() = getInstanceCall.getCallee() and
+        src.asExpr() = getInstanceCall
+      )
+    }
+
+    predicate isSink(DataFlow::Node sink) {
+      exists(JCAKeyGenerationCall call | sink.asExpr() = call.getQualifier())
+    }
+
+    predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+      exists(Call getInstanceCall, KeyGenerators kg, Call keySizeSetterCall |
+        kg.getInstance() = getInstanceCall.getCallee() and
+        node1.asExpr() = getInstanceCall.getQualifier() and
+        kg.keySizeSetter() = keySizeSetterCall.getCallee() and
+        node2.asExpr() = keySizeSetterCall.getQualifier()
+      )
+    }
+  }
+
+  module KeyDerivationOperationInstanceConfigFlow =
+    DataFlow::Global<KeyDerivationOperationInstanceConfig>;
+
+  class KeyDerivationOperationInstance extends Crypto::KeyDerivationOperationInstance instanceof Call
+  {
+    Crypto::KeyDerivationAlgorithmInstance algorithm;
+    JCAKeyGenerationCall generateCall;
+
+    KeyDerivationOperationInstance() {
+      exists(KeyDerivationAlgorithmArg arg, DataFlow::Node source, DataFlow::Node sink |
+        this = generateCall and
+        generateCall.getQualifier() = sink.asExpr() and
+        KeyDerivationOperationInstanceConfigFlow::flow(source, sink) and
+        algorithm = arg and
+        source.asExpr() = arg.getCall()
+      )
+    }
+
+    //TODO fill these out using various flows defined above
+    override Crypto::KeyDerivationAlgorithmInstance getAlgorithm() { none() }
+
+    override Crypto::KeyMaterialInstance getInputKeyMaterial() { none() }
+
+    override Crypto::KeyArtifactInstance getOutputKey() { none() }
+  }
+
+  /**
+   * A Key Object
+   */
+  class KeyInstantiation extends Crypto::KeyArtifactInstance instanceof Expr {
+    KeyInstantiation() {
+      exists(RefType t, Variable v |
+        this = v.getInitializer() and
+        v.getType().(RefType).extendsOrImplements*(t) and
+        t.hasQualifiedName("java.security", "Key")
+      )
+    }
+
+    override DataFlow::Node asOutputData() { result.asExpr() = this }
+
+    //TODO fix by using the KeyDerivationOperationInstance type in this type
+    override DataFlow::Node getInput() { result.asExpr() = this.(ClassInstanceExpr).getArgument(0) }
+
+    override DataFlow::Node getKeySize() { none() }
+  }
 }

java/ql/src/experimental/Quantum/ConstantPassword.ql

@@ -0,0 +1,38 @@
+/**
+ * @name Constant password
+ * @description Using constant passwords is not secure, because potential attackers can easily recover them from the source code.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 6.8
+ * @precision high
+ * @id java/constant-password-new-model
+ * @tags security
+ *       external/cwe/cwe-259
+ */
+
+//this query is a replica of the concept in: https://github.com/github/codeql/blob/main/swift/ql/src/queries/Security/CWE-259/ConstantPassword.ql
+//but uses the **NEW MODELLING**
+import experimental.Quantum.Language
+import semmle.code.java.dataflow.TaintTracking
+
+/**
+ * A constant password is created through either a byte array or string literals.
+ */
+class ConstantPasswordSource extends Expr {
+  ConstantPasswordSource() {
+    this instanceof CharacterLiteral or
+    this instanceof StringLiteral
+  }
+}
+
+module ConstantToKeyDerivationFlow implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ConstantPasswordSource }
+
+  predicate isSink(DataFlow::Node sink) { any(Crypto::KeyMaterialInstance i).getInput() = sink }
+}
+
+module ConstantToKeyDerivationFlowInit = TaintTracking::Global<ConstantToKeyDerivationFlow>;
+
+from DataFlow::Node source, DataFlow::Node sink
+where ConstantToKeyDerivationFlowInit::flow(source, sink)
+select sink, "Constant password $@ is used.", source, source.toString()

shared/cryptography/codeql/cryptography/Model.qll

@@ -82,7 +82,13 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
 
   abstract class HashAlgorithmInstance extends LocatableElement { }
 
-  abstract class KeyDerivationOperationInstance extends LocatableElement { }
+  abstract class KeyDerivationOperationInstance extends LocatableElement {
+    abstract KeyDerivationAlgorithmInstance getAlgorithm();
+
+    abstract KeyMaterialInstance getInputKeyMaterial();
+
+    abstract KeyArtifactInstance getOutputKey();
+  }
 
   abstract class KeyDerivationAlgorithmInstance extends LocatableElement { }
 
@@ -118,7 +124,11 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
 
   abstract class DigestArtifactInstance extends ArtifactLocatableElement { }
 
-  abstract class KeyArtifactInstance extends ArtifactLocatableElement { }
+  abstract class KeyMaterialInstance extends ArtifactLocatableElement { }
+
+  abstract class KeyArtifactInstance extends ArtifactLocatableElement {
+    abstract DataFlowNode getKeySize();
+  }
 
   abstract class NonceArtifactInstance extends ArtifactLocatableElement { }
 
@@ -130,6 +140,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
     // Artifacts (data that is not an operation or algorithm, e.g., a key)
     TDigest(DigestArtifactInstance e) or
     TKey(KeyArtifactInstance e) or
+    TKeyMaterial(KeyMaterialInstance e) or
     TNonce(NonceArtifactInstance e) or
     TRandomNumberGeneration(RandomNumberGenerationInstance e) or
     // Operations (e.g., hashing, encryption)
@@ -407,16 +418,6 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
     }
   }
 
-  /**
-   * An operation that derives one or more keys from an input value.
-   */
-  abstract class KeyDerivationOperation extends Operation, TKeyDerivationOperation {
-    final override Location getLocation() {
-      exists(LocatableElement le | this = TKeyDerivationOperation(le) and result = le.getLocation())
-    }
-    //override string getOperationType() { result = "KeyDerivationOperation" }
-  }
-
   /**
    * An algorithm that derives one or more keys from an input value.
    *
@@ -425,6 +426,8 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
    * For known algorithms, use the specialized classes, e.g., `HKDF` and `PKCS12KDF`.
    */
   abstract class KeyDerivationAlgorithm extends Algorithm, TKeyDerivationAlgorithm {
+    final LocatableElement getInstance() { this = TKeyDerivationAlgorithm(result) }
+
     final override Location getLocation() {
       exists(LocatableElement le | this = TKeyDerivationAlgorithm(le) and result = le.getLocation())
     }
@@ -1000,4 +1003,62 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
   abstract class KEMAlgorithm extends TKeyEncapsulationAlgorithm, Algorithm {
     final override string getAlgorithmType() { result = "KeyEncapsulationAlgorithm" }
   }
+
+  /**
+   * A Key Material Object
+   */
+  private class KeyMaterialImpl extends Artifact, TKeyMaterial {
+    KeyMaterialInstance instance;
+
+    KeyMaterialImpl() { this = TKeyMaterial(instance) }
+
+    final override string getInternalType() { result = "KeyMaterial" }
+
+    override Location getLocation() { result = instance.getLocation() }
+
+    override DataFlowNode asOutputData() { result = instance.asOutputData() }
+
+    override DataFlowNode getInputData() { result = instance.getInput() }
+  }
+
+  final class KeyMaterial = KeyMaterialImpl;
+
+  /**
+   * A Key Object
+   */
+  private class KeyArtifactImpl extends Artifact, TKey {
+    KeyArtifactInstance instance;
+
+    KeyArtifactImpl() { this = TKey(instance) }
+
+    final override string getInternalType() { result = "KeyMaterial" }
+
+    override Location getLocation() { result = instance.getLocation() }
+
+    override DataFlowNode asOutputData() { result = instance.asOutputData() }
+
+    override DataFlowNode getInputData() { result = instance.getInput() }
+  }
+
+  final class KeyArtifact = KeyArtifactImpl;
+
+  /**
+   * A Key Derivation operation
+   * todo: decide if derivation or generation is the right term here for the most general concept
+   */
+  private class KeyDerivationOperationImpl extends Operation, TKeyDerivationOperation {
+    KeyDerivationOperationInstance instance;
+
+    KeyDerivationOperationImpl() { this = TKeyDerivationOperation(instance) }
+
+    final override string getInternalType() { result = "KeyMaterial" }
+
+    override Location getLocation() { result = instance.getLocation() }
+
+    override KeyDerivationAlgorithm getAlgorithm() {
+      result.getInstance() = instance.getAlgorithm()
+    }
+  }
+
+  final class KeyDerivationOperation = KeyDerivationOperationImpl;
 }