Signature model refactor

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll

@@ -49,7 +49,9 @@ module KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow =
   DataFlow::Global<KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig>;
 
 module RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof OpenSslPaddingLiteral }
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof OpenSslSpecialPaddingLiteral
+  }
 
   predicate isSink(DataFlow::Node sink) {
     exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll

@@ -2,12 +2,10 @@ import cpp
 private import experimental.quantum.Language
 private import KnownAlgorithmConstants
 private import Crypto::KeyOpAlg as KeyOpAlg
-private import OpenSSLAlgorithmInstanceBase
-private import PaddingAlgorithmInstance
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
+private import experimental.quantum.OpenSSL.Operations.OpenSSLOperationBase
+private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
+private import OpenSSLAlgorithmInstances
 private import AlgToAVCFlow
-private import BlockAlgorithmInstance
 
 /**
  * Given a `KnownOpenSslCipherAlgorithmExpr`, converts this to a cipher family type.
@@ -97,10 +95,13 @@ class KnownOpenSslCipherConstantAlgorithmInstance extends OpenSslAlgorithmInstan
   }
 
   override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() {
-    //TODO: the padding is either self, or it flows through getter ctx to a set padding call
-    // like EVP_PKEY_CTX_set_rsa_padding
     result = this
-    // TODO or trace through getter ctx to set padding
+    or
+    exists(OperationStep s |
+      this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
+      s.getAlgorithmValueConsumerForInput(PaddingAlgorithmIO()) =
+        result.(OpenSslAlgorithmInstance).getAvc()
+    )
   }
 
   override string getRawAlgorithmName() {

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/MACAlgorithmInstance.qll

@@ -2,12 +2,13 @@ import cpp
 private import experimental.quantum.Language
 private import KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
-private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
+private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
 private import experimental.quantum.OpenSSL.Operations.OpenSSLOperations
+private import Crypto::KeyOpAlg as KeyOpAlg
 private import AlgToAVCFlow
 
 class KnownOpenSslMacConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
-  Crypto::MacAlgorithmInstance instanceof KnownOpenSslMacAlgorithmExpr
+  Crypto::KeyOperationAlgorithmInstance instanceof KnownOpenSslMacAlgorithmExpr
 {
   OpenSslAlgorithmValueConsumer getterCall;
 
@@ -33,17 +34,34 @@ class KnownOpenSslMacConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
 
   override OpenSslAlgorithmValueConsumer getAvc() { result = getterCall }
 
-  override string getRawMacAlgorithmName() {
+  override string getRawAlgorithmName() {
     result = this.(Literal).getValue().toString()
     or
     result = this.(Call).getTarget().getName()
   }
 
-  override Crypto::MacType getMacType() {
-    this instanceof KnownOpenSslHMacAlgorithmExpr and result = Crypto::HMAC()
-    or
-    this instanceof KnownOpenSslCMacAlgorithmExpr and result = Crypto::CMAC()
+  override Crypto::KeyOpAlg::AlgorithmType getAlgorithmType() {
+    if this instanceof KnownOpenSslHMacAlgorithmExpr
+    then result = KeyOpAlg::TMac(KeyOpAlg::HMAC())
+    else
+      if this instanceof KnownOpenSslCMacAlgorithmExpr
+      then result = KeyOpAlg::TMac(KeyOpAlg::CMAC())
+      else result = KeyOpAlg::TMac(KeyOpAlg::OtherMacAlgorithmType())
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() {
+    // TODO: trace to any key size initializer?
+    none()
+  }
+
+  override int getKeySizeFixed() {
+    // TODO: are there known fixed key sizes to consider?
+    none()
   }
+
+  override Crypto::ModeOfOperationAlgorithmInstance getModeOfOperationAlgorithm() { none() }
+
+  override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() { none() }
 }
 
 class KnownOpenSslHMacConstantAlgorithmInstance extends Crypto::HmacAlgorithmInstance,
@@ -60,9 +78,13 @@ class KnownOpenSslHMacConstantAlgorithmInstance extends Crypto::HmacAlgorithmIns
       // where the current AVC traces to a HashAlgorithmIO consuming operation step.
       // TODO: need to consider getting reset values, tracing down to the first set for now
       exists(OperationStep s, AvcContextCreationStep avc |
-        avc = this.getAvc() and
+        avc = super.getAvc() and
         avc.flowsToOperationStep(s) and
         s.getAlgorithmValueConsumerForInput(HashAlgorithmIO()) = result
       )
   }
+
+  override Crypto::ModeOfOperationAlgorithmInstance getModeOfOperationAlgorithm() { none() }
+
+  override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() { none() }
 }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll

@@ -1,10 +1,10 @@
 import cpp
 private import experimental.quantum.Language
 private import OpenSSLAlgorithmInstanceBase
+private import experimental.quantum.OpenSSL.Operations.OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import AlgToAVCFlow
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
+private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 private import codeql.quantum.experimental.Standardization::Types::KeyOpAlg as KeyOpAlg
 
 /**
@@ -18,13 +18,14 @@ private import codeql.quantum.experimental.Standardization::Types::KeyOpAlg as K
  *     # define RSA_PKCS1_WITH_TLS_PADDING 7
  *     # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
  */
-class OpenSslPaddingLiteral extends Literal {
+class OpenSslSpecialPaddingLiteral extends Literal {
   // TODO: we can be more specific about where the literal is in a larger expression
   // to avoid literals that are clealy not representing an algorithm, e.g., array indices.
-  OpenSslPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
+  OpenSslSpecialPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
 }
 
 /**
+ * Holds if `e` has the given `type`.
  * Given a `KnownOpenSslPaddingAlgorithmExpr`, converts this to a padding family type.
  * Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
  */
@@ -45,9 +46,6 @@ predicate knownOpenSslConstantToPaddingFamilyType(
   )
 }
 
-//abstract class OpenSslPaddingAlgorithmInstance extends OpenSslAlgorithmInstance, Crypto::PaddingAlgorithmInstance{}
-// TODO: need to alter this to include known padding constants which don't have the
-// same mechanics as those with known nids
 class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
   Crypto::PaddingAlgorithmInstance instanceof Expr
 {
@@ -79,7 +77,7 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
     isPaddingSpecificConsumer = false
     or
     // Possibility 3: padding-specific literal
-    this instanceof OpenSslPaddingLiteral and
+    this instanceof OpenSslSpecialPaddingLiteral and
     exists(DataFlow::Node src, DataFlow::Node sink |
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
@@ -124,44 +122,6 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
   }
 }
 
-// // Values used for EVP_PKEY_CTX_set_rsa_padding, these are
-// // not the same as 'typical' constants found in the set of known algorithm constants
-// // they do not have an NID
-// // TODO: what about setting the padding directly?
-// class KnownRSAPaddingConstant extends OpenSslPaddingAlgorithmInstance, Crypto::PaddingAlgorithmInstance instanceof Literal
-// {
-//   KnownRSAPaddingConstant() {
-//     // from rsa.h in openssl:
-//     // # define RSA_PKCS1_PADDING          1
-//     // # define RSA_NO_PADDING             3
-//     // # define RSA_PKCS1_OAEP_PADDING     4
-//     // # define RSA_X931_PADDING           5
-//     // /* EVP_PKEY_ only */
-//     // # define RSA_PKCS1_PSS_PADDING      6
-//     // # define RSA_PKCS1_WITH_TLS_PADDING 7
-//     // /* internal RSA_ only */
-//     // # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
-//     this instanceof Literal and
-//     this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8]
-//     // TODO: trace to padding-specific consumers
-//     RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow
-//   }
-//   override string getRawPaddingAlgorithmName() { result = this.(Literal).getValue().toString() }
-//   override Crypto::TPaddingType getPaddingType() {
-//     if this.(Literal).getValue().toInt() in [1, 6, 7, 8]
-//     then result = Crypto::PKCS1_v1_5()
-//     else
-//       if this.(Literal).getValue().toInt() = 3
-//       then result = Crypto::NoPadding()
-//       else
-//         if this.(Literal).getValue().toInt() = 4
-//         then result = Crypto::OAEP()
-//         else
-//           if this.(Literal).getValue().toInt() = 5
-//           then result = Crypto::ANSI_X9_23()
-//           else result = Crypto::OtherPadding()
-//   }
-// }
 class OaepPaddingAlgorithmInstance extends Crypto::OaepPaddingAlgorithmInstance,
   KnownOpenSslPaddingConstantAlgorithmInstance
 {
@@ -170,10 +130,18 @@ class OaepPaddingAlgorithmInstance extends Crypto::OaepPaddingAlgorithmInstance,
   }
 
   override Crypto::HashAlgorithmInstance getOaepEncodingHashAlgorithm() {
-    none() //TODO
+    exists(OperationStep s |
+      this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
+      s.getAlgorithmValueConsumerForInput(HashAlgorithmOaepIO()) =
+        result.(OpenSslAlgorithmInstance).getAvc()
+    )
   }
 
   override Crypto::HashAlgorithmInstance getMgf1HashAlgorithm() {
-    none() //TODO
+    exists(OperationStep s |
+      this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
+      s.getAlgorithmValueConsumerForInput(HashAlgorithmMgf1IO()) =
+        result.(OpenSslAlgorithmInstance).getAvc()
+    )
   }
 }

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/CipherOperation.qll

@@ -3,6 +3,26 @@ private import OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 import EVPPKeyCtxInitializer
 
+/**
+ * A base class for all final cipher operation steps.
+ */
+abstract class FinalCipherOperationStep extends OperationStep {
+  override OperationStepType getStepType() { result = FinalStep() }
+}
+
+/**
+ * A base configuration for all EVP cipher operations.
+ */
+abstract class EvpCipherOperationFinalStep extends FinalCipherOperationStep {
+  override DataFlow::Node getInput(IOType type) {
+    result.asExpr() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asExpr() = this.getArgument(0) and type = ContextIO()
+  }
+}
+
 /**
  * A base class for all EVP cipher operations.
  */
@@ -155,21 +175,6 @@ class EvpCipherUpdateCall extends OperationStep {
   override OperationStepType getStepType() { result = UpdateStep() }
 }
 
-/**
- * A base configuration for all EVP cipher operations.
- */
-abstract class EvpCipherOperationFinalStep extends OperationStep {
-  override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
-  }
-
-  override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
-  }
-
-  override OperationStepType getStepType() { result = FinalStep() }
-}
-
 /**
  * A Call to EVP_Cipher.
  */
@@ -216,7 +221,14 @@ class EvpCipherFinalCall extends EvpCipherOperationFinalStep {
  */
 class EvpPKeyCipherOperation extends EvpCipherOperationFinalStep {
   EvpPKeyCipherOperation() {
-    this.getTarget().getName() in ["EVP_PKEY_encrypt", "EVP_PKEY_decrypt"]
+    this.getTarget().getName() in ["EVP_PKEY_encrypt", "EVP_PKEY_decrypt"] and
+    // TODO: for now ignore this operation entirely if it is setting the cipher text to null
+    // this needs to be re-evalauted if this scenario sets other values worth tracking
+    (
+      exists(this.(Call).getArgument(1).getValue())
+      implies
+      this.(Call).getArgument(1).getValue().toInt() != 0
+    )
   }
 
   override DataFlow::Node getInput(IOType type) {
@@ -228,16 +240,31 @@ class EvpPKeyCipherOperation extends EvpCipherOperationFinalStep {
   override DataFlow::Node getOutput(IOType type) {
     super.getOutput(type) = result
     or
-    result.asExpr() = this.getArgument(1) and type = CiphertextIO()
+    result.asExpr() = this.getArgument(1) and
+    type = CiphertextIO() and
+    this.getStepType() = FinalStep()
     // TODO: could indicate text lengths here, as well
   }
+
+  override OperationStepType getStepType() {
+    // When the output buffer is null, the step is not a final step
+    // it is used to get the buffer size, if 0 consider it an initialization step
+    // NOTE/TODO: not tracing 0 to the arg, just looking for 0 directly in param
+    // the assumption is this is the common case, but we may want to make this more
+    // robust and support a dataflow.
+    result = FinalStep() and
+    (exists(super.getArgument(1).getValue()) implies super.getArgument(1).getValue().toInt() != 0)
+    or
+    result = InitializerStep() and
+    super.getArgument(1).getValue().toInt() = 0
+  }
 }
 
 /**
  * An EVP cipher operation instance.
  * Any operation step that is a final operation step for EVP cipher operation steps.
  */
-class EvpCipherOperationInstance extends Crypto::KeyOperationInstance instanceof EvpCipherOperationFinalStep
+class OpenSslCipherOperationInstance extends Crypto::KeyOperationInstance instanceof FinalCipherOperationStep
 {
   override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
     super.getPrimaryAlgorithmValueConsumer() = result

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPPKeyCtxInitializer.qll

@@ -71,19 +71,42 @@ class EvpCtxSetEcParamgenCurveNidInitializer extends OperationStep {
  * - `EVP_PKEY_CTX_set_ecdh_kdf_md`
  */
 class EvpCtxSetHashInitializer extends OperationStep {
+  boolean isOaep;
+  boolean isMgf1;
+
   EvpCtxSetHashInitializer() {
     this.getTarget().getName() in [
-        "EVP_PKEY_CTX_set_signature_md", "EVP_PKEY_CTX_set_rsa_mgf1_md_name",
-        "EVP_PKEY_CTX_set_rsa_mgf1_md", "EVP_PKEY_CTX_set_rsa_oaep_md_name",
-        "EVP_PKEY_CTX_set_rsa_oaep_md", "EVP_PKEY_CTX_set_dsa_paramgen_md",
+        "EVP_PKEY_CTX_set_signature_md", "EVP_PKEY_CTX_set_dsa_paramgen_md",
         "EVP_PKEY_CTX_set_dh_kdf_md", "EVP_PKEY_CTX_set_ecdh_kdf_md"
-      ]
+      ] and
+    isOaep = false and
+    isMgf1 = false
+    or
+    this.getTarget().getName() in [
+        "EVP_PKEY_CTX_set_rsa_mgf1_md_name", "EVP_PKEY_CTX_set_rsa_mgf1_md"
+      ] and
+    isOaep = false and
+    isMgf1 = true
+    or
+    this.getTarget().getName() in [
+        "EVP_PKEY_CTX_set_rsa_oaep_md_name",
+        "EVP_PKEY_CTX_set_rsa_oaep_md"
+      ] and
+    isOaep = true and
+    isMgf1 = false
   }
 
   override DataFlow::Node getInput(IOType type) {
     result.asExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = HashAlgorithmIO()
+    result.asExpr() = this.getArgument(1) and
+    type = HashAlgorithmIO() and
+    isOaep = false and
+    isMgf1 = false
+    or
+    result.asExpr() = this.getArgument(1) and type = HashAlgorithmOaepIO() and isOaep = true
+    or
+    result.asExpr() = this.getArgument(1) and type = HashAlgorithmMgf1IO() and isMgf1 = true
   }
 
   override DataFlow::Node getOutput(IOType type) {