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

Commit afbba9fb authored by Felipe Leme's avatar Felipe Leme
Browse files

Checks package name belongs to called UID on some content capture methods. 

Also refactored how the SecurityException is thrown back to the caller app.

Bug: 122959591
Test: manual verification
Test: atest CtsContentCaptureServiceTestCases # sanity check (minus usual flakiness)

Change-Id: I4d2a68e61dc1c801d80734a30f4bbe6fdae8555d
parent a8d33c24

core/java/android/view/contentcapture/ContentCaptureManager.java

+33 −19
Original line number Diff line number Diff line
@@ -525,15 +525,12 @@ public final class ContentCaptureManager { 
        // the service to fine tune how long-lived apps (like browsers) are whitelisted.

        if (!isContentCaptureEnabled() && !mOptions.lite) return null;



        final SyncResultReceiver resultReceiver = new SyncResultReceiver(SYNC_CALLS_TIMEOUT_MS);

        try {

            mService.getContentCaptureConditions(mContext.getPackageName(), resultReceiver);

        final SyncResultReceiver resultReceiver = syncRun(

                (r) -> mService.getContentCaptureConditions(mContext.getPackageName(), r));



        final ArrayList<ContentCaptureCondition> result = resultReceiver

                .getParcelableListResult();

        return toSet(result);

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

        }

    }



    /**
@@ -566,21 +563,14 @@ public final class ContentCaptureManager { 
    @SystemApi

    @TestApi

    public boolean isContentCaptureFeatureEnabled() {

        final SyncResultReceiver resultReceiver = new SyncResultReceiver(SYNC_CALLS_TIMEOUT_MS);

        final int resultCode;

        try {

            mService.isContentCaptureFeatureEnabled(resultReceiver);

            resultCode = resultReceiver.getIntResult();

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

        }

        final SyncResultReceiver resultReceiver = syncRun(

                (r) -> mService.isContentCaptureFeatureEnabled(r));

        final int resultCode = resultReceiver.getIntResult();

        switch (resultCode) {

            case RESULT_CODE_TRUE:

                return true;

            case RESULT_CODE_FALSE:

                return false;

            case RESULT_CODE_SECURITY_EXCEPTION:

                throw new SecurityException("caller is not user's ContentCapture service");

            default:

                Log.wtf(TAG, "received invalid result: " + resultCode);

                return false;
@@ -603,6 +593,26 @@ public final class ContentCaptureManager { 
        }

    }



    /**

     * Runs a sync method in the service, properly handling exceptions.

     *

     * @throws SecurityException if caller is not allowed to execute the method.

     */

    @NonNull

    private SyncResultReceiver syncRun(@NonNull MyRunnable r) {

        final SyncResultReceiver resultReceiver = new SyncResultReceiver(SYNC_CALLS_TIMEOUT_MS);

        try {

            r.run(resultReceiver);

            final int resultCode = resultReceiver.getIntResult();

            if (resultCode == RESULT_CODE_SECURITY_EXCEPTION) {

                throw new SecurityException(resultReceiver.getStringResult());

            }

            return resultReceiver;

        } catch (RemoteException e) {

            throw e.rethrowFromSystemServer();

        }

    }



    /** @hide */

    public void dump(String prefix, PrintWriter pw) {

        pw.print(prefix); pw.println("ContentCaptureManager");
@@ -626,4 +636,8 @@ public final class ContentCaptureManager { 
            }

        }

    }



    private interface MyRunnable {

        void run(@NonNull SyncResultReceiver receiver) throws RemoteException;

    }

}

core/java/com/android/internal/util/SyncResultReceiver.java

+1 −2
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ import android.annotation.NonNull; 
import android.annotation.Nullable;

import android.os.Bundle;

import android.os.Parcelable;

import android.os.RemoteException;



import com.android.internal.os.IResultReceiver;
@@ -183,7 +182,7 @@ public final class SyncResultReceiver extends IResultReceiver.Stub { 
    }



    /** @hide */

    public static final class TimeoutException extends RemoteException {

    public static final class TimeoutException extends RuntimeException {

        private TimeoutException(String msg) {

            super(msg);

        }

services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java

+38 −28
Original line number Diff line number Diff line
@@ -450,23 +450,16 @@ public final class ContentCaptureManagerService extends 
    }



    @GuardedBy("mLock")

    private boolean assertCalledByServiceLocked(@NonNull String methodName, @UserIdInt int userId,

            int callingUid, @NonNull IResultReceiver result) {

        final boolean isService = isCalledByServiceLocked(methodName, userId, callingUid);

        if (isService) return true;



        try {

            result.send(RESULT_CODE_SECURITY_EXCEPTION, /* resultData= */ null);

        } catch (RemoteException e) {

            Slog.w(mTag, "Unable to send isContentCaptureFeatureEnabled(): " + e);

    private void assertCalledByServiceLocked(@NonNull String methodName) {

        if (!isCalledByServiceLocked(methodName)) {

            throw new SecurityException("caller is not user's ContentCapture service");

        }

        return false;

    }



    @GuardedBy("mLock")

    private boolean isCalledByServiceLocked(@NonNull String methodName, @UserIdInt int userId,

            int callingUid) {



    private boolean isCalledByServiceLocked(@NonNull String methodName) {

        final int userId = UserHandle.getCallingUserId();

        final int callingUid = Binder.getCallingUid();

        final String serviceName = mServiceNameResolver.getServiceName(userId);

        if (serviceName == null) {

            Slog.e(mTag, methodName + ": called by UID " + callingUid
@@ -499,6 +492,27 @@ public final class ContentCaptureManagerService extends 
        return true;

    }



    /**

     * Executes the given {@code runnable} and if it throws a {@link SecurityException},

     * send it back to the receiver.

     *

     * @return whether the exception was thrown or not.

     */

    private boolean throwsSecurityException(@NonNull IResultReceiver result,

            @NonNull Runnable runable) {

        try {

            runable.run();

            return false;

        } catch (SecurityException e) {

            try {

                result.send(RESULT_CODE_SECURITY_EXCEPTION, bundleFor(e.getMessage()));

            } catch (RemoteException e2) {

                Slog.w(mTag, "Unable to send security exception (" + e + "): ", e2);

            }

        }

        return true;

    }



    @Override // from AbstractMasterSystemService

    protected void dumpLocked(String prefix, PrintWriter pw) {

        super.dumpLocked(prefix, pw);
@@ -570,7 +584,7 @@ public final class ContentCaptureManagerService extends 
        @Override

        public void removeUserData(@NonNull UserDataRemovalRequest request) {

            Preconditions.checkNotNull(request);

            // TODO(b/122959591): check caller uid owns the package name

            assertCalledByPackageOwner(request.getPackageName());



            final int userId = UserHandle.getCallingUserId();

            synchronized (mLock) {
@@ -581,13 +595,14 @@ public final class ContentCaptureManagerService extends 


        @Override

        public void isContentCaptureFeatureEnabled(@NonNull IResultReceiver result) {

            final int userId = UserHandle.getCallingUserId();

            boolean enabled;

            synchronized (mLock) {

                final boolean isService = assertCalledByServiceLocked(

                        "isContentCaptureFeatureEnabled()", userId, Binder.getCallingUid(), result);

                if (!isService) return;

                if (throwsSecurityException(result,

                        () -> assertCalledByServiceLocked("isContentCaptureFeatureEnabled()"))) {

                    return;

                }



                final int userId = UserHandle.getCallingUserId();

                enabled = !mDisabledByDeviceConfig && !isDisabledBySettingsLocked(userId);

            }

            try {
@@ -599,16 +614,9 @@ public final class ContentCaptureManagerService extends 


        @Override

        public void getServiceSettingsActivity(@NonNull IResultReceiver result) {

            try {

                enforceCallingPermissionForManagement();

            } catch (SecurityException e) {

                try {

                    result.send(RESULT_CODE_SECURITY_EXCEPTION, bundleFor(e.getMessage()));

                } catch (RemoteException e2) {

                    Slog.w(mTag, "Unable to send getServiceSettingsIntent() exception: " + e2);

            if (throwsSecurityException(result, () -> enforceCallingPermissionForManagement())) {

                return;

            }

            }



            final int userId = UserHandle.getCallingUserId();

            final ComponentName componentName;
@@ -627,7 +635,9 @@ public final class ContentCaptureManagerService extends 
        @Override

        public void getContentCaptureConditions(@NonNull String packageName,

                @NonNull IResultReceiver result) {

            // TODO(b/122959591): check caller uid owns the package name

            if (throwsSecurityException(result, () -> assertCalledByPackageOwner(packageName))) {

                return;

            }



            final int userId = UserHandle.getCallingUserId();

            final ArrayList<ContentCaptureCondition> conditions;

services/core/java/com/android/server/infra/AbstractMasterSystemService.java

+17 −0
Original line number Diff line number Diff line
@@ -616,6 +616,23 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem 
        mServicesCache.clear();

    }



    /**

     * Asserts that the given package name is owned by the UID making this call.

     *

     * @throws SecurityException when it's not...

     */

    protected final void assertCalledByPackageOwner(@NonNull String packageName) {

        Preconditions.checkNotNull(packageName);

        final int uid = Binder.getCallingUid();

        final String[] packages = getContext().getPackageManager().getPackagesForUid(uid);

        if (packages != null) {

            for (String candidate : packages) {

                if (packageName.equals(candidate)) return; // Found it

            }

        }

        throw new SecurityException("UID " + uid + " does not own " + packageName);

    }



    // TODO(b/117779333): support proto

    protected void dumpLocked(@NonNull String prefix, @NonNull PrintWriter pw) {

        boolean realDebug = debug;