Merge pull request #20690 from bdrodes/weak_symmetric_cipher_bug

Crypto: Fix bug in weak symmetric cipher query

Author: nicolaswill

java/ql/src/experimental/quantum/Examples/WeakSymmetricCipher.qll

@@ -8,7 +8,8 @@ predicate isUnapprovedSymmetricCipher(Crypto::KeyOperationAlgorithmNode alg, str
   exists(KeyOpAlg::AlgorithmType algType |
     algType = alg.getAlgorithmType() and
     msg = "Use of unapproved symmetric cipher algorithm or API: " + algType.toString() + "." and
-    algType != KeyOpAlg::TSymmetricCipher(KeyOpAlg::AES())
+    algType != KeyOpAlg::TSymmetricCipher(KeyOpAlg::AES()) and
+    algType instanceof KeyOpAlg::TSymmetricCipher
   )
   // NOTE: an org could decide to disallow very specific algorithms as well, shown below
   // (

java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownSymmetricCipher/Test.java

@@ -3,6 +3,7 @@
 import javax.crypto.SecretKey;
 import java.security.Key;
 import java.security.NoSuchAlgorithmException;
+import javax.crypto.SecretKeyFactory;
 
 public class Test {
     public static void main(String[] args) throws Exception {
@@ -73,5 +74,8 @@ public static void main(String[] args) throws Exception {
         Cipher aesGcmCipher = Cipher.getInstance("AES/GCM/NoPadding");
         aesGcmCipher.init(Cipher.ENCRYPT_MODE, aesKey);
         byte[] aesGcmEncrypted = aesGcmCipher.doFinal(data);
+
+        // GOOD: not a symmetric cipher (Sanity check)
+        SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
     }
 }

java/ql/test/experimental/query-tests/quantum/examples/WeakOrUnknownSymmetricCipher/WeakSymmetricCipher.expected

@@ -1,14 +1,14 @@
-| Test.java:12:59:12:63 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
-| Test.java:14:47:14:51 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
-| Test.java:19:62:19:69 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
-| Test.java:21:50:21:57 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
-| Test.java:26:64:26:73 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Blowfish. |
-| Test.java:28:52:28:61 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Blowfish. |
-| Test.java:33:59:33:63 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC2. |
-| Test.java:35:47:35:51 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC2. |
-| Test.java:40:59:40:63 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC4. |
-| Test.java:42:47:42:51 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC4. |
-| Test.java:47:60:47:65 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: IDEA. |
-| Test.java:49:48:49:53 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: IDEA. |
-| Test.java:54:64:54:73 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Skipjack. |
-| Test.java:56:52:56:61 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Skipjack. |
+| Test.java:13:59:13:63 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
+| Test.java:15:47:15:51 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
+| Test.java:20:62:20:69 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
+| Test.java:22:50:22:57 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: DES. |
+| Test.java:27:64:27:73 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Blowfish. |
+| Test.java:29:52:29:61 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Blowfish. |
+| Test.java:34:59:34:63 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC2. |
+| Test.java:36:47:36:51 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC2. |
+| Test.java:41:59:41:63 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC4. |
+| Test.java:43:47:43:51 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: RC4. |
+| Test.java:48:60:48:65 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: IDEA. |
+| Test.java:50:48:50:53 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: IDEA. |
+| Test.java:55:64:55:73 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Skipjack. |
+| Test.java:57:52:57:61 | KeyOperationAlgorithm | Use of unapproved symmetric cipher algorithm or API: Skipjack. |