Skip to content

Commit

Permalink
Final BouncyCastle translation and cleaning (#150)
Browse files Browse the repository at this point in the history
* Use encapsulation or decapsulation functionality for the translation of KEM

Signed-off-by: Hugo Queinnec <[email protected]>

* Remove deprecated parts of BouncyCastleInfoMap

Signed-off-by: Hugo Queinnec <[email protected]>

* fix xBcDerivationFunction

Signed-off-by: Hugo Queinnec <[email protected]>

* ECNR

Signed-off-by: Hugo Queinnec <[email protected]>

* ongoing model javadoc

Signed-off-by: Hugo Queinnec <[email protected]>

* ongoing model javadoc

Signed-off-by: Hugo Queinnec <[email protected]>

* add remaining empty javadoc

Signed-off-by: Hugo Queinnec <[email protected]>

* remove deprecated TODOs

Signed-off-by: Hugo Queinnec <[email protected]>

* todo

Signed-off-by: Hugo Queinnec <[email protected]>

---------

Signed-off-by: Hugo Queinnec <[email protected]>
  • Loading branch information
hugoqnc authored Sep 23, 2024
1 parent c538f02 commit ec6db5f
Show file tree
Hide file tree
Showing 216 changed files with 3,200 additions and 373 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public static List<IDetectionRule<Tree>> rules() {
// Wrapper
BcWrapperEngine.rules().stream(),
// BasicAgreement
// TODO: Should it be an entry point or only a depending rule of IES?
BcBasicAgreement.rules().stream(),
// DerivationFunction
BcDerivationFunction.rules().stream(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,13 @@
* Instantiate this class to easily store information related to a cryptographic class name. Using
* the function {@code putKey} and continuing with other "put" functions, information can be easily
* stored into an {@code Info} class. The information can then be retrieved using the getter
* functions of the {@code Info} class, or dedicated helpers like the {@code getDisplayName}
* function.
* functions of the {@code Info} class.
*/
public class BouncyCastleInfoMap {

private final Map<String, Info> map = new ConcurrentHashMap<>();

public static class Info {
@Nullable private String name;

/**
* use the key of the map instead
*
* @deprecated
*/
@Deprecated(since = "2.0.0")
public String getName() {
return name;
}

private void setName(@Nonnull String name) {
this.name = name;
}

@Nullable private String type;

public String getType() {
Expand Down Expand Up @@ -85,15 +68,6 @@ public InfoBuilder(String key) {

private static final String errorMessage = "Unexpected missing key in InfoBuilder: ";

public InfoBuilder putName(@Nonnull String name) {
if (!map.containsKey(key)) {
throw new IllegalArgumentException(errorMessage + key);
}
Info info = map.get(key);
info.setName(name);
return this;
}

public InfoBuilder putType(@Nonnull String type) {
if (!map.containsKey(key)) {
throw new IllegalArgumentException(errorMessage + key);
Expand Down Expand Up @@ -128,47 +102,4 @@ public InfoBuilder putKey(@Nonnull String key) {
public Set<Entry<String, Info>> entrySet() {
return map.entrySet();
}

/**
* Returns the correct name of the cryptographic asset. If the asset name was specified with
* {@code putName}, it will return this name. Otherwise it will return the {@code key}.
*
* @param key - Key of the information map, typically the cryptographic class
* @return The correct name of the cryptographic asset
* @deprecated use the key of the map instead
*/
@Deprecated(since = "2.0.0")
public String getDisplayName(@Nonnull String key) {
return getDisplayName(key, null);
}

/**
* Returns the correct name of the cryptographic asset. If the asset name was specified with
* {@code putName}, it will return this name. Otherwise it will return the {@code key}, possibly
* modified by the {@code removePart} argument.
*
* @param key - Key of the information map, typically the cryptographic class
* @param removePart - Substring to remove from the name (only when no specific name was defined
* with {@code putName}). If {@code null}, it does nothing.
* @return The correct name of the cryptographic asset
* @deprecated use the key of the map instead
*/
@Deprecated(since = "2.0.0")
public String getDisplayName(@Nonnull String key, String removePart) {
if (!map.containsKey(key)) {
throw new IllegalArgumentException("Key does not exist in the map: " + key);
}

Info info = map.get(key);

if (info.getName() != null) {
return info.getName();
} else {
if (removePart == null) {
return key;
} else {
return key.replace(removePart, "");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ private BcOCBBlockCipher() {

private static final String MODE = "OCBBlockCipher";

// TODO: add a way to distinguish between the main and the hash ciphers,
// that can be used in translation
private static final IDetectionRule<Tree> CONSTRUCTOR_1 =
new DetectionRuleBuilder<Tree>()
.createDetectionRule()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,7 @@ private static final List<IDetectionRule<Tree>> specialConstructors(
constructorsList.add(
new DetectionRuleBuilder<Tree>()
.createDetectionRule()
// TODO: forExactObjectTypes(...)
.forObjectTypes("org.bouncycastle.crypto.modes.PGPCFBBlockCipher")
.forObjectExactTypes("org.bouncycastle.crypto.modes.PGPCFBBlockCipher")
.forConstructor()
.shouldBeDetectedAs(new ValueActionFactory<>("PGPCFBBlockCipher"))
.withMethodParameter("org.bouncycastle.crypto.BlockCipher")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private BcBufferedBlockCipher() {
infoMap.putKey("DefaultBufferedBlockCipher").putType(typePrefix); // <–– parent class
infoMap.putKey("CTSBlockCipher").putType(typePrefix + "modes.");
infoMap.putKey("KXTSBlockCipher").putType(typePrefix + "modes.");
infoMap.putKey("OldCTSBlockCipher").putName("CTS").putType(typePrefix + "modes.");
infoMap.putKey("OldCTSBlockCipher").putType(typePrefix + "modes.");
infoMap.putKey("PaddedBlockCipher").putType(typePrefix + "modes.");
}

Expand Down Expand Up @@ -84,7 +84,6 @@ private BcBufferedBlockCipher() {
.forConstructor()
.shouldBeDetectedAs(new ValueActionFactory<>("NISTCTSBlockCipher"))
.withMethodParameter("int")
// TODO: "Type": should it be detected?
.withMethodParameter("org.bouncycastle.crypto.BlockCipher")
.addDependingDetectionRules(BcBlockCipher.all())
.buildForContext(new CipherContext(Map.of("kind", "BUFFERED_BLOCK_CIPHER")))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ private BcGMSSParameters() {
.inBundle(() -> "Bc")
.withoutDependingDetectionRules();

// TODO: add a test case for this constructor
private static final IDetectionRule<Tree> PRIVATE_KEY_CONSTRUCTOR_1 =
new DetectionRuleBuilder<Tree>()
.createDetectionRule()
Expand All @@ -126,7 +125,6 @@ private BcGMSSParameters() {
.inBundle(() -> "Bc")
.withoutDependingDetectionRules();

// TODO: add a test case for this constructor
private static final IDetectionRule<Tree> PRIVATE_KEY_CONSTRUCTOR_2 =
new DetectionRuleBuilder<Tree>()
.createDetectionRule()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ private BcSABERParameters() {
.withMethodParameter("int")
.shouldBeDetectedAs(new KeySizeFactory<>(Size.UnitType.BIT))
.withMethodParameter("boolean")
/*
* TODO: How to translate below?
* It comes from using SABERParameters with `usingAes == true`
*/
// .shouldBeDetectedAs(new BooleanFactory<>()) // captures `usingAes`
.withMethodParameter("boolean")
.buildForContext(new AlgorithmParameterContext())
.inBundle(() -> "Bc")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ private BcDerivationFunction() {
.withMethodParameter("org.bouncycastle.crypto.Digest")
.addDependingDetectionRules(BcDigests.rules())
.buildForContext(new KeyContext(Map.of("kind", "KDF")))
// TODO: .withDependingDetectionRules(DerivationFunctionInit.rules()));
.inBundle(() -> "Bc")
.withoutDependingDetectionRules());
}
Expand All @@ -117,7 +116,6 @@ private BcDerivationFunction() {
.withMethodParameter("org.bouncycastle.crypto.Mac")
.addDependingDetectionRules(BcMac.rules())
.buildForContext(new KeyContext(Map.of("kind", "KDF")))
// TODO: .withDependingDetectionRules(DerivationFunctionInit.rules()));
.inBundle(() -> "Bc")
.withoutDependingDetectionRules());
}
Expand All @@ -139,7 +137,6 @@ private BcDerivationFunction() {
.withMethodParameter("org.bouncycastle.crypto.Digest")
.addDependingDetectionRules(BcDigests.rules())
.buildForContext(new KeyContext(Map.of("kind", "KDF")))
// TODO: .withDependingDetectionRules(DerivationFunctionInit.rules()));
.inBundle(() -> "Bc")
.withoutDependingDetectionRules());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ private BcDSA() {
// nothing
}

/* TODO: maybe the function `extractSecret` would be a better entry point than the constructors? */

public static final List<String> dsas =
List.of(
"DSASigner",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,6 @@ private BcMac() {
.forObjectTypes("org.bouncycastle.crypto.macs.SkeinMac")
.forConstructor()
.shouldBeDetectedAs(new ValueActionFactory<>("SkeinMac"))
// TODO: Should I capture this parameter?
// Could that lead to an infinity of depending detection rules?
.withMethodParameter("org.bouncycastle.crypto.macs.SkeinMac")
.buildForContext(new MacContext())
.inBundle(() -> "Bc")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ private BcIESEngineInit() {
.withMethodParameter("org.bouncycastle.crypto.params.AsymmetricKeyParameter")
.withMethodParameter("org.bouncycastle.crypto.CipherParameters")
.addDependingDetectionRules(BcCipherParameters.rules())
// TODO: capture?
.withMethodParameter(
"org.bouncycastle.crypto.generators.EphemeralKeyPairGenerator")
.buildForContext(new CipherContext())
Expand All @@ -61,7 +60,6 @@ private BcIESEngineInit() {
.withMethodParameter("org.bouncycastle.crypto.params.AsymmetricKeyParameter")
.withMethodParameter("org.bouncycastle.crypto.CipherParameters")
.addDependingDetectionRules(BcCipherParameters.rules())
// TODO: capture?
.withMethodParameter("org.bouncycastle.crypto.KeyParser")
.buildForContext(new CipherContext())
.inBundle(() -> "Bc")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.sonar.plugins.java.api.tree.Tree;

public final class BcPQCSigner {
// TODO: Need support for MessageSigner and StateAwareMessageSigner first

private BcPQCSigner() {
// nothing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ private BcSimpleSigner() {
// nothing
}

/* TODO: maybe the function `extractSecret` would be a better entry point than the constructors? */

private static final List<String> simpleSigners =
Arrays.asList(
"Ed25519Signer" /* standard algorithm */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@ public class BcOAEPEncodingTestFile {
public byte[] encryptCEK1(final RSAPublicKey pub, final SecretKey cek)
throws RuntimeException {
try {
// TODO: This detection should optimally not happen once RSA has been detected as a child finding of OAEPEncoding
// This detection should optimally not happen once RSA has been detected as a child finding of OAEPEncoding
AsymmetricBlockCipher engine = new RSAEngine(); // Noncompliant {{RSAEngine}}

// TODO: Using intermediate variables should also work
// Digest digest = new ShortenedDigest(new SHA3Digest(), 16);
// Digest digest = new SHA3Digest();
// TODO: Duplicate detection of SHA3 (see detection store)
OAEPEncoding cipher = new OAEPEncoding(engine, new ShortenedDigest(new SHA3Digest(), 16)); // Noncompliant {{OAEPEncoding}}

BigInteger mod = pub.getModulus();
Expand All @@ -46,7 +45,7 @@ public byte[] encryptCEK1(final RSAPublicKey pub, final SecretKey cek)
public byte[] encryptCEK2(final RSAPublicKey pub, final SecretKey cek)
throws RuntimeException {
try {
// TODO: This detection should optimally not happen once RSA has been detected as a child finding of OAEPEncoding
// This detection should optimally not happen once RSA has been detected as a child finding of OAEPEncoding
AsymmetricBlockCipher engine = new RSAEngine(); // Noncompliant {{RSAEngine}}

OAEPEncoding cipher = new OAEPEncoding(engine, new NonMemoableDigest(new SHA3Digest()), new SHA512Digest(), new byte[16]); // Noncompliant {{OAEPEncoding}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

public class BcCBCBlockCipherTestFile {
public void AESCipherCBCnoPad1(byte[] key) {
// TODO: This detection should optimally not happen once AES has been detected as a child finding of CBCBlockCipher
// This detection should optimally not happen once AES has been detected as a child finding of CBCBlockCipher
org.bouncycastle.crypto.BlockCipher aes = new AESFastEngine(); // Noncompliant {{AESFastEngine}}
CBCBlockCipher cbc = new CBCBlockCipher(aes); // Noncompliant {{CBCBlockCipher}}
KeyParameter kp = new KeyParameter(key);
Expand All @@ -14,7 +14,7 @@ public void AESCipherCBCnoPad1(byte[] key) {
}

public void AESCipherCBCnoPad2(byte[] key) {
// TODO: This detection should optimally not happen once AES has been detected as a child finding of CBCBlockCipher
// This detection should optimally not happen once AES has been detected as a child finding of CBCBlockCipher
org.bouncycastle.crypto.BlockCipher aes = AESEngine.newInstance(); // Noncompliant {{AESEngine}}
CBCBlockCipher cbc = CBCBlockCipher.newInstance(aes); // Noncompliant {{CBCBlockCipher}}
KeyParameter kp = new KeyParameter(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0 || findingId == 2) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 2) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0 || findingId == 2 || findingId == 4 || findingId == 6) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 1 || findingId == 3) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0 || findingId == 2) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0 || findingId == 1) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0 || findingId == 1) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ public void asserts(
@NotNull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
@NotNull List<INode> nodes) {
/**
* TODO: Optimally, we shouldn't have these direct detections of engines, as they appear in
* the depending detection rules
* Optimally, we shouldn't have these direct detections of engines, as they appear in the
* depending detection rules
*/
if (findingId == 0) {
return;
Expand Down
Loading

0 comments on commit ec6db5f

Please sign in to comment.