Skip to content

Quantum: Add OpenSSL signature models #19705

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
merged 27 commits into from
Jun 18, 2025
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
39583ab
Crypto: Update KnownAlgoirthmConstants to make a distinction between …
bdrodes Jun 4, 2025
952bc26
Crypto: Added Signature algorithm instance and consumer
bdrodes Jun 4, 2025
33e239d
Crypto: Collapse initializer qll's into operations.
bdrodes Jun 4, 2025
f952f90
Crypto: Update CtxFlow to flow from any "source ctx" which is any ctx…
bdrodes Jun 4, 2025
98aae6a
Crypto: Add EVP key gen and signature operation (work in progress). A…
bdrodes Jun 5, 2025
4f2045b
Crypto: CtxFlow now uses an interface for additional steps. Add CTX s…
bdrodes Jun 9, 2025
729467c
Crypto: Separate out CTX parameter initialization, and add additional…
bdrodes Jun 9, 2025
7d47994
Crypto: Nop out signature operations for now until complete. Minor mo…
bdrodes Jun 10, 2025
d3cff2d
Crypto: Add support to trace keys, add support to find prior key gen …
bdrodes Jun 11, 2025
8f25380
Crypto: Consolidate tests to use node, edges, and properties.
bdrodes Jun 11, 2025
20e2c7c
Crypto: Overhaul/refactor of EVPInitialzers. Update cipher operation …
bdrodes Jun 12, 2025
eb20955
Crypto: Further simplify test caes to only use edges/nodes/properties…
bdrodes Jun 12, 2025
cf2f0f1
Crypto: Initial model of signatures. Still incomplete for verificatio…
bdrodes Jun 13, 2025
fb495bf
Crypto: Update expected files. There are failures, but accepting them…
bdrodes Jun 13, 2025
1882db7
Crypto: EVP Signature Operation cleanup.
bdrodes Jun 13, 2025
db0bc47
Merge branch 'main' into pawel_signatures_conversion
nicolaswill Jun 15, 2025
f975428
Merge branch 'main' into pawel_signatures_conversion
nicolaswill Jun 16, 2025
45fa2c9
Crypto: Code review cleanup.
bdrodes Jun 16, 2025
90e480b
Merge branch 'pawel_signatures_conversion' of https://github.com/bdro…
bdrodes Jun 16, 2025
790a607
Crypto: Acronym change from OpenSSL to OpenSsl, AVC to Avc and EVP to…
bdrodes Jun 16, 2025
7c18686
Crypto: Further ql-for-ql alert alert fixes.
bdrodes Jun 16, 2025
6c9c969
Crypto: Remove dead comments
bdrodes Jun 16, 2025
2b6a832
Crypto: Update JCA model to account for Model.qll changes.
bdrodes Jun 16, 2025
ac35634
Merge branch 'openssl_acronym_normalization' into pawel_signatures_co…
bdrodes Jun 17, 2025
f2f97c9
Crypto: QL docs fix.
bdrodes Jun 17, 2025
a81fffc
Crypto: Fix redundant override issue.
bdrodes Jun 17, 2025
b2765a2
Merge branch 'main' into pawel_signatures_conversion
bdrodes Jun 17, 2025
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
4 changes: 3 additions & 1 deletion cpp/ql/lib/experimental/quantum/Language.qll
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ module CryptoInput implements InputSig<Language::Location> {
LocatableElement dfn_to_element(DataFlow::Node node) {
result = node.asExpr() or
result = node.asParameter() or
result = node.asVariable()
result = node.asVariable() or
result = node.asDefiningArgument()
// TODO: do we need asIndirectExpr()?
}

string locationToFileBaseNameAndLineNumberString(Location location) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ private import PaddingAlgorithmInstance
*/
module KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof KnownOpenSSLAlgorithmConstant
source.asExpr() instanceof KnownOpenSSLAlgorithmExpr and
// No need to flow direct operations to AVCs
not source.asExpr() instanceof OpenSSLDirectAlgorithmOperationCall
}

predicate isSink(DataFlow::Node sink) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgor
private import AlgToAVCFlow

/**
* Given a `KnownOpenSSLBlockModeAlgorithmConstant`, converts this to a block family type.
* Given a `KnownOpenSSLBlockModeAlgorithmExpr`, converts this to a block family type.
* Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
*/
predicate knownOpenSSLConstantToBlockModeFamilyType(
KnownOpenSSLBlockModeAlgorithmConstant e, Crypto::TBlockCipherModeOfOperationType type
KnownOpenSSLBlockModeAlgorithmExpr e, Crypto::TBlockCipherModeOfOperationType type
) {
exists(string name |
name = e.getNormalizedName() and
name = e.(KnownOpenSSLAlgorithmExpr).getNormalizedName() and
(
name.matches("CBC") and type instanceof Crypto::CBC
or
Expand All @@ -40,7 +40,7 @@ predicate knownOpenSSLConstantToBlockModeFamilyType(
}

class KnownOpenSSLBlockModeConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::ModeOfOperationAlgorithmInstance instanceof KnownOpenSSLBlockModeAlgorithmConstant
Crypto::ModeOfOperationAlgorithmInstance instanceof KnownOpenSSLBlockModeAlgorithmExpr
{
OpenSSLAlgorithmValueConsumer getterCall;

Expand All @@ -49,7 +49,7 @@ class KnownOpenSSLBlockModeConstantAlgorithmInstance extends OpenSSLAlgorithmIns
// 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
this instanceof OpenSSLAlgorithmLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
Expand All @@ -60,7 +60,8 @@ class KnownOpenSSLBlockModeConstantAlgorithmInstance extends OpenSSLAlgorithmIns
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
this instanceof OpenSSLAlgorithmCall and
getterCall = this
}

override Crypto::TBlockCipherModeOfOperationType getModeType() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ private import AlgToAVCFlow
private import BlockAlgorithmInstance

/**
* Given a `KnownOpenSSLCipherAlgorithmConstant`, converts this to a cipher family type.
* Given a `KnownOpenSSLCipherAlgorithmExpr`, converts this to a cipher family type.
* Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
*/
predicate knownOpenSSLConstantToCipherFamilyType(
KnownOpenSSLCipherAlgorithmConstant e, Crypto::KeyOpAlg::TAlgorithm type
KnownOpenSSLCipherAlgorithmExpr e, Crypto::KeyOpAlg::TAlgorithm type
) {
exists(string name |
name = e.getNormalizedName() and
name = e.(KnownOpenSSLAlgorithmExpr).getNormalizedName() and
(
name.matches("AES%") and type = KeyOpAlg::TSymmetricCipher(KeyOpAlg::AES())
or
Expand Down Expand Up @@ -65,7 +65,7 @@ predicate knownOpenSSLConstantToCipherFamilyType(
}

class KnownOpenSSLCipherConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::KeyOperationAlgorithmInstance instanceof KnownOpenSSLCipherAlgorithmConstant
Crypto::KeyOperationAlgorithmInstance instanceof KnownOpenSSLCipherAlgorithmExpr
{
OpenSSLAlgorithmValueConsumer getterCall;

Expand All @@ -74,7 +74,7 @@ class KnownOpenSSLCipherConstantAlgorithmInstance extends OpenSSLAlgorithmInstan
// 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
this instanceof OpenSSLAlgorithmLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
Expand All @@ -85,7 +85,8 @@ class KnownOpenSSLCipherConstantAlgorithmInstance extends OpenSSLAlgorithmInstan
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
this instanceof OpenSSLAlgorithmCall and
getterCall = this
}

override Crypto::ModeOfOperationAlgorithmInstance getModeOfOperationAlgorithm() {
Expand All @@ -109,7 +110,7 @@ class KnownOpenSSLCipherConstantAlgorithmInstance extends OpenSSLAlgorithmInstan
}

override int getKeySizeFixed() {
this.(KnownOpenSSLCipherAlgorithmConstant).getExplicitKeySize() = result
this.(KnownOpenSSLCipherAlgorithmExpr).getExplicitKeySize() = result
}

override Crypto::KeyOpAlg::Algorithm getAlgorithmType() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgori
private import AlgToAVCFlow

class KnownOpenSSLEllipticCurveConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::EllipticCurveInstance instanceof KnownOpenSSLEllipticCurveAlgorithmConstant
Crypto::EllipticCurveInstance instanceof KnownOpenSSLEllipticCurveAlgorithmExpr
{
OpenSSLAlgorithmValueConsumer getterCall;

Expand All @@ -16,7 +16,7 @@ class KnownOpenSSLEllipticCurveConstantAlgorithmInstance extends OpenSSLAlgorith
// 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
this instanceof OpenSSLAlgorithmLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.getInputNode() and
Expand All @@ -27,7 +27,8 @@ class KnownOpenSSLEllipticCurveConstantAlgorithmInstance extends OpenSSLAlgorith
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
this instanceof OpenSSLAlgorithmCall and
getterCall = this
}

override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }
Expand All @@ -43,11 +44,11 @@ class KnownOpenSSLEllipticCurveConstantAlgorithmInstance extends OpenSSLAlgorith
}

override string getParsedEllipticCurveName() {
result = this.(KnownOpenSSLEllipticCurveAlgorithmConstant).getNormalizedName()
result = this.(KnownOpenSSLAlgorithmExpr).getNormalizedName()
}

override int getKeySize() {
Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.(KnownOpenSSLEllipticCurveAlgorithmConstant)
Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.(KnownOpenSSLAlgorithmExpr)
.getNormalizedName(), result, _)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmI
private import AlgToAVCFlow

predicate knownOpenSSLConstantToHashFamilyType(
KnownOpenSSLHashAlgorithmConstant e, Crypto::THashType type
KnownOpenSSLHashAlgorithmExpr e, Crypto::THashType type
) {
exists(string name |
name = e.getNormalizedName() and
name = e.(KnownOpenSSLAlgorithmExpr).getNormalizedName() and
(
name.matches("BLAKE2B") and type instanceof Crypto::BLAKE2B
or
Expand All @@ -29,7 +29,7 @@ predicate knownOpenSSLConstantToHashFamilyType(
or
name.matches(["SHA", "SHA1"]) and type instanceof Crypto::SHA1
or
name.matches("SHA+%") and not name.matches(["SHA1", "SHA3-"]) and type instanceof Crypto::SHA2
name.matches("SHA_%") and not name.matches(["SHA1", "SHA3-"]) and type instanceof Crypto::SHA2
Copy link
Contributor

Choose a reason for hiding this comment

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

This is redundant. SHA_ cannot match SHA1 or SHA3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The _ in matches means match one character, not any character (+ vs *), so it isn't redundant. The issue was I accidentally used the '+' syntax previously.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah! Four years of writing CodeQL, and I just learned that now...

Holds when the receiver matches the pattern. Patterns are matched by case sensitive string matching, and there are two wildcards: _ matches a single character, and % matches any sequence of characters.

Anyway, isn't _% redundant then? Just % should do the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I basically didn't want to match just "sha" and call it sha2. I wrote this awhile back so there is probably a better way, but basically it would not be correct to just use % because of that.

or
name.matches("SHA3-%") and type instanceof Crypto::SHA3
or
Expand All @@ -45,7 +45,7 @@ predicate knownOpenSSLConstantToHashFamilyType(
}

class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::HashAlgorithmInstance instanceof KnownOpenSSLHashAlgorithmConstant
Crypto::HashAlgorithmInstance instanceof KnownOpenSSLHashAlgorithmExpr
{
OpenSSLAlgorithmValueConsumer getterCall;

Expand All @@ -54,7 +54,7 @@ class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance
// 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
this instanceof OpenSSLAlgorithmLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
Expand All @@ -65,7 +65,8 @@ class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
this instanceof OpenSSLAlgorithmCall and
getterCall = this
}

override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }
Expand All @@ -83,6 +84,6 @@ class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance
}

override int getFixedDigestLength() {
this.(KnownOpenSSLHashAlgorithmConstant).getExplicitDigestLength() = result
this.(KnownOpenSSLHashAlgorithmExpr).getExplicitDigestLength() = result
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmI
private import AlgToAVCFlow

predicate knownOpenSSLConstantToKeyAgreementFamilyType(
KnownOpenSSLKeyAgreementAlgorithmConstant e, Crypto::TKeyAgreementType type
KnownOpenSSLKeyAgreementAlgorithmExpr e, Crypto::TKeyAgreementType type
) {
exists(string name |
name = e.getNormalizedName() and
name = e.(KnownOpenSSLAlgorithmExpr).getNormalizedName() and
(
name = "ECDH" and type = Crypto::ECDH()
or
Expand All @@ -23,7 +23,7 @@ predicate knownOpenSSLConstantToKeyAgreementFamilyType(
}

class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::KeyAgreementAlgorithmInstance instanceof KnownOpenSSLKeyAgreementAlgorithmConstant
Crypto::KeyAgreementAlgorithmInstance instanceof KnownOpenSSLKeyAgreementAlgorithmExpr
{
OpenSSLAlgorithmValueConsumer getterCall;

Expand All @@ -32,7 +32,7 @@ class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance
// 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
this instanceof OpenSSLAlgorithmLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.getInputNode() and
Expand All @@ -43,7 +43,9 @@ class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
this instanceof OpenSSLAlgorithmCall and
this instanceof DirectAlgorithmValueConsumer and
getterCall = this
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this ever hold? This class does not extend OpenSSLAlgorithmValueConsumer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy Pasta? I'll fix this.

}

override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }
Expand Down
Loading
Loading