From 11725e1206645e567cfdd70100d64d1e0a85180d Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Fri, 31 Jul 2020 18:46:28 -0700 Subject: [PATCH 01/10] 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 98a23f2b00750..3cb6293f07066 100644 --- a/core/java/android/app/ActivityView.java +++ b/core/java/android/app/ActivityView.java @@ -105,7 +105,8 @@ public class ActivityView extends ViewGroup implements android.window.TaskEmbedd 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 class ActivityView extends ViewGroup implements android.window.TaskEmbedd @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 9ccb4c172158d..9013da36007e4 100644 --- a/core/java/android/window/VirtualDisplayTaskEmbedder.java +++ b/core/java/android/window/VirtualDisplayTaskEmbedder.java @@ -19,6 +19,7 @@ package android.window; 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 class VirtualDisplayTaskEmbedder extends TaskEmbedder { 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 5c2841aff1d83..7597e87321530 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 daf613976358e..0f6284d22d106 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 class VirtualDisplayTest extends AndroidTestCase { 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 1e556a3ed4026..58d5776543a91 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 @@ public class BubbleExpandedView extends LinearLayout { 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 24661d69a78eb..852868616afdb 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.UserHandle; 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 final class DisplayManagerService extends SystemService { } } - 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 02/10] 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 327bca268a7bc..91b591c7b77ef 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -232,6 +232,10 @@ public abstract class DocumentsProvider extends ContentProvider { } } + 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 @@ public abstract class DocumentsProvider extends ContentProvider { 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 @@ public abstract class DocumentsProvider extends ContentProvider { 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 @@ public abstract class DocumentsProvider extends ContentProvider { 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 @@ public abstract class DocumentsProvider extends ContentProvider { 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 @@ public abstract class DocumentsProvider extends ContentProvider { } } 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 @@ public abstract class DocumentsProvider extends ContentProvider { } } 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 03/10] 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 91b591c7b77ef..4e1f81919c7db 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -218,8 +218,15 @@ public abstract class DocumentsProvider extends ContentProvider { } /** {@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 @@ public abstract class DocumentsProvider extends ContentProvider { 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 @@ public abstract class DocumentsProvider extends ContentProvider { "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 04/10] [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 23bf955ba9a67..c4f1c805398b3 100644 --- a/services/core/java/com/android/server/TelephonyRegistry.java +++ b/services/core/java/com/android/server/TelephonyRegistry.java @@ -310,11 +310,10 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 class TelephonyRegistry extends ITelephonyRegistry.Stub { + " 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 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { .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 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub { 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 05/10] 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 6a92a83c98990..bb134f11319bb 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 06/10] 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 347beab0c42fc..bcb17bc9e2dc9 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 class Vpn { */ 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 07/10] 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 dc1612575f382..13bf197aa9dcb 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 08/10] 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 69b02ceb24111..f6308202ab688 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.messages.nano.SystemMessageProto.SystemMessage; 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 @@ public class LockSettingsService extends ILockSettings.Stub { 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 @@ public class LockSettingsService extends ILockSettings.Stub { @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 ba851992cbad9..2c2fdcaab340f 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 class SyntheticPasswordTests extends BaseLockSettingsServiceTests { 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 09/10] 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 1e33f0ea50942..68757d86fb89f 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 10/10] 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 e0195e4eafc1c..4f0c84e586a28 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.ViewGroup; 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 class InputMethodService extends AbstractInputMethodService { 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).