Address PR review: add Signature.getInstance sink, HMAC/PBKDF2 whitelist, fix test APIs

- Model Signature.getInstance() as CryptoAlgoSpec sink (previously only
  Signature constructor was modeled)
- Add HMAC-based algorithms (HMACSHA1/256/384/512, HmacSHA1/256/384/512)
  and PBKDF2 to the secure algorithm whitelist
- Fix XDH/X25519/X448 tests to use KeyAgreement.getInstance() instead of
  KeyPairGenerator.getInstance() to match their key agreement semantics
- Add test cases for SHA384withECDSA, HMACSHA*, and PBKDF2WithHmacSHA1
  from user-reported false positives
- Update change note to document all additions

Author: MarkLee131

java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md

@@ -1,4 +1,5 @@
 ---
 category: minorAnalysis
 ---
-* The `java/potentially-weak-cryptographic-algorithm` query no longer flags Elliptic Curve algorithms (`EC`, `ECDSA`, `ECDH`, `EdDSA`, `Ed25519`, `Ed448`, `XDH`, `X25519`, `X448`) as potentially insecure. These are modern, secure algorithms recommended by NIST SP 800-57 and other standards bodies. Previously, these algorithms were not included in the secure algorithm whitelist, causing false positives when using standard Java cryptographic APIs such as `KeyPairGenerator.getInstance("EC")`.
+* The `java/potentially-weak-cryptographic-algorithm` query no longer flags Elliptic Curve algorithms (`EC`, `ECDSA`, `ECDH`, `EdDSA`, `Ed25519`, `Ed448`, `XDH`, `X25519`, `X448`), HMAC-based algorithms (`HMACSHA1`, `HMACSHA256`, `HMACSHA384`, `HMACSHA512`), or PBKDF2 key derivation as potentially insecure. These are modern, secure algorithms recommended by NIST and other standards bodies. Previously, these algorithms were not included in the secure algorithm whitelist, causing false positives when using standard Java cryptographic APIs such as `KeyPairGenerator.getInstance("EC")` or `new SecretKeySpec(key, "HMACSHA256")`.
+* The `Signature.getInstance(...)` method is now modeled as a `CryptoAlgoSpec` sink, alongside the existing `Signature` constructor sink. This ensures that algorithm strings passed to `Signature.getInstance(...)` are also checked by the query.

java/ql/lib/semmle/code/java/security/Encryption.qll

@@ -263,7 +263,9 @@ string getASecureAlgorithmName() {
       // Elliptic Curve algorithms: EC (key generation), ECDSA (signatures), ECDH (key agreement),
       // EdDSA/Ed25519/Ed448 (Edwards-curve signatures), XDH/X25519/X448 (key agreement).
       // These are modern, secure algorithms recommended by NIST and other standards bodies.
-      "EC", "ECDSA", "ECDH", "EdDSA", "Ed25519", "Ed448", "XDH", "X25519", "X448"
+      "EC", "ECDSA", "ECDH", "EdDSA", "Ed25519", "Ed448", "XDH", "X25519", "X448",
+      // HMAC-based algorithms and key derivation functions.
+      "HMACSHA(1|256|384|512)", "HmacSHA(1|256|384|512)", "PBKDF2"
     ]
 }
 
@@ -370,9 +372,13 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec {
     exists(Constructor c | c.getAReference() = this |
       c.getDeclaringType().hasQualifiedName("java.security", "Signature")
     )
+    or
+    exists(Method m | m.getAReference() = this |
+      m.hasQualifiedName("java.security", "Signature", "getInstance")
+    )
   }
 
-  override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
+  override Expr getAlgoSpec() { result = this.(Call).getArgument(0) }
 }
 
 /** A call to the `getInstance` method declared in `java.security.KeyPairGenerator`. */

java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java

@@ -52,7 +52,7 @@ public void test() {
 			// GOOD: EC is a secure algorithm for key pair generation
 			keyPairGenerator = KeyPairGenerator.getInstance("EC");
 
-			// GOOD: ECDSA is a secure algorithm for digital signatures
+			// GOOD: ECDSA is a secure signature algorithm
 			Signature ecdsaSig = Signature.getInstance("ECDSA");
 
 			// GOOD: ECDH is a secure algorithm for key agreement
@@ -61,24 +61,33 @@ public void test() {
 			// GOOD: EdDSA is a secure algorithm (Edwards-curve Digital Signature Algorithm)
 			keyPairGenerator = KeyPairGenerator.getInstance("EdDSA");
 
-			// GOOD: Ed25519 is a secure algorithm
+			// GOOD: Ed25519 is a secure algorithm for key pair generation
 			keyPairGenerator = KeyPairGenerator.getInstance("Ed25519");
 
-			// GOOD: Ed448 is a secure algorithm
+			// GOOD: Ed448 is a secure algorithm for key pair generation
 			keyPairGenerator = KeyPairGenerator.getInstance("Ed448");
 
 			// GOOD: XDH is a secure algorithm for key agreement
-			keyPairGenerator = KeyPairGenerator.getInstance("XDH");
+			KeyAgreement xdhKa = KeyAgreement.getInstance("XDH");
 
 			// GOOD: X25519 is a secure algorithm for key agreement
-			keyPairGenerator = KeyPairGenerator.getInstance("X25519");
+			KeyAgreement x25519Ka = KeyAgreement.getInstance("X25519");
 
 			// GOOD: X448 is a secure algorithm for key agreement
-			keyPairGenerator = KeyPairGenerator.getInstance("X448");
+			KeyAgreement x448Ka = KeyAgreement.getInstance("X448");
 
 			// GOOD: SHA256withECDSA is a secure signature algorithm
 			Signature sha256Ecdsa = Signature.getInstance("SHA256withECDSA");
 
+			// GOOD: HMAC-based SecretKeySpec should not be flagged
+			new SecretKeySpec(null, "HMACSHA1");
+			new SecretKeySpec(null, "HMACSHA256");
+			new SecretKeySpec(null, "HMACSHA384");
+			new SecretKeySpec(null, "SHA384withECDSA");
+
+			// GOOD: PBKDF2 key derivation is a secure algorithm
+			SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
+
 		} catch (Exception e) {
 			// fail
 		}