Add JCA key concepts latest model and constant pwd query

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

@@ -4,6 +4,7 @@ import semmle.code.java.controlflow.Dominance
 
 module JCAModel {
   import Language
+  import KeyConcepts
 
   // TODO: Verify that the PBEWith% case works correctly
   bindingset[algo]

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

@@ -0,0 +1,227 @@
+import Language
+
+/**
+ * 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 getOutputNode() { result.asExpr() = this }
+
+  override DataFlow::Node getInputNode() {
+    result.asExpr() = this.(ClassInstanceExpr).getArgument(0)
+  }
+
+  //TODO
+  override predicate flowsTo(Crypto::FlowAwareElement other) { none() }
+}
+
+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() { result = algorithm }
+
+  override Crypto::KeyMaterialInstance getInputKeyMaterial() { none() }
+
+  override Crypto::KeyArtifactInstance getOutputKey() { none() }
+    //exists(KeyObject key | result = key and key.getOutputNode() = this) }
+}
+
+/**
+ * A Key Object
+ */
+class KeyObject extends Crypto::KeyArtifactInstance instanceof Variable {
+  Crypto::KeyDerivationOperationInstance instantiation;
+
+  KeyObject() {
+    exists(RefType t |
+      instantiation = this.getInitializer() and
+      this.getType().(RefType).extendsOrImplements*(t) and
+      t.hasQualifiedName("java.security", "Key")
+    )
+  }
+
+  override DataFlow::Node getOutputNode() { result.asExpr() = instantiation }
+
+  //TODO fix by using the KeyDerivationOperationInstance type in this type
+  override DataFlow::Node getInputNode() {
+    result.asExpr() = instantiation
+  }
+
+  override DataFlow::Node getKeySize() { exists(DataFlow::Node src, DataFlow::Node sink, Call initcall
+  | 
+  IntToKeyGeneratorInitConfigFlow::flow(src, sink)
+  and result = src
+  and initcall.getArgument(0) = sink.asExpr()
+  //init qualifier same as keygen qualifier, we need to use more of the connected concepts tho, this is re-inventing them every time we need something
+and initcall.getQualifier() = instantiation.(JCAKeyGenerationCall).getQualifier().(VarAccess).getVariable().getAnAccess()
+
+)
+}
+
+  //TODO
+  override predicate flowsTo(Crypto::FlowAwareElement other) { 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).getInputNode() = 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()

java/ql/src/experimental/Quantum/InsuficientKeySize.ql

@@ -0,0 +1,28 @@
+/**
+ * @name Use of a cryptographic algorithm with insufficient key size
+ * @description Using cryptographic algorithms with too small a key size can
+ *              allow an attacker to compromise security.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id java/insufficient-key-size-new-model
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+//this query is a replica of the concept in: https://github.com/github/codeql/blob/main/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql
+//but uses the **NEW MODELLING**
+//AND is actually only a subset of the logic -
+//precisely this currently detects insuffficient key size for AES
+import experimental.Quantum.Language
+
+/** Returns the minimum recommended key size for AES. */
+int minSecureKeySizeAes() { result = 128 }
+
+//todo make this actually check to match the alg to AES (some JCA model improve required)
+//currently looks for any key size lower than...
+from Crypto::KeyArtifactInstance key
+where key.getKeySize().asExpr().(IntegerLiteral).getIntValue() < minSecureKeySizeAes()
+select key, "This $@ is less than the recommended key size of " + minSecureKeySizeAes() + " bits.",
+  key.getKeySize(), "key size"