Merge "Fix ADB key file reading" into tm-dev am: 9523639bb0

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/19049555

Change-Id: Iccea075182ba10344cabe1cc9d9b94c28569711c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

core/java/android/debug/AdbManagerInternal.java

@@ -54,6 +54,12 @@ public abstract class AdbManagerInternal {
      */
     public abstract File getAdbTempKeysFile();
 
+    /**
+     * Notify the AdbManager that the key files have changed and any in-memory state should be
+     * reloaded.
+     */
+    public abstract void notifyKeyFilesUpdated();
+
     /**
      * Starts adbd for a transport.
      */

services/core/java/com/android/server/adb/AdbDebuggingManager.java

@@ -19,7 +19,7 @@ package com.android.server.adb;
 import static com.android.internal.util.dump.DumpUtils.writeStringIfNotNull;
 
 import android.annotation.NonNull;
-import android.annotation.TestApi;
+import android.annotation.Nullable;
 import android.app.ActivityManager;
 import android.app.Notification;
 import android.app.NotificationChannel;
@@ -102,11 +102,26 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
- * Provides communication to the Android Debug Bridge daemon to allow, deny, or clear public keysi
+ * Provides communication to the Android Debug Bridge daemon to allow, deny, or clear public keys
  * that are authorized to connect to the ADB service itself.
+ *
+ * <p>The AdbDebuggingManager controls two files:
+ * <ol>
+ *     <li>adb_keys
+ *     <li>adb_temp_keys.xml
+ * </ol>
+ *
+ * <p>The ADB Daemon (adbd) reads <em>only</em> the adb_keys file for authorization. Public keys
+ * from registered hosts are stored in adb_keys, one entry per line.
+ *
+ * <p>AdbDebuggingManager also keeps adb_temp_keys.xml, which is used for two things
+ * <ol>
+ *     <li>Removing unused keys from the adb_keys file
+ *     <li>Managing authorized WiFi access points for ADB over WiFi
+ * </ol>
  */
 public class AdbDebuggingManager {
-    private static final String TAG = "AdbDebuggingManager";
+    private static final String TAG = AdbDebuggingManager.class.getSimpleName();
     private static final boolean DEBUG = false;
     private static final boolean MDNS_DEBUG = false;
 
@@ -118,18 +133,20 @@ public class AdbDebuggingManager {
     // as a subsequent connection occurs within the allowed duration.
     private static final String ADB_TEMP_KEYS_FILE = "adb_temp_keys.xml";
     private static final int BUFFER_SIZE = 65536;
+    private static final Ticker SYSTEM_TICKER = () -> System.currentTimeMillis();
 
     private final Context mContext;
     private final ContentResolver mContentResolver;
-    private final Handler mHandler;
+    @VisibleForTesting final AdbDebuggingHandler mHandler;
-    private AdbDebuggingThread mThread;
+    @Nullable private AdbDebuggingThread mThread;
     private boolean mAdbUsbEnabled = false;
     private boolean mAdbWifiEnabled = false;
     private String mFingerprints;
     // A key can be used more than once (e.g. USB, wifi), so need to keep a refcount
-    private final Map<String, Integer> mConnectedKeys;
+    private final Map<String, Integer> mConnectedKeys = new HashMap<>();
-    private String mConfirmComponent;
+    private final String mConfirmComponent;
-    private final File mTestUserKeyFile;
+    @Nullable private final File mUserKeyFile;
+    @Nullable private final File mTempKeysFile;
 
     private static final String WIFI_PERSISTENT_CONFIG_PROPERTY =
             "persist.adb.tls_server.enable";
@@ -138,37 +155,44 @@ public class AdbDebuggingManager {
     private static final int PAIRING_CODE_LENGTH = 6;
     private PairingThread mPairingThread = null;
     // A list of keys connected via wifi
-    private final Set<String> mWifiConnectedKeys;
+    private final Set<String> mWifiConnectedKeys = new HashSet<>();
     // The current info of the adbwifi connection.
-    private AdbConnectionInfo mAdbConnectionInfo;
+    private AdbConnectionInfo mAdbConnectionInfo = new AdbConnectionInfo();
     // Polls for a tls port property when adb wifi is enabled
     private AdbConnectionPortPoller mConnectionPortPoller;
     private final PortListenerImpl mPortListener = new PortListenerImpl();
+    private final Ticker mTicker;
 
     public AdbDebuggingManager(Context context) {
-        mHandler = new AdbDebuggingHandler(FgThread.get().getLooper());
+        this(
-        mContext = context;
+                context,
-        mContentResolver = mContext.getContentResolver();
+                /* confirmComponent= */ null,
-        mTestUserKeyFile = null;
+                getAdbFile(ADB_KEYS_FILE),
-        mConnectedKeys = new HashMap<String, Integer>();
+                getAdbFile(ADB_TEMP_KEYS_FILE),
-        mWifiConnectedKeys = new HashSet<String>();
+                /* adbDebuggingThread= */ null,
-        mAdbConnectionInfo = new AdbConnectionInfo();
+                SYSTEM_TICKER);
     }
 
     /**
      * Constructor that accepts the component to be invoked to confirm if the user wants to allow
      * an adb connection from the key.
      */
-    @TestApi
+    @VisibleForTesting
-    protected AdbDebuggingManager(Context context, String confirmComponent, File testUserKeyFile) {
+    AdbDebuggingManager(
-        mHandler = new AdbDebuggingHandler(FgThread.get().getLooper());
+            Context context,
+            String confirmComponent,
+            File testUserKeyFile,
+            File tempKeysFile,
+            AdbDebuggingThread adbDebuggingThread,
+            Ticker ticker) {
         mContext = context;
         mContentResolver = mContext.getContentResolver();
         mConfirmComponent = confirmComponent;
-        mTestUserKeyFile = testUserKeyFile;
+        mUserKeyFile = testUserKeyFile;
-        mConnectedKeys = new HashMap<String, Integer>();
+        mTempKeysFile = tempKeysFile;
-        mWifiConnectedKeys = new HashSet<String>();
+        mThread = adbDebuggingThread;
-        mAdbConnectionInfo = new AdbConnectionInfo();
+        mTicker = ticker;
+        mHandler = new AdbDebuggingHandler(FgThread.get().getLooper(), mThread);
     }
 
     static void sendBroadcastWithDebugPermission(@NonNull Context context, @NonNull Intent intent,
@@ -189,8 +213,7 @@ public class AdbDebuggingManager {
         // consisting of only letters, digits, and hyphens, must begin and end
         // with a letter or digit, must not contain consecutive hyphens, and
         // must contain at least one letter.
-        @VisibleForTesting
+        @VisibleForTesting static final String SERVICE_PROTOCOL = "adb-tls-pairing";
-        static final String SERVICE_PROTOCOL = "adb-tls-pairing";
         private final String mServiceType = String.format("_%s._tcp.", SERVICE_PROTOCOL);
         private int mPort;
 
@@ -352,16 +375,24 @@ public class AdbDebuggingManager {
         }
     }
 
-    class AdbDebuggingThread extends Thread {
+    @VisibleForTesting
+    static class AdbDebuggingThread extends Thread {
         private boolean mStopped;
         private LocalSocket mSocket;
         private OutputStream mOutputStream;
         private InputStream mInputStream;
+        private Handler mHandler;
 
+        @VisibleForTesting
         AdbDebuggingThread() {
             super(TAG);
         }
 
+        @VisibleForTesting
+        void setHandler(Handler handler) {
+            mHandler = handler;
+        }
+
         @Override
         public void run() {
             if (DEBUG) Slog.d(TAG, "Entering thread");
@@ -536,7 +567,7 @@ public class AdbDebuggingManager {
         }
     }
 
-    class AdbConnectionInfo {
+    private static class AdbConnectionInfo {
         private String mBssid;
         private String mSsid;
         private int mPort;
@@ -743,11 +774,14 @@ public class AdbDebuggingManager {
         // Notification when adbd socket is disconnected.
         static final int MSG_ADBD_SOCKET_DISCONNECTED = 27;
 
+        // === Messages from other parts of the system
+        private static final int MESSAGE_KEY_FILES_UPDATED = 28;
+
         // === Messages we can send to adbd ===========
         static final String MSG_DISCONNECT_DEVICE = "DD";
         static final String MSG_DISABLE_ADBDWIFI = "DA";
 
-        private AdbKeyStore mAdbKeyStore;
+        @Nullable @VisibleForTesting AdbKeyStore mAdbKeyStore;
 
         // Usb, Wi-Fi transports can be enabled together or separately, so don't break the framework
         // connection unless all transport types are disconnected.
@@ -762,19 +796,19 @@ public class AdbDebuggingManager {
             }
         };
 
-        AdbDebuggingHandler(Looper looper) {
+        /** Constructor that accepts the AdbDebuggingThread to which responses should be sent. */
-            super(looper);
+        @VisibleForTesting
-        }
+        AdbDebuggingHandler(Looper looper, AdbDebuggingThread thread) {
-
-        /**
-         * Constructor that accepts the AdbDebuggingThread to which responses should be sent
-         * and the AdbKeyStore to be used to store the temporary grants.
-         */
-        @TestApi
-        AdbDebuggingHandler(Looper looper, AdbDebuggingThread thread, AdbKeyStore adbKeyStore) {
             super(looper);
             mThread = thread;
-            mAdbKeyStore = adbKeyStore;
+        }
+
+        /** Initialize the AdbKeyStore so tests can grab mAdbKeyStore immediately. */
+        @VisibleForTesting
+        void initKeyStore() {
+            if (mAdbKeyStore == null) {
+                mAdbKeyStore = new AdbKeyStore();
+            }
         }
 
         // Show when at least one device is connected.
@@ -805,6 +839,7 @@ public class AdbDebuggingManager {
 
             registerForAuthTimeChanges();
             mThread = new AdbDebuggingThread();
+            mThread.setHandler(mHandler);
             mThread.start();
 
             mAdbKeyStore.updateKeyStore();
@@ -825,8 +860,7 @@ public class AdbDebuggingManager {
 
             if (!mConnectedKeys.isEmpty()) {
                 for (Map.Entry<String, Integer> entry : mConnectedKeys.entrySet()) {
-                    mAdbKeyStore.setLastConnectionTime(entry.getKey(),
+                    mAdbKeyStore.setLastConnectionTime(entry.getKey(), mTicker.currentTimeMillis());
-                            System.currentTimeMillis());
                 }
                 sendPersistKeyStoreMessage();
                 mConnectedKeys.clear();
@@ -836,9 +870,7 @@ public class AdbDebuggingManager {
         }
 
         public void handleMessage(Message msg) {
-            if (mAdbKeyStore == null) {
+            initKeyStore();
-                mAdbKeyStore = new AdbKeyStore();
-            }
 
             switch (msg.what) {
                 case MESSAGE_ADB_ENABLED:
@@ -873,7 +905,7 @@ public class AdbDebuggingManager {
                             if (!mConnectedKeys.containsKey(key)) {
                                 mConnectedKeys.put(key, 1);
                             }
-                            mAdbKeyStore.setLastConnectionTime(key, System.currentTimeMillis());
+                            mAdbKeyStore.setLastConnectionTime(key, mTicker.currentTimeMillis());
                             sendPersistKeyStoreMessage();
                             scheduleJobToUpdateAdbKeyStore();
                         }
@@ -920,9 +952,7 @@ public class AdbDebuggingManager {
                     mConnectedKeys.clear();
                     // If the key store has not yet been instantiated then do so now; this avoids
                     // the unnecessary creation of the key store when adb is not enabled.
-                    if (mAdbKeyStore == null) {
+                    initKeyStore();
-                        mAdbKeyStore = new AdbKeyStore();
-                    }
                     mWifiConnectedKeys.clear();
                     mAdbKeyStore.deleteKeyStore();
                     cancelJobToUpdateAdbKeyStore();
@@ -937,7 +967,8 @@ public class AdbDebuggingManager {
                             alwaysAllow = true;
                             int refcount = mConnectedKeys.get(key) - 1;
                             if (refcount == 0) {
-                                mAdbKeyStore.setLastConnectionTime(key, System.currentTimeMillis());
+                                mAdbKeyStore.setLastConnectionTime(
+                                        key, mTicker.currentTimeMillis());
                                 sendPersistKeyStoreMessage();
                                 scheduleJobToUpdateAdbKeyStore();
                                 mConnectedKeys.remove(key);
@@ -963,7 +994,7 @@ public class AdbDebuggingManager {
                     if (!mConnectedKeys.isEmpty()) {
                         for (Map.Entry<String, Integer> entry : mConnectedKeys.entrySet()) {
                             mAdbKeyStore.setLastConnectionTime(entry.getKey(),
-                                    System.currentTimeMillis());
+                                    mTicker.currentTimeMillis());
                         }
                         sendPersistKeyStoreMessage();
                         scheduleJobToUpdateAdbKeyStore();
@@ -984,7 +1015,7 @@ public class AdbDebuggingManager {
                         } else {
                             mConnectedKeys.put(key, mConnectedKeys.get(key) + 1);
                         }
-                        mAdbKeyStore.setLastConnectionTime(key, System.currentTimeMillis());
+                        mAdbKeyStore.setLastConnectionTime(key, mTicker.currentTimeMillis());
                         sendPersistKeyStoreMessage();
                         scheduleJobToUpdateAdbKeyStore();
                         logAdbConnectionChanged(key, AdbProtoEnums.AUTOMATICALLY_ALLOWED, true);
@@ -1206,6 +1237,10 @@ public class AdbDebuggingManager {
                     }
                     break;
                 }
+                case MESSAGE_KEY_FILES_UPDATED: {
+                    mAdbKeyStore.reloadKeyMap();
+                    break;
+                }
             }
         }
 
@@ -1377,8 +1412,7 @@ public class AdbDebuggingManager {
                 AdbDebuggingManager.sendBroadcastWithDebugPermission(mContext, intent,
                         UserHandle.ALL);
                 // Add the key into the keystore
-                mAdbKeyStore.setLastConnectionTime(publicKey,
+                mAdbKeyStore.setLastConnectionTime(publicKey, mTicker.currentTimeMillis());
-                        System.currentTimeMillis());
                 sendPersistKeyStoreMessage();
                 scheduleJobToUpdateAdbKeyStore();
             }
@@ -1449,19 +1483,13 @@ public class AdbDebuggingManager {
         extras.add(new AbstractMap.SimpleEntry<String, String>("ssid", ssid));
         extras.add(new AbstractMap.SimpleEntry<String, String>("bssid", bssid));
         int currentUserId = ActivityManager.getCurrentUser();
-        UserInfo userInfo = UserManager.get(mContext).getUserInfo(currentUserId);
+        String componentString =
-        String componentString;
+                Resources.getSystem().getString(
-        if (userInfo.isAdmin()) {
+                        R.string.config_customAdbWifiNetworkConfirmationComponent);
-            componentString = Resources.getSystem().getString(
-                    com.android.internal.R.string.config_customAdbWifiNetworkConfirmationComponent);
-        } else {
-            componentString = Resources.getSystem().getString(
-                    com.android.internal.R.string.config_customAdbWifiNetworkConfirmationComponent);
-        }
         ComponentName componentName = ComponentName.unflattenFromString(componentString);
+        UserInfo userInfo = UserManager.get(mContext).getUserInfo(currentUserId);
         if (startConfirmationActivity(componentName, userInfo.getUserHandle(), extras)
-                || startConfirmationService(componentName, userInfo.getUserHandle(),
+                || startConfirmationService(componentName, userInfo.getUserHandle(), extras)) {
-                        extras)) {
             return;
         }
         Slog.e(TAG, "Unable to start customAdbWifiNetworkConfirmation[SecondaryUser]Component "
@@ -1543,7 +1571,7 @@ public class AdbDebuggingManager {
     /**
      * Returns a new File with the specified name in the adb directory.
      */
-    private File getAdbFile(String fileName) {
+    private static File getAdbFile(String fileName) {
         File dataDir = Environment.getDataDirectory();
         File adbDir = new File(dataDir, ADB_DIRECTORY);
 
@@ -1556,66 +1584,38 @@ public class AdbDebuggingManager {
     }
 
     File getAdbTempKeysFile() {
-        return getAdbFile(ADB_TEMP_KEYS_FILE);
+        return mTempKeysFile;
     }
 
     File getUserKeyFile() {
-        return mTestUserKeyFile == null ? getAdbFile(ADB_KEYS_FILE) : mTestUserKeyFile;
+        return mUserKeyFile;
-    }
-
-    private void writeKey(String key) {
-        try {
-            File keyFile = getUserKeyFile();
-
-            if (keyFile == null) {
-                return;
-            }
-
-            FileOutputStream fo = new FileOutputStream(keyFile, true);
-            fo.write(key.getBytes());
-            fo.write('\n');
-            fo.close();
-
-            FileUtils.setPermissions(keyFile.toString(),
-                    FileUtils.S_IRUSR | FileUtils.S_IWUSR | FileUtils.S_IRGRP, -1, -1);
-        } catch (IOException ex) {
-            Slog.e(TAG, "Error writing key:" + ex);
-        }
     }
 
     private void writeKeys(Iterable<String> keys) {
-        AtomicFile atomicKeyFile = null;
+        if (mUserKeyFile == null) {
+            return;
+        }
+
+        AtomicFile atomicKeyFile = new AtomicFile(mUserKeyFile);
+        // Note: Do not use a try-with-resources with the FileOutputStream, because AtomicFile
+        // requires that it's cleaned up with AtomicFile.failWrite();
         FileOutputStream fo = null;
         try {
-            File keyFile = getUserKeyFile();
-
-            if (keyFile == null) {
-                return;
-            }
-
-            atomicKeyFile = new AtomicFile(keyFile);
             fo = atomicKeyFile.startWrite();
             for (String key : keys) {
                 fo.write(key.getBytes());
                 fo.write('\n');
             }
             atomicKeyFile.finishWrite(fo);
-
-            FileUtils.setPermissions(keyFile.toString(),
-                    FileUtils.S_IRUSR | FileUtils.S_IWUSR | FileUtils.S_IRGRP, -1, -1);
         } catch (IOException ex) {
             Slog.e(TAG, "Error writing keys: " + ex);
-            if (atomicKeyFile != null) {
+            atomicKeyFile.failWrite(fo);
-                atomicKeyFile.failWrite(fo);
+            return;
-            }
         }
-    }
 
-    private void deleteKeyFile() {
+        FileUtils.setPermissions(
-        File keyFile = getUserKeyFile();
+                mUserKeyFile.toString(),
-        if (keyFile != null) {
+                FileUtils.S_IRUSR | FileUtils.S_IWUSR | FileUtils.S_IRGRP, -1, -1);
-            keyFile.delete();
-        }
     }
 
     /**
@@ -1744,6 +1744,13 @@ public class AdbDebuggingManager {
         return mAdbWifiEnabled;
     }
 
+    /**
+     * Notify that they key files were updated so the AdbKeyManager reloads the keys.
+     */
+    public void notifyKeyFilesUpdated() {
+        mHandler.sendEmptyMessage(AdbDebuggingHandler.MESSAGE_KEY_FILES_UPDATED);
+    }
+
     /**
      * Sends a message to the handler to persist the keystore.
      */
@@ -1778,7 +1785,7 @@ public class AdbDebuggingManager {
 
         try {
             dump.write("keystore", AdbDebuggingManagerProto.KEYSTORE,
-                    FileUtils.readTextFile(getAdbTempKeysFile(), 0, null));
+                    FileUtils.readTextFile(mTempKeysFile, 0, null));
         } catch (IOException e) {
             Slog.i(TAG, "Cannot read keystore: ", e);
         }
@@ -1792,12 +1799,12 @@ public class AdbDebuggingManager {
      * ADB_ALLOWED_CONNECTION_TIME setting.
      */
     class AdbKeyStore {
-        private Map<String, Long> mKeyMap;
-        private Set<String> mSystemKeys;
-        private File mKeyFile;
         private AtomicFile mAtomicKeyFile;
 
-        private List<String> mTrustedNetworks;
+        private final Set<String> mSystemKeys;
+        private final Map<String, Long> mKeyMap = new HashMap<>();
+        private final List<String> mTrustedNetworks = new ArrayList<>();
+
         private static final int KEYSTORE_VERSION = 1;
         private static final int MAX_SUPPORTED_KEYSTORE_VERSION = 1;
         private static final String XML_KEYSTORE_START_TAG = "keyStore";
@@ -1819,26 +1826,22 @@ public class AdbDebuggingManager {
         public static final long NO_PREVIOUS_CONNECTION = 0;
 
         /**
-         * Constructor that uses the default location for the persistent adb keystore.
+         * Create an AdbKeyStore instance.
+         *
+         * <p>Upon creation, we parse {@link #mTempKeysFile} to determine authorized WiFi APs and
+         * retrieve the map of stored ADB keys and their last connected times. After that, we read
+         * the {@link #mUserKeyFile}, and any keys that exist in that file that do not exist in the
+         * map are added to the map (for backwards compatibility).
          */
         AdbKeyStore() {
-            init();
-        }
-
-        /**
-         * Constructor that uses the specified file as the location for the persistent adb keystore.
-         */
-        AdbKeyStore(File keyFile) {
-            mKeyFile = keyFile;
-            init();
-        }
-
-        private void init() {
             initKeyFile();
-            mKeyMap = getKeyMap();
+            readTempKeysFile();
-            mTrustedNetworks = getTrustedNetworks();
             mSystemKeys = getSystemKeysFromFile(SYSTEM_KEY_FILE);
-            addUserKeysToKeyStore();
+            addExistingUserKeysToKeyStore();
+        }
+
+        public void reloadKeyMap() {
+            readTempKeysFile();
         }
 
         public void addTrustedNetwork(String bssid) {
@@ -1877,7 +1880,6 @@ public class AdbDebuggingManager {
         public void removeKey(String key) {
             if (mKeyMap.containsKey(key)) {
                 mKeyMap.remove(key);
-                writeKeys(mKeyMap.keySet());
                 sendPersistKeyStoreMessage();
             }
         }
@@ -1886,12 +1888,9 @@ public class AdbDebuggingManager {
          * Initializes the key file that will be used to persist the adb grants.
          */
         private void initKeyFile() {
-            if (mKeyFile == null) {
+            // mTempKeysFile can be null if the adb file cannot be obtained
-                mKeyFile = getAdbTempKeysFile();
+            if (mTempKeysFile != null) {
-            }
+                mAtomicKeyFile = new AtomicFile(mTempKeysFile);
-            // getAdbTempKeysFile can return null if the adb file cannot be obtained
-            if (mKeyFile != null) {
-                mAtomicKeyFile = new AtomicFile(mKeyFile);
             }
         }
 
@@ -1932,201 +1931,108 @@ public class AdbDebuggingManager {
         }
 
         /**
-         * Returns the key map with the keys and last connection times from the key file.
+         * Update the key map and the trusted networks list with values parsed from the temp keys
+         * file.
          */
-        private Map<String, Long> getKeyMap() {
+        private void readTempKeysFile() {
-            Map<String, Long> keyMap = new HashMap<String, Long>();
+            mKeyMap.clear();
-            // if the AtomicFile could not be instantiated before attempt again; if it still fails
+            mTrustedNetworks.clear();
-            // return an empty key map.
             if (mAtomicKeyFile == null) {
                 initKeyFile();
                 if (mAtomicKeyFile == null) {
-                    Slog.e(TAG, "Unable to obtain the key file, " + mKeyFile + ", for reading");
+                    Slog.e(
-                    return keyMap;
+                            TAG,
+                            "Unable to obtain the key file, " + mTempKeysFile + ", for reading");
+                    return;
                 }
             }
             if (!mAtomicKeyFile.exists()) {
-                return keyMap;
+                return;
             }
             try (FileInputStream keyStream = mAtomicKeyFile.openRead()) {
-                TypedXmlPullParser parser = Xml.resolvePullParser(keyStream);
+                TypedXmlPullParser parser;
-                // Check for supported keystore version.
+                try {
-                XmlUtils.beginDocument(parser, XML_KEYSTORE_START_TAG);
+                    parser = Xml.resolvePullParser(keyStream);
-                if (parser.next() != XmlPullParser.END_DOCUMENT) {
+                    XmlUtils.beginDocument(parser, XML_KEYSTORE_START_TAG);
-                    String tagName = parser.getName();
+
-                    if (tagName == null || !XML_KEYSTORE_START_TAG.equals(tagName)) {
-                        Slog.e(TAG, "Expected " + XML_KEYSTORE_START_TAG + ", but got tag="
-                                + tagName);
-                        return keyMap;
-                    }
                     int keystoreVersion = parser.getAttributeInt(null, XML_ATTRIBUTE_VERSION);
                     if (keystoreVersion > MAX_SUPPORTED_KEYSTORE_VERSION) {
                         Slog.e(TAG, "Keystore version=" + keystoreVersion
                                 + " not supported (max_supported="
                                 + MAX_SUPPORTED_KEYSTORE_VERSION + ")");
-                        return keyMap;
+                        return;
                     }
+                } catch (XmlPullParserException e) {
+                    // This could be because the XML document doesn't start with
+                    // XML_KEYSTORE_START_TAG. Try again, instead just starting the document with
+                    // the adbKey tag (the old format).
+                    parser = Xml.resolvePullParser(keyStream);
                 }
-                while (parser.next() != XmlPullParser.END_DOCUMENT) {
+                readKeyStoreContents(parser);
-                    String tagName = parser.getName();
-                    if (tagName == null) {
-                        break;
-                    } else if (!tagName.equals(XML_TAG_ADB_KEY)) {
-                        XmlUtils.skipCurrentTag(parser);
-                        continue;
-                    }
-                    String key = parser.getAttributeValue(null, XML_ATTRIBUTE_KEY);
-                    long connectionTime;
-                    try {
-                        connectionTime = parser.getAttributeLong(null,
-                                XML_ATTRIBUTE_LAST_CONNECTION);
-                    } catch (XmlPullParserException e) {
-                        Slog.e(TAG,
-                                "Caught a NumberFormatException parsing the last connection time: "
-                                        + e);
-                        XmlUtils.skipCurrentTag(parser);
-                        continue;
-                    }
-                    keyMap.put(key, connectionTime);
-                }
             } catch (IOException e) {
                 Slog.e(TAG, "Caught an IOException parsing the XML key file: ", e);
             } catch (XmlPullParserException e) {
-                Slog.w(TAG, "Caught XmlPullParserException parsing the XML key file: ", e);
+                Slog.e(TAG, "Caught XmlPullParserException parsing the XML key file: ", e);
-                // The file could be written in a format prior to introducing keystore tag.
-                return getKeyMapBeforeKeystoreVersion();
             }
-            return keyMap;
         }
 
-
+        private void readKeyStoreContents(TypedXmlPullParser parser)
-        /**
+                throws XmlPullParserException, IOException {
-         * Returns the key map with the keys and last connection times from the key file.
+            // This parser is very forgiving. For backwards-compatibility, we simply iterate through
-         * This implementation was prior to adding the XML_KEYSTORE_START_TAG.
+            // all the tags in the file, skipping over anything that's not an <adbKey> tag or a
-         */
+            // <wifiAP> tag. Invalid tags (such as ones that don't have a valid "lastConnection"
-        private Map<String, Long> getKeyMapBeforeKeystoreVersion() {
+            // attribute) are simply ignored.
-            Map<String, Long> keyMap = new HashMap<String, Long>();
+            while ((parser.next()) != XmlPullParser.END_DOCUMENT) {
-            // if the AtomicFile could not be instantiated before attempt again; if it still fails
+                String tagName = parser.getName();
-            // return an empty key map.
+                if (XML_TAG_ADB_KEY.equals(tagName)) {
-            if (mAtomicKeyFile == null) {
+                    addAdbKeyToKeyMap(parser);
-                initKeyFile();
+                } else if (XML_TAG_WIFI_ACCESS_POINT.equals(tagName)) {
-                if (mAtomicKeyFile == null) {
+                    addTrustedNetworkToTrustedNetworks(parser);
-                    Slog.e(TAG, "Unable to obtain the key file, " + mKeyFile + ", for reading");
+                } else {
-                    return keyMap;
+                    Slog.w(TAG, "Ignoring tag '" + tagName + "'. Not recognized.");
                 }
+                XmlUtils.skipCurrentTag(parser);
             }
-            if (!mAtomicKeyFile.exists()) {
-                return keyMap;
-            }
-            try (FileInputStream keyStream = mAtomicKeyFile.openRead()) {
-                TypedXmlPullParser parser = Xml.resolvePullParser(keyStream);
-                XmlUtils.beginDocument(parser, XML_TAG_ADB_KEY);
-                while (parser.next() != XmlPullParser.END_DOCUMENT) {
-                    String tagName = parser.getName();
-                    if (tagName == null) {
-                        break;
-                    } else if (!tagName.equals(XML_TAG_ADB_KEY)) {
-                        XmlUtils.skipCurrentTag(parser);
-                        continue;
-                    }
-                    String key = parser.getAttributeValue(null, XML_ATTRIBUTE_KEY);
-                    long connectionTime;
-                    try {
-                        connectionTime = parser.getAttributeLong(null,
-                                XML_ATTRIBUTE_LAST_CONNECTION);
-                    } catch (XmlPullParserException e) {
-                        Slog.e(TAG,
-                                "Caught a NumberFormatException parsing the last connection time: "
-                                        + e);
-                        XmlUtils.skipCurrentTag(parser);
-                        continue;
-                    }
-                    keyMap.put(key, connectionTime);
-                }
-            } catch (IOException | XmlPullParserException e) {
-                Slog.e(TAG, "Caught an exception parsing the XML key file: ", e);
-            }
-            return keyMap;
         }
 
-        /**
+        private void addAdbKeyToKeyMap(TypedXmlPullParser parser) {
-         * Returns the map of trusted networks from the keystore file.
+            String key = parser.getAttributeValue(null, XML_ATTRIBUTE_KEY);
-         *
+            try {
-         * This was implemented in keystore version 1.
+                long connectionTime =
-         */
+                        parser.getAttributeLong(null, XML_ATTRIBUTE_LAST_CONNECTION);
-        private List<String> getTrustedNetworks() {
+                mKeyMap.put(key, connectionTime);
-            List<String> trustedNetworks = new ArrayList<String>();
+            } catch (XmlPullParserException e) {
-            // if the AtomicFile could not be instantiated before attempt again; if it still fails
+                Slog.e(TAG, "Error reading adbKey attributes", e);
-            // return an empty key map.
-            if (mAtomicKeyFile == null) {
-                initKeyFile();
-                if (mAtomicKeyFile == null) {
-                    Slog.e(TAG, "Unable to obtain the key file, " + mKeyFile + ", for reading");
-                    return trustedNetworks;
-                }
             }
-            if (!mAtomicKeyFile.exists()) {
+        }
-                return trustedNetworks;
+
-            }
+        private void addTrustedNetworkToTrustedNetworks(TypedXmlPullParser parser) {
-            try (FileInputStream keyStream = mAtomicKeyFile.openRead()) {
+            String bssid = parser.getAttributeValue(null, XML_ATTRIBUTE_WIFI_BSSID);
-                TypedXmlPullParser parser = Xml.resolvePullParser(keyStream);
+            mTrustedNetworks.add(bssid);
-                // Check for supported keystore version.
-                XmlUtils.beginDocument(parser, XML_KEYSTORE_START_TAG);
-                if (parser.next() != XmlPullParser.END_DOCUMENT) {
-                    String tagName = parser.getName();
-                    if (tagName == null || !XML_KEYSTORE_START_TAG.equals(tagName)) {
-                        Slog.e(TAG, "Expected " + XML_KEYSTORE_START_TAG + ", but got tag="
-                                + tagName);
-                        return trustedNetworks;
-                    }
-                    int keystoreVersion = parser.getAttributeInt(null, XML_ATTRIBUTE_VERSION);
-                    if (keystoreVersion > MAX_SUPPORTED_KEYSTORE_VERSION) {
-                        Slog.e(TAG, "Keystore version=" + keystoreVersion
-                                + " not supported (max_supported="
-                                + MAX_SUPPORTED_KEYSTORE_VERSION);
-                        return trustedNetworks;
-                    }
-                }
-                while (parser.next() != XmlPullParser.END_DOCUMENT) {
-                    String tagName = parser.getName();
-                    if (tagName == null) {
-                        break;
-                    } else if (!tagName.equals(XML_TAG_WIFI_ACCESS_POINT)) {
-                        XmlUtils.skipCurrentTag(parser);
-                        continue;
-                    }
-                    String bssid = parser.getAttributeValue(null, XML_ATTRIBUTE_WIFI_BSSID);
-                    trustedNetworks.add(bssid);
-                }
-            } catch (IOException | XmlPullParserException | NumberFormatException e) {
-                Slog.e(TAG, "Caught an exception parsing the XML key file: ", e);
-            }
-            return trustedNetworks;
         }
 
         /**
          * Updates the keystore with keys that were previously set to be always allowed before the
          * connection time of keys was tracked.
          */
-        private void addUserKeysToKeyStore() {
+        private void addExistingUserKeysToKeyStore() {
-            File userKeyFile = getUserKeyFile();
+            if (mUserKeyFile == null || !mUserKeyFile.exists()) {
+                return;
+            }
             boolean mapUpdated = false;
-            if (userKeyFile != null && userKeyFile.exists()) {
+            try (BufferedReader in = new BufferedReader(new FileReader(mUserKeyFile))) {
-                try (BufferedReader in = new BufferedReader(new FileReader(userKeyFile))) {
+                String key;
-                    long time = System.currentTimeMillis();
+                while ((key = in.readLine()) != null) {
-                    String key;
+                    // if the keystore does not contain the key from the user key file then add
-                    while ((key = in.readLine()) != null) {
+                    // it to the Map with the current system time to prevent it from expiring
-                        // if the keystore does not contain the key from the user key file then add
+                    // immediately if the user is actively using this key.
-                        // it to the Map with the current system time to prevent it from expiring
+                    if (!mKeyMap.containsKey(key)) {
-                        // immediately if the user is actively using this key.
+                        mKeyMap.put(key, mTicker.currentTimeMillis());
-                        if (!mKeyMap.containsKey(key)) {
+                        mapUpdated = true;
-                            mKeyMap.put(key, time);
-                            mapUpdated = true;
-                        }
                     }
-                } catch (IOException e) {
-                    Slog.e(TAG, "Caught an exception reading " + userKeyFile + ": " + e);
                 }
+            } catch (IOException e) {
+                Slog.e(TAG, "Caught an exception reading " + mUserKeyFile + ": " + e);
             }
             if (mapUpdated) {
                 sendPersistKeyStoreMessage();
@@ -2147,7 +2053,9 @@ public class AdbDebuggingManager {
             if (mAtomicKeyFile == null) {
                 initKeyFile();
                 if (mAtomicKeyFile == null) {
-                    Slog.e(TAG, "Unable to obtain the key file, " + mKeyFile + ", for writing");
+                    Slog.e(
+                            TAG,
+                            "Unable to obtain the key file, " + mTempKeysFile + ", for writing");
                     return;
                 }
             }
@@ -2178,17 +2086,21 @@ public class AdbDebuggingManager {
                 Slog.e(TAG, "Caught an exception writing the key map: ", e);
                 mAtomicKeyFile.failWrite(keyStream);
             }
+            writeKeys(mKeyMap.keySet());
         }
 
         private boolean filterOutOldKeys() {
-            boolean keysDeleted = false;
             long allowedTime = getAllowedConnectionTime();
-            long systemTime = System.currentTimeMillis();
+            if (allowedTime == 0) {
+                return false;
+            }
+            boolean keysDeleted = false;
+            long systemTime = mTicker.currentTimeMillis();
             Iterator<Map.Entry<String, Long>> keyMapIterator = mKeyMap.entrySet().iterator();
             while (keyMapIterator.hasNext()) {
                 Map.Entry<String, Long> keyEntry = keyMapIterator.next();
                 long connectionTime = keyEntry.getValue();
-                if (allowedTime != 0 && systemTime > (connectionTime + allowedTime)) {
+                if (systemTime > (connectionTime + allowedTime)) {
                     keyMapIterator.remove();
                     keysDeleted = true;
                 }
@@ -2212,7 +2124,7 @@ public class AdbDebuggingManager {
             if (allowedTime == 0) {
                 return minExpiration;
             }
-            long systemTime = System.currentTimeMillis();
+            long systemTime = mTicker.currentTimeMillis();
             Iterator<Map.Entry<String, Long>> keyMapIterator = mKeyMap.entrySet().iterator();
             while (keyMapIterator.hasNext()) {
                 Map.Entry<String, Long> keyEntry = keyMapIterator.next();
@@ -2233,7 +2145,9 @@ public class AdbDebuggingManager {
         public void deleteKeyStore() {
             mKeyMap.clear();
             mTrustedNetworks.clear();
-            deleteKeyFile();
+            if (mUserKeyFile != null) {
+                mUserKeyFile.delete();
+            }
             if (mAtomicKeyFile == null) {
                 return;
             }
@@ -2260,7 +2174,8 @@ public class AdbDebuggingManager {
          * is set to true the time will be set even if it is older than the previously written
          * connection time.
          */
-        public void setLastConnectionTime(String key, long connectionTime, boolean force) {
+        @VisibleForTesting
+        void setLastConnectionTime(String key, long connectionTime, boolean force) {
             // Do not set the connection time to a value that is earlier than what was previously
             // stored as the last connection time unless force is set.
             if (mKeyMap.containsKey(key) && mKeyMap.get(key) >= connectionTime && !force) {
@@ -2271,11 +2186,6 @@ public class AdbDebuggingManager {
             if (mSystemKeys.contains(key)) {
                 return;
             }
-            // if this is the first time the key is being added then write it to the key file as
-            // well.
-            if (!mKeyMap.containsKey(key)) {
-                writeKey(key);
-            }
             mKeyMap.put(key, connectionTime);
         }
 
@@ -2307,12 +2217,8 @@ public class AdbDebuggingManager {
             long allowedConnectionTime = getAllowedConnectionTime();
             // if the allowed connection time is 0 then revert to the previous behavior of always
             // allowing previously granted adb grants.
-            if (allowedConnectionTime == 0 || (System.currentTimeMillis() < (lastConnectionTime
+            return allowedConnectionTime == 0
-                    + allowedConnectionTime))) {
+                    || (mTicker.currentTimeMillis() < (lastConnectionTime + allowedConnectionTime));
-                return true;
-            } else {
-                return false;
-            }
         }
 
         /**
@@ -2324,4 +2230,15 @@ public class AdbDebuggingManager {
             return mTrustedNetworks.contains(bssid);
         }
     }
+
+    /**
+     * A Guava-like interface for getting the current system time.
+     *
+     * This allows us to swap a fake ticker in for testing to reduce "Thread.sleep()" calls and test
+     * for exact expected times instead of random ones.
+     */
+    @VisibleForTesting
+    interface Ticker {
+        long currentTimeMillis();
+    }
 }

services/core/java/com/android/server/adb/AdbService.java

@@ -151,6 +151,14 @@ public class AdbService extends IAdbManager.Stub {
             return mDebuggingManager == null ? null : mDebuggingManager.getAdbTempKeysFile();
         }
 
+        @Override
+        public void notifyKeyFilesUpdated() {
+            if (mDebuggingManager == null) {
+                return;
+            }
+            mDebuggingManager.notifyKeyFilesUpdated();
+        }
+
         @Override
         public void startAdbdForTransport(byte transportType) {
             FgThread.getHandler().sendMessage(obtainMessage(

services/core/java/com/android/server/testharness/TestHarnessModeService.java

@@ -189,6 +189,7 @@ public class TestHarnessModeService extends SystemService {
         if (adbManager.getAdbTempKeysFile() != null) {
             writeBytesToFile(persistentData.mAdbTempKeys, adbManager.getAdbTempKeysFile().toPath());
         }
+        adbManager.notifyKeyFilesUpdated();
     }
 
     private void configureUser() {

services/tests/servicestests/src/com/android/server/adb/AdbDebuggingManagerTest.java

@@ -36,8 +36,6 @@ import android.util.Log;
 
 import androidx.test.InstrumentationRegistry;
 
-import com.android.server.FgThread;
-
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -48,6 +46,11 @@ import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileReader;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CountDownLatch;
@@ -88,6 +91,7 @@ public final class AdbDebuggingManagerTest {
     private long mOriginalAllowedConnectionTime;
     private File mAdbKeyXmlFile;
     private File mAdbKeyFile;
+    private FakeTicker mFakeTicker;
 
     @Before
     public void setUp() throws Exception {
@@ -96,14 +100,25 @@ public final class AdbDebuggingManagerTest {
         if (mAdbKeyFile.exists()) {
             mAdbKeyFile.delete();
         }
-        mManager = new AdbDebuggingManager(mContext, ADB_CONFIRM_COMPONENT, mAdbKeyFile);
         mAdbKeyXmlFile = new File(mContext.getFilesDir(), "test_adb_keys.xml");
         if (mAdbKeyXmlFile.exists()) {
             mAdbKeyXmlFile.delete();
         }
+
+        mFakeTicker = new FakeTicker();
+        // Set the ticker time to October 22, 2008 (the day the T-Mobile G1 was released)
+        mFakeTicker.advance(1224658800L);
+
         mThread = new AdbDebuggingThreadTest();
-        mKeyStore = mManager.new AdbKeyStore(mAdbKeyXmlFile);
+        mManager = new AdbDebuggingManager(
-        mHandler = mManager.new AdbDebuggingHandler(FgThread.get().getLooper(), mThread, mKeyStore);
+                mContext, ADB_CONFIRM_COMPONENT, mAdbKeyFile, mAdbKeyXmlFile, mThread, mFakeTicker);
+
+        mHandler = mManager.mHandler;
+        mThread.setHandler(mHandler);
+
+        mHandler.initKeyStore();
+        mKeyStore = mHandler.mAdbKeyStore;
+
         mOriginalAllowedConnectionTime = mKeyStore.getAllowedConnectionTime();
         mBlockingQueue = new ArrayBlockingQueue<>(1);
     }
@@ -122,7 +137,7 @@ public final class AdbDebuggingManagerTest {
     private void setAllowedConnectionTime(long connectionTime) {
         Settings.Global.putLong(mContext.getContentResolver(),
                 Settings.Global.ADB_ALLOWED_CONNECTION_TIME, connectionTime);
-    };
+    }
 
     @Test
     public void testAllowNewKeyOnce() throws Exception {
@@ -158,20 +173,15 @@ public final class AdbDebuggingManagerTest {
         // Allow a connection from a new key with the 'Always allow' option selected.
         runAdbTest(TEST_KEY_1, true, true, false);
 
-        // Get the last connection time for the currently connected key to verify that it is updated
+        // Advance the clock by 10ms to ensure there's a difference
-        // after the disconnect.
+        mFakeTicker.advance(10 * 1_000_000);
-        long lastConnectionTime = mKeyStore.getLastConnectionTime(TEST_KEY_1);
-
-        // Sleep for a small amount of time to ensure a difference can be observed in the last
-        // connection time after a disconnect.
-        Thread.sleep(10);
 
         // Send the disconnect message for the currently connected key to trigger an update of the
         // last connection time.
         disconnectKey(TEST_KEY_1);
-        assertNotEquals(
+        assertEquals(
                 "The last connection time was not updated after the disconnect",
-                lastConnectionTime,
+                mFakeTicker.currentTimeMillis(),
                 mKeyStore.getLastConnectionTime(TEST_KEY_1));
     }
 
@@ -244,8 +254,8 @@ public final class AdbDebuggingManagerTest {
         // Get the current last connection time for comparison after the scheduled job is run
         long lastConnectionTime = mKeyStore.getLastConnectionTime(TEST_KEY_1);
 
-        // Sleep a small amount of time to ensure that the updated connection time changes
+        // Advance a small amount of time to ensure that the updated connection time changes
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
 
         // Send a message to the handler to update the last connection time for the active key
         updateKeyStore();
@@ -269,13 +279,13 @@ public final class AdbDebuggingManagerTest {
         persistKeyStore();
         assertTrue(
                 "The key with the 'Always allow' option selected was not persisted in the keystore",
-                mManager.new AdbKeyStore(mAdbKeyXmlFile).isKeyAuthorized(TEST_KEY_1));
+                mManager.new AdbKeyStore().isKeyAuthorized(TEST_KEY_1));
 
         // Get the current last connection time to ensure it is updated in the persisted keystore.
         long lastConnectionTime = mKeyStore.getLastConnectionTime(TEST_KEY_1);
 
-        // Sleep a small amount of time to ensure the last connection time is updated.
+        // Advance a small amount of time to ensure the last connection time is updated.
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
 
         // Send a message to the handler to update the last connection time for the active key.
         updateKeyStore();
@@ -286,7 +296,7 @@ public final class AdbDebuggingManagerTest {
         assertNotEquals(
                 "The last connection time in the key file was not updated after the update "
                         + "connection time message", lastConnectionTime,
-                mManager.new AdbKeyStore(mAdbKeyXmlFile).getLastConnectionTime(TEST_KEY_1));
+                mManager.new AdbKeyStore().getLastConnectionTime(TEST_KEY_1));
         // Verify that the key is in the adb_keys file
         assertTrue("The key was not in the adb_keys file after persisting the keystore",
                 isKeyInFile(TEST_KEY_1, mAdbKeyFile));
@@ -327,8 +337,8 @@ public final class AdbDebuggingManagerTest {
         // Set the allowed window to a small value to ensure the time is beyond the allowed window.
         setAllowedConnectionTime(1);
 
-        // Sleep for a small amount of time to exceed the allowed window.
+        // Advance a small amount of time to exceed the allowed window.
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
 
         // The AdbKeyStore has a method to get the time of the next key expiration to ensure the
         // scheduled job runs at the time of the next expiration or after 24 hours, whichever occurs
@@ -478,9 +488,12 @@ public final class AdbDebuggingManagerTest {
         // Set the current expiration time to a minute from expiration and verify this new value is
         // returned.
         final long newExpirationTime = 60000;
-        mKeyStore.setLastConnectionTime(TEST_KEY_1,
+        mKeyStore.setLastConnectionTime(
-                System.currentTimeMillis() - Settings.Global.DEFAULT_ADB_ALLOWED_CONNECTION_TIME
+                TEST_KEY_1,
-                        + newExpirationTime, true);
+                mFakeTicker.currentTimeMillis()
+                        - Settings.Global.DEFAULT_ADB_ALLOWED_CONNECTION_TIME
+                        + newExpirationTime,
+                true);
         expirationTime = mKeyStore.getNextExpirationTime();
         if (Math.abs(expirationTime - newExpirationTime) > epsilon) {
             fail("The expiration time for a key about to expire, " + expirationTime
@@ -525,7 +538,7 @@ public final class AdbDebuggingManagerTest {
         // Get the last connection time for the key to verify that it is updated when the connected
         // key message is sent.
         long connectionTime = mKeyStore.getLastConnectionTime(TEST_KEY_1);
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
         mHandler.obtainMessage(AdbDebuggingManager.AdbDebuggingHandler.MESSAGE_ADB_CONNECTED_KEY,
                 TEST_KEY_1).sendToTarget();
         flushHandlerQueue();
@@ -536,7 +549,7 @@ public final class AdbDebuggingManagerTest {
 
         // Verify that the scheduled job updates the connection time of the key.
         connectionTime = mKeyStore.getLastConnectionTime(TEST_KEY_1);
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
         updateKeyStore();
         assertNotEquals(
                 "The connection time for the key must be updated when the update keystore message"
@@ -545,7 +558,7 @@ public final class AdbDebuggingManagerTest {
 
         // Verify that the connection time is updated when the key is disconnected.
         connectionTime = mKeyStore.getLastConnectionTime(TEST_KEY_1);
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
         disconnectKey(TEST_KEY_1);
         assertNotEquals(
                 "The connection time for the key must be updated when the disconnected message is"
@@ -628,11 +641,11 @@ public final class AdbDebuggingManagerTest {
         setAllowedConnectionTime(Settings.Global.DEFAULT_ADB_ALLOWED_CONNECTION_TIME);
 
         // The untracked keys should be added to the keystore as part of the constructor.
-        AdbDebuggingManager.AdbKeyStore adbKeyStore = mManager.new AdbKeyStore(mAdbKeyXmlFile);
+        AdbDebuggingManager.AdbKeyStore adbKeyStore = mManager.new AdbKeyStore();
 
         // Verify that the connection time for each test key is within a small value of the current
         // time.
-        long time = System.currentTimeMillis();
+        long time = mFakeTicker.currentTimeMillis();
         for (String key : testKeys) {
             long connectionTime = adbKeyStore.getLastConnectionTime(key);
             if (Math.abs(time - connectionTime) > epsilon) {
@@ -651,11 +664,11 @@ public final class AdbDebuggingManagerTest {
         runAdbTest(TEST_KEY_1, true, true, false);
         runAdbTest(TEST_KEY_2, true, true, false);
 
-        // Sleep a small amount of time to ensure the connection time is updated by the scheduled
+        // Advance a small amount of time to ensure the connection time is updated by the scheduled
         // job.
         long connectionTime1 = mKeyStore.getLastConnectionTime(TEST_KEY_1);
         long connectionTime2 = mKeyStore.getLastConnectionTime(TEST_KEY_2);
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
         updateKeyStore();
         assertNotEquals(
                 "The connection time for test key 1 must be updated after the scheduled job runs",
@@ -669,7 +682,7 @@ public final class AdbDebuggingManagerTest {
         disconnectKey(TEST_KEY_2);
         connectionTime1 = mKeyStore.getLastConnectionTime(TEST_KEY_1);
         connectionTime2 = mKeyStore.getLastConnectionTime(TEST_KEY_2);
-        Thread.sleep(10);
+        mFakeTicker.advance(10);
         updateKeyStore();
         assertNotEquals(
                 "The connection time for test key 1 must be updated after another key is "
@@ -686,8 +699,6 @@ public final class AdbDebuggingManagerTest {
         // to clear the adb authorizations when adb is disabled after a boot a NullPointerException
         // was thrown as deleteKeyStore is invoked against the key store. This test ensures the
         // key store can be successfully cleared when adb is disabled.
-        mHandler = mManager.new AdbDebuggingHandler(FgThread.get().getLooper());
-
         clearKeyStore();
     }
 
@@ -723,12 +734,104 @@ public final class AdbDebuggingManagerTest {
 
         // Now remove one of the keys and make sure the other key is still there
         mKeyStore.removeKey(TEST_KEY_1);
+        // Wait for the handler queue to receive the MESSAGE_ADB_PERSIST_KEYSTORE
+        flushHandlerQueue();
+
         assertFalse("The key was still in the adb_keys file after removing the key",
                 isKeyInFile(TEST_KEY_1, mAdbKeyFile));
         assertTrue("The key was not in the adb_keys file after removing a different key",
                 isKeyInFile(TEST_KEY_2, mAdbKeyFile));
     }
 
+    @Test
+    public void testAdbKeyStore_addDuplicateKey_doesNotAddDuplicateToAdbKeyFile() throws Exception {
+        setAllowedConnectionTime(0);
+
+        runAdbTest(TEST_KEY_1, true, true, false);
+        persistKeyStore();
+        runAdbTest(TEST_KEY_1, true, true, false);
+        persistKeyStore();
+
+        assertEquals("adb_keys contains duplicate keys", 1, adbKeyFileKeys(mAdbKeyFile).size());
+    }
+
+    @Test
+    public void testAdbKeyStore_adbTempKeysFile_readsLastConnectionTimeFromXml() throws Exception {
+        long insertTime = mFakeTicker.currentTimeMillis();
+        runAdbTest(TEST_KEY_1, true, true, false);
+        persistKeyStore();
+
+        mFakeTicker.advance(10);
+        AdbDebuggingManager.AdbKeyStore newKeyStore = mManager.new AdbKeyStore();
+
+        assertEquals(
+                "KeyStore not populated from the XML file.",
+                insertTime,
+                newKeyStore.getLastConnectionTime(TEST_KEY_1));
+    }
+
+    @Test
+    public void test_notifyKeyFilesUpdated_filesDeletedRemovesPreviouslyAddedKey()
+            throws Exception {
+        runAdbTest(TEST_KEY_1, true, true, false);
+        persistKeyStore();
+
+        Files.delete(mAdbKeyXmlFile.toPath());
+        Files.delete(mAdbKeyFile.toPath());
+
+        mManager.notifyKeyFilesUpdated();
+        flushHandlerQueue();
+
+        assertFalse(
+                "Key is authorized after reloading deleted key files. Was state preserved?",
+                mKeyStore.isKeyAuthorized(TEST_KEY_1));
+    }
+
+    @Test
+    public void test_notifyKeyFilesUpdated_newKeyIsAuthorized() throws Exception {
+        runAdbTest(TEST_KEY_1, true, true, false);
+        persistKeyStore();
+
+        // Back up the existing key files
+        Path tempXmlFile = Files.createTempFile("adbKeyXmlFile", ".tmp");
+        Path tempAdbKeysFile = Files.createTempFile("adb_keys", ".tmp");
+        Files.copy(mAdbKeyXmlFile.toPath(), tempXmlFile, StandardCopyOption.REPLACE_EXISTING);
+        Files.copy(mAdbKeyFile.toPath(), tempAdbKeysFile, StandardCopyOption.REPLACE_EXISTING);
+
+        // Delete the existing key files
+        Files.delete(mAdbKeyXmlFile.toPath());
+        Files.delete(mAdbKeyFile.toPath());
+
+        // Notify the manager that adb key files have changed.
+        mManager.notifyKeyFilesUpdated();
+        flushHandlerQueue();
+
+        // Copy the files back
+        Files.copy(tempXmlFile, mAdbKeyXmlFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+        Files.copy(tempAdbKeysFile, mAdbKeyFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+
+        // Tell the manager that the key files have changed.
+        mManager.notifyKeyFilesUpdated();
+        flushHandlerQueue();
+
+        assertTrue(
+                "Key is not authorized after reloading key files.",
+                mKeyStore.isKeyAuthorized(TEST_KEY_1));
+    }
+
+    @Test
+    public void testAdbKeyStore_adbWifiConnect_storesBssidWhenAlwaysAllow() throws Exception {
+        String trustedNetwork = "My Network";
+        mKeyStore.addTrustedNetwork(trustedNetwork);
+        persistKeyStore();
+
+        AdbDebuggingManager.AdbKeyStore newKeyStore = mManager.new AdbKeyStore();
+
+        assertTrue(
+                "Persisted trusted network not found in new keystore instance.",
+                newKeyStore.isTrustedNetwork(trustedNetwork));
+    }
+
     @Test
     public void testIsValidMdnsServiceName() {
         // Longer than 15 characters
@@ -1030,28 +1133,27 @@ public final class AdbDebuggingManagerTest {
         if (key == null) {
             return false;
         }
+        return adbKeyFileKeys(keyFile).contains(key);
+    }
+
+    private static List<String> adbKeyFileKeys(File keyFile) throws Exception {
+        List<String> keys = new ArrayList<>();
         if (keyFile.exists()) {
             try (BufferedReader in = new BufferedReader(new FileReader(keyFile))) {
                 String currKey;
                 while ((currKey = in.readLine()) != null) {
-                    if (key.equals(currKey)) {
+                    keys.add(currKey);
-                        return true;
-                    }
                 }
             }
         }
-        return false;
+        return keys;
     }
 
     /**
      * Helper class that extends AdbDebuggingThread to receive the response from AdbDebuggingManager
      * indicating whether the key should be allowed to  connect.
      */
-    class AdbDebuggingThreadTest extends AdbDebuggingManager.AdbDebuggingThread {
+    private class AdbDebuggingThreadTest extends AdbDebuggingManager.AdbDebuggingThread {
-        AdbDebuggingThreadTest() {
-            mManager.super();
-        }
-
         @Override
         public void sendResponse(String msg) {
             TestResult result = new TestResult(TestResult.RESULT_RESPONSE_RECEIVED, msg);
@@ -1091,4 +1193,17 @@ public final class AdbDebuggingManagerTest {
             return "{mReturnCode = " + mReturnCode + ", mMessage = " + mMessage + "}";
         }
     }
+
+    private static class FakeTicker implements AdbDebuggingManager.Ticker {
+        private long mCurrentTime;
+
+        private void advance(long milliseconds) {
+            mCurrentTime += milliseconds;
+        }
+
+        @Override
+        public long currentTimeMillis() {
+            return mCurrentTime;
+        }
+    }
 }