Added support for BouncyCast ECDSA

This commit adds support for ECDSA. This includes tracking the
instantiated curve parameters using data flow.

It also adds SignatureArtifactInstance and SignatureOperationInstance
types to the shared model.

Author: fegge

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

@@ -1,5 +1,101 @@
 import java
 
+module Params {
+  import Language
+  import BouncyCastle.FlowAnalysis
+  import BouncyCastle.AlgorithmInstances
+
+  /**
+   * A model of the `Parameters` class in Bouncy Castle.
+   */
+  class Parameters extends RefType {
+    Parameters() {
+      // Matches `org.bouncycastle.crypto.params`, `org.bouncycastle.asn1.x9`, etc.
+      this.getPackage().getName().matches("org.bouncycastle.%") and
+      this.getName().matches("%Parameters")
+    }
+  }
+
+  class KeyParameters extends Parameters {
+    KeyParameters() {
+      this.getPackage().getName() = "org.bouncycastle.crypto.params" and
+      this.getName().matches("%KeyParameters")
+    }
+  }
+
+  /**
+   * Any call that returns a BouncyCastle parameters object. This type is used
+   * to model data flow to resolve algorithm instances like elliptic curves.
+   *
+   * Examples:
+   * ```
+   * curveParams = SECNamedCurves.getByName(...);
+   * domainParams = new ECDomainParameters(...);
+   * ```
+   */
+  class ParametersInstantiation extends Call {
+    ParametersInstantiation() {
+      // Class instantiations
+      this.(ConstructorCall)
+          .getConstructedType()
+          .getPackage()
+          .getName()
+          .matches("org.bouncycastle.%") and
+      this.(ConstructorCall).getConstructedType() instanceof Parameters
+      or
+      // (Static) factory methods
+      this.(MethodCall)
+          .getCallee()
+          .getDeclaringType()
+          .getPackage()
+          .getName()
+          .matches("org.bouncycastle.%") and
+      this.(MethodCall).getType() instanceof Parameters
+    }
+
+    // Can be overridden by subclasses which take a key size argument.
+    Expr getKeySizeArg() { none() }
+
+    Crypto::ConsumerInputDataFlowNode getAKeySizeConsumer() {
+      result.asExpr() = this.getKeySizeArg()
+    }
+
+    // Can be overridden by subclasses which take an algorithm argument.
+    Expr getAlgorithmArg() { none() }
+
+    Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
+      result.getInputNode().asExpr() = this.getAlgorithmArg()
+    }
+
+    Expr getAParametersArg() {
+      result = this.getAnArgument() and
+      result.getType() instanceof Parameters
+    }
+
+    Crypto::ConsumerInputDataFlowNode getAParametersConsumer() {
+      result.asExpr() = this.getAParametersArg()
+    }
+  }
+
+  class X9ECParametersInstantiation extends ParametersInstantiation {
+    X9ECParametersInstantiation() { this.(Expr).getType().getName() = "X9ECParameters" }
+
+    override Expr getAlgorithmArg() { result = this.getArgument(0) }
+  }
+
+  class EllipticCurveStringLiteralArg extends EllipticCurveAlgorithmValueConsumer instanceof Expr {
+    ParametersInstantiation params;
+
+    EllipticCurveStringLiteralArg() { this = params.getAlgorithmArg() }
+
+    override Crypto::ConsumerInputDataFlowNode getInputNode() { result.asExpr() = this }
+
+    override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
+      result.(EllipticCurveStringLiteralInstance).getConsumer() = this
+    }
+  }
+}
+
 /**
  * Models for the signature algorithms defined by the `org.bouncycastle.crypto.signers` package.
  */
@@ -26,6 +122,40 @@ module Signers {
     MethodCall getAMethodCall(string name) {
       result.getCallee().hasQualifiedName(this.getPackage().getName(), this.getName(), name)
     }
+
+    // Overridden by subclasses to provide the message argument.
+    Expr getMessageArg(MethodCall call) {
+      call.getCallee().getName() = "update" and
+      result = call.getArgument(0)
+    }
+
+    // Overridden by subclasses to provide the signature argument.
+    Expr getSignatureArg(MethodCall call) {
+      call.getCallee().getName() = "verifySignature" and
+      result = call.getArgument(0)
+    }
+
+    // Overridden by subclasses to provide the signature output.
+    Expr getSignatureOutput(MethodCall call) {
+      call.getCallee().getName() = "generateSignature" and
+      result = call
+    }
+  }
+
+  class ECDSASigner extends Signer {
+    ECDSASigner() { this.getName().matches("ECDSA%") }
+
+    override Expr getMessageArg(MethodCall call) {
+      // For ECDSA the message is passed directly to `generateSignature()`.
+      call.getCallee().getName().matches(["generateSignature", "verifySignature"]) and
+      result = call.getArgument(0)
+    }
+
+    override Expr getSignatureArg(MethodCall call) {
+      // For ECDSA, r and s are passed to `verifySignature()` as separate arguments.
+      call.getCallee().getName() = "verifySignature" and
+      result = call.getArgument([1, 2])
+    }
   }
 
   /**
@@ -45,7 +175,17 @@ module Signers {
 
     Expr getForSigningArg() { result = this.getArgument(0) }
 
-    Expr getKeyArg() { result = this.getArgument(1) }
+    Expr getKeyArg() {
+      this.getParameterArg().getType() instanceof Params::KeyParameters and
+      result = this.getParameterArg()
+    }
+
+    // The second argument is used to provide parameters (like the key) to the signer.
+    Expr getParameterArg() { result = this.getArgument(1) }
+
+    Crypto::ConsumerInputDataFlowNode getAParametersConsumer() {
+      result.asExpr() = this.getParameterArg()
+    }
 
     // TODO: Support dataflow for the operation sub-type.
     Crypto::KeyOperationSubtype getKeyOperationSubtype() {
@@ -68,36 +208,48 @@ module Signers {
    * verify the signature, respectively.
    */
   private class SignerUseCall extends MethodCall {
-    SignerUseCall() { this = any(Signer signer).getAUseCall() }
+    Signer signer;
+
+    SignerUseCall() { this = signer.getAUseCall() }
 
     predicate isIntermediate() { this.getCallee().getName() = "update" }
 
-    Expr getInput() { result = this.getArgument(0) }
+    Expr getMessageInput() { result = signer.getMessageArg(this) }
 
-    Expr getOutput() { result = this }
+    Expr getSignatureInput() { result = signer.getSignatureArg(this) }
+
+    Expr getSignatureOutput() { result = signer.getSignatureOutput(this) }
   }
 
   /**
    * Instantiate the flow analysis module for the `Signer` class.
    */
-  private module FlowAnalysis =
+  private module SignerFlow =
     NewToInitToUseFlowAnalysis<SignerNewCall, SignerInitCall, SignerUseCall>;
 
   /**
    * A signing operation instance is a call to either `update()`, `generateSignature()`,
    * or `verifySignature()` on a `Signer` instance.
    */
-  class SignatureOperationInstance extends Crypto::KeyOperationInstance instanceof SignerUseCall {
+  class SignatureOperationInstance extends Crypto::SignatureOperationInstance instanceof SignerUseCall
+  {
     SignatureOperationInstance() { not this.isIntermediate() }
 
     override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
-      result = FlowAnalysis::getNewFromUse(this, _, _)
+      result = this.getParameters().getAnAlgorithmValueConsumer()
+      or
+      result = SignerFlow::getNewFromUse(this, _, _)
     }
 
     override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
-      if FlowAnalysis::hasInit(this)
-      then result = this.getInitCall().getKeyOperationSubtype()
-      else result = Crypto::TUnknownKeyOperationMode()
+      // This is less expensive and more robust than resolving the subtype using
+      // dataflow from the `forSigning` argument to `init()`.
+      if super.getMethod().getName() = "generateSignature"
+      then result = Crypto::TSignMode()
+      else
+        if super.getMethod().getName() = "verifySignature"
+        then result = Crypto::TVerifyMode()
+        else result = Crypto::TUnknownKeyOperationMode()
     }
 
     override Crypto::ConsumerInputDataFlowNode getKeyConsumer() {
@@ -107,18 +259,36 @@ module Signers {
     override Crypto::ConsumerInputDataFlowNode getNonceConsumer() { none() }
 
     override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
-      result.asExpr() = this.getAnUpdateCall().getInput()
+      // Inputs to signers with streaming APIs
+      result.asExpr() = this.getAnUpdateCall().getMessageInput()
+      or
+      // Inputs to signers with one shot APIs
+      result.asExpr() = super.getMessageInput()
+    }
+
+    override Crypto::ConsumerInputDataFlowNode getSignatureArtifactConsumer() {
+      result.asExpr() = super.getSignatureInput()
     }
 
     override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
-      this.getKeyOperationSubtype() = Crypto::TSignMode() and
-      result.asExpr() = super.getOutput()
+      // Signature output
+      result.asExpr() = super.getSignatureOutput()
     }
 
-    SignerInitCall getInitCall() { result = FlowAnalysis::getInitFromUse(this, _, _) }
+    SignerInitCall getInitCall() { result = SignerFlow::getInitFromUse(this, _, _) }
 
     SignerUseCall getAnUpdateCall() {
-      result = FlowAnalysis::getAnIntermediateUseFromFinalUse(this, _, _)
+      result = SignerFlow::getAnIntermediateUseFromFinalUse(this, _, _)
+    }
+
+    Crypto::KeyArtifactOutputInstance getKey() { result.flowsTo(this.getInitCall().getKeyArg()) }
+
+    Generators::KeyGenerationOperationInstance getKeyGenerationOperationInstance() {
+      result.getKeyArtifactOutputInstance() = this.getKey()
+    }
+
+    Params::ParametersInstantiation getParameters() {
+      result = this.getKeyGenerationOperationInstance().getParameters()
     }
   }
 }
@@ -194,6 +364,11 @@ module Generators {
     // TODO: We may need to model this using the `parameters` argument passed to
     // the `init()` method.
     Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() { none() }
+
+    // The `KeyGenerationParameters` argument used to configure the key generator.
+    Crypto::ConsumerInputDataFlowNode getAParametersConsumer() {
+      result.asExpr() = this.getArgument(0)
+    }
   }
 
   /**
@@ -216,6 +391,14 @@ module Generators {
   private module KeyGeneratorFlow =
     NewToInitToUseFlowAnalysis<KeyGeneratorNewCall, KeyGeneratorInitCall, KeyGeneratorUseCall>;
 
+  private module ParametersFlow =
+    ParametersToInitFlowAnalysis<Params::ParametersInstantiation, KeyGeneratorInitCall>;
+
+  Params::ParametersInstantiation getParametersFromInit(KeyGeneratorInitCall init) {
+    result = ParametersFlow::getParametersFromInit(init, _, _) and
+    result instanceof Params::X9ECParametersInstantiation
+  }
+
   /**
    * A key generation operation instance is a call to `generateKey()` or
    * `generateKeyPair()` on a key generator defined under
@@ -224,6 +407,10 @@ module Generators {
   class KeyGenerationOperationInstance extends Crypto::KeyGenerationOperationInstance instanceof KeyGeneratorUseCall
   {
     override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
+      // The algorithm value consumer flows through a parameters argument to `init()`
+      result = this.getParameters().getAnAlgorithmValueConsumer()
+      or
+      // The algorithm is implicit in the key generator type
       result = KeyGeneratorFlow::getNewFromUse(this, _, _)
     }
 
@@ -240,17 +427,12 @@ module Generators {
     override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() {
       result = KeyGeneratorFlow::getInitFromUse(this, _, _).getKeySizeConsumer()
     }
-  }
-}
 
-/**
- * Models for cryptographic parameters defined by the `org.bouncycastle.crypto.params` package.
- */
-module Parameters {
-  class KeyGenerationParameters extends RefType {
-    KeyGenerationParameters() {
-      this.getPackage().getName() = "org.bouncycastle.crypto.params" and
-      this.getName().matches("%KeyGenerationParameters")
+    Params::ParametersInstantiation getParameters() {
+      exists(KeyGeneratorInitCall init |
+        init = KeyGeneratorFlow::getInitFromUse(this, _, _) and
+        result = ParametersFlow::getParametersFromInit(init, _, _)
+      )
     }
   }
 }