Quantum: Initial support for BouncyCastle signature algorithms #19568
Conversation
fegge
force-pushed
the
fegge/bouncycastle
branch
2 times, most recently
from
8d2ec44 to
5884c71
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
The BouncyCastle stubs need a copy of the license file:
https://github.com/bcgit/bc-java/blob/main/LICENSE.md
Aside from that, please resolve the QL for QL Code Scanning alerts (with the exception of class naming to match camelCase or PascalCase for acronyms... we can do that later).
I have not yet reviewed all of the modeling.
This commit adds the `KeyGenerationOperationInstance` and `KeyGenerationAlgorithmInstance` types to the BouncyCastle model. It also adds data flow support from key pairs to the corresponding public and private components.
This commit also adds associated elliptic curves to the key generation and key nodes.
This commit adds support for ECDSA. This includes tracking the instantiated curve parameters using data flow. It also adds SignatureArtifactInstance and SignatureOperationInstance types to the shared model.
This commit adds EllipticCurveConsumingAlgorithmInstance to the shared model, allowing us to model and graph elliptic curve algorithms.
Co-authored-by: Arthur Baars <aibaars@github.com>
| * algorithm and elliptic curve are implicitly defined by the underlying type. | ||
| */ | ||
| abstract class KnownEllipticCurveSignatureAlgorithmInstance extends KnownEllipticCurveInstance, | ||
| SignatureAlgorithmInstance |
Check warning
Code scanning / CodeQL
Class QLDoc style. Warning
| /** | ||
| * A flow step for parameters created from other parameters. | ||
| * | ||
| * As an example, below we want to track the flow from the `X9ECParameters` | ||
| * constructor call to the `keyPairGenerator.init()` call to be able to | ||
| * determine the curve associated with the generator. | ||
| * | ||
| * Example: | ||
| * ``` | ||
| * X9ECParameters ecParams = SECNamedCurves.getByName("secp256r1"); | ||
| * ECDomainParameters domainParams = new ECDomainParameters(ecParams); | ||
| * ECKeyGenerationParameters keyGenParams = new ECKeyGenerationParameters(domainParams, ...); | ||
| * ECKeyPairGenerator keyPairGenerator = new ECKeyPairGenerator(); | ||
| * keyPairGenerator.init(keyGenParams); | ||
| * ``` | ||
| * | ||
| * We also want to track flow from parameters to the `init()` call | ||
| * via a curve instantiation. E.g. via a call to `getCurve()` as follows: | ||
| * | ||
| * Example: | ||
| * ``` | ||
| * X9ECParameters ecParams = SECNamedCurves.getByName("secp256r1"); | ||
| * ECCurve curve = ecParams.getCurve(); | ||
| * ECDomainParameters domainParams = new ECDomainParameters(curve, ...); | ||
| * ECKeyGenerationParameters keyGenParams = new ECKeyGenerationParameters(domainParams, ...); | ||
| * ECKeyPairGenerator keyPairGenerator = new ECKeyPairGenerator(); | ||
| * keyPairGenerator.init(keyGenParams); | ||
| */ |
Check warning
Code scanning / CodeQL
Predicate QLDoc style. Warning
| * ECKeyPairGenerator keyPairGenerator = new ECKeyPairGenerator(); | ||
| * keyPairGenerator.init(keyGenParams); | ||
| * ``` | ||
| * |
Check notice
Code scanning / CodeQL
Use of regexp to match a set of constant string Note
| * ``` | ||
| * | ||
| * We also want to track flow from parameters to the `init()` call | ||
| * via a curve instantiation. E.g. via a call to `getCurve()` as follows: |
Check notice
Code scanning / CodeQL
Use of regexp to match a set of constant string Note
| import java | ||
|
|
||
| module Params { | ||
| import FlowAnalysis |
Check warning
Code scanning / CodeQL
Redundant import Warning
| * Models for the key generation algorithms defined by the `org.bouncycastle.crypto.generators` package. | ||
| */ | ||
| module Generators { | ||
| import FlowAnalysis |
Check warning
Code scanning / CodeQL
Redundant import Warning
| /** | ||
| * This type is used to model data flow from a key pair to the private and | ||
| * public components of the key pair. | ||
| */ |
Check warning
Code scanning / CodeQL
Class QLDoc style. Warning
| * `init()` which takes a single `KeyGenerationParameters` argument. | ||
| */ | ||
| private class KeyGeneratorInitCall extends MethodCall { | ||
| KeyGenerator gen; |
Check notice
Code scanning / CodeQL
Field only used in CharPred Note
| * `init()` which takes a single `KeyGenerationParameters` argument. | ||
| */ | ||
| private class BlockCipherModeInitCall extends MethodCall { | ||
| BlockCipherMode mode; |
Check notice
Code scanning / CodeQL
Field only used in CharPred Note
| * decrypt data. | ||
| */ | ||
| private class BlockCipherModeUseCall extends MethodCall { | ||
| BlockCipherMode mode; |
Check notice
Code scanning / CodeQL
Field only used in CharPred Note
java/ql/lib/experimental/quantum/BouncyCastle/OperationInstances.qll
Outdated
Show resolved
Hide resolved
| KeyGenerator gen; | ||
|
|
||
| KeyGeneratorInitCall() { this = gen.getAnInitCall() } |
There was a problem hiding this comment.
| KeyGenerator gen; | |
| KeyGeneratorInitCall() { this = gen.getAnInitCall() } | |
| KeyGeneratorInitCall() { this = any(KeyGenerator gen).getAnInitCall() } |
| BlockCipherMode mode; | ||
|
|
||
| BlockCipherModeInitCall() { this = mode.getAnInitCall() } |
There was a problem hiding this comment.
| BlockCipherMode mode; | |
| BlockCipherModeInitCall() { this = mode.getAnInitCall() } | |
| BlockCipherModeInitCall() { this = any(BlockCipherMode mode).getAnInitCall() } |
| BlockCipherMode mode; | ||
|
|
||
| BlockCipherModeUseCall() { this = mode.getAUseCall() } |
There was a problem hiding this comment.
| BlockCipherMode mode; | |
| BlockCipherModeUseCall() { this = mode.getAUseCall() } | |
| BlockCipherModeUseCall() { this = any(BlockCipherMode mode).getAUseCall() } |
| /** | ||
| * Models the named elliptic curve passed to `X9ECParameters.getCurve()`. | ||
| */ |
There was a problem hiding this comment.
Those QLDoc style warnings are a bit nitpicky, but it would be great to maintain the style. Typically things start with something like An X9ECParametersInstantiation is ... or The class X9ECParametersInstantiation represents ... . Have a look through the API docs to get an idea of the style.
…es.qll Co-authored-by: Arthur Baars <aibaars@github.com>
To be reviewed by @nicolaswill.