Merge pull request #19607 from trailofbits/openssl-base-classes

Quantum: Add base classes for OpenSSL EVP methods

Author: nicolaswill

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

@@ -4,42 +4,15 @@ private import OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 private import semmle.code.cpp.dataflow.new.DataFlow
 
-private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source)
-  }
-
-  predicate isSink(DataFlow::Node sink) {
-    exists(ECKeyGenOperation c | c.getAlgorithmArg() = sink.asExpr())
-  }
-}
-
-private module AlgGetterToAlgConsumerFlow = DataFlow::Global<AlgGetterToAlgConsumerConfig>;
-
 class ECKeyGenOperation extends OpenSSLOperation, Crypto::KeyGenerationOperationInstance {
   ECKeyGenOperation() { this.(Call).getTarget().getName() = "EC_KEY_generate_key" }
 
-  override Expr getOutputArg() {
-    result = this.(Call) // return value of call
-  }
-
-  Expr getAlgorithmArg() { result = this.(Call).getArgument(0) }
-
-  override Expr getInputArg() {
-    // there is no 'input', in the sense that no data is being manipulated by the operation.
-    // There is an input of an algorithm, but that is not the intention of the operation input arg.
-    none()
-  }
+  override Expr getAlgorithmArg() { result = this.(Call).getArgument(0) }
 
   override Crypto::KeyArtifactType getOutputKeyType() { result = Crypto::TAsymmetricKeyType() }
 
   override Crypto::ArtifactOutputDataFlowNode getOutputKeyArtifact() {
-    result = this.getOutputNode()
-  }
-
-  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
-    AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(),
-      DataFlow::exprNode(this.getAlgorithmArg()))
+    result.asExpr() = this.(Call).getArgument(0)
   }
 
   override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() {

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

@@ -5,6 +5,7 @@
 
 private import experimental.quantum.Language
 private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow
+private import OpenSSLOperationBase
 
 module EncValToInitEncArgConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr().getValue().toInt() in [0, 1] }
@@ -34,19 +35,12 @@ Crypto::KeyOperationSubtype intToCipherOperationSubtype(int i) {
 }
 
 // TODO: need to add key consumer
-abstract class EVP_Cipher_Initializer extends Call {
-  Expr getContextArg() { result = this.(Call).getArgument(0) }
+abstract class EVP_Cipher_Initializer extends EVPInitialize {
+  override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) }
 
-  Expr getAlgorithmArg() { result = this.(Call).getArgument(1) }
-
-  abstract Expr getKeyArg();
-
-  abstract Expr getIVArg();
-
-  //   abstract Crypto::CipherOperationSubtype getCipherOperationSubtype();
   abstract Expr getOperationSubtypeArg();
 
-  Crypto::KeyOperationSubtype getCipherOperationSubtype() {
+  override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
     if this.(Call).getTarget().getName().toLowerCase().matches("%encrypt%")
     then result instanceof Crypto::TEncryptMode
     else

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

@@ -4,36 +4,23 @@ private import EVPCipherInitializer
 private import OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 
-private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source)
+class EVP_Cipher_Update_Call extends EVPUpdate {
+  EVP_Cipher_Update_Call() {
+    this.(Call).getTarget().getName() in [
+        "EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate"
+      ]
   }
 
-  predicate isSink(DataFlow::Node sink) {
-    exists(EVP_Cipher_Operation c | c.getAlgorithmArg() = sink.asExpr())
-  }
-}
+  override Expr getInputArg() { result = this.(Call).getArgument(3) }
 
-private module AlgGetterToAlgConsumerFlow = DataFlow::Global<AlgGetterToAlgConsumerConfig>;
+  override Expr getOutputArg() { result = this.(Call).getArgument(1) }
+}
 
-// import experimental.quantum.OpenSSL.AlgorithmValueConsumers.AlgorithmValueConsumers
-// import OpenSSLOperation
-// class EVPCipherOutput extends CipherOutputArtifact {
-//   EVPCipherOutput() { exists(EVP_Cipher_Operation op | op.getOutputArg() = this) }
-//   override DataFlow::Node getOutputNode() { result.asDefiningArgument() = this }
-// }
-//
 /**
  * see: https://docs.openssl.org/master/man3/EVP_EncryptInit/#synopsis
  * Base configuration for all EVP cipher operations.
- * NOTE: cannot extend instance of OpenSSLOperation, as we need to override
- * elements of OpenSSLOperation (i.e., we are creating an instance)
  */
-abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperationInstance {
-  Expr getContextArg() { result = this.(Call).getArgument(0) }
-
-  Expr getAlgorithmArg() { this.getInitCall().getAlgorithmArg() = result }
-
+abstract class EVP_Cipher_Operation extends EVPOperation, Crypto::KeyOperationInstance {
   override Expr getOutputArg() { result = this.(Call).getArgument(1) }
 
   override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
@@ -43,81 +30,48 @@ abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperati
     result instanceof Crypto::TDecryptMode and
     this.(Call).getTarget().getName().toLowerCase().matches("%decrypt%")
     or
-    result = this.getInitCall().getCipherOperationSubtype() and
+    result = this.getInitCall().getKeyOperationSubtype() and
     this.(Call).getTarget().getName().toLowerCase().matches("%cipher%")
   }
 
-  EVP_Cipher_Initializer getInitCall() {
-    CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
-  }
-
   override Crypto::ConsumerInputDataFlowNode getNonceConsumer() {
     this.getInitCall().getIVArg() = result.asExpr()
   }
 
-  override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
-
   override Crypto::ConsumerInputDataFlowNode getKeyConsumer() {
     this.getInitCall().getKeyArg() = result.asExpr()
+    // todo: or track to the EVP_PKEY_CTX_new
   }
 
-  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() }
+  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
+    result = EVPOperation.super.getOutputArtifact()
+  }
 
-  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
-    AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(),
-      DataFlow::exprNode(this.getInitCall().getAlgorithmArg()))
+  override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
+    result = EVPOperation.super.getInputConsumer()
   }
 }
 
-class EVP_Cipher_Call extends EVP_Cipher_Operation {
+class EVP_Cipher_Call extends EVPOperation, EVP_Cipher_Operation {
   EVP_Cipher_Call() { this.(Call).getTarget().getName() = "EVP_Cipher" }
 
   override Expr getInputArg() { result = this.(Call).getArgument(2) }
 }
 
-// NOTE: not modeled as cipher operations, these are intermediate calls
-class EVP_Cipher_Update_Call extends Call {
-  EVP_Cipher_Update_Call() {
-    this.(Call).getTarget().getName() in [
-        "EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate"
-      ]
-  }
-
-  Expr getInputArg() { result = this.(Call).getArgument(3) }
-
-  DataFlow::Node getInputNode() { result.asExpr() = this.getInputArg() }
-
-  Expr getContextArg() { result = this.(Call).getArgument(0) }
-}
-
-class EVP_Cipher_Final_Call extends EVP_Cipher_Operation {
+class EVP_Cipher_Final_Call extends EVPFinal, EVP_Cipher_Operation {
   EVP_Cipher_Final_Call() {
     this.(Call).getTarget().getName() in [
         "EVP_EncryptFinal_ex", "EVP_DecryptFinal_ex", "EVP_CipherFinal_ex", "EVP_EncryptFinal",
         "EVP_DecryptFinal", "EVP_CipherFinal"
       ]
   }
 
-  EVP_Cipher_Update_Call getUpdateCalls() {
-    CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
-  }
-
-  override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() }
-
-  override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
-}
-
-class EVP_PKEY_Operation extends EVP_Cipher_Operation {
-  EVP_PKEY_Operation() {
-    this.(Call).getTarget().getName() in ["EVP_PKEY_decrypt", "EVP_PKEY_encrypt"]
+  /**
+   * Output is both from update calls and from the final call.
+   */
+  override Expr getOutputArg() {
+    result = EVPFinal.super.getOutputArg()
+    or
+    result = EVP_Cipher_Operation.super.getOutputArg()
   }
-
-  override Expr getInputArg() { result = this.(Call).getArgument(3) }
-  // TODO: how PKEY is initialized is different that symmetric cipher
-  // Consider making an entirely new class for this and specializing
-  // the get init call
-}
-
-class EVPCipherInputArgument extends Expr {
-  EVPCipherInputArgument() { exists(EVP_Cipher_Operation op | op.getInputArg() = this) }
 }

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

@@ -1,10 +1,7 @@
 import cpp
+private import OpenSSLOperationBase
 
-abstract class EVP_Hash_Initializer extends Call {
-  Expr getContextArg() { result = this.(Call).getArgument(0) }
-
-  abstract Expr getAlgorithmArg();
-}
+abstract class EVP_Hash_Initializer extends EVPInitialize { }
 
 class EVP_DigestInit_Variant_Calls extends EVP_Hash_Initializer {
   EVP_DigestInit_Variant_Calls() {

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

@@ -8,118 +8,78 @@ private import OpenSSLOperationBase
 private import EVPHashInitializer
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 
-// import EVPHashConsumers
-abstract class EVP_Hash_Operation extends OpenSSLOperation, Crypto::HashOperationInstance {
-  Expr getContextArg() { result = this.(Call).getArgument(0) }
+class EVP_Digest_Update_Call extends EVPUpdate {
+  EVP_Digest_Update_Call() { this.(Call).getTarget().getName() = "EVP_DigestUpdate" }
 
-  Expr getAlgorithmArg() { result = this.getInitCall().getAlgorithmArg() }
-
-  EVP_Hash_Initializer getInitCall() {
-    CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
-  }
-
-  /**
-   * By default, the algorithm value comes from the init call.
-   * There are variants where this isn't true, in which case the
-   * subclass should override this method.
-   */
-  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
-    AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(),
-      DataFlow::exprNode(this.getAlgorithmArg()))
-  }
-}
-
-private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source)
-  }
-
-  predicate isSink(DataFlow::Node sink) {
-    exists(EVP_Hash_Operation c | c.getAlgorithmArg() = sink.asExpr())
-  }
+  override Expr getInputArg() { result = this.(Call).getArgument(1) }
 }
 
-private module AlgGetterToAlgConsumerFlow = DataFlow::Global<AlgGetterToAlgConsumerConfig>;
-
 //https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis
-class EVP_Q_Digest_Operation extends EVP_Hash_Operation {
+class EVP_Q_Digest_Operation extends EVPOperation, Crypto::HashOperationInstance {
   EVP_Q_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Q_digest" }
 
-  //override Crypto::AlgorithmConsumer getAlgorithmConsumer() {  }
+  override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) }
+
   override EVP_Hash_Initializer getInitCall() {
     // This variant of digest does not use an init
     // and even if it were used, the init would be ignored/undefined
     none()
   }
 
-  override Expr getOutputArg() { result = this.(Call).getArgument(5) }
-
   override Expr getInputArg() { result = this.(Call).getArgument(3) }
 
-  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() }
-
-  override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
+  override Expr getOutputArg() { result = this.(Call).getArgument(5) }
 
-  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
-    // The operation is a direct algorithm consumer
-    // NOTE: the operation itself is already modeld as a value consumer, so we can
-    // simply return 'this', see modeled hash algorithm consuers for EVP_Q_Digest
-    this = result
+  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
+    result = EVPOperation.super.getOutputArtifact()
   }
 
-  override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) }
+  override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
+    result = EVPOperation.super.getInputConsumer()
+  }
 }
 
-class EVP_Digest_Operation extends EVP_Hash_Operation {
+class EVP_Digest_Operation extends EVPOperation, Crypto::HashOperationInstance {
   EVP_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Digest" }
 
   // There is no context argument for this function
   override Expr getContextArg() { none() }
 
+  override Expr getAlgorithmArg() { result = this.(Call).getArgument(4) }
+
   override EVP_Hash_Initializer getInitCall() {
     // This variant of digest does not use an init
     // and even if it were used, the init would be ignored/undefined
     none()
   }
 
-  override Expr getAlgorithmArg() { result = this.(Call).getArgument(4) }
-
-  override Expr getOutputArg() { result = this.(Call).getArgument(2) }
-
   override Expr getInputArg() { result = this.(Call).getArgument(0) }
 
-  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() }
-
-  override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
-}
-
-// NOTE: not modeled as hash operations, these are intermediate calls
-class EVP_Digest_Update_Call extends Call {
-  EVP_Digest_Update_Call() { this.(Call).getTarget().getName() in ["EVP_DigestUpdate"] }
-
-  Expr getInputArg() { result = this.(Call).getArgument(1) }
+  override Expr getOutputArg() { result = this.(Call).getArgument(2) }
 
-  DataFlow::Node getInputNode() { result.asExpr() = this.getInputArg() }
+  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
+    result = EVPOperation.super.getOutputArtifact()
+  }
 
-  Expr getContextArg() { result = this.(Call).getArgument(0) }
+  override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
+    result = EVPOperation.super.getInputConsumer()
+  }
 }
 
-class EVP_Digest_Final_Call extends EVP_Hash_Operation {
+class EVP_Digest_Final_Call extends EVPFinal, Crypto::HashOperationInstance {
   EVP_Digest_Final_Call() {
     this.(Call).getTarget().getName() in [
         "EVP_DigestFinal", "EVP_DigestFinal_ex", "EVP_DigestFinalXOF"
       ]
   }
 
-  EVP_Digest_Update_Call getUpdateCalls() {
-    CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
-  }
-
-  override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() }
-
-  override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
-
   override Expr getOutputArg() { result = this.(Call).getArgument(1) }
 
-  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() }
+  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
+    result = EVPFinal.super.getOutputArtifact()
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
+    result = EVPFinal.super.getInputConsumer()
+  }
 }