Quantum: Add base classes for OpenSSL EVP methods #19607
Conversation
…ound through tests, and updating CODEOWNERS for quantum tests
add initial work for openssl signatures add basic C test files for ciphers and signatures more signature classes, comments for evp base classes more signature tests fix super calls for input consumers fix getOutputArtifact for tests formatting delete redundant test files move algorithm methods to OpenSSLOperation refactor ECKeyGenOperation for new EVP classes formatting fix getOutputArtifact fix cipher and digest operation test results mv openssl signature to another PR
GrosQuildu
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds foundational QL modeling for OpenSSL operations under the experimental quantum API and introduces corresponding library-tests for both hash and cipher operations.
- Introduces base and specialized QL classes for EVP crypto operations (hash, cipher, keygen, etc.)
- Adds new
.qlqueries underlibrary-tests/quantum/openssland their expected result files - Updates CODEOWNERS to include deeper experimental paths and imports
EVPSignatureOperation
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/test/experimental/library-tests/quantum/openssl/*.ql | New test queries for hash, cipher, key, and nonce sources |
| cpp/ql/test/experimental/library-tests/quantum/openssl/*.expected | Expected output for the new test queries |
| cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll | Imported EVPSignatureOperation into main operations |
| cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll | Added context flow, algorithm consumer, and base methods |
| cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVP*.qll | Refactored individual EVP operation classes |
| cpp/ql/lib/experimental/quantum/OpenSSL/CtxFlow.qll | Extended context type matching for CTX flow |
| CODEOWNERS | Broadened experimental/quantum path pattern |
Comments suppressed due to low confidence (3)
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll:5
- You’ve added
EVPSignatureOperationto the main operations import but there are no corresponding tests for signature flows. Consider adding library-tests for signature init, update, and final calls to ensure full coverage.
import EVPSignatureOperation
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll:111
- [nitpick] The newly added abstract method
getOutputArglacks a doc comment. Adding a brief description will help maintainers understand its contract and intended use.
abstract Expr getOutputArg();
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll:15
- In
getOutputKeyArtifactyou’re usingresult.asExpr()whereas most other output artifacts useasDefiningArgument(). Verify that this correctly models the artifact’s definition point in the dataflow.
result.asExpr() = this.(Call).getArgument(0)
|
|
||
| override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() } | ||
| override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { |
There was a problem hiding this comment.
[nitpick] There’s repeated boilerplate in many EVP* classes for overriding getOutputArtifact and getInputConsumer just to call the super implementation. Consider moving those common overrides into a shared intermediate base to reduce duplication.
Copilot uses AI. Check for mistakes.
| @@ -16,7 +16,7 @@ | |||
| /java/ql/test-kotlin2/ @github/codeql-kotlin | |||
|
|
|||
| # Experimental CodeQL cryptography | |||
| **/experimental/quantum/ @github/ps-codeql | |||
| **/experimental/**/quantum/ @github/ps-codeql | |||
There was a problem hiding this comment.
[nitpick] The new glob **/experimental/**/quantum/ may match more paths than intended. If you only want the QL modules under a specific directory, consider narrowing the pattern to avoid unintended CODEOWNERS assignments.
| **/experimental/**/quantum/ @github/ps-codeql | |
| /experimental/cryptography/quantum/ @github/ps-codeql |
Copilot uses AI. Check for mistakes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -1,21 +1,157 @@ | |||
| private import experimental.quantum.Language | |||
| private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow | |||
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| /** | ||
| * All OpenSSL operations. | ||
| */ |
Check warning
Code scanning / CodeQL
Class QLDoc style. Warning
| /** | ||
| * Calls to initialization functions of EVP API. | ||
| * These are not operations in the sense of Crypto::OperationInstance, | ||
| * but they are used to initialize the context for the operation. | ||
| */ |
Check warning
Code scanning / CodeQL
Class QLDoc style. Warning
| * These are not operations in the sense of Crypto::OperationInstance, | ||
| * but they are used to initialize the context for the operation. | ||
| */ | ||
| abstract class EVPInitialize extends Call { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| /** | ||
| * The context argument that ties together initialization, updates and/or final calls. | ||
| */ |
Check warning
Code scanning / CodeQL
Predicate QLDoc style. Warning
| /** | ||
| * Final calls of EVP API. | ||
| */ |
Check warning
Code scanning / CodeQL
Class QLDoc style. Warning
| /** | ||
| * Final calls of EVP API. | ||
| */ | ||
| abstract class EVPFinal extends EVPOperation { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| /** | ||
| * The input data was provided to all update calls. | ||
| */ |
Check warning
Code scanning / CodeQL
Predicate QLDoc style. Warning
| /** | ||
| * One-shot calls of EVP API. | ||
| */ |
Check warning
Code scanning / CodeQL
Class QLDoc style. Warning
| /** | ||
| * One-shot calls of EVP API. | ||
| */ | ||
| abstract class EVPOneShot extends EVPOperation { } |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll
Outdated
Show resolved
Hide resolved
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll
Outdated
Show resolved
Hide resolved
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll
Outdated
Show resolved
Hide resolved
|
|
||
| abstract Expr getIVArg(); | ||
|
|
||
| // abstract Crypto::CipherOperationSubtype getCipherOperationSubtype(); | ||
| abstract Expr getOperationSubtypeArg(); |
There was a problem hiding this comment.
Should this be part of the EVP initialize interface too?
There was a problem hiding this comment.
No, I would say this is cipher-specific. If we find otherwise we can move that to EVP initialize later.
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll
Outdated
Show resolved
Hide resolved
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll
Outdated
Show resolved
Hide resolved
…ationBase.qll Co-authored-by: Ben Rodes <[email protected]>
Co-authored-by: Ben Rodes <[email protected]>
Changes
OpenSSLOperationand adds base classes for OpenSSL's EVP API. Mostly a refactor to make existing code nicer.The PR fixes some tests too.
Made on top of #19564