From 221ea892dcc661bd07d6f36ff012edca2c48aed4 Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Sun, 4 Aug 2013 16:50:16 -0700 Subject: [PATCH] Start restricting service calls with implicit intents. The bindService() and startService() calls have always had undefined behavior when used with an implicit Intent and there are multiple matching services. Because of this, it is not safe for applications to use such Intents when interacting with services, yet the platform would merrily go about doing... something. In KLP I want to cause this case to be invalid, resulting in an exception thrown back to the app. Unfortunately there are lots of (scary) things relying on this behavior, so we can't immediately turn it into an exception, even one qualified by the caller's target SDK version. In this change, we start loggin a WTF when such a call happens, and clean up some stuff in Bluetooth that was doing this behavior. Change-Id: I62e25d07890588d2362104e20b054aebb6c0e007 --- core/java/android/app/ContextImpl.java | 24 ++++++++++++ .../java/android/bluetooth/BluetoothA2dp.java | 17 ++++++--- .../android/bluetooth/BluetoothHeadset.java | 17 ++++++--- .../android/bluetooth/BluetoothHealth.java | 17 ++++++--- .../bluetooth/BluetoothInputDevice.java | 18 ++++++--- core/java/android/bluetooth/BluetoothPan.java | 25 ++++++++----- .../java/android/bluetooth/BluetoothPbap.java | 19 ++++++---- core/java/android/content/Context.java | 26 ++++++------- core/java/android/content/Intent.java | 37 ++++++++++++++++++- core/java/android/os/Build.java | 8 ++++ .../server/BluetoothManagerService.java | 24 +++++++----- 11 files changed, 170 insertions(+), 62 deletions(-) diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index ab1190334eb09..60c028870d6a1 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -1444,6 +1444,14 @@ class ContextImpl extends Context { @Override public ComponentName startServiceAsUser(Intent service, UserHandle user) { try { + if (service.getComponent() == null && service.getPackage() == null) { + if (getApplicationInfo().targetSdkVersion >= Build.VERSION_CODES.KEY_LIME_PIE) { + IllegalArgumentException ex = new IllegalArgumentException( + "Service Intent must be explicit: " + service); + Log.wtf(TAG, "This will become an error", ex); + //throw ex; + } + } service.prepareToLeaveProcess(); ComponentName cn = ActivityManagerNative.getDefault().startService( mMainThread.getApplicationThread(), service, @@ -1468,6 +1476,14 @@ class ContextImpl extends Context { @Override public boolean stopServiceAsUser(Intent service, UserHandle user) { try { + if (service.getComponent() == null && service.getPackage() == null) { + if (getApplicationInfo().targetSdkVersion >= Build.VERSION_CODES.KEY_LIME_PIE) { + IllegalArgumentException ex = new IllegalArgumentException( + "Service Intent must be explicit: " + service); + Log.wtf(TAG, "This will become an error", ex); + //throw ex; + } + } service.prepareToLeaveProcess(); int res = ActivityManagerNative.getDefault().stopService( mMainThread.getApplicationThread(), service, @@ -1503,6 +1519,14 @@ class ContextImpl extends Context { } else { throw new RuntimeException("Not supported in system context"); } + if (service.getComponent() == null && service.getPackage() == null) { + if (getApplicationInfo().targetSdkVersion >= Build.VERSION_CODES.KEY_LIME_PIE) { + IllegalArgumentException ex = new IllegalArgumentException( + "Service Intent must be explicit: " + service); + Log.wtf(TAG, "This will become an error", ex); + //throw ex; + } + } try { IBinder token = getActivityToken(); if (token == null && (flags&BIND_AUTO_CREATE) == 0 && mPackageInfo != null diff --git a/core/java/android/bluetooth/BluetoothA2dp.java b/core/java/android/bluetooth/BluetoothA2dp.java index 6fdf3b47b85f8..d7d8cdbe439f8 100644 --- a/core/java/android/bluetooth/BluetoothA2dp.java +++ b/core/java/android/bluetooth/BluetoothA2dp.java @@ -128,9 +128,7 @@ public final class BluetoothA2dp implements BluetoothProfile { try { if (mService == null) { if (VDBG) Log.d(TAG,"Binding service..."); - if (!mContext.bindService(new Intent(IBluetoothA2dp.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth A2DP Service"); - } + doBind(); } } catch (Exception re) { Log.e(TAG,"",re); @@ -157,9 +155,18 @@ public final class BluetoothA2dp implements BluetoothProfile { } } - if (!context.bindService(new Intent(IBluetoothA2dp.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth A2DP Service"); + doBind(); + } + + boolean doBind() { + Intent intent = new Intent(IBluetoothA2dp.class.getName()); + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindService(intent, mConnection, 0)) { + Log.e(TAG, "Could not bind to Bluetooth A2DP Service with " + intent); + return false; } + return true; } /*package*/ void close() { diff --git a/core/java/android/bluetooth/BluetoothHeadset.java b/core/java/android/bluetooth/BluetoothHeadset.java index 963e9fcb21113..5a5764d1d0aef 100644 --- a/core/java/android/bluetooth/BluetoothHeadset.java +++ b/core/java/android/bluetooth/BluetoothHeadset.java @@ -241,9 +241,7 @@ public final class BluetoothHeadset implements BluetoothProfile { try { if (mService == null) { if (VDBG) Log.d(TAG,"Binding service..."); - if (!mContext.bindService(new Intent(IBluetoothHeadset.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth Headset Service"); - } + doBind(); } } catch (Exception re) { Log.e(TAG,"",re); @@ -270,9 +268,18 @@ public final class BluetoothHeadset implements BluetoothProfile { } } - if (!context.bindService(new Intent(IBluetoothHeadset.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth Headset Service"); + doBind(); + } + + boolean doBind() { + Intent intent = new Intent(IBluetoothHeadset.class.getName()); + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindService(intent, mConnection, 0)) { + Log.e(TAG, "Could not bind to Bluetooth Headset Service with " + intent); + return false; } + return true; } /** diff --git a/core/java/android/bluetooth/BluetoothHealth.java b/core/java/android/bluetooth/BluetoothHealth.java index cb23662f6c111..b1a084a7b3005 100644 --- a/core/java/android/bluetooth/BluetoothHealth.java +++ b/core/java/android/bluetooth/BluetoothHealth.java @@ -117,9 +117,7 @@ public final class BluetoothHealth implements BluetoothProfile { try { if (mService == null) { if (VDBG) Log.d(TAG,"Binding service..."); - if (!mContext.bindService(new Intent(IBluetoothHealth.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth Health Service"); - } + doBind(); } } catch (Exception re) { Log.e(TAG,"",re); @@ -483,9 +481,18 @@ public final class BluetoothHealth implements BluetoothProfile { } } - if (!context.bindService(new Intent(IBluetoothHealth.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth Health Service"); + doBind(); + } + + boolean doBind() { + Intent intent = new Intent(IBluetoothHealth.class.getName()); + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindService(intent, mConnection, 0)) { + Log.e(TAG, "Could not bind to Bluetooth Health Service with " + intent); + return false; } + return true; } /*package*/ void close() { diff --git a/core/java/android/bluetooth/BluetoothInputDevice.java b/core/java/android/bluetooth/BluetoothInputDevice.java index db7e424d40ee2..f9c789c58a4ca 100644 --- a/core/java/android/bluetooth/BluetoothInputDevice.java +++ b/core/java/android/bluetooth/BluetoothInputDevice.java @@ -206,9 +206,7 @@ public final class BluetoothInputDevice implements BluetoothProfile { try { if (mService == null) { if (VDBG) Log.d(TAG,"Binding service..."); - if (!mContext.bindService(new Intent(IBluetoothInputDevice.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth HID Service"); - } + doBind(); } } catch (Exception re) { Log.e(TAG,"",re); @@ -237,10 +235,18 @@ public final class BluetoothInputDevice implements BluetoothProfile { } } - if (!context.bindService(new Intent(IBluetoothInputDevice.class.getName()), - mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth HID Service"); + doBind(); + } + + boolean doBind() { + Intent intent = new Intent(IBluetoothInputDevice.class.getName()); + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindService(intent, mConnection, 0)) { + Log.e(TAG, "Could not bind to Bluetooth HID Service with " + intent); + return false; } + return true; } /*package*/ void close() { diff --git a/core/java/android/bluetooth/BluetoothPan.java b/core/java/android/bluetooth/BluetoothPan.java index e25ec86a8667b..83d4329e971d4 100644 --- a/core/java/android/bluetooth/BluetoothPan.java +++ b/core/java/android/bluetooth/BluetoothPan.java @@ -137,12 +137,20 @@ public final class BluetoothPan implements BluetoothProfile { } catch (RemoteException re) { Log.w(TAG,"Unable to register BluetoothStateChangeCallback",re); } - Log.d(TAG, "BluetoothPan() call bindService"); - if (!context.bindService(new Intent(IBluetoothPan.class.getName()), - mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth HID Service"); + if (VDBG) Log.d(TAG, "BluetoothPan() call bindService"); + doBind(); + if (VDBG) Log.d(TAG, "BluetoothPan(), bindService called"); + } + + boolean doBind() { + Intent intent = new Intent(IBluetoothPan.class.getName()); + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindService(intent, mConnection, 0)) { + Log.e(TAG, "Could not bind to Bluetooth Pan Service with " + intent); + return false; } - Log.d(TAG, "BluetoothPan(), bindService called"); + return true; } /*package*/ void close() { @@ -170,11 +178,8 @@ public final class BluetoothPan implements BluetoothProfile { //Handle enable request to bind again. if (on) { Log.d(TAG, "onBluetoothStateChange(on) call bindService"); - if (!mContext.bindService(new Intent(IBluetoothPan.class.getName()), - mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth HID Service"); - } - Log.d(TAG, "BluetoothPan(), bindService called"); + doBind(); + if (VDBG) Log.d(TAG, "BluetoothPan(), bindService called"); } else { if (VDBG) Log.d(TAG,"Unbinding service..."); synchronized (mConnection) { diff --git a/core/java/android/bluetooth/BluetoothPbap.java b/core/java/android/bluetooth/BluetoothPbap.java index b5280e5337783..c42251f4367b2 100644 --- a/core/java/android/bluetooth/BluetoothPbap.java +++ b/core/java/android/bluetooth/BluetoothPbap.java @@ -129,11 +129,7 @@ public class BluetoothPbap { try { if (mService == null) { if (VDBG) Log.d(TAG,"Binding service..."); - if (!mContext.bindService( - new Intent(IBluetoothPbap.class.getName()), - mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth PBAP Service"); - } + doBind(); } } catch (Exception re) { Log.e(TAG,"",re); @@ -158,9 +154,18 @@ public class BluetoothPbap { Log.e(TAG,"",e); } } - if (!context.bindService(new Intent(IBluetoothPbap.class.getName()), mConnection, 0)) { - Log.e(TAG, "Could not bind to Bluetooth Pbap Service"); + doBind(); + } + + boolean doBind() { + Intent intent = new Intent(IBluetoothPbap.class.getName()); + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindService(intent, mConnection, 0)) { + Log.e(TAG, "Could not bind to Bluetooth Pbap Service with " + intent); + return false; } + return true; } protected void finalize() throws Throwable { diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index 0b1127c98ad53..5c37206459f86 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -976,7 +976,7 @@ public abstract class Context { * May be null if there are no options. See {@link android.app.ActivityOptions} * for how to build the Bundle supplied here; there are no supported definitions * for building it manually. - * @param user The UserHandle of the user to start this activity for. + * @param userId The UserHandle of the user to start this activity for. * @throws ActivityNotFoundException   * @hide */ @@ -1573,9 +1573,11 @@ public abstract class Context { /** * Request that a given application service be started. The Intent - * can either contain the complete class name of a specific service - * implementation to start, or an abstract definition through the - * action and other fields of the kind of service to start. If this service + * should contain either contain the complete class name of a specific service + * implementation to start or a specific package name to target. If the + * Intent is less specified, it will either throw an {@link IllegalArgumentException} + * (if the caller targets {@link android.os.Build.VERSION_CODES#KEY_LIME_PIE} or later), + * or which of multiple matching services it finds and uses will be undefined. If this service * is not already running, it will be instantiated and started (creating a * process for it if needed); if it is running then it remains running. * @@ -1601,10 +1603,9 @@ public abstract class Context { *

This function will throw {@link SecurityException} if you do not * have permission to start the given service. * - * @param service Identifies the service to be started. The Intent may - * specify either an explicit component name to start, or a logical - * description (action, category, etc) to match an - * {@link IntentFilter} published by a service. Additional values + * @param service Identifies the service to be started. The Intent must be either + * fully explicit (supplying a component name) or specify a specific package + * name it is targetted to. Additional values * may be included in the Intent extras to supply arguments along with * this specific start call. * @@ -1634,10 +1635,9 @@ public abstract class Context { *

This function will throw {@link SecurityException} if you do not * have permission to stop the given service. * - * @param service Description of the service to be stopped. The Intent may - * specify either an explicit component name to start, or a logical - * description (action, category, etc) to match an - * {@link IntentFilter} published by a service. + * @param service Description of the service to be stopped. The Intent must be either + * fully explicit (supplying a component name) or specify a specific package + * name it is targetted to. * * @return If there is a service matching the given Intent that is already * running, then it is stopped and {@code true} is returned; else {@code false} is returned. @@ -2296,7 +2296,7 @@ public abstract class Context { * camera devices. * * @see #getSystemService - * @see android.hardware.camera.CameraManager + * @see android.hardware.camera2.CameraManager */ public static final String CAMERA_SERVICE = "camera"; diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index 30ea3f957cb09..205ca6bd05d8b 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -16,6 +16,7 @@ package android.content; +import android.content.pm.ApplicationInfo; import android.util.ArraySet; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -44,6 +45,7 @@ import java.io.Serializable; import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Iterator; +import java.util.List; import java.util.Locale; import java.util.Set; @@ -5033,6 +5035,39 @@ public class Intent implements Parcelable, Cloneable { return ai; } + /** + * Special function for use by the system to resolve service + * intents to system apps. Throws an exception if there are + * multiple potential matches to the Intent. Returns null if + * there are no matches. + * @hide + */ + public ComponentName resolveSystemService(PackageManager pm, int flags) { + if (mComponent != null) { + return mComponent; + } + + List results = pm.queryIntentServices(this, flags); + if (results == null) { + return null; + } + ComponentName comp = null; + for (int i=0; iApplications targeting this or a later release will get these + * new changes in behavior:

+ * */ public static final int KEY_LIME_PIE = CUR_DEVELOPMENT; } diff --git a/services/java/com/android/server/BluetoothManagerService.java b/services/java/com/android/server/BluetoothManagerService.java index a5cf362856488..d2d5280208224 100644 --- a/services/java/com/android/server/BluetoothManagerService.java +++ b/services/java/com/android/server/BluetoothManagerService.java @@ -643,10 +643,9 @@ class BluetoothManagerService extends IBluetoothManager.Stub { Message timeoutMsg = mHandler.obtainMessage(MESSAGE_TIMEOUT_BIND); mHandler.sendMessageDelayed(timeoutMsg,TIMEOUT_BIND_MS); Intent i = new Intent(IBluetooth.class.getName()); - if (!mContext.bindServiceAsUser(i, mConnection, - Context.BIND_AUTO_CREATE, UserHandle.CURRENT)) { + if (!doBind(i, mConnection, + Context.BIND_AUTO_CREATE, UserHandle.CURRENT)) { mHandler.removeMessages(MESSAGE_TIMEOUT_BIND); - Log.e(TAG, "fail to bind to: " + IBluetooth.class.getName()); } else { mBinding = true; } @@ -1028,10 +1027,8 @@ class BluetoothManagerService extends IBluetoothManager.Stub { mHandler.sendMessageDelayed(timeoutMsg,TIMEOUT_BIND_MS); mConnection.setGetNameAddressOnly(false); Intent i = new Intent(IBluetooth.class.getName()); - if (!mContext.bindServiceAsUser(i, mConnection,Context.BIND_AUTO_CREATE, - UserHandle.CURRENT)) { + if (!doBind(i, mConnection,Context.BIND_AUTO_CREATE, UserHandle.CURRENT)) { mHandler.removeMessages(MESSAGE_TIMEOUT_BIND); - Log.e(TAG, "Fail to bind to: " + IBluetooth.class.getName()); } else { mBinding = true; } @@ -1070,6 +1067,16 @@ class BluetoothManagerService extends IBluetoothManager.Stub { } } + boolean doBind(Intent intent, ServiceConnection conn, int flags, UserHandle user) { + ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0); + intent.setComponent(comp); + if (comp == null || !mContext.bindServiceAsUser(intent, conn, flags, user)) { + Log.e(TAG, "Fail to bind to: " + intent); + return false; + } + return true; + } + private void handleDisable() { synchronized(mConnection) { // don't need to disable if GetNameAddressOnly is set, @@ -1122,10 +1129,7 @@ class BluetoothManagerService extends IBluetoothManager.Stub { if (mContext.getPackageManager().hasSystemFeature( PackageManager.FEATURE_BLUETOOTH_LE)) { Intent i = new Intent(IBluetoothGatt.class.getName()); - if (!mContext.bindServiceAsUser(i, mConnection, Context.BIND_AUTO_CREATE, - UserHandle.CURRENT)) { - Log.e(TAG, "Fail to bind to: " + IBluetoothGatt.class.getName()); - } + doBind(i, mConnection, Context.BIND_AUTO_CREATE, UserHandle.CURRENT); } } else { //If Bluetooth is off, send service down event to proxy objects, and unbind