Skip to content

Crypto: Refactor Model and signatures, fix models, add unit tests - #19944

Merged
nicolaswill merged 31 commits into
github:mainfrom
bdrodes:signature_model_refactor
Oct 2, 2025
Merged

Crypto: Refactor Model and signatures, fix models, add unit tests#19944
nicolaswill merged 31 commits into
github:mainfrom
bdrodes:signature_model_refactor

Conversation

@bdrodes

@bdrodesbdrodes commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

A complete overhaul of how signatures are modeled to take into account that some operations are ambiguous, they can be either MAC or Signature. Significant updates to the OpenSSL models both to use the new signature/mac models but also misc. fixes and additional modeling to make the scope of the OpenSSL models wider and more accurate. Updates to the JCA models also to account for the new signature/mac models.

This PR is a culmination of many misc. updates necessary to get the openssl models to a consistent representation, with rigorous unit tests, especially as represented in the graph. Next steps are to ensure other models (e.g., the JCA) are similarly consistent with rigorous unit tests.

# Conflicts:
#	cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll
#	cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll
…for APIs having one function to do both. Incomplete. Work in progress.
…ests for openssl, refactor still required for JCA.
Comment threadshared/quantum/codeql/quantum/experimental/Model.qll Fixed
bdrodesand others added 10 commits July 1, 2025 12:21
…model) and simplified unit tests in terms of the graph complexity to aid visual assessments of model correctness.
…is now just a passthrough, it is not a known implicit operation call. Some final operations generating null outputs are now removed from possible final operartions (typically used to determine buffer lenghth and not actually performing the operation). Misc. false positive/error fixes and code clean up, and added missing models.
Comment threadjava/ql/lib/experimental/quantum/JCA.qll Fixed
@bdrodes
bdrodes marked this pull request as ready for review October 2, 2025 13:02
@bdrodes
bdrodes requested a review from a team as a code ownerOctober 2, 2025 13:02
CopilotAI review requested due to automatic review settings October 2, 2025 13:02

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the signature model architecture to consolidate MAC and signature operations under a unified hierarchy. The key changes include:

  • Merges MAC algorithm types into the general key operation algorithm type system
  • Introduces a new SignatureOrMacOperationInstance base class to handle APIs that can perform either signature or MAC operations
  • Updates OpenSSL operation modeling to better distinguish between signature-only and signature/MAC operations
  • Adds comprehensive verification operation support alongside existing signing operations

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.

Show a summary per file
FileDescription
shared/quantum/codeql/quantum/experimental/Standardization.qllIntegrates MAC algorithm types into the main key operation algorithm hierarchy
shared/quantum/codeql/quantum/experimental/Model.qllRefactors signature/MAC operation class hierarchy and removes standalone MAC classes
java/ql/lib/experimental/quantum/JCA.qllUpdates JCA model to use new MAC algorithm types and fixes hash algorithm name parsing
cpp/ql/test/experimental/library-tests/quantum/openssl_signature.cRestructures test code into focused individual test functions
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/SignatureOperation.qllAdds comprehensive signature/verification operations and introduces SignatureOrMacFinalOperation
cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/MACAlgorithmInstance.qllUpdates MAC instances to extend KeyOperationAlgorithmInstance instead of MacAlgorithmInstance
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qllSwitches from DataFlow to TaintTracking and adds new IO types for hash algorithms

Comment threadjava/ql/lib/experimental/quantum/JCA.qll Fixed
…ures to make the inheritance decomposition and caveats clearer, and 2) use getConsumer to establish the hash and hmac consumer. Update the Model to expect hash node types specifically for HMAC getHashALgorithmOrUnknown.
Comment threadjava/ql/lib/experimental/quantum/JCA.qll Dismissed
Comment threadjava/ql/lib/experimental/quantum/JCA.qll Dismissed
Comment threadjava/ql/lib/experimental/quantum/JCA.qll Dismissed
Comment threadcpp/ql/lib/experimental/quantum/Language.qll Outdated
@nicolaswillnicolaswill changed the title Signature model refactorCrypto: Refactor Model.qll and signatures, update JCA and OpenSSL, add unit testsOct 2, 2025
@nicolaswillnicolaswill changed the title Crypto: Refactor Model.qll and signatures, update JCA and OpenSSL, add unit testsCrypto: Refactor Model and signatures, fix models, add unit testsOct 2, 2025
@nicolaswill
nicolaswill merged commit cbe34f1 into github:mainOct 2, 2025
42 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@bdrodes@github-advanced-security@nicolaswill