From 5e2ef660148f2840c3497f4ea3f483d6cbb35ae2 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Thu, 6 Oct 2022 00:40:42 -0400 Subject: [PATCH] refactoring to use both dataflow configs; commit before deleting unused code --- .../security/InsufficientKeySizeQuery.qll | 141 +++++++++- ...BeforeAddingTaintFlowBackForAsymmetric.qll | 264 ++++++++++++++++++ .../CWE/CWE-326/InsufficientKeySize.ql | 18 +- .../CWE-326/InsufficientKeySizeTest.java | 245 ++++++++++------ 4 files changed, 579 insertions(+), 89 deletions(-) create mode 100644 java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery_BeforeAddingTaintFlowBackForAsymmetric.qll diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll index 720ed112981..869f3c7cb0f 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll @@ -2,7 +2,7 @@ import semmle.code.java.security.Encryption import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.DataFlow -//import DataFlow::PathGraph +// ******* DATAFLOW ******************************************************************************* /** * Asymmetric (RSA, DSA, DH) key length data flow tracking configuration. */ @@ -14,6 +14,8 @@ class AsymmetricKeyTrackingConfiguration extends DataFlow::Configuration { integer.getIntValue() < 2048 and source.asExpr() = integer or + // The below only handles cases when variables are used (both locally in a method and between methods) + // The above adds handling for direct use of integers as well var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 2048 and source.asExpr() = var.getVariable().getInitializer() @@ -28,11 +30,101 @@ class AsymmetricKeyTrackingConfiguration extends DataFlow::Configuration { } } +/** + * Symmetric (AES) key length data flow tracking configuration. + */ +class SymmetricKeyTrackingConfiguration extends DataFlow::Configuration { + SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration" } + + override predicate isSource(DataFlow::Node source) { + exists(IntegerLiteral integer, VarAccess var | + integer.getIntValue() < 128 and + source.asExpr() = integer + or + // The below only handles cases when variables are used (both locally in a method and between methods) + // The above adds handling for direct use of integers as well + var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and + var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 128 and + source.asExpr() = var.getVariable().getInitializer() + ) + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof KeyGeneratorInitMethod and + sink.asExpr() = ma.getArgument(0) + ) + } +} + +/** + * Symmetric (AES) key length data flow tracking configuration. + */ +class SymmetricKeyTrackingConfiguration2 extends DataFlow::Configuration { + SymmetricKeyTrackingConfiguration2() { this = "SymmetricKeyTrackingConfiguration2" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof IntegerLiteral and + source.toString().toInt() < 128 + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof KeyGeneratorInitMethod and + sink.asExpr() = ma.getArgument(0) + ) + } +} + +class UnsafeSymmetricKeySize extends IntegerLiteral { + UnsafeSymmetricKeySize() { this.getIntValue() < 128 } +} + +class UnsafeAsymmetricKeySize extends IntegerLiteral { + UnsafeAsymmetricKeySize() { this.getIntValue() < 2048 } +} + +class UnsafeKeySize extends IntegerLiteral { + UnsafeKeySize() { + this instanceof UnsafeAsymmetricKeySize and + exists(MethodAccess ma | ma.getMethod() instanceof KeyPairGeneratorInitMethod) + or + this instanceof UnsafeSymmetricKeySize and + exists(MethodAccess ma | ma.getMethod() instanceof KeyGeneratorInitMethod) + } +} + +class KeyInitMethod extends Method { + KeyInitMethod() { + this instanceof KeyGeneratorInitMethod or + this instanceof KeyPairGeneratorInitMethod + } +} + +/** + * key length data flow tracking configuration. + */ +class KeyTrackingConfiguration extends DataFlow::Configuration { + KeyTrackingConfiguration() { this = "KeyTrackingConfiguration" } + + override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof UnsafeKeySize } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof KeyInitMethod and + sink.asExpr() = ma.getArgument(0) + ) + } +} + +// ******* DATAFLOW ******************************************************************************* +// ! move to Encryption.qll? /** The Java class `java.security.spec.ECGenParameterSpec`. */ private class ECGenParameterSpec extends RefType { ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") } } +// ! move to Encryption.qll? /** The `init` method declared in `javax.crypto.KeyGenerator`. */ private class KeyGeneratorInitMethod extends Method { KeyGeneratorInitMethod() { @@ -41,6 +133,7 @@ private class KeyGeneratorInitMethod extends Method { } } +// ! move to Encryption.qll? /** The `initialize` method declared in `java.security.KeyPairGenerator`. */ private class KeyPairGeneratorInitMethod extends Method { KeyPairGeneratorInitMethod() { @@ -97,6 +190,46 @@ private class KeyPairGeneratorInitConfiguration extends TaintTracking::Configura } } +/** + * Holds if a symmetric `KeyGenerator` implementing encryption algorithm + * `type` and initialized by `ma` uses an insufficient key size. + * + * `msg` provides a human-readable description of the problem. + */ +//bindingset[type] +private predicate hasShortSymmetricKey_TEST() { + exists( + SymmetricKeyTrackingConfiguration2 cfg, DataFlow::PathNode source, DataFlow::PathNode sink + | + cfg.hasFlowPath(source, sink) + ) + // ma.getMethod() instanceof KeyGeneratorInitMethod and + // // flow needed to correctly determine algorithm type and + // // not match to ANY symmetric algorithm (although doesn't really matter since only have AES currently...) + // exists( + // JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration cc, DataFlow::PathNode source, + // DataFlow::PathNode dest + // | + // jcg.getAlgoSpec().(StringLiteral).getValue() = type and + // source.getNode().asExpr() = jcg and + // dest.getNode().asExpr() = ma.getQualifier() and + // cc.hasFlowPath(source, dest) + // ) and + // ( + // // VarAccess case needed to handle FN of key-size stored in a variable + // // Note: cannot use CompileTimeConstantExpr since will miss cases when variable is not a compile-time constant + // // (e.g. not declared `final` in Java) + // exists(VarAccess var | + // var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and + // var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 128 and + // ma.getArgument(0) = var + // ) + // or + // ma.getArgument(0).(IntegerLiteral).getIntValue() < 128 + // ) and + // msg = "Key size should be at least 128 bits for " + type + " encryption." +} + /** * Holds if a symmetric `KeyGenerator` implementing encryption algorithm * `type` and initialized by `ma` uses an insufficient key size. @@ -262,3 +395,9 @@ predicate hasInsufficientKeySize(Expr e, string msg) { hasShortRsaKeyPair(e, msg) or hasShortECKeyPair(e, msg) } + +predicate hasInsufficientKeySize2(DataFlow::PathNode source, DataFlow::PathNode sink) { + exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) + or + exists(SymmetricKeyTrackingConfiguration2 config2 | config2.hasFlowPath(source, sink)) +} diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery_BeforeAddingTaintFlowBackForAsymmetric.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery_BeforeAddingTaintFlowBackForAsymmetric.qll new file mode 100644 index 00000000000..720ed112981 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery_BeforeAddingTaintFlowBackForAsymmetric.qll @@ -0,0 +1,264 @@ +import semmle.code.java.security.Encryption +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.DataFlow + +//import DataFlow::PathGraph +/** + * Asymmetric (RSA, DSA, DH) key length data flow tracking configuration. + */ +class AsymmetricKeyTrackingConfiguration extends DataFlow::Configuration { + AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" } + + override predicate isSource(DataFlow::Node source) { + exists(IntegerLiteral integer, VarAccess var | + integer.getIntValue() < 2048 and + source.asExpr() = integer + or + var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and + var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 2048 and + source.asExpr() = var.getVariable().getInitializer() + ) + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + sink.asExpr() = ma.getArgument(0) + ) + } +} + +/** The Java class `java.security.spec.ECGenParameterSpec`. */ +private class ECGenParameterSpec extends RefType { + ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") } +} + +/** The `init` method declared in `javax.crypto.KeyGenerator`. */ +private class KeyGeneratorInitMethod extends Method { + KeyGeneratorInitMethod() { + this.getDeclaringType() instanceof KeyGenerator and + this.hasName("init") + } +} + +/** The `initialize` method declared in `java.security.KeyPairGenerator`. */ +private class KeyPairGeneratorInitMethod extends Method { + KeyPairGeneratorInitMethod() { + this.getDeclaringType() instanceof KeyPairGenerator and + this.hasName("initialize") + } +} + +/** Returns the key size in the EC algorithm string */ +bindingset[algorithm] +private int getECKeySize(string algorithm) { + algorithm.matches("sec%") and // specification such as "secp256r1" + result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt() + or + algorithm.matches("X9.62%") and //specification such as "X9.62 prime192v2" + result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt() + or + (algorithm.matches("prime%") or algorithm.matches("c2tnb%")) and //specification such as "prime192v2" + result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt() +} + +/** Taint configuration tracking flow from a key generator to a `init` method call. */ +private class KeyGeneratorInitConfiguration extends TaintTracking::Configuration { + KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof JavaxCryptoKeyGenerator + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof KeyGeneratorInitMethod and + sink.asExpr() = ma.getQualifier() + ) + } +} + +/** + * Taint configuration tracking flow from a keypair generator to + * an `initialize` method call. + */ +private class KeyPairGeneratorInitConfiguration extends TaintTracking::Configuration { + KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof JavaSecurityKeyPairGenerator + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + sink.asExpr() = ma.getQualifier() + ) + } +} + +/** + * Holds if a symmetric `KeyGenerator` implementing encryption algorithm + * `type` and initialized by `ma` uses an insufficient key size. + * + * `msg` provides a human-readable description of the problem. + */ +bindingset[type] +private predicate hasShortSymmetricKey(MethodAccess ma, string msg, string type) { + ma.getMethod() instanceof KeyGeneratorInitMethod and + // flow needed to correctly determine algorithm type and + // not match to ANY symmetric algorithm (although doesn't really matter since only have AES currently...) + exists( + JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration cc, DataFlow::PathNode source, + DataFlow::PathNode dest + | + jcg.getAlgoSpec().(StringLiteral).getValue() = type and + source.getNode().asExpr() = jcg and + dest.getNode().asExpr() = ma.getQualifier() and + cc.hasFlowPath(source, dest) + ) and + ( + // VarAccess case needed to handle FN of key-size stored in a variable + // Note: cannot use CompileTimeConstantExpr since will miss cases when variable is not a compile-time constant + // (e.g. not declared `final` in Java) + exists(VarAccess var | + var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and + var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 128 and + ma.getArgument(0) = var + ) + or + // exists(CompileTimeConstantExpr var | + // //var.getUnderlyingExpr() instanceof IntegerLiteral and // can't include this... + // var.getIntValue() < 128 and + // ma.getArgument(0) = var + // ) + // or + ma.getArgument(0).(IntegerLiteral).getIntValue() < 128 + ) and + msg = "Key size should be at least 128 bits for " + type + " encryption." +} + +/** + * Holds if an AES `KeyGenerator` initialized by `ma` uses an insufficient key size. + * `msg` provides a human-readable description of the problem. + */ +private predicate hasShortAESKey(MethodAccess ma, string msg) { + hasShortSymmetricKey(ma, msg, "AES") +} + +/** + * Holds if an asymmetric `KeyPairGenerator` implementing encryption algorithm + * `type` and initialized by `ma` uses an insufficient key size. + * + * `msg` provides a human-readable description of the problem. + */ +bindingset[type] +private predicate hasShortAsymmetricKeyPair(MethodAccess ma, string msg, string type) { + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + //ma.getQualifier() instanceof JavaSecurityKeyPairGenerator and + //ma.getQualifier().getBasicBlock() instanceof JavaSecurityKeyPairGenerator and + // * USE BELOW + ma.getQualifier().getBasicBlock().getAPredecessor() instanceof JavaSecurityKeyPairGenerator and + // * USE ABOVE + //ma.getQualifier().getBasicBlock().getNode(2) instanceof JavaSecurityKeyPairGenerator and + // ma.getQualifier() + // .getBasicBlock() + // .getANode() + // .(JavaSecurityKeyPairGenerator) + // .getAlgoSpec() + // .(StringLiteral) + // .getValue() + // .toUpperCase() = type and + //ma.getQualifier().getBasicBlock().getAPredecessor() instanceof JavaSecurityKeyPairGenerator and + // * USE BELOW + ma.getQualifier() + .getBasicBlock() + .getAPredecessor() + .(JavaSecurityKeyPairGenerator) + .getAlgoSpec() + .(StringLiteral) + .getValue() + .toUpperCase() = type and + // * USE ABOVE + // flow needed to correctly determine algorithm type and + // not match to ANY asymmetric algorithm + // * REMOVE BELOW + // exists( + // JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kc, + // DataFlow::PathNode source, DataFlow::PathNode dest + // | + // jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = type and + // source.getNode().asExpr() = jpg and + // dest.getNode().asExpr() = ma.getQualifier() and + // kc.hasFlowPath(source, dest) + // ) and + // * REMOVE ABOVE + // VarAccess case needed to handle FN of key-size stored in a variable + // Note: cannot use CompileTimeConstantExpr since will miss cases when variable is not a compile-time constant + // (e.g. not declared `final` in Java) + ( + exists(VarAccess var | + var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and + var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 2048 and + ma.getArgument(0) = var + ) + or + ma.getArgument(0).(IntegerLiteral).getIntValue() < 2048 + // or + // exists( + // AsymmetricKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink + // | + // cfg.hasFlowPath(source, sink) + // ) + ) and + msg = "Key size should be at least 2048 bits for " + type + " encryption." +} + +/** + * Holds if a DSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. + * + * `msg` provides a human-readable description of the problem. + */ +private predicate hasShortDsaKeyPair(MethodAccess ma, string msg) { + hasShortAsymmetricKeyPair(ma, msg, "DSA") or + hasShortAsymmetricKeyPair(ma, msg, "DH") +} + +/** + * Holds if a RSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. + * + * `msg` provides a human-readable description of the problem. + */ +private predicate hasShortRsaKeyPair(MethodAccess ma, string msg) { + hasShortAsymmetricKeyPair(ma, msg, "RSA") +} + +/** + * Holds if an EC `KeyPairGenerator` initialized by `ma` uses an insufficient key size. + * + * `msg` provides a human-readable description of the problem. + */ +private predicate hasShortECKeyPair(MethodAccess ma, string msg) { + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + exists( + JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kc, + DataFlow::PathNode source, DataFlow::PathNode dest, ClassInstanceExpr cie + | + jpg.getAlgoSpec().(StringLiteral).getValue().matches("EC%") and // ECC variants such as ECDH and ECDSA + source.getNode().asExpr() = jpg and + dest.getNode().asExpr() = ma.getQualifier() and + kc.hasFlowPath(source, dest) and + DataFlow::localExprFlow(cie, ma.getArgument(0)) and + ma.getArgument(0).getType() instanceof ECGenParameterSpec and + getECKeySize(cie.getArgument(0).(StringLiteral).getValue()) < 256 + ) and + msg = "Key size should be at least 256 bits for EC encryption." +} + +// ! refactor this so can use 'path-problem' select clause instead? +predicate hasInsufficientKeySize(Expr e, string msg) { + hasShortAESKey(e, msg) or + hasShortDsaKeyPair(e, msg) or + hasShortRsaKeyPair(e, msg) or + hasShortECKeyPair(e, msg) +} diff --git a/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql b/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql index 66bf459f5f7..2979c9116c4 100644 --- a/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql +++ b/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql @@ -18,7 +18,17 @@ import DataFlow::PathGraph // from Expr e, string msg // where hasInsufficientKeySize(e, msg) // select e, msg -from AsymmetricKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) -select sink.getNode(), source, sink, "The $@ of an asymmetric key should be at least 2048 bits.", - sink.getNode(), "size" +// from +// AsymmetricKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, +// KeyTrackingConfiguration cfg2 //, DataFlow::PathNode source2, DataFlow::PathNode sink2 +// where +// //cfg.hasFlowPath(source, sink) //or +// cfg2.hasFlowPath(source, sink) +// select sink.getNode(), source, sink, "The $@ of an asymmetric key should be at least 2048 bits.", +// sink.getNode(), "size" +from DataFlow::PathNode source, DataFlow::PathNode sink +where + //hasInsufficientKeySize2(source, sink) + exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or + exists(SymmetricKeyTrackingConfiguration2 config2 | config2.hasFlowPath(source, sink)) +select sink.getNode(), source, sink, "This $@ is too small.", sink.getNode(), "key size" diff --git a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java index 9d9f5688520..68c3e7f322e 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java @@ -3,115 +3,192 @@ import java.security.spec.ECGenParameterSpec; import javax.crypto.KeyGenerator; public class InsufficientKeySizeTest { - public void cryptoMethod() throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - KeyGenerator keyGen1 = KeyGenerator.getInstance("AES"); - // BAD: Key size is less than 128 - keyGen1.init(64); // $ hasInsufficientKeySize + public void keySizeTesting() throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - KeyGenerator keyGen2 = KeyGenerator.getInstance("AES"); - // GOOD: Key size is no less than 128 - keyGen2.init(128); // Safe + // Test basic key generation for all algos - KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA"); - // BAD: Key size is less than 2048 - keyPairGen1.initialize(1024); // $ hasInsufficientKeySize + // AES (Symmetric) + { + // BAD: Key size is less than 128 + KeyGenerator keyGen1 = KeyGenerator.getInstance("AES"); + keyGen1.init(64); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA"); - // GOOD: Key size is no less than 2048 - keyPairGen2.initialize(2048); // Safe + // GOOD: Key size is no less than 128 + KeyGenerator keyGen2 = KeyGenerator.getInstance("AES"); + keyGen2.init(128); // Safe + } - KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA"); - // BAD: Key size is less than 2048 - keyPairGen3.initialize(1024); // $ hasInsufficientKeySize + // RSA (Asymmetric) + { + // BAD: Key size is less than 2048 + KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA"); + keyPairGen1.initialize(1024); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); - // GOOD: Key size is no less than 2048 - keyPairGen4.initialize(2048); // Safe + // GOOD: Key size is no less than 2048 + KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA"); + keyPairGen2.initialize(2048); // Safe + } - KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); - keyPairGen5.initialize(ecSpec1); // $ hasInsufficientKeySize + // DSA (Asymmetric) + { + // BAD: Key size is less than 2048 + KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA"); + keyPairGen3.initialize(1024); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - keyPairGen6.initialize(new ECGenParameterSpec("secp112r1")); // $ hasInsufficientKeySize + // GOOD: Key size is no less than 2048 + KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); + keyPairGen4.initialize(2048); // Safe + } - KeyPairGenerator keyPairGen7 = KeyPairGenerator.getInstance("EC"); - // GOOD: Key size is no less than 256 - ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); - keyPairGen7.initialize(ecSpec2); // Safe + // DH (Asymmetric) + { + // BAD: Key size is less than 2048 + KeyPairGenerator keyPairGen16 = KeyPairGenerator.getInstance("dh"); + keyPairGen16.initialize(1024); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen8 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - ECGenParameterSpec ecSpec3 = new ECGenParameterSpec("X9.62 prime192v2"); - keyPairGen8.initialize(ecSpec3); // $ hasInsufficientKeySize + // GOOD: Key size is no less than 2048 + KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH"); + keyPairGen17.initialize(2048); // Safe + } - KeyPairGenerator keyPairGen9 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - ECGenParameterSpec ecSpec4 = new ECGenParameterSpec("X9.62 c2tnb191v3"); - keyPairGen9.initialize(ecSpec4); // $ hasInsufficientKeySize + // EC (Asymmetric) + // ! Check if I can re-use the same KeyPairGenerator instance with all of the below? + { + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); + keyPairGen5.initialize(ecSpec1); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen10 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - ECGenParameterSpec ecSpec5 = new ECGenParameterSpec("sect163k1"); - keyPairGen10.initialize(ecSpec5); // $ hasInsufficientKeySize + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); + keyPairGen6.initialize(new ECGenParameterSpec("secp112r1")); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen11 = KeyPairGenerator.getInstance("EC"); - // GOOD: Key size is no less than 256 - ECGenParameterSpec ecSpec6 = new ECGenParameterSpec("X9.62 c2tnb359v1"); - keyPairGen11.initialize(ecSpec6); // Safe + // GOOD: Key size is no less than 256 + KeyPairGenerator keyPairGen7 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); + keyPairGen7.initialize(ecSpec2); // Safe - KeyPairGenerator keyPairGen12 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - ECGenParameterSpec ecSpec7 = new ECGenParameterSpec("prime192v2"); - keyPairGen12.initialize(ecSpec7); // $ hasInsufficientKeySize + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen8 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec3 = new ECGenParameterSpec("X9.62 prime192v2"); + keyPairGen8.initialize(ecSpec3); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen13 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is no less than 256 // ! I think this comment is wrong - double-check - ECGenParameterSpec ecSpec8 = new ECGenParameterSpec("prime256v1"); - keyPairGen13.initialize(ecSpec8); // Safe + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen9 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec4 = new ECGenParameterSpec("X9.62 c2tnb191v3"); + keyPairGen9.initialize(ecSpec4); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen14 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is less than 256 - ECGenParameterSpec ecSpec9 = new ECGenParameterSpec("c2tnb191v1"); - keyPairGen14.initialize(ecSpec9); // $ hasInsufficientKeySize + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen10 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec5 = new ECGenParameterSpec("sect163k1"); + keyPairGen10.initialize(ecSpec5); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen15 = KeyPairGenerator.getInstance("EC"); - // BAD: Key size is no less than 256 // ! I think this comment is wrong - double-check - ECGenParameterSpec ecSpec10 = new ECGenParameterSpec("c2tnb431r1"); - keyPairGen15.initialize(ecSpec10); // Safe + // GOOD: Key size is no less than 256 + KeyPairGenerator keyPairGen11 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec6 = new ECGenParameterSpec("X9.62 c2tnb359v1"); + keyPairGen11.initialize(ecSpec6); // Safe - KeyPairGenerator keyPairGen16 = KeyPairGenerator.getInstance("dh"); - // BAD: Key size is less than 2048 - keyPairGen16.initialize(1024); // $ hasInsufficientKeySize + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen12 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec7 = new ECGenParameterSpec("prime192v2"); + keyPairGen12.initialize(ecSpec7); // $ hasInsufficientKeySize - KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH"); - // GOOD: Key size is no less than 2048 - keyPairGen17.initialize(2048); // Safe + // GOOD: Key size is no less than 256 + KeyPairGenerator keyPairGen13 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec8 = new ECGenParameterSpec("prime256v1"); + keyPairGen13.initialize(ecSpec8); // Safe + + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen14 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec9 = new ECGenParameterSpec("c2tnb191v1"); + keyPairGen14.initialize(ecSpec9); // $ hasInsufficientKeySize + + // GOOD: Key size is no less than 256 + KeyPairGenerator keyPairGen15 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec10 = new ECGenParameterSpec("c2tnb431r1"); + keyPairGen15.initialize(ecSpec10); // Safe + } + + // ! FN Testing Additions: + + // Test local variable usage - Symmetric + { + final int size1 = 64; // compile-time constant + int size2 = 64; // NOT a compile-time constant + + // BAD: Key size is less than 128 + KeyGenerator keyGen3 = KeyGenerator.getInstance("AES"); + keyGen3.init(size1); // $ hasInsufficientKeySize + + // BAD: Key size is less than 128 + KeyGenerator keyGen4 = KeyGenerator.getInstance("AES"); + keyGen4.init(size2); // $ hasInsufficientKeySize + } + + // Test local variable usage - Asymmetric, Not EC + { + final int size1 = 1024; // compile-time constant + int size2 = 1024; // NOT a compile-time constant + + // BAD: Key size is less than 2048 + KeyPairGenerator keyPairGen18 = KeyPairGenerator.getInstance("RSA"); + keyPairGen18.initialize(size1); // $ hasInsufficientKeySize + + // BAD: Key size is less than 2048 + KeyPairGenerator keyPairGen19 = KeyPairGenerator.getInstance("RSA"); + keyPairGen19.initialize(size2); // $ hasInsufficientKeySize + } - // FN: Test with variables as numbers - final int size1 = 64; - KeyGenerator keyGen3 = KeyGenerator.getInstance("AES"); - // BAD: Key size is less than 128 - keyGen3.init(size1); // $ hasInsufficientKeySize + // Test variable passed to other method(s) - Symmetric + { + int size = 64; // test integer variable + KeyGenerator keyGen = KeyGenerator.getInstance("AES"); // test KeyGenerator variable + testSymmetric(size, keyGen); + } - int size2 = 1024; - KeyPairGenerator keyPairGen18 = KeyPairGenerator.getInstance("RSA"); - // BAD: Key size is less than 2048 - keyPairGen18.initialize(size2); // $ hasInsufficientKeySize - int keysize = 1024; - KeyPairGenerator keyPairGen20 = KeyPairGenerator.getInstance("DSA"); - test(keysize, keyPairGen20); + // Test variables passed to other method(s) - Asymmetric, Not EC + { + int size = 1024; // test integer variable + KeyPairGenerator keyPairGen21 = KeyPairGenerator.getInstance("RSA"); // test KeyPairGenerator variable + testAsymmetricNonEC(size, keyPairGen21); + } + + // Test variable passed to other method(s) - Asymmetric, EC + { + ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); // test ECGenParameterSpec variable + KeyPairGenerator keyPairGen22 = KeyPairGenerator.getInstance("EC"); // test KeyPairGenerator variable + testAsymmetricEC(ecSpec, keyPairGen22); + } + } - public static void test(int keySize, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - KeyPairGenerator keyPairGen19 = KeyPairGenerator.getInstance("RSA"); - // BAD: Key size is less than 128 - keyPairGen19.initialize(keySize); // $ hasInsufficientKeySize + public static void testSymmetric(int keySize, KeyGenerator kg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + // BAD: Key size is less than 2048 + KeyGenerator keyGen = KeyGenerator.getInstance("AES"); + keyGen.init(keySize); // $ hasInsufficientKeySize + // BAD: Key size is less than 2048 + kg.init(64); // $ hasInsufficientKeySize + } + + public static void testAsymmetricNonEC(int keySize, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + // BAD: Key size is less than 2048 + KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("RSA"); + keyPairGen.initialize(keySize); // $ hasInsufficientKeySize + + // BAD: Key size is less than 2048 kpg.initialize(1024); // $ hasInsufficientKeySize } + + public static void testAsymmetricEC(ECGenParameterSpec spec, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + // BAD: Key size is less than 256 + KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("EC"); + keyPairGen.initialize(spec); // $ hasInsufficientKeySize + + // BAD: Key size is less than 256 + ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); + kpg.initialize(ecSpec); // $ hasInsufficientKeySize + } }