Added support for EllipticCurveConsumingAlgorithm

Author: fegge

java/ql/lib/experimental/quantum/BouncyCastle/AlgorithmInstances.qll

@@ -1,20 +1,9 @@
 import java
 import experimental.quantum.Language
 import AlgorithmValueConsumers
+import OperationInstances
 import FlowAnalysis
 
-/**
- * Elliptic curve algorithms.
- */
-abstract private class EllipticCurveConsumingAlgorithmInstance extends Crypto::EllipticCurveConsumingAlgorithmInstance
-{
-  string getFixedEllipticCurveName() { none() }
-
-  override Crypto::AlgorithmValueConsumer getEllipticCurveConsumer() {
-    result.(ImplicitEllipticCurveInstance).getAlgorithm() = this
-  }
-}
-
 /**
  * A string literal that represents an elliptic curve name.
  */
@@ -42,28 +31,12 @@ class EllipticCurveStringLiteralInstance extends Crypto::EllipticCurveInstance i
 }
 
 /**
- * Represents an elliptic curve that is implicitly defined by the underlying
- * algorithm. In this case, we view the algorithm and elliptic curve as being
- * implicitly defined by the constructor call.
+ * Represents an elliptic curve algorithm where the elliptic curve is implicitly
+ * defined by the underlying type.
  */
-class ImplicitEllipticCurveInstance extends Crypto::EllipticCurveInstance,
-  EllipticCurveAlgorithmValueConsumer instanceof ClassInstanceExpr
+abstract class KnownEllipticCurveInstance extends Crypto::EllipticCurveInstance,
+  Crypto::EllipticCurveConsumingAlgorithmInstance, Crypto::AlgorithmValueConsumer instanceof ClassInstanceExpr
 {
-  EllipticCurveConsumingAlgorithmInstance algorithm;
-
-  ImplicitEllipticCurveInstance() {
-    this = algorithm and
-    exists(algorithm.getFixedEllipticCurveName())
-  }
-
-  EllipticCurveConsumingAlgorithmInstance getAlgorithm() { result = this }
-
-  override string getRawEllipticCurveName() { result = algorithm.getFixedEllipticCurveName() }
-
-  override Crypto::ConsumerInputDataFlowNode getInputNode() { none() }
-
-  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
-
   override Crypto::TEllipticCurveType getEllipticCurveType() {
     Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.getRawEllipticCurveName().toUpperCase(),
       _, result)
@@ -73,6 +46,8 @@ class ImplicitEllipticCurveInstance extends Crypto::EllipticCurveInstance,
     Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.getRawEllipticCurveName().toUpperCase(),
       result, _)
   }
+
+  override Crypto::AlgorithmValueConsumer getEllipticCurveConsumer() { result = this }
 }
 
 /**
@@ -112,6 +87,18 @@ abstract class SignatureAlgorithmInstance extends Crypto::KeyOperationAlgorithmI
   DataFlow::Node getEllipticCurveInput() { none() }
 }
 
+/**
+ * Represents an elliptic curve signature algorithm where both the signature
+ * algorithm and elliptic curve are implicitly defined by the underlying type.
+ */
+abstract class KnownEllipticCurveSignatureAlgorithmInstance extends KnownEllipticCurveInstance,
+  SignatureAlgorithmInstance
+{
+  override Crypto::ConsumerInputDataFlowNode getInputNode() { none() }
+
+  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
+}
+
 /**
  * DSA and DSADigest signers.
  */
@@ -129,22 +116,24 @@ class DSASignatureAlgorithmInstance extends SignatureAlgorithmInstance instanceo
 /**
  * Ed25519, Ed25519ph, and Ed25519ctx signers.
  */
-class Ed25519SignatureAlgorithmInstance extends SignatureAlgorithmInstance,
-  EllipticCurveConsumingAlgorithmInstance instanceof ClassInstanceExpr
+class Ed25519SignatureAlgorithmInstance extends KnownEllipticCurveSignatureAlgorithmInstance instanceof ClassInstanceExpr
 {
   Ed25519SignatureAlgorithmInstance() {
     super.getConstructedType() instanceof Signers::Signer and
     super.getConstructedType().getName().matches("Ed25519%")
   }
 
-  override string getFixedEllipticCurveName() { result = "Curve25519" }
+  override string getRawAlgorithmName() {
+    typeNameToRawAlgorithmName(super.getConstructedType().getName(), result)
+  }
+
+  override string getRawEllipticCurveName() { result = "Curve25519" }
 }
 
 /**
  * Ed448 and Ed448ph signers.
  */
-class Ed448SignatureAlgorithmInstance extends SignatureAlgorithmInstance,
-  EllipticCurveConsumingAlgorithmInstance instanceof ClassInstanceExpr
+class Ed448SignatureAlgorithmInstance extends KnownEllipticCurveSignatureAlgorithmInstance instanceof ClassInstanceExpr
 {
   Ed448SignatureAlgorithmInstance() {
     super.getConstructedType() instanceof Signers::Signer and
@@ -155,7 +144,7 @@ class Ed448SignatureAlgorithmInstance extends SignatureAlgorithmInstance,
     typeNameToRawAlgorithmName(super.getConstructedType().getName(), result)
   }
 
-  override string getFixedEllipticCurveName() { result = "Curve448" }
+  override string getRawEllipticCurveName() { result = "Curve448" }
 }
 
 /**
@@ -171,7 +160,7 @@ class Ed448SignatureAlgorithmInstance extends SignatureAlgorithmInstance,
 class ECDSASignatureAlgorithmInstance extends SignatureAlgorithmInstance instanceof ClassInstanceExpr
 {
   ECDSASignatureAlgorithmInstance() {
-    super.getConstructedType() instanceof Signers::Signer and
+    super.getConstructedType() instanceof Signers::OneShotSigner and
     super.getConstructedType().getName().matches("ECDSA%")
   }
 
@@ -186,6 +175,9 @@ class ECDSASignatureAlgorithmInstance extends SignatureAlgorithmInstance instanc
   override int getKeySizeFixed() { none() }
 }
 
+/**
+ * LMS signers.
+ */
 class LMSSignatureAlgorithmInstance extends SignatureAlgorithmInstance instanceof ClassInstanceExpr {
   LMSSignatureAlgorithmInstance() {
     super.getConstructedType() instanceof Signers::Signer and
@@ -239,50 +231,79 @@ abstract class KeyGenerationAlgorithmInstance extends Crypto::KeyOperationAlgori
   DataFlow::Node getEllipticCurveInput() { none() }
 }
 
-class Ed25519KeyGenerationAlgorithmInstance extends KeyGenerationAlgorithmInstance,
-  EllipticCurveConsumingAlgorithmInstance instanceof ClassInstanceExpr
+/**
+ * Represents an elliptic curve key generation algorithm where both the key
+ * generation algorithm and elliptic curve are implicitly defined by the
+ * underlying type.
+ */
+abstract class KnownEllipticCurveKeyGenerationAlgorithmInstance extends KnownEllipticCurveInstance,
+  KeyGenerationAlgorithmInstance
+{
+  override Crypto::ConsumerInputDataFlowNode getInputNode() { none() }
+
+  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
+}
+
+class Ed25519KeyGenerationAlgorithmInstance extends KnownEllipticCurveKeyGenerationAlgorithmInstance instanceof ClassInstanceExpr
 {
   Ed25519KeyGenerationAlgorithmInstance() {
     super.getConstructedType() instanceof Generators::KeyGenerator and
     super.getConstructedType().getName().matches("Ed25519%")
   }
 
-  override string getFixedEllipticCurveName() { result = "Curve25519" }
+  override string getRawEllipticCurveName() { result = "Curve25519" }
 }
 
-class Ed448KeyGenerationAlgorithmInstance extends KeyGenerationAlgorithmInstance,
-  EllipticCurveConsumingAlgorithmInstance instanceof ClassInstanceExpr
+class Ed448KeyGenerationAlgorithmInstance extends KnownEllipticCurveKeyGenerationAlgorithmInstance instanceof ClassInstanceExpr
 {
   Ed448KeyGenerationAlgorithmInstance() {
     super.getConstructedType() instanceof Generators::KeyGenerator and
     super.getConstructedType().getName().matches("Ed448%")
   }
 
-  override string getFixedEllipticCurveName() { result = "Curve448" }
+  override string getRawEllipticCurveName() { result = "Curve448" }
 }
 
 /**
  * Represents a generic `ECKeyPairGenerator` instance.
  */
-class GenericEllipticCurveKeyGenerationAlgorithmInstance extends KeyGenerationAlgorithmInstance instanceof ClassInstanceExpr
+class GenericEllipticCurveKeyGenerationAlgorithmInstance extends KeyGenerationAlgorithmInstance,
+  Crypto::EllipticCurveConsumingAlgorithmInstance instanceof ClassInstanceExpr
 {
   GenericEllipticCurveKeyGenerationAlgorithmInstance() {
     super.getConstructedType() instanceof Generators::KeyGenerator and
     super.getConstructedType().getName().matches("EC%")
   }
 
   override string getRawAlgorithmName() {
-    // TODO: The generator constructs an elliptic curve key pair, but the
-    // algorithm is not determined at key generation. As an example, the key
-    // could be used for either ECDSA or ECDH. For this reason, we just return
-    // "EllipticCurve".
-    result = "EllipticCurve"
+    // TODO: The generator constructs an elliptic curve key pair. The curve used
+    // is determined using data flow. If this fails we would like to report
+    // something useful, so we use "UnknownCurve". However, this should probably
+    // be handled at the node layer.
+    if exists(this.getConsumedEllipticCurve())
+    then result = this.getConsumedEllipticCurve().getRawEllipticCurveName()
+    else result = "UnknownCurve"
   }
 
   override Crypto::KeyOpAlg::Algorithm getAlgorithmType() {
-    // The algorithm type is not known. See above.
+    // TODO: There is currently to algorithm type for elliptic curve key
+    // generation.
     result = Crypto::KeyOpAlg::TUnknownKeyOperationAlgorithmType()
   }
+
+  override Crypto::AlgorithmValueConsumer getEllipticCurveConsumer() {
+    // The elliptic curve is resolved recursively from the parameters passed to
+    // the `init()` call.
+    exists(MethodCall init |
+      init = Generators::KeyGeneratorFlow::getInitFromNew(this, _, _) and
+      result =
+        Generators::ParametersFlow::getParametersFromInit(init, _, _).getAnAlgorithmValueConsumer()
+    )
+  }
+
+  Crypto::EllipticCurveInstance getConsumedEllipticCurve() {
+    result = this.getEllipticCurveConsumer().getAKnownAlgorithmSource()
+  }
 }
 
 /**

java/ql/lib/experimental/quantum/BouncyCastle/FlowAnalysis.qll

@@ -161,6 +161,12 @@ module NewToInitToUseFlowAnalysis<NewCallSig New, InitCallSig Init, UseCallSig U
     NewToInitToUseFlow::flowPath(src, sink)
   }
 
+  Init getInitFromNew(New new, NewToInitToUseFlow::PathNode src, NewToInitToUseFlow::PathNode sink) {
+    src.getNode().asExpr() = new and
+    sink.getNode().asExpr() = result.(MethodCall).getQualifier() and
+    NewToInitToUseFlow::flowPath(src, sink)
+  }
+
   Init getInitFromUse(Use use, NewToInitToUseFlow::PathNode src, NewToInitToUseFlow::PathNode sink) {
     src.getNode().asExpr() = result.(MethodCall).getQualifier() and
     sink.getNode().asExpr() = use.(MethodCall).getQualifier() and
@@ -211,7 +217,7 @@ private class CurveInstantiation extends MethodCall {
  * represents a parameter object or a curve.
  */
 module ParametersToInitFlowAnalysis<NewCallSig New, InitCallSig Init> {
-  module ParametersToInitConfig implements DataFlow::ConfigSig {
+  private module ParametersToInitConfig implements DataFlow::ConfigSig {
     predicate isSource(DataFlow::Node source) { source.asExpr() instanceof New }
 
     predicate isSink(DataFlow::Node sink) { exists(Init init | sink = init.getParametersInput()) }
@@ -261,7 +267,7 @@ module ParametersToInitFlowAnalysis<NewCallSig New, InitCallSig Init> {
     }
   }
 
-  module ParametersToInitFlow = DataFlow::Global<ParametersToInitConfig>;
+  private module ParametersToInitFlow = DataFlow::Global<ParametersToInitConfig>;
 
   New getParametersFromInit(
     Init init, ParametersToInitFlow::PathNode src, ParametersToInitFlow::PathNode sink

java/ql/lib/experimental/quantum/BouncyCastle/OperationInstances.qll

@@ -173,6 +173,7 @@ module Signers {
 
     override Expr getSignatureArg(MethodCall call) {
       // For ECDSA, r and s are passed to `verifySignature()` as separate arguments.
+      // For LMS, the signature is passed as a single argument in position 1.
       call.getCallee().getName() = "verifySignature" and
       result = call.getArgument([1, 2])
     }
@@ -204,20 +205,6 @@ module Signers {
     Expr getParametersArg() { result = this.getArgument(1) }
 
     DataFlow::Node getParametersInput() { result.asExpr() = this.getParametersArg() }
-
-    // TODO: Support dataflow for the operation sub-type.
-    Crypto::KeyOperationSubtype getKeyOperationSubtype() {
-      if this.isOperationSubTypeKnown()
-      then
-        this.getForSigningArg().(BooleanLiteral).getBooleanValue() = true and
-        result = Crypto::TSignMode()
-        or
-        this.getForSigningArg().(BooleanLiteral).getBooleanValue() = false and
-        result = Crypto::TVerifyMode()
-      else result = Crypto::TUnknownKeyOperationMode()
-    }
-
-    predicate isOperationSubTypeKnown() { this.getForSigningArg() instanceof BooleanLiteral }
   }
 
   /**
@@ -390,21 +377,12 @@ module Generators {
     Expr getOutput() { result = this }
   }
 
-  private module KeyGeneratorFlow =
+  module KeyGeneratorFlow =
     NewToInitToUseFlowAnalysis<KeyGeneratorNewCall, KeyGeneratorInitCall, KeyGeneratorUseCall>;
 
-  private module ParametersFlow =
+  module ParametersFlow =
     ParametersToInitFlowAnalysis<Params::ParametersInstantiation, KeyGeneratorInitCall>;
 
-  Params::ParametersInstantiation getParametersFromInit(KeyGeneratorInitCall init) {
-    result = ParametersFlow::getParametersFromInit(init, _, _)
-  }
-
-  // TODO: Remove this.
-  Params::ParametersInstantiation getParametersFromUse(KeyGeneratorInitCall init) {
-    result = ParametersFlow::getParametersFromInit(init, _, _)
-  }
-
   /**
    * A key generation operation instance is a call to `generateKey()` or
    * `generateKeyPair()` on a key generator defined under
@@ -413,7 +391,7 @@ module Generators {
   class KeyGenerationOperationInstance extends Crypto::KeyGenerationOperationInstance instanceof KeyGeneratorUseCall
   {
     override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
-      // The algorithm is implicit in the key generator type
+      // The algorithm is implicitly defined by the key generator type
       result = KeyGeneratorFlow::getNewFromUse(this, _, _)
     }