From b07f4854b5ebc232135f77413934a17aa84e4509 Mon Sep 17 00:00:00 2001 From: Victor Hsieh Date: Thu, 26 Mar 2020 12:33:58 -0700 Subject: [PATCH] Respect app-ops permission in FileIntegrityService Previous permission doesn't consider REQUEST_INSTALL_PACKAGES permission as an app-ops permission. Bug: 152009905 Test: atest GtsPlayFsiTestCases Test: remove appops setup from AndroidTest.xml, the same test failed Change-Id: Icdbf6bb35fe146c5be8a97e29c4c554b3ce91b5d --- .../android/app/SystemServiceRegistry.java | 2 +- .../security/FileIntegrityManager.java | 7 ++- .../security/IFileIntegrityService.aidl | 2 +- .../server/security/FileIntegrityService.java | 43 ++++++++++++++----- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/core/java/android/app/SystemServiceRegistry.java b/core/java/android/app/SystemServiceRegistry.java index 1aabd24d6df68..054e5e0945f3c 100644 --- a/core/java/android/app/SystemServiceRegistry.java +++ b/core/java/android/app/SystemServiceRegistry.java @@ -1310,7 +1310,7 @@ public final class SystemServiceRegistry { throws ServiceNotFoundException { IBinder b = ServiceManager.getServiceOrThrow( Context.FILE_INTEGRITY_SERVICE); - return new FileIntegrityManager( + return new FileIntegrityManager(ctx.getOuterContext(), IFileIntegrityService.Stub.asInterface(b)); }}); //CHECKSTYLE:ON IndentationCheck diff --git a/core/java/android/security/FileIntegrityManager.java b/core/java/android/security/FileIntegrityManager.java index cdd6584e9b35c..266046e57cd5f 100644 --- a/core/java/android/security/FileIntegrityManager.java +++ b/core/java/android/security/FileIntegrityManager.java @@ -31,9 +31,11 @@ import java.security.cert.X509Certificate; @SystemService(Context.FILE_INTEGRITY_SERVICE) public final class FileIntegrityManager { @NonNull private final IFileIntegrityService mService; + @NonNull private final Context mContext; /** @hide */ - public FileIntegrityManager(@NonNull IFileIntegrityService service) { + public FileIntegrityManager(@NonNull Context context, @NonNull IFileIntegrityService service) { + mContext = context; mService = service; } @@ -69,7 +71,8 @@ public final class FileIntegrityManager { public boolean isAppSourceCertificateTrusted(@NonNull X509Certificate certificate) throws CertificateEncodingException { try { - return mService.isAppSourceCertificateTrusted(certificate.getEncoded()); + return mService.isAppSourceCertificateTrusted( + certificate.getEncoded(), mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/security/IFileIntegrityService.aidl b/core/java/android/security/IFileIntegrityService.aidl index ebb8bcb853504..dff347ec758a1 100644 --- a/core/java/android/security/IFileIntegrityService.aidl +++ b/core/java/android/security/IFileIntegrityService.aidl @@ -22,5 +22,5 @@ package android.security; */ interface IFileIntegrityService { boolean isApkVeritySupported(); - boolean isAppSourceCertificateTrusted(in byte[] certificateBytes); + boolean isAppSourceCertificateTrusted(in byte[] certificateBytes, in String packageName); } diff --git a/services/core/java/com/android/server/security/FileIntegrityService.java b/services/core/java/com/android/server/security/FileIntegrityService.java index 482090a020253..841aca5a8d6f4 100644 --- a/services/core/java/com/android/server/security/FileIntegrityService.java +++ b/services/core/java/com/android/server/security/FileIntegrityService.java @@ -18,14 +18,19 @@ package com.android.server.security; import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.AppOpsManager; import android.content.Context; import android.content.pm.PackageManager; +import android.content.pm.PackageManagerInternal; +import android.os.Binder; import android.os.Build; import android.os.IBinder; import android.os.SystemProperties; +import android.os.UserHandle; import android.security.IFileIntegrityService; import android.util.Slog; +import com.android.server.LocalServices; import com.android.server.SystemService; import java.io.ByteArrayInputStream; @@ -58,10 +63,10 @@ public class FileIntegrityService extends SystemService { } @Override - public boolean isAppSourceCertificateTrusted(@Nullable byte[] certificateBytes) { - enforceAnyCallingPermissions( - android.Manifest.permission.REQUEST_INSTALL_PACKAGES, - android.Manifest.permission.INSTALL_PACKAGES); + public boolean isAppSourceCertificateTrusted(@Nullable byte[] certificateBytes, + @NonNull String packageName) { + checkCallerPermission(packageName); + try { if (!isApkVeritySupported()) { return false; @@ -77,14 +82,30 @@ public class FileIntegrityService extends SystemService { } } - private void enforceAnyCallingPermissions(String ...permissions) { - for (String permission : permissions) { - if (getContext().checkCallingPermission(permission) - == PackageManager.PERMISSION_GRANTED) { - return; - } + private void checkCallerPermission(String packageName) { + final int callingUid = Binder.getCallingUid(); + final int callingUserId = UserHandle.getUserId(callingUid); + final PackageManagerInternal packageManager = + LocalServices.getService(PackageManagerInternal.class); + final int packageUid = packageManager.getPackageUid( + packageName, 0 /*flag*/, callingUserId); + if (callingUid != packageUid) { + throw new SecurityException( + "Calling uid " + callingUid + " does not own package " + packageName); + } + + if (getContext().checkCallingPermission(android.Manifest.permission.INSTALL_PACKAGES) + == PackageManager.PERMISSION_GRANTED) { + return; + } + + final AppOpsManager appOpsManager = getContext().getSystemService(AppOpsManager.class); + final int mode = appOpsManager.checkOpNoThrow( + AppOpsManager.OP_REQUEST_INSTALL_PACKAGES, callingUid, packageName); + if (mode != AppOpsManager.MODE_ALLOWED) { + throw new SecurityException( + "Caller should have INSTALL_PACKAGES or REQUEST_INSTALL_PACKAGES"); } - throw new SecurityException("Insufficient permission"); } };