Write adb public key to devices during provision.

build/android/provision_devices.py

Index: build/android/provision_devices.py
diff --git a/build/android/provision_devices.py b/build/android/provision_devices.py
index 824e64202e78787d1096eedd73ddaa4e7b704dc1..f6de2e28afb99f32dc36dc003c62bd39ec53651c 100755
--- a/build/android/provision_devices.py
+++ b/build/android/provision_devices.py
@@ -113,6 +113,15 @@ def _ConfigureLocalProperties(device, java_debug=True):
 
   # LOCAL_PROPERTIES_PATH = '/data/local.prop'
 
+def WriteAdbKeysFile(device, adb_keys_string):
+  path_list = constants.ADB_KEYS_FILE.split('/')
+  dir_path = '/'.join(path_list[:len(path_list)-1])

[perezju, 2015/02/25 09:35:41]

nit: maybe that's just me, but I would go for:
  dir_path = posixpath.dirname(constants.ADB_KEYS_FILE)

[jbudorick, 2015/02/25 14:09:07]

I didn't know about posixpath. We should be using that elsewhere for device path
manipulations...

[navabi, 2015/02/25 18:57:25]

Yeah I don't like this split. But how about using:
dir_path = os.path.dirname(constants.ADB_KEYS_FILE) 

Is posixpath preferred over os.path.dirname? I changed to use os.path.dirname
for now.

[jbudorick, 2015/02/25 21:47:38]

It depends on the path being manipulated:
 - for host paths, os.path is preferred
 - for device paths, we have historically handrolled path manipulation (e.g.
'/'.join(path_components), path.split('/')). However, posixpath handles paths
the way we want for devices.

+  device.RunShellCommand('mkdir -p %s' % dir_path, as_root=True)
+  device.RunShellCommand('restorecon %s' % dir_path, as_root=True)
+  device.WriteFile(constants.ADB_KEYS_FILE, adb_keys_string, as_root=True)
+  device.RunShellCommand('restorecon %s' % constants.ADB_KEYS_FILE,
+                         as_root=True)
+
 
 def WipeDeviceData(device):
   """Wipes data from device, keeping only the adb_keys for authorization.
@@ -131,13 +140,7 @@ def WipeDeviceData(device):
     adb_keys = device.ReadFile(constants.ADB_KEYS_FILE, as_root=True)
   device.RunShellCommand('wipe data', as_root=True)
   if device_authorized:
-    path_list = constants.ADB_KEYS_FILE.split('/')
-    dir_path = '/'.join(path_list[:len(path_list)-1])
-    device.RunShellCommand('mkdir -p %s' % dir_path, as_root=True)
-    device.RunShellCommand('restorecon %s' % dir_path, as_root=True)
-    device.WriteFile(constants.ADB_KEYS_FILE, adb_keys, as_root=True)
-    device.RunShellCommand('restorecon %s' % constants.ADB_KEYS_FILE,
-                           as_root=True)
+    WriteAdbKeysFile(device, adb_keys)
 
 
 def WipeDeviceIfPossible(device, timeout):
@@ -158,6 +161,15 @@ def ProvisionDevice(device, options):
   else:
     reboot_timeout = _DEFAULT_TIMEOUTS.PRE_LOLLIPOP
 
+  if options.adb_key_files:
+    adb_keys = set()

[perezju, 2015/02/25 09:35:41]

nit: does it really need to be a set? do we expect passing more than one file
*and* have the same public key appear in multiple files?

[navabi, 2015/02/25 18:57:25]

Not the way the current adb keys on the host are, but following up on your
comment on 168, let's keep it a set in case in the future we end up with
multiple adb keys in a adb key file on the host. Then there may be an
intersection of adb keys between multiple files that are passed in.

+    for adb_key_file in options.adb_key_files:
+      with open(adb_key_file, 'r') as f:
+        adb_public_key = f.readlines()[0]

[perezju, 2015/02/25 09:35:41]

why do we keep only the first line? (earlier versions of your CL kept all of the
lines)

Also, if you only want the first line you can avoid reading the whole file by
doing instead:
  adb_public_key = next(f).rstrip()

[navabi, 2015/02/25 18:57:25]

Because before we were reading the adb keys file on the device which have
multiple lines. Here we are reading individual adb keys on the host, and the way
we have it set up is that we have multiple adb key files each with one adb key.
It *could* have multiple lines per adb key on the host, so I changed it.

+      adb_keys.add(adb_public_key)
+    adb_key_contents = '\n'.join(adb_keys)
+    WriteAdbKeysFile(device, adb_key_contents)

[jbudorick, 2015/02/25 14:09:07]

This will wipe existing keys from the device. Would that cause problems before
the hosts have the private keys? Should this preserve any keys on the device and
just add the keys from the key files?

[navabi, 2015/02/25 18:57:25]

This will not cause problems before the hosts have the private keys. The reason
is when we run this command to put the shared public adb key onto the devices
for the first time, we will be careful to also include the existing public key
on the host, which is currently used to authenticate adb on the devices.

I intend this option to overwrite the adb keys. It is up to the caller to make
sure they overwrite the adb keys the right way (e.g. include the existing public
keys so we don't lose authentication). We have plans on how to do that. We will
test first on one bot before deploying everywhere.

[jbudorick, 2015/02/25 21:47:38]

This makes me a little nervous, but ok.

+
   try:
     if not options.skip_wipe:
       WipeDeviceIfPossible(device, reboot_timeout)
@@ -276,6 +288,8 @@ def main():
   parser.add_argument('-r', '--auto-reconnect', action='store_true',
                       help='push binary which will reboot the device on adb'
                       ' disconnections')
+  parser.add_argument('--adb-key-files', type=str, nargs='+',
+                      help='list of adb keys to push to device')
   args = parser.parse_args()
   constants.SetBuildType(args.target)