From 01065a5719414b0855da2820beb9bd4a6459ba1b Mon Sep 17 00:00:00 2001 From: "Tadashi G. Takaoka" Date: Wed, 19 Jul 2017 14:10:24 +0900 Subject: [PATCH] Lock down IME switcher notification handling This CL fixes 1) broken IME switcher notification (Bug 63644555) and 2) a possible security bypass that any background application can virtually call IMM#showInputMethodPicker() by sending an explicit intent to the com.android.settings (Bug 64008672), and 3) wrong hint color for the notification. 1) From Android-O, an implicit broadcast intent doesn't get delivered to background apps [1]. So that the implicit broadcast intent of action "android.settings.SHOW_INPUT_METHOD_PICKER" isn't always delivered to Settings app, especially from the notification. So that InputMethodManagerService should use an explicit broadcast intent for a pending intent in the IME switcher notification. And it should also implement broadcast receiver of the intent by itself and remove InputMethodDialogReceiver from com.android.settings app [2]. 2) In addition to the existing security check [3], the explict broadcast intent mentioned in the above 1) must be locked down to the system by using protected-broadcast [4]. [1]: https://developer.android.com/preview/features/background.html#broadcasts [2]: Id990c66516c9b3ed7ada6891746ec0e0eecbe545 Settings app [3]: I4f0fc21268200c64d12b31ca54416acfbf62f37b InputMethodManagerService [4]: Ib58d2931cc8db3b88eab64352ba445be67eaec68 CTS permission2 Test: Modified InputMethodManagerService.updateSystemUiLocked() method to show IME switcher notification, and confirmed IME picker can be shown from notification bar. Test: Confirmed the following command causes error. $ adb shell am broadcast \ -a com.android.server.InputMethodManagerService.SHOW_INPUT_METHOD_PICKER java.lang.SecurityException: Permission Denial: not allowed to send broadcast com.android.server.InputMethodManagerService.SHOW_INPUT_METHOD_PICKER from pid=xxxx, uid=xxxx Fixes: 63644555 Bug: 64008672 Change-Id: Id36c8c34159bea8b72557b40bcf024d401f580b6 --- core/res/AndroidManifest.xml | 1 + .../server/InputMethodManagerService.java | 26 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index f36e1ad64ae5a..a0ee555149a25 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -545,6 +545,7 @@ + diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index a8e2f32272406..ce062aaa7d92f 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -50,6 +50,7 @@ import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlSerializer; import android.annotation.BinderThread; +import android.annotation.ColorInt; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -231,6 +232,13 @@ public class InputMethodManagerService extends IInputMethodManager.Stub int WIRED_AFFORDANCE = 1; } + /** + * A protected broadcast intent action for internal use for {@link PendingIntent} in + * the notification. + */ + private static final String ACTION_SHOW_INPUT_METHOD_PICKER = + "com.android.server.InputMethodManagerService.SHOW_INPUT_METHOD_PICKER"; + final Context mContext; final Resources mRes; final Handler mHandler; @@ -836,6 +844,16 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } } else if (Intent.ACTION_LOCALE_CHANGED.equals(action)) { onActionLocaleChanged(); + } else if (ACTION_SHOW_INPUT_METHOD_PICKER.equals(action)) { + // ACTION_SHOW_INPUT_METHOD_PICKER action is a protected-broadcast and it is + // guaranteed to be send only from the system, so that there is no need for extra + // security check such as + // {@link #canShowInputMethodPickerLocked(IInputMethodClient)}. + mHandler.obtainMessage( + MSG_SHOW_IM_SUBTYPE_PICKER, + InputMethodManager.SHOW_IM_PICKER_MODE_INCLUDE_AUXILIARY_SUBTYPES, + 0 /* arg2 */) + .sendToTarget(); } else { Slog.w(TAG, "Unexpected intent " + intent); } @@ -1285,6 +1303,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub Bundle extras = new Bundle(); extras.putBoolean(Notification.EXTRA_ALLOW_DURING_SETUP, true); + @ColorInt final int accentColor = mContext.getColor( + com.android.internal.R.color.system_notification_accent_color); mImeSwitcherNotification = new Notification.Builder(mContext, SystemNotificationChannels.VIRTUAL_KEYBOARD) .setSmallIcon(com.android.internal.R.drawable.ic_notification_ime_default) @@ -1292,9 +1312,10 @@ public class InputMethodManagerService extends IInputMethodManager.Stub .setOngoing(true) .addExtras(extras) .setCategory(Notification.CATEGORY_SYSTEM) - .setColor(com.android.internal.R.color.system_notification_accent_color); + .setColor(accentColor); - Intent intent = new Intent(Settings.ACTION_SHOW_INPUT_METHOD_PICKER); + Intent intent = new Intent(ACTION_SHOW_INPUT_METHOD_PICKER) + .setPackage(mContext.getPackageName()); mImeSwitchPendingIntent = PendingIntent.getBroadcast(mContext, 0, intent, 0); mShowOngoingImeSwitcherForPhones = false; @@ -1445,6 +1466,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub broadcastFilter.addAction(Intent.ACTION_USER_REMOVED); broadcastFilter.addAction(Intent.ACTION_SETTING_RESTORED); broadcastFilter.addAction(Intent.ACTION_LOCALE_CHANGED); + broadcastFilter.addAction(ACTION_SHOW_INPUT_METHOD_PICKER); mContext.registerReceiver(new ImmsBroadcastReceiver(), broadcastFilter); buildInputMethodListLocked(true /* resetDefaultEnabledIme */);