From 65672df61f1ce5a1c458313daff4ec45360594d7 Mon Sep 17 00:00:00 2001 From: Lingfeng Guan Date: Thu, 11 Nov 2021 21:31:42 -0800 Subject: [PATCH 1/2] SignApk - use existing password mechanism when using keystore Summary: In my last diff, I've added mechanism to load private key from keystore. However, that mechanism will reveal password as part of the java param. This diff tries to use existing ANDROID_PW_FILE mechanism to support password for keystore private keys (through stdin) This diff also fix a null pointer bug in the existing password handling Test: This diff has been tested locally, and could sign correctly with our keystore with or without password Tags: Change-Id: Ie291ea8702a3b4d270b0f8689b023c3f290980a7 --- .../src/com/android/signapk/SignApk.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/signapk/src/com/android/signapk/SignApk.java b/tools/signapk/src/com/android/signapk/SignApk.java index 232e119985..e29a654951 100644 --- a/tools/signapk/src/com/android/signapk/SignApk.java +++ b/tools/signapk/src/com/android/signapk/SignApk.java @@ -204,13 +204,14 @@ class SignApk { * If a console doesn't exist, reads the password from stdin * If a console exists, reads the password from console and returns it as a string. * - * @param keyFile The file containing the private key. Used to prompt the user. + * @param keyFileName Name of the file containing the private key. Used to prompt the user. */ - private static String readPassword(File keyFile) { + private static String readPassword(String keyFileName) { Console console; char[] pwd; if ((console = System.console()) == null) { - System.out.print("Enter password for " + keyFile + " (password will not be hidden): "); + System.out.print( + "Enter password for " + keyFileName + " (password will not be hidden): "); System.out.flush(); BufferedReader stdin = new BufferedReader(new InputStreamReader(System.in)); try { @@ -219,7 +220,7 @@ class SignApk { return null; } } else { - if ((pwd = console.readPassword("[%s]", "Enter password for " + keyFile)) != null) { + if ((pwd = console.readPassword("[%s]", "Enter password for " + keyFileName)) != null) { return String.valueOf(pwd); } else { return null; @@ -246,11 +247,11 @@ class SignApk { return null; } - char[] password = readPassword(keyFile).toCharArray(); + final String password = readPassword(keyFile.getPath()); SecretKeyFactory skFactory = SecretKeyFactory.getInstance(epkInfo.getAlgName()); - Key key = skFactory.generateSecret(new PBEKeySpec(password)); - + Key key = skFactory.generateSecret( + new PBEKeySpec(password != null ? password.toCharArray() : null)); Cipher cipher = Cipher.getInstance(epkInfo.getAlgName()); cipher.init(Cipher.DECRYPT_MODE, key, epkInfo.getAlgParameters()); @@ -305,10 +306,11 @@ class SignApk { /** Get a PKCS#11 private key from keyStore */ private static PrivateKey loadPrivateKeyFromKeyStore( - final KeyStore keyStore, final String keyName, final String password) + final KeyStore keyStore, final String keyName) throws CertificateException, KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException, UnrecoverableEntryException { - final Key key = keyStore.getKey(keyName, password == null ? null : password.toCharArray()); + final String password = readPassword(keyName); + final Key key = keyStore.getKey(keyName, password != null ? password.toCharArray() : null); final PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) keyStore.getEntry(keyName, null); if (privateKeyEntry == null) { throw new Error( @@ -1201,10 +1203,8 @@ class SignApk { if (keyStore == null) { privateKey[i] = readPrivateKey(new File(args[argNum])); } else { - String[] splits = args[argNum].split(":", 2); - final String keyAlias = splits[0]; - final String password = splits.length > 1 ? splits[1] : null; - privateKey[i] = loadPrivateKeyFromKeyStore(keyStore, keyAlias, password); + final String keyAlias = args[argNum]; + privateKey[i] = loadPrivateKeyFromKeyStore(keyStore, keyAlias); } } inputJar = new JarFile(new File(inputFilename), false); // Don't verify. From c5498416177d8d2e34b799acf8140a9bf5eb434a Mon Sep 17 00:00:00 2001 From: Lingfeng Guan Date: Tue, 23 Nov 2021 15:44:36 -0800 Subject: [PATCH 2/2] SignApk - change signature of readPassword to use char[] instead Summary: Use char[] is more conventional for password handling. See this question for reference. https://stackoverflow.com/questions/8881291 This is to address a concern raised in https://android-review.googlesource.com/c/platform/build/+/1890395/2 Test: mma Change-Id: I8d60efc557d7641c057e49a2aa4613fea67cd1e6 --- .../src/com/android/signapk/SignApk.java | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/tools/signapk/src/com/android/signapk/SignApk.java b/tools/signapk/src/com/android/signapk/SignApk.java index e29a654951..c127dbe9de 100644 --- a/tools/signapk/src/com/android/signapk/SignApk.java +++ b/tools/signapk/src/com/android/signapk/SignApk.java @@ -206,25 +206,21 @@ class SignApk { * * @param keyFileName Name of the file containing the private key. Used to prompt the user. */ - private static String readPassword(String keyFileName) { + private static char[] readPassword(String keyFileName) { Console console; - char[] pwd; if ((console = System.console()) == null) { System.out.print( "Enter password for " + keyFileName + " (password will not be hidden): "); System.out.flush(); BufferedReader stdin = new BufferedReader(new InputStreamReader(System.in)); try { - return stdin.readLine(); + String result = stdin.readLine(); + return result == null ? null : result.toCharArray(); } catch (IOException ex) { return null; } } else { - if ((pwd = console.readPassword("[%s]", "Enter password for " + keyFileName)) != null) { - return String.valueOf(pwd); - } else { - return null; - } + return console.readPassword("[%s]", "Enter password for " + keyFileName); } } @@ -247,11 +243,8 @@ class SignApk { return null; } - final String password = readPassword(keyFile.getPath()); - SecretKeyFactory skFactory = SecretKeyFactory.getInstance(epkInfo.getAlgName()); - Key key = skFactory.generateSecret( - new PBEKeySpec(password != null ? password.toCharArray() : null)); + Key key = skFactory.generateSecret(new PBEKeySpec(readPassword(keyFile.getPath()))); Cipher cipher = Cipher.getInstance(epkInfo.getAlgName()); cipher.init(Cipher.DECRYPT_MODE, key, epkInfo.getAlgParameters()); @@ -309,8 +302,7 @@ class SignApk { final KeyStore keyStore, final String keyName) throws CertificateException, KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException, UnrecoverableEntryException { - final String password = readPassword(keyName); - final Key key = keyStore.getKey(keyName, password != null ? password.toCharArray() : null); + final Key key = keyStore.getKey(keyName, readPassword(keyName)); final PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) keyStore.getEntry(keyName, null); if (privateKeyEntry == null) { throw new Error(