From 674019065bceb4150190bfb1aa63cda9de0a8560 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Wed, 10 Jun 2015 09:38:42 -0700 Subject: [PATCH] Fix USB access control when adb is disabled. When adb is disabled, the default usb mode would be "none", which would turn off the driver and prevent UsbDeviceManager from receiving any new USB connect / disconnect messages. This prevents the user from ever enabling MTP and sharing data when adb is turned off. As discussed in bug 21429947, we work around this problem by keeping the USB driver in MTP mode most of the time, so that we continue to receive USB connect / disconnect messages. To avoid leaking confidential user photos, this change introduces an unlocked state. Setting the mtp enabled function is now decoupled from exposing data on the USB connection. Only if MTP is enabled and USB data has been unlocked is confidential user data allowed to be shared. Bug: 21429947 Change-Id: Iefb5c7e22dc4962bf5226f2ed3d0155b5c7b413c --- .../android/hardware/usb/IUsbManager.aidl | 10 +++++ .../java/android/hardware/usb/UsbManager.java | 40 +++++++++++++++++++ .../android/server/usb/UsbDeviceManager.java | 40 +++++++++++++++++-- .../com/android/server/usb/UsbService.java | 12 ++++++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/core/java/android/hardware/usb/IUsbManager.aidl b/core/java/android/hardware/usb/IUsbManager.aidl index 881dc0f321aaa..31a6a962e2b9a 100644 --- a/core/java/android/hardware/usb/IUsbManager.aidl +++ b/core/java/android/hardware/usb/IUsbManager.aidl @@ -85,6 +85,16 @@ interface IUsbManager /* Sets the current USB function. */ void setCurrentFunction(String function); + /* Sets whether USB data (for example, MTP exposed pictures) should be made + * available on the USB connection. Unlocking data should only be done with + * user involvement, since exposing pictures or other data could leak sensitive + * user information. + */ + void setUsbDataUnlocked(boolean unlock); + + /* Returns true iff sensitive user data is exposed on the USB connection. */ + boolean isUsbDataUnlocked(); + /* Allow USB debugging from the attached host. If alwaysAllow is true, add the * the public key to list of host keys that the user has approved. */ diff --git a/core/java/android/hardware/usb/UsbManager.java b/core/java/android/hardware/usb/UsbManager.java index 000d41fe4985a..c83f4668b81a0 100644 --- a/core/java/android/hardware/usb/UsbManager.java +++ b/core/java/android/hardware/usb/UsbManager.java @@ -141,6 +141,16 @@ public class UsbManager { */ public static final String USB_CONFIGURED = "configured"; + /** + * Boolean extra indicating whether confidential user data, such as photos, should be + * made available on the USB connection. This variable will only be set when the user + * has explicitly asked for this data to be unlocked. + * Used in extras for the {@link #ACTION_USB_STATE} broadcast. + * + * {@hide} + */ + public static final String USB_DATA_UNLOCKED = "unlocked"; + /** * Name of the USB mass storage USB function. * Used in extras for the {@link #ACTION_USB_STATE} broadcast @@ -464,4 +474,34 @@ public class UsbManager { Log.e(TAG, "RemoteException in setCurrentFunction", e); } } + + /** + * Sets whether USB data (for example, MTP exposed pictures) should be made available + * on the USB connection. Unlocking usb data should only be done with user involvement, + * since exposing pictures or other data could leak sensitive user information. + * + * {@hide} + */ + public void setUsbDataUnlocked(boolean unlocked) { + try { + mService.setUsbDataUnlocked(unlocked); + } catch (RemoteException e) { + Log.e(TAG, "RemoteException in setUsbDataUnlocked", e); + } + } + + /** + * Returns {@code true} iff access to sensitive USB data is currently allowed. + * + * {@hide} + */ + public boolean isUsbDataUnlocked() { + try { + return mService.isUsbDataUnlocked(); + } catch (RemoteException e) { + Log.e(TAG, "RemoteException in isUsbDataUnlocked", e); + } + return false; + } + } diff --git a/services/usb/java/com/android/server/usb/UsbDeviceManager.java b/services/usb/java/com/android/server/usb/UsbDeviceManager.java index d6a7dd16902a3..588f8c6030016 100644 --- a/services/usb/java/com/android/server/usb/UsbDeviceManager.java +++ b/services/usb/java/com/android/server/usb/UsbDeviceManager.java @@ -91,6 +91,7 @@ public class UsbDeviceManager { private static final int MSG_SYSTEM_READY = 3; private static final int MSG_BOOT_COMPLETED = 4; private static final int MSG_USER_SWITCHED = 5; + private static final int MSG_SET_USB_DATA_UNLOCKED = 6; private static final int AUDIO_MODE_SOURCE = 1; @@ -314,6 +315,7 @@ public class UsbDeviceManager { // current USB state private boolean mConnected; private boolean mConfigured; + private boolean mUsbDataUnlocked; private String mCurrentFunctions; private UsbAccessory mCurrentAccessory; private int mUsbNotificationId; @@ -350,7 +352,7 @@ public class UsbDeviceManager { SystemProperties.get(UsbManager.ADB_PERSISTENT_PROPERTY, "adb"), UsbManager.USB_FUNCTION_ADB); - mCurrentFunctions = mAdbEnabled ? "adb" : "none"; + mCurrentFunctions = mAdbEnabled ? "adb" : UsbManager.USB_FUNCTION_MTP; String state = FileUtils.readTextFile(new File(STATE_PATH), 0, null).trim(); updateState(state); @@ -459,6 +461,15 @@ public class UsbDeviceManager { } } + /** + * Stop and start the USB driver. This is needed to close all outstanding + * USB connections. + */ + private void restartCurrentFunction() { + setUsbConfig("none"); + setUsbConfig(mCurrentFunctions); + } + private void setEnabledFunctions(String functions) { if (DEBUG) Slog.d(TAG, "setEnabledFunctions " + functions); @@ -531,6 +542,7 @@ public class UsbDeviceManager { intent.addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING); intent.putExtra(UsbManager.USB_CONNECTED, mConnected); intent.putExtra(UsbManager.USB_CONFIGURED, mConfigured); + intent.putExtra(UsbManager.USB_DATA_UNLOCKED, mUsbDataUnlocked); if (mCurrentFunctions != null) { String[] functions = mCurrentFunctions.split(","); @@ -599,6 +611,10 @@ public class UsbDeviceManager { case MSG_UPDATE_STATE: mConnected = (msg.arg1 == 1); mConfigured = (msg.arg2 == 1); + if (!mConnected) { + // When a disconnect occurs, relock access to sensitive user data + mUsbDataUnlocked = false; + } updateUsbNotification(); updateAdbNotification(); if (containsFunction(mCurrentFunctions, @@ -621,6 +637,12 @@ public class UsbDeviceManager { String functions = (String)msg.obj; setEnabledFunctions(functions); break; + case MSG_SET_USB_DATA_UNLOCKED: + mUsbDataUnlocked = (msg.arg1 == 1); + updateUsbNotification(); + updateUsbState(); + restartCurrentFunction(); + break; case MSG_SYSTEM_READY: setUsbConfig(mCurrentFunctions); updatePersistentProperty(); @@ -676,7 +698,9 @@ public class UsbDeviceManager { int id = 0; Resources r = mContext.getResources(); if (mConnected) { - if (containsFunction(mCurrentFunctions, UsbManager.USB_FUNCTION_MTP)) { + if (!mUsbDataUnlocked) { + id = com.android.internal.R.string.usb_charging_notification_title; + } else if (containsFunction(mCurrentFunctions, UsbManager.USB_FUNCTION_MTP)) { id = com.android.internal.R.string.usb_mtp_notification_title; } else if (containsFunction(mCurrentFunctions, UsbManager.USB_FUNCTION_PTP)) { id = com.android.internal.R.string.usb_ptp_notification_title; @@ -771,7 +795,7 @@ public class UsbDeviceManager { } private String getDefaultFunctions() { - return "none"; + return UsbManager.USB_FUNCTION_MTP; } public void dump(FileDescriptor fd, PrintWriter pw) { @@ -817,6 +841,16 @@ public class UsbDeviceManager { mHandler.sendMessage(MSG_SET_CURRENT_FUNCTIONS, functions); } + public void setUsbDataUnlocked(boolean unlocked) { + if (DEBUG) Slog.d(TAG, "setUsbDataUnlocked(" + unlocked + ")"); + mHandler.sendMessage(MSG_SET_USB_DATA_UNLOCKED, unlocked); + } + + public boolean isUsbDataUnlocked() { + if (DEBUG) Slog.d(TAG, "isUsbDataUnlocked() -> " + mHandler.mUsbDataUnlocked); + return mHandler.mUsbDataUnlocked; + } + private void readOemUsbOverrideConfig() { String[] configList = mContext.getResources().getStringArray( com.android.internal.R.array.config_oemUsbModeOverride); diff --git a/services/usb/java/com/android/server/usb/UsbService.java b/services/usb/java/com/android/server/usb/UsbService.java index 51281a2ff4489..7a3426c5e18ee 100644 --- a/services/usb/java/com/android/server/usb/UsbService.java +++ b/services/usb/java/com/android/server/usb/UsbService.java @@ -270,6 +270,18 @@ public class UsbService extends IUsbManager.Stub { } } + @Override + public void setUsbDataUnlocked(boolean unlocked) { + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.MANAGE_USB, null); + mDeviceManager.setUsbDataUnlocked(unlocked); + } + + @Override + public boolean isUsbDataUnlocked() { + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.MANAGE_USB, null); + return mDeviceManager.isUsbDataUnlocked(); + } + @Override public void allowUsbDebugging(boolean alwaysAllow, String publicKey) { mContext.enforceCallingOrSelfPermission(android.Manifest.permission.MANAGE_USB, null);