Skip to content

Commit 357ae92

Browse files
feggefegge
committed
Updated Bouncy Castle algorithm instances
- Signature operations are now handled by a single algorithm instance - All key generation operations except generic EC key generation operations are now handled by a single algorithm instance - Ed25519 and Ed448 key generation have the algorithm set to Ed25519 and Ed448 respectively - For generic EC key generation operations the algorithm is given by the corresponding curve (since these could be used for either ECDSA or ECDH)
1 parent 1e5bb5f commit 357ae92

File tree

11 files changed

+163
-308
lines changed

11 files changed

+163
-308
lines changed

‎java/ql/lib/experimental/quantum/BouncyCastle/AlgorithmInstances.qll‎

Lines changed: 83 additions & 233 deletions
Large diffs are not rendered by default.

‎java/ql/lib/experimental/quantum/BouncyCastle/AlgorithmValueConsumers.qll‎

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,11 @@ class EllipticCurveStringLiteralArg extends EllipticCurveAlgorithmValueConsumer
2222
}
2323

2424
/**
25-
* An AVC for a signature algorithm where the algorithm is implicitly defined by
26-
* the constructor.
25+
* An AVC representing the block cipher argument passed to an block cipher mode
26+
* constructor.
2727
*/
28-
abstract class SignatureAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer {
29-
override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
30-
31-
override Crypto::ConsumerInputDataFlowNode getInputNode() { none() }
32-
}
33-
34-
/**
35-
* An AVC for a key generation algorithm where the algorithm is implicitly
36-
* defined by the constructor.
37-
*/
38-
abstract class KeyGenerationAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer {
39-
override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
40-
41-
override Crypto::ConsumerInputDataFlowNode getInputNode() { none() }
42-
}
43-
44-
/**
45-
* A block cipher argument passed to an block cipher mode constructor.
46-
*/
47-
class BlockCipherAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer instanceof Expr {
48-
BlockCipherAlgorithmValueConsumer() {
28+
class BlockCipherAlgorithmArg extends Crypto::AlgorithmValueConsumer instanceof Expr {
29+
BlockCipherAlgorithmArg() {
4930
this = any(BlockCipherModeAlgorithmInstance mode).getBlockCipherArg()
5031
}
5132

@@ -57,14 +38,9 @@ class BlockCipherAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer i
5738
}
5839

5940
/**
60-
* An AVC for a block cipher mode implicitly defined by the constructor.
41+
* An AVC for an algorithm that is implicitly defined by the instance.
6142
*/
62-
abstract class BlockCipherModeAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer instanceof ClassInstanceExpr
63-
{
64-
BlockCipherModeAlgorithmValueConsumer() {
65-
this.getType() instanceof Modes::UnpaddedBlockCipherMode
66-
}
67-
43+
abstract class ImplicitAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer {
6844
override Crypto::AlgorithmInstance getAKnownAlgorithmSource() { result = this }
6945

7046
override Crypto::ConsumerInputDataFlowNode getInputNode() { none() }

‎java/ql/lib/experimental/quantum/BouncyCastle/FlowAnalysis.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ module EllipticCurveStringLiteralToConsumerFlow {
362362
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof StringLiteral }
363363

364364
predicate isSink(DataFlow::Node sink) {
365-
exists(EllipticCurveAlgorithmValueConsumer consumer | sink = consumer.getInputNode())
365+
exists(Crypto::AlgorithmValueConsumer consumer | sink = consumer.getInputNode())
366366
}
367367

368368
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
@@ -374,7 +374,7 @@ module EllipticCurveStringLiteralToConsumerFlow {
374374
private module EllipticCurveStringLiteralToAlgorithmValueConsumerFlow =
375375
DataFlow::Global<EllipticCurveStringLiteralToAlgorithmValueConsumerConfig>;
376376

377-
EllipticCurveAlgorithmValueConsumer getConsumerFromLiteral(
377+
Crypto::AlgorithmValueConsumer getConsumerFromLiteral(
378378
StringLiteral literal, EllipticCurveStringLiteralToAlgorithmValueConsumerFlow::PathNode src,
379379
EllipticCurveStringLiteralToAlgorithmValueConsumerFlow::PathNode sink
380380
) {

‎java/ql/lib/experimental/quantum/BouncyCastle/OperationInstances.qll‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ module Signers {
233233
* BouncyCastle algorithms are instantiated by calling the constructor of the
234234
* corresponding class, which also represents the algorithm instance.
235235
*/
236-
private class SignerNewCall = SignatureAlgorithmInstance;
236+
private class SignerNewCall = ImplicitSignatureClassInstanceExpr;
237237

238238
/**
239239
* The type is instantiated by a constructor call and initialized by a call to
@@ -389,7 +389,17 @@ module Generators {
389389
* BouncyCastle algorithms are instantiated by calling the constructor of the
390390
* corresponding class, which also represents the algorithm instance.
391391
*/
392-
private class KeyGeneratorNewCall = KeyGenerationAlgorithmInstance;
392+
private class KeyGeneratorNewCall extends ClassInstanceExpr {
393+
KeyGeneratorNewCall() { this.getConstructedType() instanceof KeyGenerator }
394+
395+
// Used for data flow from elliptic curve string literals to the algorithm
396+
// instance.
397+
DataFlow::Node getParametersInput() { none() }
398+
399+
// Used for data flow from elliptic curve string literals to the algorithm
400+
// instance.
401+
DataFlow::Node getEllipticCurveInput() { none() }
402+
}
393403

394404
/**
395405
* A call to a key generator `init()` method.
@@ -414,18 +424,19 @@ module Generators {
414424

415425
KeyGeneratorUseCall() { this = gen.getAUseCall() }
416426

417-
// Since key generators don't have `update()` methods, this is always false.
427+
// This is used to define the `NewToInitToUse` data flow. Since key
428+
// generators don't have `update()` methods, this is always false.
418429
predicate isIntermediate() { none() }
419430

420431
Crypto::KeyArtifactType getKeyType() { result = gen.getKeyType() }
421432

422433
Expr getOutput() { result = this }
423434
}
424435

425-
module KeyGeneratorFlow =
436+
private module KeyGeneratorFlow =
426437
NewToInitToUseFlow<KeyGeneratorNewCall, KeyGeneratorInitCall, KeyGeneratorUseCall>;
427438

428-
module ParametersFlow =
439+
private module ParametersFlow =
429440
ParametersToInitFlow<Params::ParametersInstantiation, KeyGeneratorInitCall>;
430441

431442
/**
@@ -436,8 +447,12 @@ module Generators {
436447
class KeyGenerationOperationInstance extends Crypto::KeyGenerationOperationInstance instanceof KeyGeneratorUseCall
437448
{
438449
override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
439-
// The algorithm is implicitly defined by the key generator type, which is
440-
// determined by the constructor call.
450+
// For most Bouncy Castle key generators, the algorithm is implicitly
451+
// defined by the type, and so we use the constructor call to represent
452+
// the AVC.
453+
//
454+
// TODO: Avoid using data flow for this and use the operation instance to
455+
// represent the AVC and algorithm instance whenever it makes sense.
441456
result = KeyGeneratorFlow::getNewFromUse(this, _, _)
442457
}
443458

@@ -448,7 +463,14 @@ module Generators {
448463
override Crypto::KeyArtifactType getOutputKeyType() { result = super.getKeyType() }
449464

450465
override int getKeySizeFixed() {
451-
result = KeyGeneratorFlow::getNewFromUse(this, _, _).getKeySizeFixed()
466+
// Either the algorithm is a key operation algorithm or an elliptic curve.
467+
// For elliptic curve, the key size depends on the curve, so we don't
468+
// return anything.
469+
result =
470+
this.getAnAlgorithmValueConsumer()
471+
.getAKnownAlgorithmSource()
472+
.(Crypto::KeyOperationAlgorithmInstance)
473+
.getKeySizeFixed()
452474
}
453475

454476
override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() {
@@ -462,6 +484,25 @@ module Generators {
462484
)
463485
}
464486
}
487+
488+
/**
489+
* A generic elliptic curve key generation operation using `ECKeyPairGenerator`.
490+
*/
491+
class EllipticCurveKeyGenerationOperationInstance extends KeyGenerationOperationInstance instanceof KeyGeneratorUseCall
492+
{
493+
EllipticCurveKeyGenerationOperationInstance() {
494+
super.getQualifier().getType().getName().matches("EC%")
495+
}
496+
497+
// We attempt to resolve the elliptic curve algorithm from the
498+
// `KeyGenerationParameters` argument to `init()`.
499+
override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
500+
exists(KeyGeneratorInitCall init |
501+
init = KeyGeneratorFlow::getInitFromUse(this, _, _) and
502+
result = ParametersFlow::getParametersFromInit(init, _, _).getAnAlgorithmValueConsumer()
503+
)
504+
}
505+
}
465506
}
466507

467508
module Modes {

java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/ECDSAP256SignAndVerify.java renamed to java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/ECDSASignAndVerify.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@
2121
/**
2222
* Test Bouncy Castle's low-level ECDSA API
2323
*/
24-
public class ECDSAP256SignAndVerify {
24+
public class ECDSASignAndVerify {
2525

2626
public static void main(String[] args) {
2727
// Add Bouncy Castle provider
2828
Security.addProvider(new BouncyCastleProvider());
2929

3030
try {
31-
byte[] message = "Hello, ECDSA P-256 signature!".getBytes("UTF-8");
31+
byte[] message = "Hello, ECDSA signature!".getBytes("UTF-8");
3232

3333
// Test different key generation methods
3434
signWithKeyPair(generateKeyPair1(), message);

java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/LMSSignature.java renamed to java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/LMSSignAndVerify.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* low-level API.
1717
*
1818
*/
19-
public class LMSSignature {
19+
public class LMSSignAndVerify {
2020
public static void main(String[] args) {
2121
Security.addProvider(new BouncyCastleProvider());
2222

@@ -37,18 +37,15 @@ public static void main(String[] args) {
3737

3838
byte[] message = "Hello, LMS signature!".getBytes("UTF-8");
3939

40-
LMSSigner signer = new LMSSigner();
41-
4240
// Sign the message
41+
LMSSigner signer = new LMSSigner();
4342
signer.init(true, privateKey); // true for signing
4443
byte[] signature = signer.generateSignature(message);
4544

4645
// Verify the signature
47-
//
48-
// TODO: Using the same signer instance for verification causes both
49-
// keys to be reported. This should be handled using dataflow.
50-
signer.init(false, publicKey);
51-
boolean verified = signer.verifySignature(message, signature);
46+
LMSSigner verifier = new LMSSigner();
47+
verifier.init(false, publicKey);
48+
boolean verified = verifier.verifySignature(message, signature);
5249

5350
System.out.println("Signature verified: " + verified);
5451
} catch (Exception e) {

‎java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/key_artifacts.expected‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
1-
| ECDSAP256SignAndVerify.java:115:27:115:36 | Key | ECDSAP256SignAndVerify.java:57:16:57:49 | Key |
2-
| ECDSAP256SignAndVerify.java:115:27:115:36 | Key | ECDSAP256SignAndVerify.java:80:16:80:49 | Key |
3-
| ECDSAP256SignAndVerify.java:115:27:115:36 | Key | ECDSAP256SignAndVerify.java:103:16:103:49 | Key |
4-
| ECDSAP256SignAndVerify.java:120:30:120:38 | Key | ECDSAP256SignAndVerify.java:57:16:57:49 | Key |
5-
| ECDSAP256SignAndVerify.java:120:30:120:38 | Key | ECDSAP256SignAndVerify.java:80:16:80:49 | Key |
6-
| ECDSAP256SignAndVerify.java:120:30:120:38 | Key | ECDSAP256SignAndVerify.java:103:16:103:49 | Key |
7-
| Ed448SignAndVerify.java:30:31:30:40 | Key | Ed448SignAndVerify.java:21:47:21:80 | Key |
8-
| Ed448SignAndVerify.java:38:34:38:42 | Key | Ed448SignAndVerify.java:21:47:21:80 | Key |
9-
| Ed25519SignAndVerify.java:30:31:30:40 | Key | Ed25519SignAndVerify.java:21:47:21:80 | Key |
10-
| Ed25519SignAndVerify.java:38:34:38:42 | Key | Ed25519SignAndVerify.java:21:47:21:80 | Key |
11-
| LMSSignature.java:43:31:43:40 | Key | LMSSignature.java:33:47:33:74 | Key |
12-
| LMSSignature.java:50:32:50:40 | Key | LMSSignature.java:33:47:33:74 | Key |
1+
| ECDSASignAndVerify.java:57:16:57:49 | Key | secp256r1 |
2+
| ECDSASignAndVerify.java:80:16:80:49 | Key | secp256k1 |
3+
| Ed448SignAndVerify.java:21:47:21:80 | Key | Ed448 |
4+
| Ed25519SignAndVerify.java:21:47:21:80 | Key | Ed25519 |
5+
| LMSSignAndVerify.java:33:47:33:74 | Key | LMS |

‎java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/key_artifacts.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ import java
22
import experimental.quantum.Language
33

44
from Crypto::KeyArtifactNode n
5-
select n, n.getSourceNode()
5+
select n, n.getAKnownAlgorithm()

‎java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/key_generation_operations.expected‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
| ECDSAP256SignAndVerify.java:57:16:57:49 | KeyGeneration | ECDSAP256SignAndVerify.java:57:16:57:49 | Key |
2-
| ECDSAP256SignAndVerify.java:80:16:80:49 | KeyGeneration | ECDSAP256SignAndVerify.java:80:16:80:49 | Key |
3-
| ECDSAP256SignAndVerify.java:103:16:103:49 | KeyGeneration | ECDSAP256SignAndVerify.java:103:16:103:49 | Key |
4-
| Ed448SignAndVerify.java:21:47:21:80 | KeyGeneration | Ed448SignAndVerify.java:21:47:21:80 | Key |
5-
| Ed25519SignAndVerify.java:21:47:21:80 | KeyGeneration | Ed25519SignAndVerify.java:21:47:21:80 | Key |
6-
| LMSSignature.java:33:47:33:74 | KeyGeneration | LMSSignature.java:33:47:33:74 | Key |
1+
| ECDSASignAndVerify.java:57:16:57:49 | KeyGeneration | ECDSASignAndVerify.java:47:28:47:38 | EllipticCurve | ECDSASignAndVerify.java:57:16:57:49 | Key |
2+
| ECDSASignAndVerify.java:80:16:80:49 | KeyGeneration | ECDSASignAndVerify.java:65:28:65:38 | EllipticCurve | ECDSASignAndVerify.java:80:16:80:49 | Key |
3+
| Ed448SignAndVerify.java:21:47:21:80 | KeyGeneration | Ed448SignAndVerify.java:19:54:19:80 | KeyOperationAlgorithm | Ed448SignAndVerify.java:21:47:21:80 | Key |
4+
| Ed25519SignAndVerify.java:21:47:21:80 | KeyGeneration | Ed25519SignAndVerify.java:19:56:19:84 | KeyOperationAlgorithm | Ed25519SignAndVerify.java:21:47:21:80 | Key |
5+
| LMSSignAndVerify.java:33:47:33:74 | KeyGeneration | LMSSignAndVerify.java:31:46:31:70 | KeyOperationAlgorithm | LMSSignAndVerify.java:33:47:33:74 | Key |

‎java/ql/test/experimental/library-tests/quantum/BouncyCastle/signers/key_generation_operations.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ import java
22
import experimental.quantum.Language
33

44
from Crypto::KeyGenerationOperationNode n
5-
select n, n.getOutputKeyArtifact()
5+
select n, n.getAKnownAlgorithm(), n.getOutputKeyArtifact()

0 commit comments

Comments
 (0)