+
+
+
+Biometric authentication, such as fingerprint recognition, can be used alongside cryptographic keys stored in the Android KeyStore to protect sensitive parts of the application. However,
+when a key generated for this purpose has certain parameters set insecurely, an attacker with physical access can bypass the
+authentication check using application hooking tools such as Frida.
+
+
+
+
+
+When generating a key for use with biometric authentication, ensure that the following parameters of KeyGenParameterSpec.Builder are set:
+
+
+setUserAuthenticationRequired should be set to true; otherwise, the key can be used without user authentication.
+setInvalidatedByBiometricEnrollment should be set to true (the default); otherwise, an attacker can use the key by enrolling additional biometrics on the device.
+setUserAuthenticationValidityDurationSeconds, if used, should be set to -1; otherwise, non-biometric (less secure) credentials can be used to access the key. We recommend using setUserAuthenticationParameters instead to explicitly set both the timeout and the types of credentials that may be used.
+
+
+
+
+
+The following example demonstrates a key that is configured with secure paramaters:
+
+
+In each of the following cases, a parameter is set insecurely:
+
+
+
+
+
+WithSecure: How Secure is your Android Keystore Authentication?.
+
+
+Android Developers: KeyGenParameterSpec.Builder.
+
+
+
+
diff --git a/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeys.ql b/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeys.ql
new file mode 100644
index 00000000000..52aaf8d885d
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeys.ql
@@ -0,0 +1,18 @@
+/**
+ * @name Insecurely generated keys for local authentication
+ * @description Generation of keys with insecure parameters for local biometric authentication can allow attackers with physical access to bypass authentication checks.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 4.4
+ * @precision medium
+ * @id java/android/insecure-local-key-gen
+ * @tags security
+ * external/cwe/cwe-287
+ */
+
+import java
+import semmle.code.java.security.AndroidLocalAuthQuery
+
+from InsecureBiometricKeyParamCall call
+where usesLocalAuth()
+select call, "This key is not secure for biometric authentication."
diff --git a/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysBad.java b/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysBad.java
new file mode 100644
index 00000000000..deb14d7a5e7
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysBad.java
@@ -0,0 +1,47 @@
+private void generateSecretKey() {
+ KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(
+ "MySecretKey",
+ KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT)
+ .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+ .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+ // BAD: User authentication is not required to use this key.
+ .setUserAuthenticationRequired(false)
+ .build();
+ KeyGenerator keyGenerator = KeyGenerator.getInstance(
+ KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");
+ keyGenerator.init(keyGenParameterSpec);
+ keyGenerator.generateKey();
+}
+
+private void generateSecretKey() {
+ KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(
+ "MySecretKey",
+ KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT)
+ .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+ .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+ .setUserAuthenticationRequired(true)
+ // BAD: An attacker can access this key by enrolling additional biometrics.
+ .setInvalidatedByBiometricEnrollment(false)
+ .build();
+ KeyGenerator keyGenerator = KeyGenerator.getInstance(
+ KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");
+ keyGenerator.init(keyGenParameterSpec);
+ keyGenerator.generateKey();
+}
+
+private void generateSecretKey() {
+ KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(
+ "MySecretKey",
+ KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT)
+ .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+ .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+ .setUserAuthenticationRequired(true)
+ .setInvalidatedByBiometricEnrollment(true)
+ // BAD: This key can be accessed using non-biometric credentials.
+ .setUserAuthenticationValidityDurationSeconds(30)
+ .build();
+ KeyGenerator keyGenerator = KeyGenerator.getInstance(
+ KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");
+ keyGenerator.init(keyGenParameterSpec);
+ keyGenerator.generateKey();
+}
\ No newline at end of file
diff --git a/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysGood.java b/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysGood.java
new file mode 100644
index 00000000000..64f9c94f9ee
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysGood.java
@@ -0,0 +1,16 @@
+private void generateSecretKey() {
+ KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(
+ "MySecretKey",
+ KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT)
+ .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+ .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+ // GOOD: Secure parameters are used to generate a key for biometric authentication.
+ .setUserAuthenticationRequired(true)
+ .setInvalidatedByBiometricEnrollment(true)
+ .setUserAuthenticationParameters(0, KeyProperties.AUTH_BIOMETRIC_STRONG)
+ .build();
+ KeyGenerator keyGenerator = KeyGenerator.getInstance(
+ KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");
+ keyGenerator.init(keyGenParameterSpec);
+ keyGenerator.generateKey();
+}
\ No newline at end of file
diff --git a/java/ql/src/change-notes/2024-02-12-android-insecure-keys.md b/java/ql/src/change-notes/2024-02-12-android-insecure-keys.md
new file mode 100644
index 00000000000..1de07727796
--- /dev/null
+++ b/java/ql/src/change-notes/2024-02-12-android-insecure-keys.md
@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query `java/android/insecure-local-key-gen` for finding instances of keys generated for biometric authentication in an insecure way.
\ No newline at end of file
diff --git a/java/ql/test/query-tests/security/CWE-287/InsecureLocalAuth.expected b/java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/InsecureKeys.expected
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-287/InsecureLocalAuth.expected
rename to java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/InsecureKeys.expected
diff --git a/java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/InsecureKeys.ql b/java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/InsecureKeys.ql
new file mode 100644
index 00000000000..eec3b62dfc2
--- /dev/null
+++ b/java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/InsecureKeys.ql
@@ -0,0 +1,19 @@
+import java
+import TestUtilities.InlineExpectationsTest
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.security.AndroidLocalAuthQuery
+
+module InsecureKeysTest implements TestSig {
+ string getARelevantTag() { result = "insecure-key" }
+
+ predicate hasActualResult(Location location, string element, string tag, string value) {
+ tag = "insecure-key" and
+ exists(InsecureBiometricKeyParamCall call | usesLocalAuth() |
+ call.getLocation() = location and
+ element = call.toString() and
+ value = ""
+ )
+ }
+}
+
+import MakeTest