refactoring to use both dataflow configs; commit before deleting unused code

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))
+}

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)
+}

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"

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
+    }
 }