Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Final BouncyCastle translation and cleaning #150

Merged
merged 9 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading