From c281625fe3fdb1a1449903f867d0394ae184709c Mon Sep 17 00:00:00 2001 From: Rich Cannings Date: Tue, 26 Mar 2019 15:57:54 -0700 Subject: Trigger a bug report upon a key store error Bug: 128318105 Test: Manual (I triggered an error) and ran tests/wifitests/runtests.sh Change-Id: I31fd18540895d9a8e50ea5e9aaf697092643df8c --- .../server/wifi/util/DataIntegrityChecker.java | 125 ++++++++------------- 1 file changed, 45 insertions(+), 80 deletions(-) (limited to 'service') diff --git a/service/java/com/android/server/wifi/util/DataIntegrityChecker.java b/service/java/com/android/server/wifi/util/DataIntegrityChecker.java index 7ea334cbb..38ecb07ec 100644 --- a/service/java/com/android/server/wifi/util/DataIntegrityChecker.java +++ b/service/java/com/android/server/wifi/util/DataIntegrityChecker.java @@ -17,21 +17,19 @@ package com.android.server.wifi.util; import android.annotation.NonNull; +import android.os.SystemProperties; import android.security.keystore.KeyGenParameterSpec; import android.security.keystore.KeyProperties; import android.text.TextUtils; import android.util.Log; -import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStreamReader; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import java.io.PrintStream; import java.security.DigestException; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; @@ -42,8 +40,6 @@ import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.UnrecoverableEntryException; import java.security.cert.CertificateException; -import java.util.ArrayList; -import java.util.List; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; @@ -67,6 +63,12 @@ public class DataIntegrityChecker { private static final int GCM_TAG_LENGTH = 128; private static final String KEY_STORE = "AndroidKeyStore"; + /** + * When KEYSTORE_FAILURE_RETURN_VALUE is true, all cryptographic operation failures will not + * enforce security and {@link #isOk(byte[])} always return true. + */ + private static final boolean KEYSTORE_FAILURE_RETURN_VALUE = true; + private final File mIntegrityFile; /** @@ -97,7 +99,7 @@ public class DataIntegrityChecker { public void update(byte[] data) { if (data == null || data.length < 1) { Log.e(TAG, "No data to update."); - writeErrorToFile(new Exception("No data to update")); + reportException(new Exception("No data to update")); return; } byte[] digest = getDigest(data); @@ -109,7 +111,7 @@ public class DataIntegrityChecker { if (integrityData != null) { writeIntegrityData(integrityData, mIntegrityFile); } else { - writeErrorToFile(new Exception("integrityData null upon update")); + reportException(new Exception("integrityData null upon update")); } } @@ -129,11 +131,11 @@ public class DataIntegrityChecker { */ public boolean isOk(byte[] data) throws DigestException { if (data == null || data.length < 1) { - return false; + return KEYSTORE_FAILURE_RETURN_VALUE; } byte[] currentDigest = getDigest(data); if (currentDigest == null || currentDigest.length < 1) { - return false; + return KEYSTORE_FAILURE_RETURN_VALUE; } EncryptedData encryptedData = readIntegrityData(mIntegrityFile); if (encryptedData == null) { @@ -141,7 +143,7 @@ public class DataIntegrityChecker { } byte[] storedDigest = decrypt(encryptedData); if (storedDigest == null) { - return false; + return KEYSTORE_FAILURE_RETURN_VALUE; } return constantTimeEquals(storedDigest, currentDigest); } @@ -151,7 +153,7 @@ public class DataIntegrityChecker { return MessageDigest.getInstance(DIGEST_ALGORITHM).digest(data); } catch (NoSuchAlgorithmException e) { Log.e(TAG, "getDigest could not find algorithm: " + DIGEST_ALGORITHM); - writeErrorToFile(e); + reportException(e); return null; } } @@ -165,23 +167,23 @@ public class DataIntegrityChecker { cipher.init(Cipher.ENCRYPT_MODE, secretKeyReference); encryptedData = new EncryptedData(cipher.doFinal(data), cipher.getIV(), keyAlias); } else { - writeErrorToFile(new Exception("secretKeyReference is null.")); + reportException(new Exception("secretKeyReference is null.")); } } catch (NoSuchAlgorithmException e) { Log.e(TAG, "encrypt could not find the algorithm: " + CIPHER_ALGORITHM); - writeErrorToFile(e); + reportException(e); } catch (NoSuchPaddingException e) { Log.e(TAG, "encrypt had a padding exception"); - writeErrorToFile(e); + reportException(e); } catch (InvalidKeyException e) { Log.e(TAG, "encrypt received an invalid key"); - writeErrorToFile(e); + reportException(e); } catch (BadPaddingException e) { Log.e(TAG, "encrypt had a padding problem"); - writeErrorToFile(e); + reportException(e); } catch (IllegalBlockSizeException e) { Log.e(TAG, "encrypt had an illegal block size"); - writeErrorToFile(e); + reportException(e); } return encryptedData; } @@ -198,22 +200,22 @@ public class DataIntegrityChecker { } } catch (NoSuchAlgorithmException e) { Log.e(TAG, "decrypt could not find cipher algorithm " + CIPHER_ALGORITHM); - writeErrorToFile(e); + reportException(e); } catch (NoSuchPaddingException e) { Log.e(TAG, "decrypt could not find padding algorithm"); - writeErrorToFile(e); + reportException(e); } catch (IllegalBlockSizeException e) { Log.e(TAG, "decrypt had a illegal block size"); - writeErrorToFile(e); + reportException(e); } catch (BadPaddingException e) { Log.e(TAG, "decrypt had bad padding"); - writeErrorToFile(e); + reportException(e); } catch (InvalidKeyException e) { Log.e(TAG, "decrypt had an invalid key"); - writeErrorToFile(e); + reportException(e); } catch (InvalidAlgorithmParameterException e) { Log.e(TAG, "decrypt had an invalid algorithm parameter"); - writeErrorToFile(e); + reportException(e); } return decryptedData; } @@ -229,7 +231,7 @@ public class DataIntegrityChecker { if (secretKeyEntry != null) { secretKey = secretKeyEntry.getSecretKey(); } else { - writeErrorToFile(new Exception("keystore contains the alias and the secret key " + reportException(new Exception("keystore contains the alias and the secret key " + "entry was null")); } } else { // The key does not exist in key store. Create the key and store it. @@ -247,25 +249,25 @@ public class DataIntegrityChecker { } } catch (CertificateException e) { Log.e(TAG, "getOrCreateSecretKey had a certificate exception."); - writeErrorToFile(e); + reportException(e); } catch (InvalidAlgorithmParameterException e) { Log.e(TAG, "getOrCreateSecretKey had an invalid algorithm parameter"); - writeErrorToFile(e); + reportException(e); } catch (IOException e) { Log.e(TAG, "getOrCreateSecretKey had an IO exception."); - writeErrorToFile(e); + reportException(e); } catch (KeyStoreException e) { Log.e(TAG, "getOrCreateSecretKey cannot find the keystore: " + KEY_STORE); - writeErrorToFile(e); + reportException(e); } catch (NoSuchAlgorithmException e) { Log.e(TAG, "getOrCreateSecretKey cannot find algorithm"); - writeErrorToFile(e); + reportException(e); } catch (NoSuchProviderException e) { Log.e(TAG, "getOrCreateSecretKey cannot find crypto provider"); - writeErrorToFile(e); + reportException(e); } catch (UnrecoverableEntryException e) { Log.e(TAG, "getOrCreateSecretKey had an unrecoverable entry exception."); - writeErrorToFile(e); + reportException(e); } return secretKey; } @@ -276,10 +278,10 @@ public class DataIntegrityChecker { oos.writeObject(encryptedData); } catch (FileNotFoundException e) { Log.e(TAG, "writeIntegrityData could not find the integrity file"); - writeErrorToFile(e); + reportException(e); } catch (IOException e) { Log.e(TAG, "writeIntegrityData had an IO exception"); - writeErrorToFile(e); + reportException(e); } } @@ -289,13 +291,13 @@ public class DataIntegrityChecker { return (EncryptedData) ois.readObject(); } catch (FileNotFoundException e) { Log.e(TAG, "readIntegrityData could not find integrity file"); - writeErrorToFile(e); + reportException(e); } catch (IOException e) { Log.e(TAG, "readIntegrityData had an IO exception"); - writeErrorToFile(e); + reportException(e); } catch (ClassNotFoundException e) { Log.e(TAG, "readIntegrityData could not find the class EncryptedData"); - writeErrorToFile(e); + reportException(e); } return null; } @@ -316,51 +318,14 @@ public class DataIntegrityChecker { return (differenceAccumulator == 0); } - /* TODO(b/128526030): Remove this error reporting code upon resolving the bug. */ - private static final boolean DEBUG = true; - private static final int LOGCAT_LINES = 1024; - private static final String ERROR_SUFFIX = ".error"; - - private void writeErrorToFile(Exception exception) { - if (DEBUG) { - try (PrintStream printStream = new PrintStream( - mIntegrityFile.getAbsolutePath() + ERROR_SUFFIX)) { - printStream.println("DataIntegrityChecker Error"); - printStream.println("Exception: " + exception); - printStream.println("Stacktrace:"); - exception.printStackTrace(printStream); - printStream.println("Logcat:"); - List logcatLines = getLogcat(); - for (String line : logcatLines) { - printStream.println(line); - } - } catch (FileNotFoundException e) { - Log.e(TAG, "Could not write error log."); - } - } - } - - private static List getLogcat() { - List lines = new ArrayList<>(LOGCAT_LINES); - try { - Process process = Runtime.getRuntime().exec( - String.format("logcat -t %d", LOGCAT_LINES)); - BufferedReader reader = new BufferedReader( - new InputStreamReader(process.getInputStream())); - String line; - while ((line = reader.readLine()) != null) { - lines.add(line); - } - reader = new BufferedReader( - new InputStreamReader(process.getErrorStream())); - while ((line = reader.readLine()) != null) { - lines.add(line); - } - process.waitFor(); - } catch (InterruptedException | IOException e) { - Log.e(TAG, "Exception while capturing logcat: " + e.toString()); + private static final boolean REQUEST_BUG_REPORT = true; + private void reportException(Exception exception) { + Log.wtf(TAG, "An irrecoverable key store error was encountered. " + + "KEYSTORE_FAILURE_RETURN_VALUE is set to " + KEYSTORE_FAILURE_RETURN_VALUE); + if (REQUEST_BUG_REPORT) { + SystemProperties.set("dumpstate.options", "bugreportwifi"); + SystemProperties.set("ctl.start", "bugreport"); } - return lines; } } -- cgit v1.2.3