Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1d194f92 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Uri permission grants improvements, tests, fixes.

The primary work in this CL is fixing two subtle security bugs that
have recently been reported in how checkGrantUriPermission() handles
advanced grant features like "persistable" and "prefix".  In general,
these advanced features should only be available when the underlying
provider has enabled permission grants.  The only narrow case where
we should allow granting without provider consent is for simple "read"
or "write" grants when crossing user boundaries.

To verify these fixes, this change adds thorough unit testing of
the core granting functionality.  This helped uncover a few subtle
bugs in how prefix grants were being issued, and in how persistable
grants were being released.

To support mocking in tests, shift all AM/PM calls to using Internal
interfaces, and initialize using best-practice onBootPhase().  This
also means we no longer have to handle RemoteExceptions.

Shift NeededUriGrants to using an ArraySet to avoid duplication of
grant data structures.

Define TEST_MAPPING to ensure future changes are tested.

Bug: 140729426, 138791358
Test: atest FrameworksServicesTests:com.android.server.uri
Test: atest CtsAppSecurityHostTestCases:android.appsecurity.cts.AppSecurityTests#testPermissionDiffCert
Change-Id: I8dac08280981c3cd15071226319efe9ebd8b4db5
parent 51ecd453
Loading
Loading
Loading
Loading
+4 −5
Original line number Original line Diff line number Diff line
@@ -2681,7 +2681,6 @@ public class ActivityManagerService extends IActivityManager.Stub
        Slog.d("AppOps", "AppOpsService published");
        Slog.d("AppOps", "AppOpsService published");
        LocalServices.addService(ActivityManagerInternal.class, mInternal);
        LocalServices.addService(ActivityManagerInternal.class, mInternal);
        mActivityTaskManager.onActivityManagerInternalAdded();
        mActivityTaskManager.onActivityManagerInternalAdded();
        mUgmInternal.onActivityManagerInternalAdded();
        mPendingIntentController.onActivityManagerInternalAdded();
        mPendingIntentController.onActivityManagerInternalAdded();
        // Wait for the synchronized block started in mProcessCpuThread,
        // Wait for the synchronized block started in mProcessCpuThread,
        // so that any other access to mProcessCpuTracker from main thread
        // so that any other access to mProcessCpuTracker from main thread
@@ -6410,7 +6409,7 @@ public class ActivityManagerService extends IActivityManager.Stub
        if (pid == MY_PID) {
        if (pid == MY_PID) {
            return PackageManager.PERMISSION_GRANTED;
            return PackageManager.PERMISSION_GRANTED;
        }
        }
        return mUgmInternal.checkUriPermission(new GrantUri(userId, uri, false), uid, modeFlags)
        return mUgmInternal.checkUriPermission(new GrantUri(userId, uri, modeFlags), uid, modeFlags)
                ? PackageManager.PERMISSION_GRANTED : PackageManager.PERMISSION_DENIED;
                ? PackageManager.PERMISSION_GRANTED : PackageManager.PERMISSION_DENIED;
    }
    }
@@ -6422,7 +6421,7 @@ public class ActivityManagerService extends IActivityManager.Stub
    public void grantUriPermission(IApplicationThread caller, String targetPkg, Uri uri,
    public void grantUriPermission(IApplicationThread caller, String targetPkg, Uri uri,
            final int modeFlags, int userId) {
            final int modeFlags, int userId) {
        enforceNotIsolatedCaller("grantUriPermission");
        enforceNotIsolatedCaller("grantUriPermission");
        GrantUri grantUri = new GrantUri(userId, uri, false);
        GrantUri grantUri = new GrantUri(userId, uri, modeFlags);
        synchronized(this) {
        synchronized(this) {
            final ProcessRecord r = getRecordForAppLocked(caller);
            final ProcessRecord r = getRecordForAppLocked(caller);
            if (r == null) {
            if (r == null) {
@@ -6480,8 +6479,8 @@ public class ActivityManagerService extends IActivityManager.Stub
                return;
                return;
            }
            }
            mUgmInternal.revokeUriPermission(targetPackage, r.uid, new GrantUri(userId, uri, false),
            mUgmInternal.revokeUriPermission(targetPackage, r.uid,
                    modeFlags);
                    new GrantUri(userId, uri, modeFlags), modeFlags);
        }
        }
    }
    }
+7 −6
Original line number Original line Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.server.uri;


import android.content.ContentProvider;
import android.content.ContentProvider;
import android.content.ContentResolver;
import android.content.ContentResolver;
import android.content.Intent;
import android.net.Uri;
import android.net.Uri;
import android.util.proto.ProtoOutputStream;
import android.util.proto.ProtoOutputStream;


@@ -27,12 +28,12 @@ import com.android.server.am.GrantUriProto;
public class GrantUri {
public class GrantUri {
    public final int sourceUserId;
    public final int sourceUserId;
    public final Uri uri;
    public final Uri uri;
    public boolean prefix;
    public final boolean prefix;


    public GrantUri(int sourceUserId, Uri uri, boolean prefix) {
    public GrantUri(int sourceUserId, Uri uri, int modeFlags) {
        this.sourceUserId = sourceUserId;
        this.sourceUserId = sourceUserId;
        this.uri = uri;
        this.uri = uri;
        this.prefix = prefix;
        this.prefix = (modeFlags & Intent.FLAG_GRANT_PREFIX_URI_PERMISSION) != 0;
    }
    }


    @Override
    @Override
@@ -74,12 +75,12 @@ public class GrantUri {
        proto.end(token);
        proto.end(token);
    }
    }


    public static GrantUri resolve(int defaultSourceUserHandle, Uri uri) {
    public static GrantUri resolve(int defaultSourceUserHandle, Uri uri, int modeFlags) {
        if (ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) {
        if (ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) {
            return new GrantUri(ContentProvider.getUserIdFromUri(uri, defaultSourceUserHandle),
            return new GrantUri(ContentProvider.getUserIdFromUri(uri, defaultSourceUserHandle),
                    ContentProvider.getUriWithoutUserId(uri), false);
                    ContentProvider.getUriWithoutUserId(uri), modeFlags);
        } else {
        } else {
            return new GrantUri(defaultSourceUserHandle, uri, false);
            return new GrantUri(defaultSourceUserHandle, uri, modeFlags);
        }
        }
    }
    }
}
}
+6 −5
Original line number Original line Diff line number Diff line
@@ -16,22 +16,23 @@


package com.android.server.uri;
package com.android.server.uri;


import android.util.ArraySet;
import android.util.proto.ProtoOutputStream;
import android.util.proto.ProtoOutputStream;


import com.android.server.am.NeededUriGrantsProto;
import com.android.server.am.NeededUriGrantsProto;


import java.util.ArrayList;

/** List of {@link GrantUri} a process needs. */
/** List of {@link GrantUri} a process needs. */
public class NeededUriGrants extends ArrayList<GrantUri> {
public class NeededUriGrants {
    final String targetPkg;
    final String targetPkg;
    final int targetUid;
    final int targetUid;
    final int flags;
    final int flags;
    final ArraySet<GrantUri> uris;


    public NeededUriGrants(String targetPkg, int targetUid, int flags) {
    public NeededUriGrants(String targetPkg, int targetUid, int flags) {
        this.targetPkg = targetPkg;
        this.targetPkg = targetPkg;
        this.targetUid = targetUid;
        this.targetUid = targetUid;
        this.flags = flags;
        this.flags = flags;
        this.uris = new ArraySet<>();
    }
    }


    public void dumpDebug(ProtoOutputStream proto, long fieldId) {
    public void dumpDebug(ProtoOutputStream proto, long fieldId) {
@@ -40,9 +41,9 @@ public class NeededUriGrants extends ArrayList<GrantUri> {
        proto.write(NeededUriGrantsProto.TARGET_UID, targetUid);
        proto.write(NeededUriGrantsProto.TARGET_UID, targetUid);
        proto.write(NeededUriGrantsProto.FLAGS, flags);
        proto.write(NeededUriGrantsProto.FLAGS, flags);


        final int N = this.size();
        final int N = uris.size();
        for (int i = 0; i < N; i++) {
        for (int i = 0; i < N; i++) {
            this.get(i).dumpDebug(proto, NeededUriGrantsProto.GRANTS);
            uris.valueAt(i).dumpDebug(proto, NeededUriGrantsProto.GRANTS);
        }
        }
        proto.end(token);
        proto.end(token);
    }
    }
+30 −0
Original line number Original line Diff line number Diff line
{
    "presubmit": [
        {
            "name": "FrameworksServicesTests",
            "options": [
                {
                    "include-filter": "com.android.server.uri."
                }
            ]
        }
    ],
    "postsubmit": [
        {
            "name": "CtsAppSecurityHostTestCases",
            "options": [
                {
                    "include-filter": "android.appsecurity.cts.AppSecurityTests#testPermissionDiffCert"
                }
            ]
        },
        {
            "name": "CtsWindowManagerDeviceTestCases",
            "options": [
                {
                    "include-filter": "android.server.wm.CrossAppDragAndDropTests"
                }
            ]
        }
    ]
}
+0 −1
Original line number Original line Diff line number Diff line
@@ -30,7 +30,6 @@ import java.io.PrintWriter;
 */
 */
public interface UriGrantsManagerInternal {
public interface UriGrantsManagerInternal {
    void onSystemReady();
    void onSystemReady();
    void onActivityManagerInternalAdded();
    void removeUriPermissionIfNeeded(UriPermission perm);
    void removeUriPermissionIfNeeded(UriPermission perm);
    void grantUriPermission(int callingUid, String targetPkg, GrantUri grantUri,
    void grantUriPermission(int callingUid, String targetPkg, GrantUri grantUri,
            final int modeFlags, UriPermissionOwner owner, int targetUserId);
            final int modeFlags, UriPermissionOwner owner, int targetUserId);
Loading