Skip to content

Crypto: Add OpenSSL elliptic curve algorithm instances and consumers #19528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import cpp
import semmle.code.cpp.dataflow.new.DataFlow
import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import experimental.quantum.Language
private import semmle.code.cpp.dataflow.new.DataFlow

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
experimental.quantum.Language
.
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers

/**
* Traces 'known algorithms' to AVCs, specifically
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import cpp
import experimental.quantum.Language
import OpenSSLAlgorithmInstanceBase
import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
import AlgToAVCFlow
private import experimental.quantum.Language
private import OpenSSLAlgorithmInstanceBase
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import AlgToAVCFlow

/**
* Given a `KnownOpenSSLBlockModeAlgorithmConstant`, converts this to a block family type.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import cpp
import experimental.quantum.Language
import KnownAlgorithmConstants
import Crypto::KeyOpAlg as KeyOpAlg
import OpenSSLAlgorithmInstanceBase
import PaddingAlgorithmInstance
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
import AlgToAVCFlow
import BlockAlgorithmInstance
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 AlgToAVCFlow
private import BlockAlgorithmInstance

/**
* Given a `KnownOpenSSLCipherAlgorithmConstant`, converts this to a cipher family type.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import cpp
private import experimental.quantum.Language
private import KnownAlgorithmConstants
private import OpenSSLAlgorithmInstanceBase
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
private import AlgToAVCFlow

class KnownOpenSSLEllipticCurveConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::EllipticCurveInstance instanceof KnownOpenSSLEllipticCurveAlgorithmConstant

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in KnownOpenSSLEllipticCurveConstantAlgorithmInstance should be PascalCase/camelCase.
{
OpenSSLAlgorithmValueConsumer getterCall;

KnownOpenSSLEllipticCurveConstantAlgorithmInstance() {
// Two possibilities:
// 1) The source is a literal and flows to a getter, then we know we have an instance
// 2) The source is a KnownOpenSSLAlgorithm is call, and we know we have an instance immediately from that
// Possibility 1:
this instanceof Literal and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.getInputNode() and
// Source is `this`
src.asExpr() = this and
// This traces to a getter
KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
}

override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in getAVC should be PascalCase/camelCase.

override string getRawEllipticCurveName() { result = this.(Literal).getValue().toString() }

override Crypto::TEllipticCurveType getEllipticCurveType() {
Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.(KnownOpenSSLEllipticCurveAlgorithmConstant)
.getNormalizedName(), _, result)
}

override int getKeySize() {
Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.(KnownOpenSSLEllipticCurveAlgorithmConstant)
.getNormalizedName(), result, _)
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import cpp
import experimental.quantum.Language
import KnownAlgorithmConstants
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
import AlgToAVCFlow
private import experimental.quantum.Language
private import KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
private import AlgToAVCFlow

predicate knownOpenSSLConstantToHashFamilyType(
KnownOpenSSLHashAlgorithmConstant e, Crypto::THashType type
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import cpp
import experimental.quantum.OpenSSL.LibraryDetector
private import experimental.quantum.OpenSSL.LibraryDetector

predicate resolveAlgorithmFromExpr(Expr e, string normalizedName, string algType) {
resolveAlgorithmFromCall(e, normalizedName, algType)
Expand Down Expand Up @@ -67,6 +67,15 @@
}
}

class KnownOpenSSLEllipticCurveAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in KnownOpenSSLEllipticCurveAlgorithmConstant should be PascalCase/camelCase.
KnownOpenSSLEllipticCurveAlgorithmConstant() {
exists(string algType |
resolveAlgorithmFromExpr(this, _, algType) and
algType.toLowerCase().matches("elliptic_curve")
)
}
}

/**
* Resolves a call to a 'direct algorithm getter', e.g., EVP_MD5()
* This approach to fetching algorithms was used in OpenSSL 1.0.2.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import experimental.quantum.Language
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase

abstract class OpenSSLAlgorithmInstance extends Crypto::AlgorithmInstance {
abstract OpenSSLAlgorithmValueConsumer getAVC();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ import CipherAlgorithmInstance
import PaddingAlgorithmInstance
import BlockAlgorithmInstance
import HashAlgorithmInstance
import EllipticCurveAlgorithmInstance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend going through all of these and doing a private import of OpenSSLAlgorithmInstanceBase (and other modules where possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed

Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import cpp
import experimental.quantum.Language
import OpenSSLAlgorithmInstanceBase
import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import AlgToAVCFlow
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
private import experimental.quantum.Language
private import OpenSSLAlgorithmInstanceBase
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import AlgToAVCFlow
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase

/**
* Given a `KnownOpenSSLPaddingAlgorithmConstant`, converts this to a padding family type.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import cpp
import experimental.quantum.Language
import experimental.quantum.OpenSSL.LibraryDetector
import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
import OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.LibraryDetector
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
private import OpenSSLAlgorithmValueConsumerBase

abstract class CipherAlgorithmValueConsumer extends OpenSSLAlgorithmValueConsumer { }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cpp
import experimental.quantum.Language
import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase

// TODO: can self referential to itself, which is also an algorithm (Known algorithm)
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import cpp
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.LibraryDetector
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances

Copy link
Preview

Copilot AI May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Add a doc comment explaining the purpose and usage of EllipticCurveValueConsumer to improve maintainability.

Suggested change
/**
* An abstract base class for consumers of elliptic curve algorithm values.
*
* This class is designed to be extended by specific implementations that
* handle elliptic curve-related cryptographic operations in OpenSSL.
* It provides a common interface and shared functionality for such consumers.
*
* Subclasses should implement the necessary methods to process elliptic curve
* algorithm values and integrate with the OpenSSL cryptographic library.
*/

Copilot uses AI. Check for mistakes.

abstract class EllipticCurveValueConsumer extends OpenSSLAlgorithmValueConsumer { }

//https://docs.openssl.org/3.0/man3/EC_KEY_new/#name
class EVPEllipticCurveAlgorithmConsumer extends EllipticCurveValueConsumer {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in EVPEllipticCurveAlgorithmConsumer should be PascalCase/camelCase.
DataFlow::Node valueArgNode;
DataFlow::Node resultNode;

EVPEllipticCurveAlgorithmConsumer() {
resultNode.asExpr() = this.(Call) and // in all cases the result is the return
isPossibleOpenSSLFunction(this.(Call).getTarget()) and
(
this.(Call).getTarget().getName() in ["EVP_EC_gen", "EC_KEY_new_by_curve_name"] and
valueArgNode.asExpr() = this.(Call).getArgument(0)
or
this.(Call).getTarget().getName() in [
"EC_KEY_new_by_curve_name_ex", "EVP_PKEY_CTX_set_ec_paramgen_curve_nid"
] and
valueArgNode.asExpr() = this.(Call).getArgument(2)
)
}

override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
exists(OpenSSLAlgorithmInstance i | i.getAVC() = this and result = i)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a generic algorithm class here, and why does the algorithm itself bind itself to the AVC as opposed to what is actually using/related to the algorithm consumed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct in my reasoning below?

"An algorithm instance exists if and only if it is a string literal that flows to a consumer. Consequently, the definition of an algorithm instance is inherently constrained by the consumer to which it flows, establishing a dependent relationship between the instance and its consuming context."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically a literal must flow to something that consumes it, if not, we aren't calling it an algorithm.

There is a flip side, the direct algorithms (functions like AES()), these... well we could say are algorithms in their own right, but I didn't model it that way. So if these don't flow to something, they also don't exist as an algorithm. We may need to re-address that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are indeed algorithms -- the instance where you define them would be be modeled by extending an algorithm, operation, and AVC (assuming AES() also performs some sort of operation using AES).

}

override DataFlow::Node getResultNode() { result = resultNode }

override Crypto::ConsumerInputDataFlowNode getInputNode() { result = valueArgNode }
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
// import EVPHashInitializer
// import EVPHashOperation
// import EVPHashAlgorithmSource
import cpp
import experimental.quantum.Language
import semmle.code.cpp.dataflow.new.DataFlow
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
import experimental.quantum.OpenSSL.LibraryDetector
private import experimental.quantum.Language
private import semmle.code.cpp.dataflow.new.DataFlow

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
experimental.quantum.Language
.
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
private import experimental.quantum.OpenSSL.LibraryDetector

abstract class HashAlgorithmValueConsumer extends OpenSSLAlgorithmValueConsumer { }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import experimental.quantum.Language
import semmle.code.cpp.dataflow.new.DataFlow
private import experimental.quantum.Language
private import semmle.code.cpp.dataflow.new.DataFlow

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
experimental.quantum.Language
.

abstract class OpenSSLAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer instanceof Call {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ import CipherAlgorithmValueConsumer
import DirectAlgorithmValueConsumer
import PaddingAlgorithmValueConsumer
import HashAlgorithmValueConsumer
import EllipticCurveAlgorithmValueConsumer
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import cpp
import experimental.quantum.Language
import experimental.quantum.OpenSSL.LibraryDetector
import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
import OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.LibraryDetector
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
private import OpenSSLAlgorithmValueConsumerBase

abstract class PaddingAlgorithmValueConsumer extends OpenSSLAlgorithmValueConsumer { }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* Models cipher initialization for EVP cipher operations.
*/

import experimental.quantum.Language
import experimental.quantum.OpenSSL.CtxFlow as CTXFlow
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in CTXFlow should be PascalCase/camelCase.

module EncValToInitEncArgConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source.asExpr().getValue().toInt() in [0, 1] }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import experimental.quantum.Language
import experimental.quantum.OpenSSL.CtxFlow as CTXFlow
import EVPCipherInitializer
import OpenSSLOperationBase
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in CTXFlow should be PascalCase/camelCase.
private import EVPCipherInitializer
private import OpenSSLOperationBase
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers

private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
* https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis
*/

import experimental.quantum.Language
import experimental.quantum.OpenSSL.CtxFlow as CTXFlow
import experimental.quantum.OpenSSL.LibraryDetector
import OpenSSLOperationBase
import EVPHashInitializer
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in CTXFlow should be PascalCase/camelCase.
private import experimental.quantum.OpenSSL.LibraryDetector
private import OpenSSLOperationBase
private import EVPHashInitializer
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers

// import EVPHashConsumers
abstract class EVP_Hash_Operation extends OpenSSLOperation, Crypto::HashOperationInstance {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import experimental.quantum.Language
private import experimental.quantum.Language

abstract class OpenSSLOperation extends Crypto::OperationInstance instanceof Call {
abstract Expr getInputArg();
Expand Down
9 changes: 2 additions & 7 deletions java/ql/lib/experimental/quantum/JCA.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1606,13 +1606,8 @@ module JCAModel {
else result = Crypto::OtherEllipticCurveType()
}

override string getKeySize() {
exists(int keySize |
Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.getRawEllipticCurveName(), keySize,
_)
|
result = keySize.toString()
)
override int getKeySize() {
Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.getRawEllipticCurveName(), result, _)
}

EllipticCurveAlgorithmValueConsumer getConsumer() { result = consumer }
Expand Down
4 changes: 2 additions & 2 deletions shared/quantum/codeql/quantum/experimental/Model.qll
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {

abstract TEllipticCurveType getEllipticCurveType();

abstract string getKeySize();
abstract int getKeySize();

/**
* The 'parsed' curve name, e.g., "P-256" or "secp256r1"
Expand Down Expand Up @@ -2613,7 +2613,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
or
// [ONLY_KNOWN]
key = "KeySize" and
value = instance.asAlg().getKeySize() and
value = instance.asAlg().getKeySize().toString() and
location = this.getLocation()
or
// [KNOWN_OR_UNKNOWN]
Expand Down
Loading