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 e6a61d7bb4c..3e6a1649ceb 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java @@ -10,244 +10,197 @@ import javax.crypto.spec.DHGenParameterSpec; public class InsufficientKeySizeTest { public void keySizeTesting() throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - // Test basic key generation for all algos - - // AES (Symmetric) + /* AES (Symmetric): minimum recommended key size is 128 */ { - // BAD: Key size is less than 128 + /* Test with keysize as int */ KeyGenerator keyGen1 = KeyGenerator.getInstance("AES"); keyGen1.init(64); // $ hasInsufficientKeySize - // GOOD: Key size is no less than 128 KeyGenerator keyGen2 = KeyGenerator.getInstance("AES"); - keyGen2.init(128); // Safe + keyGen2.init(128); // Safe: Key size is no less than 128 + + /* Test with local variable as keysize */ + final int size1 = 64; // compile-time constant + int size2 = 64; // not a compile-time constant + + KeyGenerator keyGen3 = KeyGenerator.getInstance("AES"); + keyGen3.init(size1); // $ hasInsufficientKeySize + + KeyGenerator keyGen4 = KeyGenerator.getInstance("AES"); + keyGen4.init(size2); // $ hasInsufficientKeySize + + /* Test variables passed to another method */ + KeyGenerator keyGen = KeyGenerator.getInstance("AES"); // MISSING: test KeyGenerator variable as argument + testSymmetricVariable(size2, keyGen); // test with variable as key size + testSymmetricInt(64); // test with int literal as key size } - // RSA (Asymmetric) + // RSA (Asymmetric): minimum recommended key size is 2048 { - // BAD: Key size is less than 2048 + /* Test with keysize as int */ KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA"); keyPairGen1.initialize(1024); // $ hasInsufficientKeySize - // GOOD: Key size is no less than 2048 KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA"); - keyPairGen2.initialize(2048); // Safe + keyPairGen2.initialize(2048); // Safe: Key size is no less than 2048 - // test with spec - // BAD: Key size is less than 2048 KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("RSA"); RSAKeyGenParameterSpec rsaSpec = new RSAKeyGenParameterSpec(1024, null); // $ hasInsufficientKeySize keyPairGen3.initialize(rsaSpec); - // BAD: Key size is less than 2048 KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("RSA"); keyPairGen4.initialize(new RSAKeyGenParameterSpec(1024, null)); // $ hasInsufficientKeySize + + /* Test with local variable as keysize */ + final int size1 = 1024; // compile-time constant + int size2 = 1024; // not a compile-time constant + + KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("RSA"); + keyPairGen5.initialize(size1); // $ hasInsufficientKeySize + + KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("RSA"); + keyPairGen6.initialize(size2); // $ hasInsufficientKeySize + + /* Test variables passed to another method */ + KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("RSA"); // MISSING: test KeyGenerator variable as argument + testAsymmetricNonEcVariable(size2, keyPairGen); // test with variable as key size + testAsymmetricNonEcInt(1024); // test with int literal as key size } - // DSA (Asymmetric) + // DSA (Asymmetric): minimum recommended key size is 2048 { - // BAD: Key size is less than 2048 + /* Test with keysize as int */ + KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("DSA"); + keyPairGen1.initialize(1024); // $ hasInsufficientKeySize + + KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("DSA"); + keyPairGen2.initialize(2048); // Safe: Key size is no less than 2048 + KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA"); - keyPairGen3.initialize(1024); // $ hasInsufficientKeySize - - // GOOD: Key size is no less than 2048 - KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); - keyPairGen4.initialize(2048); // Safe - - // test with spec - // BAD: Key size is less than 2048 - KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("DSA"); DSAGenParameterSpec dsaSpec = new DSAGenParameterSpec(1024, 0); // $ hasInsufficientKeySize - keyPairGen5.initialize(dsaSpec); + keyPairGen3.initialize(dsaSpec); - // BAD: Key size is less than 2048 - KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("DSA"); - keyPairGen6.initialize(new DSAGenParameterSpec(1024, 0)); // $ hasInsufficientKeySize + KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); + keyPairGen4.initialize(new DSAGenParameterSpec(1024, 0)); // $ hasInsufficientKeySize } - // DH (Asymmetric) + // DH (Asymmetric): minimum recommended key size is 2048 { - // BAD: Key size is less than 2048 - KeyPairGenerator keyPairGen16 = KeyPairGenerator.getInstance("dh"); - keyPairGen16.initialize(1024); // $ hasInsufficientKeySize + /* Test with keysize as int */ + KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("dh"); + keyPairGen1.initialize(1024); // $ hasInsufficientKeySize - // GOOD: Key size is no less than 2048 - KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH"); - keyPairGen17.initialize(2048); // Safe + KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("DH"); + keyPairGen2.initialize(2048); // Safe: Key size is no less than 2048 - // test with spec - // BAD: Key size is less than 2048 KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DH"); DHGenParameterSpec dhSpec = new DHGenParameterSpec(1024, 0); // $ hasInsufficientKeySize keyPairGen3.initialize(dhSpec); - // BAD: Key size is less than 2048 KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DH"); keyPairGen4.initialize(new DHGenParameterSpec(1024, 0)); // $ hasInsufficientKeySize } - // EC (Asymmetric) - // ! Check if I can re-use the same KeyPairGenerator instance with all of the below? + // EC (Asymmetric): minimum recommended key size is 256 { - // BAD: Key size is less than 256 - KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC"); + /* Test with keysize as int */ + KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("EC"); + keyPairGen1.initialize(128); // $ hasInsufficientKeySize + + /* Test with keysize as curve name in spec */ + KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); // $ hasInsufficientKeySize - keyPairGen5.initialize(ecSpec1); + keyPairGen2.initialize(ecSpec1); - // BAD: Key size is less than 256 - KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); - keyPairGen6.initialize(new ECGenParameterSpec("secp112r1")); // $ hasInsufficientKeySize + KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("EC"); + keyPairGen3.initialize(new ECGenParameterSpec("secp112r1")); // $ hasInsufficientKeySize - // GOOD: Key size is no less than 256 - KeyPairGenerator keyPairGen7 = KeyPairGenerator.getInstance("EC"); - ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); - keyPairGen7.initialize(ecSpec2); // Safe + KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); // Safe: Key size is no less than 256 + keyPairGen4.initialize(ecSpec2); - // BAD: Key size is less than 256 - KeyPairGenerator keyPairGen8 = KeyPairGenerator.getInstance("EC"); + KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec3 = new ECGenParameterSpec("X9.62 prime192v2"); // $ hasInsufficientKeySize - keyPairGen8.initialize(ecSpec3); + keyPairGen5.initialize(ecSpec3); - // BAD: Key size is less than 256 - KeyPairGenerator keyPairGen9 = KeyPairGenerator.getInstance("EC"); + KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec4 = new ECGenParameterSpec("X9.62 c2tnb191v3"); // $ hasInsufficientKeySize - keyPairGen9.initialize(ecSpec4); + keyPairGen6.initialize(ecSpec4); - // BAD: Key size is less than 256 - KeyPairGenerator keyPairGen10 = KeyPairGenerator.getInstance("EC"); + KeyPairGenerator keyPairGen7 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec5 = new ECGenParameterSpec("sect163k1"); // $ hasInsufficientKeySize - keyPairGen10.initialize(ecSpec5); + keyPairGen7.initialize(ecSpec5); - // 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 keyPairGen8 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec6 = new ECGenParameterSpec("X9.62 c2tnb359v1"); // Safe: Key size is no less than 256 + keyPairGen8.initialize(ecSpec6); - // BAD: Key size is less than 256 - KeyPairGenerator keyPairGen12 = KeyPairGenerator.getInstance("EC"); + KeyPairGenerator keyPairGen9 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec7 = new ECGenParameterSpec("prime192v2"); // $ hasInsufficientKeySize - keyPairGen12.initialize(ecSpec7); + keyPairGen9.initialize(ecSpec7); - // GOOD: Key size is no less than 256 - KeyPairGenerator keyPairGen13 = KeyPairGenerator.getInstance("EC"); - ECGenParameterSpec ecSpec8 = new ECGenParameterSpec("prime256v1"); - keyPairGen13.initialize(ecSpec8); // Safe + KeyPairGenerator keyPairGen10 = KeyPairGenerator.getInstance("EC"); + ECGenParameterSpec ecSpec8 = new ECGenParameterSpec("prime256v1"); // Safe: Key size is no less than 256 + keyPairGen10.initialize(ecSpec8); - // BAD: Key size is less than 256 KeyPairGenerator keyPairGen14 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec9 = new ECGenParameterSpec("c2tnb191v1"); // $ hasInsufficientKeySize keyPairGen14.initialize(ecSpec9); - // GOOD: Key size is no less than 256 KeyPairGenerator keyPairGen15 = KeyPairGenerator.getInstance("EC"); ECGenParameterSpec ecSpec10 = new ECGenParameterSpec("c2tnb431r1"); - keyPairGen15.initialize(ecSpec10); // Safe + keyPairGen15.initialize(ecSpec10); // Safe: Key size is no less than 256 + + /* Test variables passed to another method */ + ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); // $ hasInsufficientKeySize + KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("EC"); // MISSING: test KeyGenerator variable as argument + testAsymmetricEC(ecSpec, keyPairGen); // test spec as an argument } - - // ! 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 - } - - - // 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); // test with variable as key size - testSymmetric2(64); // test with int literal as key size - } - - - // 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 with variable as key size - testAsymmetricNonEC2(1024); // test with int literal as key size - } - - // Test variable passed to other method(s) - Asymmetric, EC - { - ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); // $ hasInsufficientKeySize // test ECGenParameterSpec variable - KeyPairGenerator keyPairGen22 = KeyPairGenerator.getInstance("EC"); // test KeyPairGenerator variable - testAsymmetricEC(ecSpec, keyPairGen22); - - } - } - public static void testSymmetric(int keySize, KeyGenerator kg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - // BAD: Key size is less than 2048 + public static void testSymmetricVariable(int keySize, KeyGenerator kg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { KeyGenerator keyGen = KeyGenerator.getInstance("AES"); keyGen.init(keySize); // $ hasInsufficientKeySize - - // BAD: Key size is less than 2048 kg.init(64); // $ MISSING: hasInsufficientKeySize } - //! refactor this to use expected-value tag and combine with above method - public static void testSymmetric2(int keySize) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - // BAD: Key size is less than 2048 + public static void testSymmetricInt(int keySize) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { KeyGenerator keyGen = KeyGenerator.getInstance("AES"); keyGen.init(keySize); // $ hasInsufficientKeySize } - public static void testAsymmetricNonEC(int keySize, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - // BAD: Key size is less than 2048 + public static void testAsymmetricNonEcVariable(int keySize, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("RSA"); keyPairGen.initialize(keySize); // $ hasInsufficientKeySize - - // BAD: Key size is less than 2048 kpg.initialize(1024); // $ MISSING: hasInsufficientKeySize } - //! refactor this to use expected-value tag and combine with above method - public static void testAsymmetricNonEC2(int keySize) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - // BAD: Key size is less than 2048 + public static void testAsymmetricNonEcInt(int keySize) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("RSA"); keyPairGen.initialize(keySize); // $ hasInsufficientKeySize } - public static void testAsymmetricEC(ECGenParameterSpec spec, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { - // BAD: Key size is less than 256 + public static void testAsymmetricEcVariable(ECGenParameterSpec spec, KeyPairGenerator kpg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("EC"); - keyPairGen.initialize(spec); // sink is now at above where `spec` variable is initialized + keyPairGen.initialize(spec); // sink is above where `spec` variable is initialized - // BAD: Key size is less than 256 ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); // $ hasInsufficientKeySize - kpg.initialize(ecSpec); + kpg.initialize(ecSpec); // MISSING: test KeyGenerator variable as argument } - // ToDo testing: - // ? todo #1: add tests for keysize variable passed to specs - not needed if spec is sink now - // ? todo #3: add test for retrieving a key from elsewhere? - // ? todo #4: add barrier-guard tests (see FP from OpenIdentityPlatform/OpenAM) - // ? todo #5: add tests for updated keysize variable?: e.g. keysize = 1024; keysize += 1024; so when it's used it is correctly 2048. - // ? todo #6: consider if some flow paths for keysize variables will be too hard to track how the keysize is updated (e.g. if calling some other method to get keysize, etc....) + public static void testAsymmetricEcInt(int keySize) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("EC"); + keyPairGen.initialize(keySize); // $ hasInsufficientKeySize + } + + // public static void testVariable(int keySize, KeyGenerator kg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + // KeyGenerator keyGen = KeyGenerator.getInstance("AES"); + // keyGen.init(keySize); // $ hasInsufficientKeySize + + // // BAD: Key size is less than 2048 + // kg.init(64); // $ MISSING: hasInsufficientKeySize + // } + + // public static void testInt(int keySize, KeyGenerator kg) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + // } } diff --git a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql index 5dc7ecdf467..59b8a39ed0d 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql @@ -2,7 +2,6 @@ import java import TestUtilities.InlineExpectationsTest import semmle.code.java.security.InsufficientKeySizeQuery -//import DataFlow::PathGraph // Note: importing this messes up tests - adds edges and nodes to actual file... class InsufficientKeySizeTest extends InlineExpectationsTest { InsufficientKeySizeTest() { this = "InsufficientKeySize" } @@ -11,12 +10,8 @@ class InsufficientKeySizeTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasInsufficientKeySize" and exists(DataFlow::PathNode source, DataFlow::PathNode sink | - exists(KeySizeConfiguration config1 | config1.hasFlowPath(source, sink)) + exists(KeySizeConfiguration cfg | cfg.hasFlowPath(source, sink)) | - //or - // exists(AsymmetricNonECKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) or - // exists(AsymmetricECKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) or - // exists(SymmetricKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) sink.getNode().getLocation() = location and element = sink.getNode().toString() and value = ""