Address reveiws - Add BAD example to doc, add doc example to tests and fix typo.

Author: joefarebrother

java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeys.qhelp

@@ -26,6 +26,9 @@ When generating a key for use with biometric authentication, ensure that the fol
 <example>
 <p>The following example demonstrates a key that is configured with secure paramaters:</p>
 <sample src="AndroidInsecureKeysGood.java"/>
+
+<p>In each of the following cases, a parameter is set insecurely:</p>
+<sample src="AndroidInsecureKeysBad.java"/>
 </example>
 
 <references>

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

java/ql/src/Security/CWE/CWE-287/AndroidInsecureKeysGood.java

@@ -7,7 +7,7 @@ private void generateSecretKey() {
         // GOOD: Secure parameters are used to generate a key for biometric authentication.
         .setUserAuthenticationRequired(true)
         .setInvalidatedByBiometricEnrollment(true)
-        .setUserAuthenticationParamters(0, KeyProperties.AUTH_BIOMETRIC_STRONG)
+        .setUserAuthenticationParameters(0, KeyProperties.AUTH_BIOMETRIC_STRONG)
         .build();
     KeyGenerator keyGenerator = KeyGenerator.getInstance(
             KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore");

java/ql/test/query-tests/security/CWE-287/InsecureKeys/Test1/Test.java

@@ -1,6 +1,7 @@
 import android.security.keystore.KeyGenParameterSpec;
 import android.hardware.biometrics.BiometricPrompt;
 import android.security.keystore.KeyProperties;
+import javax.crypto.KeyGenerator;
 
 class Test {
     void test() {
@@ -9,6 +10,23 @@ void test() {
         builder.setInvalidatedByBiometricEnrollment(false); // $insecure-key
         builder.setUserAuthenticationValidityDurationSeconds(30); // $insecure-key
     }
+
+    private void generateSecretKey() throws Exception {
+        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();
+    }
 }
 
 class Callback extends BiometricPrompt.AuthenticationCallback {