From 16a691f87b8b00fe32f2faf1d7e222d770da7745 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 24 Sep 2020 19:49:35 +0200 Subject: [PATCH 01/24] Don't dump binder proxies with the lock held. There's still a path where the AMS lock is held when we're trying to dump binder proxies; in this case, just don't dump any proxies at all, since there's no safe way to do it with the AMS lock held. Bug: 168353824 Test: adb shell am dump binder-proxies Change-Id: Ia9b23821a953ac73fe6d67cc169998548680083d (cherry picked from commit 7174ebbf55f9fe2dc53dbf17e812298b220d98db) --- .../com/android/server/am/ActivityManagerService.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 491579a5a29..bf92f4e03c4 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -10530,7 +10530,7 @@ protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { private void dumpEverything(FileDescriptor fd, PrintWriter pw, String[] args, int opti, boolean dumpAll, String dumpPackage, boolean dumpClient, boolean dumpNormalPriority, - int dumpAppId) { + int dumpAppId, boolean dumpProxies) { ActiveServices.ServiceDumper sdumper; @@ -10589,7 +10589,7 @@ private void dumpEverything(FileDescriptor fd, PrintWriter pw, String[] args, in } sdumper.dumpWithClient(); } - if (dumpPackage == null) { + if (dumpPackage == null && dumpProxies) { // Intentionally dropping the lock for this, because dumpBinderProxies() will make many // outgoing binder calls to retrieve interface descriptors; while that is system code, // there is nothing preventing an app from overriding this implementation by talking to @@ -10998,13 +10998,14 @@ private void doDump(FileDescriptor fd, PrintWriter pw, String[] args, boolean us // dumpEverything() will take the lock when needed, and momentarily drop // it for dumping client state. dumpEverything(fd, pw, args, opti, dumpAll, dumpPackage, dumpClient, - dumpNormalPriority, dumpAppId); + dumpNormalPriority, dumpAppId, true /* dumpProxies */); } else { // Take the lock here, so we get a consistent state for the entire dump; - // dumpEverything() will take the lock as well, but that is fine. + // dumpEverything() will take the lock as well, which is fine for everything + // except dumping proxies, which can take a long time; exclude them. synchronized(this) { dumpEverything(fd, pw, args, opti, dumpAll, dumpPackage, dumpClient, - dumpNormalPriority, dumpAppId); + dumpNormalPriority, dumpAppId, false /* dumpProxies */); } } } From 11725e1206645e567cfdd70100d64d1e0a85180d Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Fri, 31 Jul 2020 18:46:28 -0700 Subject: [PATCH 02/24] Require permission to create trusted displays Bug: 162627132 Test: atest VirtualDisplayTest#testTrustedVirtualDisplay Test: atest frameworks/base/packages/SystemUI/tests/src/com/android/systemui/bubbles Test: atest DisplayTest Test: atest VirtualDisplayTest#testTrustedVirtualDisplay Test: atest VirtualDisplayTest#testUntrustedSysDecorVirtualDisplay Test: adb logcat -b events Change-Id: Id06b2013ef5fdeadf321f14f8b611c733031d54d Merged-In: Id06b2013ef5fdeadf321f14f8b611c733031d54d (cherry picked from commit ef7b1333f0e87cd07af115719b94dd37e2de54dc) --- core/java/android/app/ActivityView.java | 15 +++++++++++++-- .../window/VirtualDisplayTaskEmbedder.java | 9 ++++++++- core/tests/coretests/AndroidManifest.xml | 1 + .../hardware/display/VirtualDisplayTest.java | 19 +++++++++++++++++++ .../systemui/bubbles/BubbleExpandedView.java | 2 +- .../server/display/DisplayManagerService.java | 15 +++++++++++---- 6 files changed, 53 insertions(+), 8 deletions(-) diff --git a/core/java/android/app/ActivityView.java b/core/java/android/app/ActivityView.java index 98a23f2b007..3cb6293f070 100644 --- a/core/java/android/app/ActivityView.java +++ b/core/java/android/app/ActivityView.java @@ -105,7 +105,8 @@ public ActivityView(Context context, AttributeSet attrs, int defStyle, public ActivityView( @NonNull Context context, @NonNull AttributeSet attrs, int defStyle, boolean singleTaskInstance, boolean usePublicVirtualDisplay) { - this(context, attrs, defStyle, singleTaskInstance, usePublicVirtualDisplay, false); + this(context, attrs, defStyle, singleTaskInstance, usePublicVirtualDisplay, + false /* disableSurfaceViewBackgroundLayer */); } /** @hide */ @@ -113,12 +114,22 @@ public ActivityView( @NonNull Context context, @NonNull AttributeSet attrs, int defStyle, boolean singleTaskInstance, boolean usePublicVirtualDisplay, boolean disableSurfaceViewBackgroundLayer) { + this(context, attrs, defStyle, singleTaskInstance, usePublicVirtualDisplay, + disableSurfaceViewBackgroundLayer, false /* useTrustedDisplay */); + } + + // TODO(b/162901735): Refactor ActivityView with Builder + /** @hide */ + public ActivityView( + @NonNull Context context, @NonNull AttributeSet attrs, int defStyle, + boolean singleTaskInstance, boolean usePublicVirtualDisplay, + boolean disableSurfaceViewBackgroundLayer, boolean useTrustedDisplay) { super(context, attrs, defStyle); if (useTaskOrganizer()) { mTaskEmbedder = new TaskOrganizerTaskEmbedder(context, this); } else { mTaskEmbedder = new VirtualDisplayTaskEmbedder(context, this, singleTaskInstance, - usePublicVirtualDisplay); + usePublicVirtualDisplay, useTrustedDisplay); } mSurfaceView = new SurfaceView(context, null, 0, 0, disableSurfaceViewBackgroundLayer); // Since ActivityView#getAlpha has been overridden, we should use parent class's alpha diff --git a/core/java/android/window/VirtualDisplayTaskEmbedder.java b/core/java/android/window/VirtualDisplayTaskEmbedder.java index 9ccb4c17215..9013da36007 100644 --- a/core/java/android/window/VirtualDisplayTaskEmbedder.java +++ b/core/java/android/window/VirtualDisplayTaskEmbedder.java @@ -19,6 +19,7 @@ import static android.hardware.display.DisplayManager.VIRTUAL_DISPLAY_FLAG_DESTROY_CONTENT_ON_REMOVAL; import static android.hardware.display.DisplayManager.VIRTUAL_DISPLAY_FLAG_OWN_CONTENT_ONLY; import static android.hardware.display.DisplayManager.VIRTUAL_DISPLAY_FLAG_PUBLIC; +import static android.hardware.display.DisplayManager.VIRTUAL_DISPLAY_FLAG_TRUSTED; import static android.view.Display.INVALID_DISPLAY; import android.app.ActivityManager; @@ -63,6 +64,7 @@ public class VirtualDisplayTaskEmbedder extends TaskEmbedder { private int mDisplayDensityDpi; private final boolean mSingleTaskInstance; private final boolean mUsePublicVirtualDisplay; + private final boolean mUseTrustedDisplay; private VirtualDisplay mVirtualDisplay; private Insets mForwardedInsets; private DisplayMetrics mTmpDisplayMetrics; @@ -77,10 +79,12 @@ public class VirtualDisplayTaskEmbedder extends TaskEmbedder { * only applicable if virtual displays are used */ public VirtualDisplayTaskEmbedder(Context context, VirtualDisplayTaskEmbedder.Host host, - boolean singleTaskInstance, boolean usePublicVirtualDisplay) { + boolean singleTaskInstance, boolean usePublicVirtualDisplay, + boolean useTrustedDisplay) { super(context, host); mSingleTaskInstance = singleTaskInstance; mUsePublicVirtualDisplay = usePublicVirtualDisplay; + mUseTrustedDisplay = useTrustedDisplay; } /** @@ -103,6 +107,9 @@ public boolean onInitialize() { if (mUsePublicVirtualDisplay) { virtualDisplayFlags |= VIRTUAL_DISPLAY_FLAG_PUBLIC; } + if (mUseTrustedDisplay) { + virtualDisplayFlags |= VIRTUAL_DISPLAY_FLAG_TRUSTED; + } mVirtualDisplay = displayManager.createVirtualDisplay( DISPLAY_NAME + "@" + System.identityHashCode(this), mHost.getWidth(), diff --git a/core/tests/coretests/AndroidManifest.xml b/core/tests/coretests/AndroidManifest.xml index 5c2841aff1d..7597e873215 100644 --- a/core/tests/coretests/AndroidManifest.xml +++ b/core/tests/coretests/AndroidManifest.xml @@ -129,6 +129,7 @@ + diff --git a/core/tests/coretests/src/android/hardware/display/VirtualDisplayTest.java b/core/tests/coretests/src/android/hardware/display/VirtualDisplayTest.java index daf61397635..0f6284d22d1 100644 --- a/core/tests/coretests/src/android/hardware/display/VirtualDisplayTest.java +++ b/core/tests/coretests/src/android/hardware/display/VirtualDisplayTest.java @@ -247,6 +247,25 @@ public void testSecurePublicPresentationVirtualDisplay() throws Exception { assertDisplayUnregistered(display); } + /** + * Ensures that an application can create a trusted virtual display with the permission + * {@code ADD_TRUSTED_DISPLAY}. + */ + public void testTrustedVirtualDisplay() throws Exception { + VirtualDisplay virtualDisplay = mDisplayManager.createVirtualDisplay(NAME, + WIDTH, HEIGHT, DENSITY, mSurface, + DisplayManager.VIRTUAL_DISPLAY_FLAG_TRUSTED); + assertNotNull("virtual display must not be null", virtualDisplay); + + Display display = virtualDisplay.getDisplay(); + try { + assertDisplayRegistered(display, Display.FLAG_PRIVATE | Display.FLAG_TRUSTED); + } finally { + virtualDisplay.release(); + } + assertDisplayUnregistered(display); + } + private void assertDisplayRegistered(Display display, int flags) { assertNotNull("display object must not be null", display); assertTrue("display must be valid", display.isValid()); diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java index 1e556a3ed40..58d5776543a 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java @@ -301,7 +301,7 @@ protected void onFinishInflate() { mActivityView = new ActivityView(mContext, null /* attrs */, 0 /* defStyle */, true /* singleTaskInstance */, false /* usePublicVirtualDisplay*/, - true /* disableSurfaceViewBackgroundLayer */); + true /* disableSurfaceViewBackgroundLayer */, true /* useTrustedDisplay */); // Set ActivityView's alpha value as zero, since there is no view content to be shown. setContentVisibility(false); diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 24661d69a78..852868616af 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -86,6 +86,7 @@ import android.os.UserManager; import android.provider.Settings; import android.text.TextUtils; +import android.util.EventLog; import android.util.IntArray; import android.util.Pair; import android.util.Slog; @@ -2191,10 +2192,16 @@ public int createVirtualDisplay(VirtualDisplayConfig virtualDisplayConfig, } } - if (callingUid == Process.SYSTEM_UID - || checkCallingPermission(ADD_TRUSTED_DISPLAY, "createVirtualDisplay()")) { - flags |= VIRTUAL_DISPLAY_FLAG_TRUSTED; - } else { + if (callingUid != Process.SYSTEM_UID && (flags & VIRTUAL_DISPLAY_FLAG_TRUSTED) != 0) { + if (!checkCallingPermission(ADD_TRUSTED_DISPLAY, "createVirtualDisplay()")) { + EventLog.writeEvent(0x534e4554, "162627132", callingUid, + "Attempt to create a trusted display without holding permission!"); + throw new SecurityException("Requires ADD_TRUSTED_DISPLAY permission to " + + "create a trusted virtual display."); + } + } + + if ((flags & VIRTUAL_DISPLAY_FLAG_TRUSTED) == 0) { flags &= ~VIRTUAL_DISPLAY_FLAG_SHOULD_SHOW_SYSTEM_DECORATIONS; } From cdb913418a5a19c253daff3d6d9c6c2fc5ff0d61 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Thu, 13 Aug 2020 12:21:15 +0100 Subject: [PATCH 03/24] Validate user-supplied URIs in DocumentsProvider calls Some URIs are used without validating their authorities which can lead to exploitation by malicious apps. Bug: 157294893 Test: Manual using test app in b/157294893 Change-Id: I799509ed5ff7e69140e84d796fe7f96d9dbfd32f Merged-In: I799509ed5ff7e69140e84d796fe7f96d9dbfd32f (cherry picked from commit 75f984bd32a3ee8115d5cea09ab1bd237537ab54) (cherry picked from commit e4bb1d7b6cd538acf423c7d8864dd26819fe8757) --- .../android/provider/DocumentsProvider.java | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java index 327bca268a7..91b591c7b77 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -232,6 +232,10 @@ private void enforceTree(Uri documentUri) { } } + private Uri validateIncomingNullableUri(@Nullable Uri uri) { + return uri == null ? null : validateIncomingUri(uri); + } + /** * Create a new document and return its newly generated * {@link Document#COLUMN_DOCUMENT_ID}. You must allocate a new @@ -1076,11 +1080,18 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) final Context context = getContext(); final Bundle out = new Bundle(); + final Uri extraUri = validateIncomingNullableUri( + extras.getParcelable(DocumentsContract.EXTRA_URI)); + final Uri extraTargetUri = validateIncomingNullableUri( + extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI)); + final Uri extraParentUri = validateIncomingNullableUri( + extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI)); + if (METHOD_EJECT_ROOT.equals(method)) { // Given that certain system apps can hold MOUNT_UNMOUNT permission, but only apps // signed with platform signature can hold MANAGE_DOCUMENTS, we are going to check for // MANAGE_DOCUMENTS or associated URI permission here instead - final Uri rootUri = extras.getParcelable(DocumentsContract.EXTRA_URI); + final Uri rootUri = extraUri; enforceWritePermissionInner(rootUri, getCallingPackage(), getCallingAttributionTag(), null); @@ -1090,7 +1101,7 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) return out; } - final Uri documentUri = extras.getParcelable(DocumentsContract.EXTRA_URI); + final Uri documentUri = extraUri; final String authority = documentUri.getAuthority(); final String documentId = DocumentsContract.getDocumentId(documentUri); @@ -1106,7 +1117,7 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) enforceReadPermissionInner(documentUri, getCallingPackage(), getCallingAttributionTag(), null); - final Uri childUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); + final Uri childUri = extraTargetUri; final String childAuthority = childUri.getAuthority(); final String childId = DocumentsContract.getDocumentId(childUri); @@ -1173,7 +1184,7 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) revokeDocumentPermission(documentId); } else if (METHOD_COPY_DOCUMENT.equals(method)) { - final Uri targetUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); + final Uri targetUri = extraTargetUri; final String targetId = DocumentsContract.getDocumentId(targetUri); enforceReadPermissionInner(documentUri, getCallingPackage(), @@ -1197,9 +1208,9 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) } } else if (METHOD_MOVE_DOCUMENT.equals(method)) { - final Uri parentSourceUri = extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI); + final Uri parentSourceUri = extraParentUri; final String parentSourceId = DocumentsContract.getDocumentId(parentSourceUri); - final Uri targetUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); + final Uri targetUri = extraTargetUri; final String targetId = DocumentsContract.getDocumentId(targetUri); enforceWritePermissionInner(documentUri, getCallingPackage(), @@ -1225,7 +1236,7 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) } } else if (METHOD_REMOVE_DOCUMENT.equals(method)) { - final Uri parentSourceUri = extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI); + final Uri parentSourceUri = extraParentUri; final String parentSourceId = DocumentsContract.getDocumentId(parentSourceUri); enforceReadPermissionInner(parentSourceUri, getCallingPackage(), From 0b4cd450afbe085def06025b9ac1f6996217bfcb Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Wed, 12 Aug 2020 17:34:22 +0100 Subject: [PATCH 04/24] Validate user-supplied tree URIs in DocumentsProvider calls Currently we only validate DocumentsContract.EXTRA_URI, this change validates other URIs suchs as DocumentsContract.EXTRA_TARGET_URI and DocumentsContract.EXTRA_PARENT_URI as well Bug: 157320716 Test: Manually using the test app in b/157320716#comment1 Change-Id: I90fd1e62aa7dc333bf32eb80ccc5b181a1d54e41 Merged-In: I90fd1e62aa7dc333bf32eb80ccc5b181a1d54e41 (cherry picked from commit b9f4fb792812f9a38ac54e69be6f121f7367c017) (cherry picked from commit eca247f2d33b18d14e0568512a7ee003cbbcd4a9) --- .../android/provider/DocumentsProvider.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java index 91b591c7b77..4e1f81919c7 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -218,8 +218,15 @@ public boolean isChildDocument(String parentDocumentId, String documentId) { } /** {@hide} */ - private void enforceTree(Uri documentUri) { - if (isTreeUri(documentUri)) { + private void enforceTreeForExtraUris(Bundle extras) { + enforceTree(extras.getParcelable(DocumentsContract.EXTRA_URI)); + enforceTree(extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI)); + enforceTree(extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI)); + } + + /** {@hide} */ + private void enforceTree(@Nullable Uri documentUri) { + if (documentUri != null && isTreeUri(documentUri)) { final String parent = getTreeDocumentId(documentUri); final String child = getDocumentId(documentUri); if (Objects.equals(parent, child)) { @@ -1080,6 +1087,9 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) final Context context = getContext(); final Bundle out = new Bundle(); + // If the URI is a tree URI performs some validation. + enforceTreeForExtraUris(extras); + final Uri extraUri = validateIncomingNullableUri( extras.getParcelable(DocumentsContract.EXTRA_URI)); final Uri extraTargetUri = validateIncomingNullableUri( @@ -1110,9 +1120,6 @@ private Bundle callUnchecked(String method, String arg, Bundle extras) "Requested authority " + authority + " doesn't match provider " + mAuthority); } - // If the URI is a tree URI performs some validation. - enforceTree(documentUri); - if (METHOD_IS_CHILD_DOCUMENT.equals(method)) { enforceReadPermissionInner(documentUri, getCallingPackage(), getCallingAttributionTag(), null); From af35aa5ac57a8c7c4534d82d8cd6cfb4f049bbfe Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Tue, 26 May 2020 19:02:59 -0700 Subject: [PATCH 05/24] [BACKPORT] Improve location checks in TelephonyRegistry Improve location checking for apps targeting SDK28 or earlier. Bug: 158484422 Test: (cts) atest TelephonyLocationTests; atest PhoneStateListenerTest Merged-In: I8caa3ea409836ce8469d8d8210b481a0284594f2 Change-Id: I8caa3ea409836ce8469d8d8210b481a0284594f2 (cherry picked from commit 4e0c7d16fd76bd7743a7f46ba63c75e8c65d63be) (cherry picked from commit cc584e777db03316f98298a18a1844e51592ebef) --- .../com/android/server/TelephonyRegistry.java | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/TelephonyRegistry.java b/services/core/java/com/android/server/TelephonyRegistry.java index 23bf955ba9a..c4f1c805398 100644 --- a/services/core/java/com/android/server/TelephonyRegistry.java +++ b/services/core/java/com/android/server/TelephonyRegistry.java @@ -310,11 +310,10 @@ public boolean isRegistrationLimitEnabledInPlatformCompat(int uid) { private List> mPreciseDataConnectionStates = new ArrayList>(); - static final int ENFORCE_COARSE_LOCATION_PERMISSION_MASK = - PhoneStateListener.LISTEN_REGISTRATION_FAILURE - | PhoneStateListener.LISTEN_BARRING_INFO; - - static final int ENFORCE_FINE_LOCATION_PERMISSION_MASK = + // Starting in Q, almost all cellular location requires FINE location enforcement. + // Prior to Q, cellular was available with COARSE location enforcement. Bits in this + // list will be checked for COARSE on apps targeting P or earlier and FINE on Q or later. + static final int ENFORCE_LOCATION_PERMISSION_MASK = PhoneStateListener.LISTEN_CELL_LOCATION | PhoneStateListener.LISTEN_CELL_INFO | PhoneStateListener.LISTEN_REGISTRATION_FAILURE @@ -371,7 +370,7 @@ public void handleMessage(Message msg) { + " newDefaultPhoneId=" + newDefaultPhoneId); } - //Due to possible risk condition,(notify call back using the new + //Due to possible race condition,(notify call back using the new //defaultSubId comes before new defaultSubId update) we need to recall all //possible missed notify callback synchronized (mRecords) { @@ -904,7 +903,8 @@ private void listen(String callingPackage, @Nullable String callingFeatureId, if (validateEventsAndUserLocked(r, PhoneStateListener.LISTEN_CELL_LOCATION)) { try { if (DBG_LOC) log("listen: mCellIdentity = " + mCellIdentity[phoneId]); - if (checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + if (checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE) + && checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { // null will be translated to empty CellLocation object in client. r.callback.onCellLocationChanged(mCellIdentity[phoneId]); } @@ -959,7 +959,8 @@ private void listen(String callingPackage, @Nullable String callingFeatureId, try { if (DBG_LOC) log("listen: mCellInfo[" + phoneId + "] = " + mCellInfo.get(phoneId)); - if (checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + if (checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE) + && checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { r.callback.onCellInfoChanged(mCellInfo.get(phoneId)); } } catch (RemoteException ex) { @@ -1513,7 +1514,8 @@ public void notifyCellInfoForSubscriber(int subId, List cellInfo) { for (Record r : mRecords) { if (validateEventsAndUserLocked(r, PhoneStateListener.LISTEN_CELL_INFO) && idMatch(r.subId, subId, phoneId) && - checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + (checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE) + && checkFineLocationAccess(r, Build.VERSION_CODES.Q))) { try { if (DBG_LOC) { log("notifyCellInfoForSubscriber: mCellInfo=" + cellInfo @@ -1845,7 +1847,8 @@ public void notifyCellLocationForSubscriber(int subId, CellIdentity cellLocation for (Record r : mRecords) { if (validateEventsAndUserLocked(r, PhoneStateListener.LISTEN_CELL_LOCATION) && idMatch(r.subId, subId, phoneId) && - checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + (checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE) + && checkFineLocationAccess(r, Build.VERSION_CODES.Q))) { try { if (DBG_LOC) { log("notifyCellLocation: cellLocation=" + cellLocation @@ -2624,19 +2627,13 @@ private boolean checkListenerPermission(int events, int subId, String callingPac .setCallingPid(Binder.getCallingPid()) .setCallingUid(Binder.getCallingUid()); - boolean shouldCheckLocationPermissions = false; - if ((events & ENFORCE_COARSE_LOCATION_PERMISSION_MASK) != 0) { - locationQueryBuilder.setMinSdkVersionForCoarse(0); - shouldCheckLocationPermissions = true; - } - - if ((events & ENFORCE_FINE_LOCATION_PERMISSION_MASK) != 0) { + if ((events & ENFORCE_LOCATION_PERMISSION_MASK) != 0) { // Everything that requires fine location started in Q. So far... locationQueryBuilder.setMinSdkVersionForFine(Build.VERSION_CODES.Q); - shouldCheckLocationPermissions = true; - } + // If we're enforcing fine starting in Q, we also want to enforce coarse even for + // older SDK versions. + locationQueryBuilder.setMinSdkVersionForCoarse(0); - if (shouldCheckLocationPermissions) { LocationAccessPolicy.LocationPermissionResult result = LocationAccessPolicy.checkLocationPermission( mContext, locationQueryBuilder.build()); @@ -2803,8 +2800,16 @@ private void checkPossibleMissNotify(Record r, int phoneId) { try { if (VDBG) log("checkPossibleMissNotify: onServiceStateChanged state=" + mServiceState[phoneId]); - r.callback.onServiceStateChanged( - new ServiceState(mServiceState[phoneId])); + ServiceState ss = new ServiceState(mServiceState[phoneId]); + if (checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + r.callback.onServiceStateChanged(ss); + } else if (checkCoarseLocationAccess(r, Build.VERSION_CODES.Q)) { + r.callback.onServiceStateChanged( + ss.createLocationInfoSanitizedCopy(false)); + } else { + r.callback.onServiceStateChanged( + ss.createLocationInfoSanitizedCopy(true)); + } } catch (RemoteException ex) { mRemoveList.add(r.binder); } @@ -2849,7 +2854,8 @@ private void checkPossibleMissNotify(Record r, int phoneId) { log("checkPossibleMissNotify: onCellInfoChanged[" + phoneId + "] = " + mCellInfo.get(phoneId)); } - if (checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + if (checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE) + && checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { r.callback.onCellInfoChanged(mCellInfo.get(phoneId)); } } catch (RemoteException ex) { @@ -2915,7 +2921,8 @@ private void checkPossibleMissNotify(Record r, int phoneId) { log("checkPossibleMissNotify: onCellLocationChanged mCellIdentity = " + mCellIdentity[phoneId]); } - if (checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { + if (checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE) + && checkFineLocationAccess(r, Build.VERSION_CODES.Q)) { // null will be translated to empty CellLocation object in client. r.callback.onCellLocationChanged(mCellIdentity[phoneId]); } From a13cfc03e1030a59de4f4e1a6ced03a72353237f Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Fri, 31 Jul 2020 16:11:23 -0700 Subject: [PATCH 06/24] Protect bluetooth.device.action.ALIAS_CHANGED Bug: 157472962 Tag: #security Test: build Change-Id: I7737c4f1ad4bf5fec3127526465c78808de03693 (cherry picked from commit a6c09bb2c638cf8be5d98c9e6fcea8caa2b4cbe8) --- core/res/AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index 6a92a83c989..bb134f11319 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -144,7 +144,7 @@ - + From 61b620ad4f773e86c03e0719ae24268babcc62a9 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Mon, 5 Oct 2020 20:00:07 +0800 Subject: [PATCH 07/24] Fix storing the wrong value of mLockdown in setting When user is stopped, the Vpn#onUserStopped() will be called and the value of mLockdown will be set to false then store into setting. This is a wrong behavior because user doesn't change it, so for this kind of case, there is no need to store the value of mLockdown in setting. In fact, there is no need to call Vpn#saveAlwaysOnPackage() when user is stopped because there is nothing changed. Bug: 168500792 Test: atest FrameworksNetTests Change-Id: Ie85a347216614b7873bfdf199165d89527ada3a8 (cherry picked from commit 9226fc3723a477751705011cd7eecf063b1c3707) --- services/core/java/com/android/server/connectivity/Vpn.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 347beab0c42..bcb17bc9e2d 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -1601,7 +1601,7 @@ public void onUserRemoved(int userHandle) { */ public synchronized void onUserStopped() { // Switch off networking lockdown (if it was enabled) - setLockdown(false); + setVpnForcedLocked(false); mAlwaysOn = false; // Quit any active connections From cf9d5d571f97fdce3d100ece113694ec2cd4bd7a Mon Sep 17 00:00:00 2001 From: Howard Ro Date: Tue, 18 Aug 2020 17:13:40 -0700 Subject: [PATCH 08/24] Fix out of bound error of IncidentService Before this change, it was possible for the code to suffer an out of bound error. Bug: 150706572 Test: make Change-Id: I3e8d37f2ee3c942bc9b176edee043557b005c757 (cherry picked from commit 8ff5315e989c1348e313bcb8170b77adc80b2fce) (cherry picked from commit e592700068db0335c83934f191fc9efcbd8037ec) --- cmds/incidentd/src/IncidentService.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp index dc1612575f3..13bf197aa9d 100644 --- a/cmds/incidentd/src/IncidentService.cpp +++ b/cmds/incidentd/src/IncidentService.cpp @@ -554,6 +554,10 @@ status_t IncidentService::command(FILE* in, FILE* out, FILE* err, Vector Date: Wed, 16 Sep 2020 14:10:21 +0100 Subject: [PATCH 09/24] Do not re-initialize synthetic password A bug was introduced in R where LSS ends up regenerating SP when an escrow token is being auto-activated on unsecured user, due to a logic error in shouldMigrateToSyntheticPasswordLocked(). Fix the bug and add some safeguards as well as unit test to prevent future regressions. Bug: 168692734 Test: atest com.android.server.locksettings Change-Id: If35f2fd26b49faf6e3d0d75c10b1b3bb95f247c2 (cherry picked from commit efc1d53df3a2e7116d7ed83bca9bf8e384d32740) (cherry picked from commit 2d51788b08aa85afdb27af4f4586ac40dc949097) --- .../locksettings/LockSettingsService.java | 7 ++++++- .../locksettings/SyntheticPasswordTests.java | 18 ++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 69b02ceb241..f6308202ab6 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -113,6 +113,7 @@ import com.android.internal.notification.SystemNotificationChannels; import com.android.internal.util.DumpUtils; import com.android.internal.util.IndentingPrintWriter; +import com.android.internal.util.Preconditions; import com.android.internal.widget.ICheckCredentialProgressCallback; import com.android.internal.widget.ILockSettings; import com.android.internal.widget.LockPatternUtils; @@ -2618,6 +2619,10 @@ private void callToAuthSecretIfNeeded(@UserIdInt int userId, protected AuthenticationToken initializeSyntheticPasswordLocked(byte[] credentialHash, LockscreenCredential credential, int userId) { Slog.i(TAG, "Initialize SyntheticPassword for user: " + userId); + Preconditions.checkState( + getSyntheticPasswordHandleLocked(userId) == SyntheticPasswordManager.DEFAULT_HANDLE, + "Cannot reinitialize SP"); + final AuthenticationToken auth = mSpManager.newSyntheticPasswordAndSid( getGateKeeperService(), credentialHash, credential, userId); if (auth == null) { @@ -2678,7 +2683,7 @@ private boolean isSyntheticPasswordBasedCredentialLocked(int userId) { @VisibleForTesting protected boolean shouldMigrateToSyntheticPasswordLocked(int userId) { - return true; + return getSyntheticPasswordHandleLocked(userId) == SyntheticPasswordManager.DEFAULT_HANDLE; } private VerifyCredentialResponse spBasedDoVerifyCredential(LockscreenCredential userCredential, diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index ba851992cba..2c2fdcaab34 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -519,10 +519,24 @@ public void testPasswordChange_NoOrphanedFilesLeft() throws Exception { LockscreenCredential password = newPassword("password"); initializeCredentialUnderSP(password, PRIMARY_USER_ID); assertTrue(mService.setLockCredential(password, password, PRIMARY_USER_ID)); + assertNoOrphanedFilesLeft(PRIMARY_USER_ID); + } + + @Test + public void testAddingEscrowToken_NoOrphanedFilesLeft() throws Exception { + final byte[] token = "some-high-entropy-secure-token".getBytes(); + for (int i = 0; i < 16; i++) { + long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); + assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); + mLocalService.removeEscrowToken(handle, PRIMARY_USER_ID); + } + assertNoOrphanedFilesLeft(PRIMARY_USER_ID); + } + private void assertNoOrphanedFilesLeft(int userId) { String handleString = String.format("%016x", - mService.getSyntheticPasswordHandleLocked(PRIMARY_USER_ID)); - File directory = mStorage.getSyntheticPasswordDirectoryForUser(PRIMARY_USER_ID); + mService.getSyntheticPasswordHandleLocked(userId)); + File directory = mStorage.getSyntheticPasswordDirectoryForUser(userId); for (File file : directory.listFiles()) { String[] parts = file.getName().split("\\."); if (!parts[0].equals(handleString) && !parts[0].equals("0000000000000000")) { From a37060dbba3ccdbb3a9385a8e51a76b5ea1124d9 Mon Sep 17 00:00:00 2001 From: Edwin Wong Date: Mon, 28 Sep 2020 21:07:23 -0700 Subject: [PATCH 10/24] Use shared libdrmframeworkcommon. The libdrmframeworkcommon was statically linked to multiple libraries used by libfwdlockengine. When the shared libraries closes, the same block of static memory will be freed twice. Test: CTS forwardlock tests atest CtsDrmTestCases Bug: 155647761 Change-Id: I45113549772d48e925082d15659b1409cbed6499 (cherry picked from commit 4ed2e6b76dc955a400ed252ce162ee6335276858) --- drm/jni/Android.bp | 1 + 1 file changed, 1 insertion(+) diff --git a/drm/jni/Android.bp b/drm/jni/Android.bp index 1e33f0ea509..68757d86fb8 100644 --- a/drm/jni/Android.bp +++ b/drm/jni/Android.bp @@ -21,6 +21,7 @@ cc_library_shared { shared_libs: [ "libdrmframework", + "libdrmframeworkcommon", "liblog", "libutils", "libandroid_runtime", From 11d7861456ddf42d2c22f02dc300cca80bca1de4 Mon Sep 17 00:00:00 2001 From: Tiger Huang Date: Wed, 14 Oct 2020 11:40:09 +0000 Subject: [PATCH 11/24] DO NOT MERGE: Revert "Don't let IME window fit status bar" This reverts commit 3cd311415ba00d5d75660a8607d1ece5e68f0534. Reason for revert: The CL causes the regression b/170474494 And it also makes status bar color incorrect while FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS is cleared Fix: 170474494 Change-Id: I26bed08456197721d07f2fab563be0c54e43efd2 (cherry picked from commit 427bdc1c86904649dc7c12cf917c05d61f2fe3fa) --- .../InputMethodService.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java index e0195e4eafc..4f0c84e586a 100644 --- a/core/java/android/inputmethodservice/InputMethodService.java +++ b/core/java/android/inputmethodservice/InputMethodService.java @@ -16,11 +16,11 @@ package android.inputmethodservice; -import static android.graphics.Color.TRANSPARENT; import static android.view.ViewGroup.LayoutParams.MATCH_PARENT; import static android.view.ViewGroup.LayoutParams.WRAP_CONTENT; import static android.view.ViewRootImpl.NEW_INSETS_MODE_NONE; import static android.view.WindowInsets.Type.navigationBars; +import static android.view.WindowInsets.Type.statusBars; import static android.view.WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS; import static java.lang.annotation.RetentionPolicy.SOURCE; @@ -69,6 +69,7 @@ import android.view.ViewRootImpl; import android.view.ViewTreeObserver; import android.view.Window; +import android.view.WindowInsets; import android.view.WindowInsets.Side; import android.view.WindowManager; import android.view.animation.AnimationUtils; @@ -1202,22 +1203,25 @@ public boolean enableHardwareAcceleration() { Context.LAYOUT_INFLATER_SERVICE); mWindow = new SoftInputWindow(this, "InputMethod", mTheme, null, null, mDispatcherState, WindowManager.LayoutParams.TYPE_INPUT_METHOD, Gravity.BOTTOM, false); - mWindow.getWindow().getAttributes().setFitInsetsTypes(navigationBars()); + mWindow.getWindow().getAttributes().setFitInsetsTypes(statusBars() | navigationBars()); mWindow.getWindow().getAttributes().setFitInsetsSides(Side.all() & ~Side.BOTTOM); mWindow.getWindow().getAttributes().setFitInsetsIgnoringVisibility(true); - // Our window will extend into the status bar area no matter the bar is visible or not. - // We don't want the ColorView to be visible when status bar is shown. - mWindow.getWindow().setStatusBarColor(TRANSPARENT); - - // Automotive devices may request the navigation bar to be hidden when the IME shows up - // (controlled via config_automotiveHideNavBarForKeyboard) in order to maximize the visible - // screen real estate. When this happens, the IME window should animate from the bottom of - // the screen to reduce the jank that happens from the lack of synchronization between the - // bottom system window and the IME window. + // IME layout should always be inset by navigation bar, no matter its current visibility, + // unless automotive requests it. Automotive devices may request the navigation bar to be + // hidden when the IME shows up (controlled via config_automotiveHideNavBarForKeyboard) + // in order to maximize the visible screen real estate. When this happens, the IME window + // should animate from the bottom of the screen to reduce the jank that happens from the + // lack of synchronization between the bottom system window and the IME window. if (mIsAutomotive && mAutomotiveHideNavBarForKeyboard) { mWindow.getWindow().setDecorFitsSystemWindows(false); } + mWindow.getWindow().getDecorView().setOnApplyWindowInsetsListener( + (v, insets) -> v.onApplyWindowInsets( + new WindowInsets.Builder(insets).setInsets( + navigationBars(), + insets.getInsetsIgnoringVisibility(navigationBars())) + .build())); // For ColorView in DecorView to work, FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS needs to be set // by default (but IME developers can opt this out later if they want a new behavior). From a185996c829a159bb27446697329b01464ab3c03 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 16 Jul 2020 16:49:06 -0700 Subject: [PATCH 12/24] Fix the issue provider can be wrong when requesting slice permission SlicePermissionActivity reads provider_pkg from intent, which can be modified at will. As a result user might see incorrect package name in the dialog granting slice permission. Bug: 159145361 Test: manual Merged-In: I8b66c02786df4096dad74b7e76255d5ddd1d609d Change-Id: I8b66c02786df4096dad74b7e76255d5ddd1d609d (cherry picked from commit 4344e632953b103910b48d43f4eb226b38ed5048) --- .../java/android/app/slice/SliceProvider.java | 1 + .../systemui/SlicePermissionActivity.java | 31 ++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/core/java/android/app/slice/SliceProvider.java b/core/java/android/app/slice/SliceProvider.java index bd1eea51f8a..46be54814dc 100644 --- a/core/java/android/app/slice/SliceProvider.java +++ b/core/java/android/app/slice/SliceProvider.java @@ -153,6 +153,7 @@ public abstract class SliceProvider extends ContentProvider { */ public static final String EXTRA_PKG = "pkg"; /** + * @Deprecated provider pkg is now being extracted in SlicePermissionActivity * @hide */ public static final String EXTRA_PROVIDER_PKG = "provider_pkg"; diff --git a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java index 449ed8c3bcd..1b241b74324 100644 --- a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java +++ b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java @@ -16,6 +16,7 @@ import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS; +import android.annotation.Nullable; import android.app.Activity; import android.app.AlertDialog; import android.app.slice.SliceManager; @@ -29,6 +30,7 @@ import android.net.Uri; import android.os.Bundle; import android.text.BidiFormatter; +import android.util.EventLog; import android.util.Log; import android.widget.CheckBox; import android.widget.TextView; @@ -50,10 +52,12 @@ protected void onCreate(Bundle savedInstanceState) { mUri = getIntent().getParcelableExtra(SliceProvider.EXTRA_BIND_URI); mCallingPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PKG); - mProviderPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG); try { PackageManager pm = getPackageManager(); + mProviderPkg = pm.resolveContentProvider(mUri.getAuthority(), + PackageManager.GET_META_DATA).applicationInfo.packageName; + verifyCallingPkg(); CharSequence app1 = BidiFormatter.getInstance().unicodeWrap(pm.getApplicationInfo( mCallingPkg, 0).loadSafeLabel(pm, PackageItemInfo.DEFAULT_MAX_LABEL_SIZE_PX, PackageItemInfo.SAFE_LABEL_FLAG_TRIM @@ -97,4 +101,29 @@ public void onClick(DialogInterface dialog, int which) { public void onDismiss(DialogInterface dialog) { finish(); } + + private void verifyCallingPkg() { + final String providerPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG); + if (providerPkg == null || mProviderPkg.equals(providerPkg)) return; + final String callingPkg = getCallingPkg(); + EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg), String.format( + "pkg %s (disguised as %s) attempted to request permission to show %s slices in %s", + callingPkg, providerPkg, mProviderPkg, mCallingPkg)); + } + + @Nullable + private String getCallingPkg() { + final Uri referrer = getReferrer(); + if (referrer == null) return null; + return referrer.getHost(); + } + + private int getUid(@Nullable final String pkg) { + if (pkg == null) return -1; + try { + return getPackageManager().getApplicationInfo(pkg, 0).uid; + } catch (NameNotFoundException e) { + } + return -1; + } } From 9ac71ec05aecd46d4569405a63f2033c06fe38ac Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Fri, 28 Aug 2020 14:55:48 -0700 Subject: [PATCH 13/24] Enforce permission checks in getting app exit reasons Bug: 165595677 Test: atest CtsSecurityTestCases:ActivityManagerTest Change-Id: Ia758d32bce6b2ac4c7145a96eccf68a962f0748b (cherry picked from commit e9b1dd415fed5415bc21abbd8e2f653fda5cf30c) --- .../java/com/android/server/am/ActivityManagerService.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 3b851800430..091c77ecaac 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -10459,12 +10459,10 @@ int enforceDumpPermissionForPackage(String packageName, int userId, int callingU } finally { Binder.restoreCallingIdentity(identity); } - if (uid == Process.INVALID_UID) { - return Process.INVALID_UID; - } + // If the uid is Process.INVALID_UID, the below 'if' check will be always true if (UserHandle.getAppId(uid) != UserHandle.getAppId(callingUid)) { // Requires the DUMP permission if the target package doesn't belong - // to the caller. + // to the caller or it doesn't exist. enforceCallingPermission(android.Manifest.permission.DUMP, function); } return uid; From 9588cd7de1f84a3ec8de273fb7d75921024189d8 Mon Sep 17 00:00:00 2001 From: Curtis Belmonte Date: Thu, 1 Oct 2020 17:57:27 -0700 Subject: [PATCH 14/24] DO NOT MERGE Check fingerprint client against top activity in auth callback Due to a race condition with activity task stack broadcasts, it's currently possible for fingerprint authentication to succeed for a non-top activity. This means, for example, that a malicious overlay could be drawn in order to mislead the user about what they are authenticating for. This commit addresses the issue by adding a check to the biometric authentication client interface that ensures the authenticating activity is on top at the time of authentication. Otherwise, the pending authentication will fail, as if an incorrect biometric had been presented. Test: Follow steps from b/159249069: 1. Install com.pro100svitlo.fingerprintauthdemo from the Play store. 2. Install the PoC attack app from b/159249069. 3. Start the PoC attack app and press the "Launch PoC attack" button. 4. Use fingerprint to authenticate while the overlay is showing. Before: Authentication succeeds, and a new activity is launched. After: Authentication fails, and no new activity is launched. Bug: 159249069 Change-Id: Ie5a0f8c3e9b92d348a78678a6ed192d440c45ffc Merged-In: I289d67e5c7055ed60f7a96725c523d07cd047b23 Merged-In: I3a810cd7e6b97333f648c978e44242662342ec57 (cherry picked from commit 09c1b8ebf9e58cd402ec6a7ae9b1948cf83982d1) --- .../biometrics/AuthenticationClient.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/services/core/java/com/android/server/biometrics/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/AuthenticationClient.java index edc8f15a9a0..8ca37b813b6 100644 --- a/services/core/java/com/android/server/biometrics/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/AuthenticationClient.java @@ -16,16 +16,22 @@ package com.android.server.biometrics; +import android.app.ActivityManager; +import android.app.ActivityTaskManager; +import android.content.ComponentName; import android.content.Context; +import android.content.pm.ApplicationInfo; import android.hardware.biometrics.BiometricAuthenticator; import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.BiometricsProtoEnums; import android.os.IBinder; import android.os.RemoteException; import android.security.KeyStore; +import android.util.EventLog; import android.util.Slog; import java.util.ArrayList; +import java.util.List; /** * A class to keep track of the authentication state for a given client. @@ -148,7 +154,54 @@ public boolean onAuthenticated(BiometricAuthenticator.Identifier identifier, + ", requireConfirmation: " + mRequireConfirmation + ", user: " + getTargetUserId()); + // Ensure authentication only succeeds if the client activity is on top or is keyguard. + boolean isBackgroundAuth = false; + if (authenticated && !Utils.isKeyguard(getContext(), getOwnerString())) { + try { + final List tasks = + ActivityTaskManager.getService().getTasks(1); + if (tasks == null || tasks.isEmpty()) { + Slog.e(TAG, "No running tasks reported"); + isBackgroundAuth = true; + } else { + final ComponentName topActivity = tasks.get(0).topActivity; + if (topActivity == null) { + Slog.e(TAG, "Unable to get top activity"); + isBackgroundAuth = true; + } else { + final String topPackage = topActivity.getPackageName(); + if (!topPackage.contentEquals(getOwnerString())) { + Slog.e(TAG, "Background authentication detected, top: " + topPackage + + ", client: " + this); + isBackgroundAuth = true; + } + } + } + } catch (RemoteException e) { + Slog.e(TAG, "Unable to get running tasks", e); + isBackgroundAuth = true; + } + } + + // Fail authentication if we can't confirm the client activity is on top. + if (isBackgroundAuth) { + Slog.e(TAG, "Failing possible background authentication"); + authenticated = false; + + // SafetyNet logging for exploitation attempts of b/159249069. + final ApplicationInfo appInfo = getContext().getApplicationInfo(); + EventLog.writeEvent(0x534e4554, "159249069", appInfo != null ? appInfo.uid : -1, + "Attempted background authentication"); + } + if (authenticated) { + // SafetyNet logging for b/159249069 if constraint is violated. + if (isBackgroundAuth) { + final ApplicationInfo appInfo = getContext().getApplicationInfo(); + EventLog.writeEvent(0x534e4554, "159249069", appInfo != null ? appInfo.uid : -1, + "Successful background authentication!"); + } + mAlreadyDone = true; if (listener != null) { From e237a83f95767f669b83508bb1f594091cbd6bac Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 8 Oct 2020 10:14:05 -0700 Subject: [PATCH 15/24] remove sensitive pii from safetynet logging Bug: 159145361 Test: manual Change-Id: I8f1be55971672c7e8f5aa8848f65b1b9d9f40fb5 Merged-In: I8f1be55971672c7e8f5aa8848f65b1b9d9f40fb5 (cherry picked from commit 3b6905bf6a39de7789f93a7ce6ca5d65a3fe589e) (cherry picked from commit 1d50760050a7c4e819d5a25d074dfaca04e68fde) --- .../src/com/android/systemui/SlicePermissionActivity.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java index 1b241b74324..57e656827f1 100644 --- a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java +++ b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java @@ -106,9 +106,7 @@ private void verifyCallingPkg() { final String providerPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG); if (providerPkg == null || mProviderPkg.equals(providerPkg)) return; final String callingPkg = getCallingPkg(); - EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg), String.format( - "pkg %s (disguised as %s) attempted to request permission to show %s slices in %s", - callingPkg, providerPkg, mProviderPkg, mCallingPkg)); + EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg)); } @Nullable From 0c17049d39b5a8867f030f6f36433564140e124a Mon Sep 17 00:00:00 2001 From: Eugene Susla Date: Mon, 12 Oct 2020 16:23:15 -0700 Subject: [PATCH 16/24] RESTRICT AUTOMERGE Fix CDM package check CDM was using a pckage check that returns a value intead of throwing, resulting in failing to throw on querying other package's associations Test: ensure attached bug no longer reproduces Bug: 167244818 Change-Id: I21319b6f5495dcae681541c76b847aad0c00b8ab (cherry picked from commit 30b022a8d207573aeaf735a33439884afe60c684) --- .../server/companion/CompanionDeviceManagerService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index d6759b3e2cc..29fc1674bab 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -372,7 +372,10 @@ private void checkCallerIsSystemOr(String pkg, int userId) throws RemoteExceptio checkArgument(getCallingUserId() == userId, "Must be called by either same user or system"); - mAppOpsManager.checkPackage(Binder.getCallingUid(), pkg); + int callingUid = Binder.getCallingUid(); + if (mAppOpsManager.checkPackage(callingUid, pkg) != AppOpsManager.MODE_ALLOWED) { + throw new SecurityException(pkg + " doesn't belong to uid " + callingUid); + } } @Override From 03da463b2a94c36e3b46f0a110ec43710b82d404 Mon Sep 17 00:00:00 2001 From: Nate Myren Date: Wed, 14 Oct 2020 15:14:04 -0700 Subject: [PATCH 17/24] Ensure permissions are revoked on state changes If a permission owner changes, or a permission level is upgraded, revoke the permission from all packages Test: Manual Bug: 154505240 Merged-In: I0dec9eb7c2fecd3147e33e04d3f79f6dffcf7721 Change-Id: I0dec9eb7c2fecd3147e33e04d3f79f6dffcf7721 (cherry picked from commit a28931a09814a89e1c55816c794c1e1f20dc0c91) (cherry picked from commit 84c1247a7e6d446822e5c72c9718db8373011313) --- .../server/pm/PackageManagerService.java | 24 ++++- .../server/pm/permission/BasePermission.java | 19 ++++ .../permission/PermissionManagerService.java | 90 +++++++++++++++++-- .../PermissionManagerServiceInternal.java | 16 +++- 4 files changed, 139 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index c20a912152c..99e1388dcd5 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -330,6 +330,7 @@ import com.android.internal.os.Zygote; import com.android.internal.telephony.CarrierAppUtils; import com.android.internal.util.ArrayUtils; +import com.android.internal.util.CollectionUtils; import com.android.internal.util.ConcurrentUtils; import com.android.internal.util.DumpUtils; import com.android.internal.util.FastXmlSerializer; @@ -12441,12 +12442,17 @@ private void commitPackageSettings(AndroidPackage pkg, mPermissionManager.addAllPermissionGroups(pkg, chatty); } + // If a permission has had its defining app changed, or it has had its protection + // upgraded, we need to revoke apps that hold it + final List permissionsWithChangedDefinition; // Don't allow ephemeral applications to define new permissions. if ((scanFlags & SCAN_AS_INSTANT_APP) != 0) { + permissionsWithChangedDefinition = null; Slog.w(TAG, "Permissions from package " + pkg.getPackageName() + " ignored: instant apps cannot define new permissions."); } else { - mPermissionManager.addAllPermissions(pkg, chatty); + permissionsWithChangedDefinition = + mPermissionManager.addAllPermissions(pkg, chatty); } int collectionSize = ArrayUtils.size(pkg.getInstrumentations()); @@ -12475,7 +12481,10 @@ private void commitPackageSettings(AndroidPackage pkg, } } - if (oldPkg != null) { + boolean hasOldPkg = oldPkg != null; + boolean hasPermissionDefinitionChanges = + !CollectionUtils.isEmpty(permissionsWithChangedDefinition); + if (hasOldPkg || hasPermissionDefinitionChanges) { // We need to call revokeRuntimePermissionsIfGroupChanged async as permission // revoke callbacks from this method might need to kill apps which need the // mPackages lock on a different thread. This would dead lock. @@ -12486,9 +12495,16 @@ private void commitPackageSettings(AndroidPackage pkg, // won't be granted yet, hence new packages are no problem. final ArrayList allPackageNames = new ArrayList<>(mPackages.keySet()); - AsyncTask.execute(() -> + AsyncTask.execute(() -> { + if (hasOldPkg) { mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg, - allPackageNames)); + allPackageNames); + } + if (hasPermissionDefinitionChanges) { + mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsWithChangedDefinition, allPackageNames); + } + }); } } diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index cfa0449aaf3..2d4b5012bf1 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -83,6 +83,8 @@ public final class BasePermission { final @PermissionType int type; + private boolean mPermissionDefinitionChanged; + String sourcePackageName; int protectionLevel; @@ -126,6 +128,11 @@ public int getProtectionLevel() { public String getSourcePackageName() { return sourcePackageName; } + + public boolean isPermissionDefinitionChanged() { + return mPermissionDefinitionChanged; + } + public int getType() { return type; } @@ -140,6 +147,10 @@ public void setPermission(@Nullable ParsedPermission perm) { this.perm = perm; } + public void setPermissionDefinitionChanged(boolean shouldOverride) { + mPermissionDefinitionChanged = shouldOverride; + } + public int[] computeGids(int userId) { if (perUser) { final int[] userGids = new int[gids.length]; @@ -322,6 +333,7 @@ static BasePermission createOrUpdate(PackageManagerInternal packageManagerIntern final PackageSettingBase pkgSetting = (PackageSettingBase) packageManagerInternal.getPackageSetting(pkg.getPackageName()); // Allow system apps to redefine non-system permissions + boolean ownerChanged = false; if (bp != null && !Objects.equals(bp.sourcePackageName, p.getPackageName())) { final boolean currentOwnerIsSystem; if (bp.perm == null) { @@ -347,6 +359,7 @@ static BasePermission createOrUpdate(PackageManagerInternal packageManagerIntern String msg = "New decl " + pkg + " of permission " + p.getName() + " is system; overriding " + bp.sourcePackageName; PackageManagerService.reportSettingsProblem(Log.WARN, msg); + ownerChanged = true; bp = null; } } @@ -354,6 +367,7 @@ static BasePermission createOrUpdate(PackageManagerInternal packageManagerIntern if (bp == null) { bp = new BasePermission(p.getName(), p.getPackageName(), TYPE_NORMAL); } + boolean wasNormal = bp.isNormal(); StringBuilder r = null; if (bp.perm == null) { if (bp.sourcePackageName == null @@ -397,6 +411,11 @@ static BasePermission createOrUpdate(PackageManagerInternal packageManagerIntern && Objects.equals(bp.perm.getName(), p.getName())) { bp.protectionLevel = p.getProtectionLevel(); } + if (bp.isRuntime() && (ownerChanged || wasNormal)) { + // If this is a runtime permission and the owner has changed, or this was a normal + // permission, then permission state should be cleaned up + bp.mPermissionDefinitionChanged = true; + } if (PackageManagerService.DEBUG_PACKAGE_SCANNING && r != null) { Log.d(TAG, " Permissions: " + r); } diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index 66d8b597426..3ffca028b1c 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -2344,8 +2344,74 @@ private void revokeRuntimePermissionsIfGroupChanged( } } - private void addAllPermissions(AndroidPackage pkg, boolean chatty) { + /** + * If permissions are upgraded to runtime, or their owner changes to the system, then any + * granted permissions must be revoked. + * + * @param permissionsToRevoke A list of permission names to revoke + * @param allPackageNames All package names + * @param permissionCallback Callback for permission changed + */ + private void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List permissionsToRevoke, + @NonNull ArrayList allPackageNames, + @NonNull PermissionCallback permissionCallback) { + + final int[] userIds = mUserManagerInt.getUserIds(); + final int numPermissions = permissionsToRevoke.size(); + final int numUserIds = userIds.length; + final int numPackages = allPackageNames.size(); + final int callingUid = Binder.getCallingUid(); + + for (int permNum = 0; permNum < numPermissions; permNum++) { + String permName = permissionsToRevoke.get(permNum); + BasePermission bp = mSettings.getPermission(permName); + if (bp == null || !bp.isRuntime()) { + continue; + } + for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) { + final int userId = userIds[userIdNum]; + for (int packageNum = 0; packageNum < numPackages; packageNum++) { + final String packageName = allPackageNames.get(packageNum); + final int uid = mPackageManagerInt.getPackageUid(packageName, 0, userId); + if (uid < Process.FIRST_APPLICATION_UID) { + // do not revoke from system apps + continue; + } + final int permissionState = checkPermissionImpl(permName, packageName, + userId); + final int flags = getPermissionFlags(permName, packageName, userId); + final int flagMask = FLAG_PERMISSION_SYSTEM_FIXED + | FLAG_PERMISSION_POLICY_FIXED + | FLAG_PERMISSION_GRANTED_BY_DEFAULT + | FLAG_PERMISSION_GRANTED_BY_ROLE; + if (permissionState == PackageManager.PERMISSION_GRANTED + && (flags & flagMask) == 0) { + EventLog.writeEvent(0x534e4554, "154505240", uid, + "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + EventLog.writeEvent(0x534e4554, "168319670", uid, + "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + Slog.e(TAG, "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + try { + revokeRuntimePermissionInternal(permName, packageName, + false, callingUid, userId, null, permissionCallback); + } catch (Exception e) { + Slog.e(TAG, "Could not revoke " + permName + " from " + + packageName, e); + } + } + } + } + bp.setPermissionDefinitionChanged(false); + } + } + + private List addAllPermissions(AndroidPackage pkg, boolean chatty) { final int N = ArrayUtils.size(pkg.getPermissions()); + ArrayList definitionChangedPermissions = new ArrayList<>(); for (int i=0; i permissionsToRevoke, + @NonNull ArrayList allPackageNames) { + PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsToRevoke, allPackageNames, mDefaultPermissionCallback); + } + @Override - public void addAllPermissions(AndroidPackage pkg, boolean chatty) { - PermissionManagerService.this.addAllPermissions(pkg, chatty); + public List addAllPermissions(AndroidPackage pkg, boolean chatty) { + return PermissionManagerService.this.addAllPermissions(pkg, chatty); } @Override public void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) { diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java index 2e83b23f57d..31a65ba35f5 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -253,13 +253,27 @@ public abstract void revokeRuntimePermissionsIfGroupChanged( @NonNull AndroidPackage oldPackage, @NonNull ArrayList allPackageNames); + /** + * Some permissions might have been owned by a non-system package, and the system then defined + * said permission. Some other permissions may one have been install permissions, but are now + * runtime or higher. These permissions should be revoked. + * + * @param permissionsToRevoke A list of permission names to revoke + * @param allPackageNames All packages + */ + public abstract void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List permissionsToRevoke, + @NonNull ArrayList allPackageNames); + /** * Add all permissions in the given package. *

* NOTE: argument {@code groupTEMP} is temporary until mPermissionGroups is moved to * the permission settings. + * + * @return A list of BasePermissions that were updated, and need to be revoked from packages */ - public abstract void addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); + public abstract List addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); public abstract void addAllPermissionGroups(@NonNull AndroidPackage pkg, boolean chatty); public abstract void removeAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); From c4ce178c261e6a2dc1aa4c1e1d570f0efd980e47 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Tue, 3 Nov 2020 15:12:44 -0800 Subject: [PATCH 18/24] Revoke permission on non-runtime -> runtime upgrade Not only on normal -> runtime. Test: atest PermissionEscalationTest Bug: 154505240, 168319670 Change-Id: If3b420067b4d7111dcf67ae6f98e42176158b679 Merged-In: If3b420067b4d7111dcf67ae6f98e42176158b679 (cherry picked from commit 33e24c883c5797fe7ba05468bde967fb0e46e66d) --- .../java/com/android/server/pm/permission/BasePermission.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index 2d4b5012bf1..5e04171a3bc 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -367,7 +367,7 @@ static BasePermission createOrUpdate(PackageManagerInternal packageManagerIntern if (bp == null) { bp = new BasePermission(p.getName(), p.getPackageName(), TYPE_NORMAL); } - boolean wasNormal = bp.isNormal(); + boolean wasNonRuntime = !bp.isRuntime(); StringBuilder r = null; if (bp.perm == null) { if (bp.sourcePackageName == null @@ -411,7 +411,7 @@ static BasePermission createOrUpdate(PackageManagerInternal packageManagerIntern && Objects.equals(bp.perm.getName(), p.getName())) { bp.protectionLevel = p.getProtectionLevel(); } - if (bp.isRuntime() && (ownerChanged || wasNormal)) { + if (bp.isRuntime() && (ownerChanged || wasNonRuntime)) { // If this is a runtime permission and the owner has changed, or this was a normal // permission, then permission state should be cleaned up bp.mPermissionDefinitionChanged = true; From 90cfe17643aa4ecbe7cbfb1c787217456f764e08 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Wed, 28 Oct 2020 14:00:32 -0700 Subject: [PATCH 19/24] Hide overlays over uninstall confirm dialog Test: atest CtsPackageUninstallTestCases Fixes: 171221302 Change-Id: I38b6d85871064d76f2911e20acc74b4ab76406b3 (cherry picked from commit cdec871e660202ac20fea9805c35ca8e7fc66b20) --- .../src/com/android/packageinstaller/UninstallerActivity.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java b/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java index be778e92787..94829b506f9 100755 --- a/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java +++ b/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java @@ -17,6 +17,7 @@ package com.android.packageinstaller; import static android.app.AppOpsManager.MODE_ALLOWED; +import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS; import static com.android.packageinstaller.PackageUtil.getMaxTargetSdkVersionForUid; @@ -87,6 +88,8 @@ public static class DialogInfo { @Override public void onCreate(Bundle icicle) { + getWindow().addSystemFlags(SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS); + // Never restore any state, esp. never create any fragments. The data in the fragment might // be stale, if e.g. the app was uninstalled while the activity was destroyed. super.onCreate(null); From 828fe0b915f30e22fec03dc1ed2e66220ceebd3e Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Mon, 9 Nov 2020 16:44:08 -0800 Subject: [PATCH 20/24] Protect GrantCredentialsPermissionActivity against overlay. Bug: 169763814 Test: manual Change-Id: I15dd22791fcc61ef02b06ad51d9e4409d11c0181 (cherry picked from commit f45dcfe1f5ed74dcec15799f2abd9e5ecbbed4b7) --- .../android/accounts/GrantCredentialsPermissionActivity.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/java/android/accounts/GrantCredentialsPermissionActivity.java b/core/java/android/accounts/GrantCredentialsPermissionActivity.java index af74b036a79..32b61b5ed8c 100644 --- a/core/java/android/accounts/GrantCredentialsPermissionActivity.java +++ b/core/java/android/accounts/GrantCredentialsPermissionActivity.java @@ -47,6 +47,9 @@ public class GrantCredentialsPermissionActivity extends Activity implements View protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + getWindow().addSystemFlags( + android.view.WindowManager.LayoutParams + .SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS); setContentView(R.layout.grant_credentials_permission); setTitle(R.string.grant_permissions_header_text); From 0b610a27ba60047842b9416dd0537c68f0dd22b2 Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Tue, 10 Nov 2020 16:01:35 -0800 Subject: [PATCH 21/24] Ignore GrantCredentials call with unexpected calling uid. Activity can be used only in two cases. 1) Calling uid matches uid grantee. 2) Calling uid is is system. This flow is used by getToken methods with notifyAuthFailure=true. Test: Existing CTS tests Bug: 158480899 Change-Id: I1421c333b6cebb4f7cddcdd8766298f6872e933b (cherry picked from commit 10d8a114bbd91c07f2feb5009328b16203ff50b1) --- .../GrantCredentialsPermissionActivity.java | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/core/java/android/accounts/GrantCredentialsPermissionActivity.java b/core/java/android/accounts/GrantCredentialsPermissionActivity.java index 32b61b5ed8c..5dc6e602e5d 100644 --- a/core/java/android/accounts/GrantCredentialsPermissionActivity.java +++ b/core/java/android/accounts/GrantCredentialsPermissionActivity.java @@ -16,16 +16,23 @@ package android.accounts; import android.app.Activity; -import android.content.res.Resources; -import android.os.Bundle; -import android.widget.TextView; -import android.widget.LinearLayout; -import android.view.View; -import android.view.LayoutInflater; +import android.app.ActivityTaskManager; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; +import android.content.res.Resources; +import android.os.Bundle; +import android.os.IBinder; +import android.os.Process; +import android.os.RemoteException; +import android.os.UserHandle; import android.text.TextUtils; +import android.util.Log; +import android.view.LayoutInflater; +import android.view.View; +import android.widget.LinearLayout; +import android.widget.TextView; + import com.android.internal.R; import java.io.IOException; @@ -42,6 +49,7 @@ public class GrantCredentialsPermissionActivity extends Activity implements View private Account mAccount; private String mAuthTokenType; private int mUid; + private int mCallingUid; private Bundle mResultBundle = null; protected LayoutInflater mInflater; @@ -77,6 +85,20 @@ protected void onCreate(Bundle savedInstanceState) { return; } + try { + IBinder activityToken = getActivityToken(); + mCallingUid = ActivityTaskManager.getService().getLaunchedFromUid(activityToken); + } catch (RemoteException re) { + // Couldn't figure out caller details + Log.w(getClass().getSimpleName(), "Unable to get caller identity \n" + re); + } + + if (!UserHandle.isSameApp(mCallingUid, Process.SYSTEM_UID) && mCallingUid != mUid) { + setResult(Activity.RESULT_CANCELED); + finish(); + return; + } + String accountTypeLabel; try { accountTypeLabel = getAccountLabel(mAccount); From 714ee76928c64c067cc405b724ee37d794c27f5b Mon Sep 17 00:00:00 2001 From: Danny Lin Date: Mon, 5 Oct 2020 12:36:35 -0700 Subject: [PATCH 22/24] Add support for app signature spoofing This is needed by microG GmsCore to pretend to be the official Google Play Services package, because client apps check the package signature to make sure it matches Google's official certificate. Changes made for Android 11: - Updated PackageInfo calls - Added new permission to public API surface, needed for PermissionController which is now an updatable APEX on 11 - Added a dummy permission group to allow users to manage the permission through the PermissionController UI (by Vachounet ) - Updated location provider comment for conciseness Change-Id: Ied7d6ce0b83a2d2345c3abba0429998d86494a88 Signed-off-by: Pavel Dubrova --- api/current.txt | 2 ++ core/res/AndroidManifest.xml | 15 ++++++++++++ core/res/res/values/config.xml | 2 ++ core/res/res/values/strings.xml | 12 ++++++++++ non-updatable-api/current.txt | 2 ++ .../server/pm/PackageManagerService.java | 23 +++++++++++++++++-- 6 files changed, 54 insertions(+), 2 deletions(-) diff --git a/api/current.txt b/api/current.txt index 952ccdad992..17cf63d7faa 100644 --- a/api/current.txt +++ b/api/current.txt @@ -79,6 +79,7 @@ package android { field public static final String DUMP = "android.permission.DUMP"; field public static final String EXPAND_STATUS_BAR = "android.permission.EXPAND_STATUS_BAR"; field public static final String FACTORY_TEST = "android.permission.FACTORY_TEST"; + field public static final String FAKE_PACKAGE_SIGNATURE = "android.permission.FAKE_PACKAGE_SIGNATURE"; field public static final String FOREGROUND_SERVICE = "android.permission.FOREGROUND_SERVICE"; field public static final String GET_ACCOUNTS = "android.permission.GET_ACCOUNTS"; field public static final String GET_ACCOUNTS_PRIVILEGED = "android.permission.GET_ACCOUNTS_PRIVILEGED"; @@ -182,6 +183,7 @@ package android { field public static final String CALL_LOG = "android.permission-group.CALL_LOG"; field public static final String CAMERA = "android.permission-group.CAMERA"; field public static final String CONTACTS = "android.permission-group.CONTACTS"; + field public static final String FAKE_PACKAGE = "android.permission-group.FAKE_PACKAGE"; field public static final String LOCATION = "android.permission-group.LOCATION"; field public static final String MICROPHONE = "android.permission-group.MICROPHONE"; field public static final String PHONE = "android.permission-group.PHONE"; diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index bb134f11319..7beb32a7d4a 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -2829,6 +2829,21 @@ android:description="@string/permdesc_getPackageSize" android:protectionLevel="normal" /> + + + + + + diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index 550601af0fa..ac81d6ba18d 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -1653,6 +1653,8 @@ com.android.location.fused + + com.google.android.gms diff --git a/core/res/res/values/strings.xml b/core/res/res/values/strings.xml index d6ee28b93f9..d5b3f54afc5 100644 --- a/core/res/res/values/strings.xml +++ b/core/res/res/values/strings.xml @@ -847,6 +847,18 @@ + + Spoof package signature + + Allows the app to pretend to be a different app. Malicious applications might be able to use this to access private application data. Legitimate uses include an emulator pretending to be what it emulates. Grant this permission with caution only! + + Spoof package signature + + allow to spoof package signature + + Allow + <b>%1$s</b> to spoof package signature? + disable or modify status bar diff --git a/non-updatable-api/current.txt b/non-updatable-api/current.txt index 5f15216e840..57748a8090a 100644 --- a/non-updatable-api/current.txt +++ b/non-updatable-api/current.txt @@ -79,6 +79,7 @@ package android { field public static final String DUMP = "android.permission.DUMP"; field public static final String EXPAND_STATUS_BAR = "android.permission.EXPAND_STATUS_BAR"; field public static final String FACTORY_TEST = "android.permission.FACTORY_TEST"; + field public static final String FAKE_PACKAGE_SIGNATURE = "android.permission.FAKE_PACKAGE_SIGNATURE"; field public static final String FOREGROUND_SERVICE = "android.permission.FOREGROUND_SERVICE"; field public static final String GET_ACCOUNTS = "android.permission.GET_ACCOUNTS"; field public static final String GET_ACCOUNTS_PRIVILEGED = "android.permission.GET_ACCOUNTS_PRIVILEGED"; @@ -182,6 +183,7 @@ package android { field public static final String CALL_LOG = "android.permission-group.CALL_LOG"; field public static final String CAMERA = "android.permission-group.CAMERA"; field public static final String CONTACTS = "android.permission-group.CONTACTS"; + field public static final String FAKE_PACKAGE = "android.permission-group.FAKE_PACKAGE"; field public static final String LOCATION = "android.permission-group.LOCATION"; field public static final String MICROPHONE = "android.permission-group.MICROPHONE"; field public static final String PHONE = "android.permission-group.PHONE"; diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 99e1388dcd5..de2f24ce23e 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -4405,8 +4405,9 @@ private PackageInfo generatePackageInfo(PackageSetting ps, int flags, int userId }); } - PackageInfo packageInfo = PackageInfoUtils.generate(p, gids, flags, - ps.firstInstallTime, ps.lastUpdateTime, permissions, state, userId, ps); + PackageInfo packageInfo = mayFakeSignature(p, PackageInfoUtils.generate(p, gids, flags, + ps.firstInstallTime, ps.lastUpdateTime, permissions, state, userId, ps), + permissions); if (packageInfo == null) { return null; @@ -4442,6 +4443,24 @@ private PackageInfo generatePackageInfo(PackageSetting ps, int flags, int userId } } + private PackageInfo mayFakeSignature(AndroidPackage p, PackageInfo pi, + Set permissions) { + try { + if (permissions.contains("android.permission.FAKE_PACKAGE_SIGNATURE") + && p.getTargetSdkVersion() > Build.VERSION_CODES.LOLLIPOP_MR1 + && p.getMetaData() != null) { + String sig = p.getMetaData().getString("fake-signature"); + if (sig != null) { + pi.signatures = new Signature[] {new Signature(sig)}; + } + } + } catch (Throwable t) { + // We should never die because of any failures, this is system code! + Log.w("PackageManagerService.FAKE_PACKAGE_SIGNATURE", t); + } + return pi; + } + @Override public void checkPackageStartable(String packageName, int userId) { final int callingUid = Binder.getCallingUid(); From d49f263e66385e0311ac862edaa618cd799bf50f Mon Sep 17 00:00:00 2001 From: Daniele Laudani Date: Wed, 3 Feb 2021 09:05:44 +0100 Subject: [PATCH 23/24] add Seedvault as backup provider --- packages/SettingsProvider/res/values/defaults.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/SettingsProvider/res/values/defaults.xml b/packages/SettingsProvider/res/values/defaults.xml index 51f69a95e16..5b2fd89b6d5 100644 --- a/packages/SettingsProvider/res/values/defaults.xml +++ b/packages/SettingsProvider/res/values/defaults.xml @@ -50,8 +50,8 @@ true true - false - com.android.localtransport/.LocalTransport + true + com.stevesoltys.seedvault.transport.ConfigurableBackupTransport From d89397c73627a510b85cac36bd337239ae5332f9 Mon Sep 17 00:00:00 2001 From: Daniele Laudani Date: Wed, 3 Feb 2021 09:08:45 +0100 Subject: [PATCH 24/24] Set backup transport programmatically --- .../providers/settings/SettingsProvider.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index b95d34f2966..55118b83f2d 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -4882,6 +4882,33 @@ Global.NOTIFICATION_BUBBLES, getContext().getResources().getBoolean( currentVersion = 191; } + if (currentVersion == 183) { + // Version 184: Update default Backup app to Seedvault + final SettingsState secureSettings = getSecureSettingsLocked(userId); + Setting currentBackupTransportSetting = secureSettings.getSettingLocked( + Secure.BACKUP_TRANSPORT); + if (currentBackupTransportSetting.isDefaultFromSystem()) { + secureSettings.insertSettingLocked( + Settings.Secure.BACKUP_TRANSPORT, + getContext().getResources().getString( + R.string.def_backup_transport), + null, true, + SettingsState.SYSTEM_PACKAGE_NAME); + } + + Setting currentBackupEnabledSetting = secureSettings.getSettingLocked( + Secure.BACKUP_ENABLED); + if (currentBackupEnabledSetting.isDefaultFromSystem()) { + secureSettings.insertSettingLocked( + Settings.Secure.BACKUP_ENABLED, + getContext().getResources().getBoolean( + R.bool.def_backup_enabled)? "1" : "0", + null, true, + SettingsState.SYSTEM_PACKAGE_NAME); + } + currentVersion = 184; + } + // vXXX: Add new settings above this point. if (currentVersion != newVersion) {