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

Commit a244ceeb authored by Soonil Nagarkar's avatar Soonil Nagarkar Committed by Android (Google) Code Review
Browse files

Merge "Fix bugs in LocationManagerService and AppOpsPolicy" into sc-dev

parents 41e7f367 4aa42bdb
Loading
Loading
Loading
Loading
+144 −12
Original line number Diff line number Diff line
@@ -23,20 +23,26 @@ import android.annotation.TestApi;
import android.util.ArrayMap;
import android.util.ArraySet;

import com.android.internal.annotations.Immutable;

import java.io.PrintWriter;
import java.util.Collection;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

/**
 * A list of packages and associated attribution tags that supports easy membership checks.
 * A list of packages and associated attribution tags that supports easy membership checks. Supports
 * "wildcard" attribution tags (ie, matching any attribution tag under a package) in additional to
 * standard checks.
 *
 * @hide
 */
@TestApi
@Immutable
public final class PackageTagsList implements Parcelable {

    // an empty set value matches any attribution tag
    // an empty set value matches any attribution tag (ie, wildcard)
    private final ArrayMap<String, ArraySet<String>> mPackageTags;

    private PackageTagsList(@NonNull ArrayMap<String, ArraySet<String>> packageTags) {
@@ -51,14 +57,33 @@ public final class PackageTagsList implements Parcelable {
    }

    /**
     * Returns true if the given package is represented within this instance. If this returns true
     * this does not imply anything about whether any given attribution tag under the given package
     * name is present.
     * Returns true if the given package is found within this instance. If this returns true this
     * does not imply anything about whether any given attribution tag under the given package name
     * is present.
     */
    public boolean includes(@NonNull String packageName) {
        return mPackageTags.containsKey(packageName);
    }

    /**
     * Returns true if the given attribution tag is found within this instance under any package.
     * Only returns true if the attribution tag literal is found, not if any package contains the
     * set of all attribution tags.
     *
     * @hide
     */
    public boolean includesTag(@NonNull String attributionTag) {
        final int size = mPackageTags.size();
        for (int i = 0; i < size; i++) {
            ArraySet<String> tags = mPackageTags.valueAt(i);
            if (tags.contains(attributionTag)) {
                return true;
            }
        }

        return false;
    }

    /**
     * Returns true if all attribution tags under the given package are contained within this
     * instance.
@@ -76,6 +101,7 @@ public final class PackageTagsList implements Parcelable {
        if (tags == null) {
            return false;
        } else if (tags.isEmpty()) {
            // our tags are the full set, so we contain any attribution tag
            return true;
        } else {
            return tags.contains(attributionTag);
@@ -98,10 +124,12 @@ public final class PackageTagsList implements Parcelable {
                return false;
            }
            if (tags.isEmpty()) {
                // our tags are the full set, so we contain whatever the other tags are
                continue;
            }
            ArraySet<String> otherTags = packageTagsList.mPackageTags.valueAt(i);
            if (otherTags.isEmpty()) {
                // other tags are the full set, so we can't contain them
                return false;
            }
            if (!tags.containsAll(otherTags)) {
@@ -247,6 +275,31 @@ public final class PackageTagsList implements Parcelable {
            return this;
        }

        /**
         * Adds the specified package and set of attribution tags to the builder.
         *
         * @hide
         */
        @SuppressLint("MissingGetterMatchingBuilder")
        public @NonNull Builder add(@NonNull String packageName,
                @NonNull Collection<String> attributionTags) {
            if (attributionTags.isEmpty()) {
                // the input is not allowed to specify a full set by passing in an empty collection
                return this;
            }

            ArraySet<String> tags = mPackageTags.get(packageName);
            if (tags == null) {
                tags = new ArraySet<>(attributionTags);
                mPackageTags.put(packageName, tags);
            } else if (!tags.isEmpty()) {
                // if we contain the full set, already done, otherwise add all the tags
                tags.addAll(attributionTags);
            }

            return this;
        }

        /**
         * Adds the specified {@link PackageTagsList} to the builder.
         */
@@ -267,13 +320,92 @@ public final class PackageTagsList implements Parcelable {
                if (newTags.isEmpty()) {
                    add(entry.getKey());
                } else {
                    ArraySet<String> tags = mPackageTags.get(entry.getKey());
                    if (tags == null) {
                        tags = new ArraySet<>(newTags);
                        mPackageTags.put(entry.getKey(), tags);
                    } else if (!tags.isEmpty()) {
                        tags.addAll(newTags);
                    add(entry.getKey(), newTags);
                }
            }

            return this;
        }

        /**
         * Removes all attribution tags under the specified package from the builder.
         *
         * @hide
         */
        @SuppressLint("MissingGetterMatchingBuilder")
        public @NonNull Builder remove(@NonNull String packageName) {
            mPackageTags.remove(packageName);
            return this;
        }

        /**
         * Removes the specified package and attribution tag from the builder if and only if the
         * specified attribution tag is listed explicitly under the package. If the package contains
         * all possible attribution tags, then nothing will be removed.
         *
         * @hide
         */
        @SuppressLint("MissingGetterMatchingBuilder")
        public @NonNull Builder remove(@NonNull String packageName,
                @Nullable String attributionTag) {
            ArraySet<String> tags = mPackageTags.get(packageName);
            if (tags != null && tags.remove(attributionTag) && tags.isEmpty()) {
                mPackageTags.remove(packageName);
            }
            return this;
        }

        /**
         * Removes the specified package and set of attribution tags from the builder if and only if
         * the specified set of attribution tags are listed explicitly under the package. If the
         * package contains all possible attribution tags, then nothing will be removed.
         *
         * @hide
         */
        @SuppressLint("MissingGetterMatchingBuilder")
        public @NonNull Builder remove(@NonNull String packageName,
                @NonNull Collection<String> attributionTags) {
            if (attributionTags.isEmpty()) {
                // the input is not allowed to specify a full set by passing in an empty collection
                return this;
            }

            ArraySet<String> tags = mPackageTags.get(packageName);
            if (tags != null && tags.removeAll(attributionTags) && tags.isEmpty()) {
                mPackageTags.remove(packageName);
            }
            return this;
        }

        /**
         * Removes the specified {@link PackageTagsList} from the builder. If a package contains all
         * possible attribution tags, it will only be removed if the package in the removed
         * {@link PackageTagsList} also contains all possible attribution tags.
         *
         * @hide
         */
        @SuppressLint("MissingGetterMatchingBuilder")
        public @NonNull Builder remove(@NonNull PackageTagsList packageTagsList) {
            return remove(packageTagsList.mPackageTags);
        }

        /**
         * Removes the given map of package to attribution tags to the builder. An empty set of
         * attribution tags is interpreted to imply all attribution tags under that package. If a
         * package contains all possible attribution tags, it will only be removed if the package in
         * the removed map also contains all possible attribution tags.
         *
         * @hide
         */
        @SuppressLint("MissingGetterMatchingBuilder")
        public @NonNull Builder remove(@NonNull Map<String, ? extends Set<String>> packageTagsMap) {
            for (Map.Entry<String, ? extends Set<String>> entry : packageTagsMap.entrySet()) {
                Set<String> removedTags = entry.getValue();
                if (removedTags.isEmpty()) {
                    // if removing the full set, drop the package completely
                    remove(entry.getKey());
                } else {
                    remove(entry.getKey(), removedTags);
                }
            }

+51 −1
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;

import java.util.Arrays;
import java.util.Collections;

@Presubmit
@RunWith(AndroidJUnit4.class)
@@ -40,7 +41,8 @@ public class PackageTagsListTest {
        PackageTagsList.Builder builder = new PackageTagsList.Builder()
                .add("package1", "attr1")
                .add("package1", "attr2")
                .add("package2");
                .add("package2")
                .add("package4", Arrays.asList("attr1", "attr2"));
        PackageTagsList list = builder.build();

        assertTrue(list.contains(builder.build()));
@@ -49,10 +51,13 @@ public class PackageTagsListTest {
        assertTrue(list.contains("package2", "attr1"));
        assertTrue(list.contains("package2", "attr2"));
        assertTrue(list.contains("package2", "attr3"));
        assertTrue(list.contains("package4", "attr1"));
        assertTrue(list.contains("package4", "attr2"));
        assertTrue(list.containsAll("package2"));
        assertTrue(list.includes("package1"));
        assertTrue(list.includes("package2"));
        assertFalse(list.contains("package1", "attr3"));
        assertFalse(list.contains("package4", "attr3"));
        assertFalse(list.containsAll("package1"));
        assertFalse(list.includes("package3"));

@@ -91,6 +96,51 @@ public class PackageTagsListTest {
        assertFalse(list.contains(bigList));
    }

    @Test
    public void testPackageTagsList_Remove() {
        PackageTagsList.Builder builder = new PackageTagsList.Builder()
                .add("package1", "attr1")
                .add("package1", "attr2")
                .add("package2")
                .add("package4", Arrays.asList("attr1", "attr2", "attr3"))
                .add("package3", "attr1")
                .remove("package1", "attr1")
                .remove("package1", "attr2")
                .remove("package2", "attr1")
                .remove("package4", Arrays.asList("attr1", "attr2"))
                .remove("package3");
        PackageTagsList list = builder.build();

        assertTrue(list.contains(builder.build()));
        assertFalse(list.contains("package1", "attr1"));
        assertFalse(list.contains("package1", "attr2"));
        assertTrue(list.contains("package2", "attr1"));
        assertTrue(list.contains("package2", "attr2"));
        assertTrue(list.contains("package2", "attr3"));
        assertFalse(list.contains("package3", "attr1"));
        assertFalse(list.contains("package4", "attr1"));
        assertFalse(list.contains("package4", "attr2"));
        assertTrue(list.contains("package4", "attr3"));
        assertTrue(list.containsAll("package2"));
        assertFalse(list.includes("package1"));
        assertTrue(list.includes("package2"));
        assertFalse(list.includes("package3"));
        assertTrue(list.includes("package4"));
    }

    @Test
    public void testPackageTagsList_EmptyCollections() {
        PackageTagsList.Builder builder = new PackageTagsList.Builder()
                .add("package1", Collections.emptyList())
                .add("package2")
                .remove("package2", Collections.emptyList());
        PackageTagsList list = builder.build();

        assertTrue(list.contains(builder.build()));
        assertFalse(list.contains("package1", "attr1"));
        assertTrue(list.contains("package2", "attr2"));
    }

    @Test
    public void testWriteToParcel() {
        PackageTagsList list = new PackageTagsList.Builder()
+8 −65
Original line number Diff line number Diff line
@@ -16,14 +16,10 @@

package android.location;


import android.annotation.NonNull;
import android.annotation.Nullable;
import android.location.util.identity.CallerIdentity;

import com.android.internal.annotations.Immutable;

import java.util.Set;
import android.os.PackageTagsList;

/**
 * Location manager local system service interface.
@@ -43,18 +39,14 @@ public abstract class LocationManagerInternal {
    }

    /**
     * Interface for getting callbacks when a location provider's location tags change.
     *
     * @see LocationTagInfo
     * Interface for getting callbacks when an app id's location provider package tags change.
     */
    public interface OnProviderLocationTagsChangeListener {
    public interface LocationPackageTagsListener {

        /**
         * Called when the location tags for a provider change.
         *
         * @param providerLocationTagInfo The tag info for a provider.
         * Called when the package tags for a location provider change for a uid.
         */
        void onLocationTagsChanged(@NonNull LocationTagInfo providerLocationTagInfo);
        void onLocationPackageTagsChanged(int uid, @NonNull PackageTagsList packageTagsList);
    }

    /**
@@ -109,58 +101,9 @@ public abstract class LocationManagerInternal {
    public abstract @Nullable LocationTime getGnssTimeMillis();

    /**
     * Sets a listener for changes in the location providers' tags. Passing
     * Sets a listener for changes in an app id's location provider package tags. Passing
     * {@code null} clears the current listener.
     *
     * @param listener The listener.
     */
    public abstract void setOnProviderLocationTagsChangeListener(
            @Nullable OnProviderLocationTagsChangeListener listener);

    /**
     * This class represents the location permission tags used by the location provider
     * packages in a given UID. These tags are strictly used for accessing state guarded
     * by the location permission(s) by a location provider which are required for the
     * provider to fulfill its function as being a location provider.
     */
    @Immutable
    public static class LocationTagInfo {
        private final int mUid;

        @NonNull
        private final String mPackageName;

        @Nullable
        private final Set<String> mLocationTags;

        public LocationTagInfo(int uid, @NonNull String packageName,
                @Nullable Set<String> locationTags) {
            mUid = uid;
            mPackageName = packageName;
            mLocationTags = locationTags;
        }

        /**
         * @return The UID for which tags are related.
         */
        public int getUid() {
            return mUid;
        }

        /**
         * @return The package for which tags are related.
     */
        @NonNull
        public String getPackageName() {
            return mPackageName;
        }

        /**
         * @return The tags for the package used for location related accesses.
         */
        @Nullable
        public Set<String> getTags() {
            return mLocationTags;
        }
    }
    public abstract void setLocationPackageTagsListener(
            @Nullable LocationPackageTagsListener listener);
}
+72 −31
Original line number Diff line number Diff line
@@ -65,7 +65,7 @@ import android.location.LastLocationRequest;
import android.location.Location;
import android.location.LocationManager;
import android.location.LocationManagerInternal;
import android.location.LocationManagerInternal.OnProviderLocationTagsChangeListener;
import android.location.LocationManagerInternal.LocationPackageTagsListener;
import android.location.LocationProvider;
import android.location.LocationRequest;
import android.location.LocationTime;
@@ -93,6 +93,7 @@ import android.util.Log;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.Preconditions;
import com.android.server.FgThread;
import com.android.server.LocalServices;
import com.android.server.SystemService;
import com.android.server.location.eventlog.LocationEventLog;
@@ -259,7 +260,7 @@ public class LocationManagerService extends ILocationManager.Stub implements
            new CopyOnWriteArrayList<>();

    @GuardedBy("mLock")
    @Nullable OnProviderLocationTagsChangeListener mOnProviderLocationTagsChangeListener;
    @Nullable LocationPackageTagsListener mLocationTagsChangedListener;

    LocationManagerService(Context context, Injector injector) {
        mContext = context.createAttributionContext(ATTRIBUTION_TAG);
@@ -1363,32 +1364,28 @@ public class LocationManagerService extends ILocationManager.Stub implements
            refreshAppOpsRestrictions(UserHandle.USER_ALL);
        }

        OnProviderLocationTagsChangeListener listener;
        synchronized (mLock) {
            listener = mOnProviderLocationTagsChangeListener;
        }

        if (listener != null) {
        if (!oldState.extraAttributionTags.equals(newState.extraAttributionTags)
                || !Objects.equals(oldState.identity, newState.identity)) {
                if (oldState.identity != null) {
                    listener.onLocationTagsChanged(
                            new LocationManagerInternal.LocationTagInfo(
                                    oldState.identity.getUid(),
                                    oldState.identity.getPackageName(),
                                    Collections.emptySet()));
            // since we're potentially affecting the tag lists for two different uids, acquire the
            // lock to ensure providers cannot change while we're looping over the providers
            // multiple times, which could lead to inconsistent results.
            synchronized (mLock) {
                LocationPackageTagsListener listener = mLocationTagsChangedListener;
                if (listener != null) {
                    int oldUid = oldState.identity != null ? oldState.identity.getUid() : -1;
                    int newUid = newState.identity != null ? newState.identity.getUid() : -1;
                    if (oldUid != -1) {
                        PackageTagsList tags = calculateAppOpsLocationSourceTags(oldUid);
                        FgThread.getHandler().post(
                                () -> listener.onLocationPackageTagsChanged(oldUid, tags));
                    }
                    // if the new app id is the same as the old app id, no need to invoke the
                    // listener twice, it's already been taken care of
                    if (newUid != -1 && newUid != oldUid) {
                        PackageTagsList tags = calculateAppOpsLocationSourceTags(newUid);
                        FgThread.getHandler().post(
                                () -> listener.onLocationPackageTagsChanged(newUid, tags));
                    }
                if (newState.identity != null) {
                    ArraySet<String> attributionTags = new ArraySet<>(
                            newState.extraAttributionTags.size() + 1);
                    attributionTags.addAll(newState.extraAttributionTags);
                    attributionTags.add(newState.identity.getAttributionTag());

                    listener.onLocationTagsChanged(
                            new LocationManagerInternal.LocationTagInfo(
                                    newState.identity.getUid(),
                                    newState.identity.getPackageName(),
                                    attributionTags));
                }
            }
        }
@@ -1436,6 +1433,31 @@ public class LocationManagerService extends ILocationManager.Stub implements
                userId);
    }

    PackageTagsList calculateAppOpsLocationSourceTags(int uid) {
        PackageTagsList.Builder builder = new PackageTagsList.Builder();
        for (LocationProviderManager manager : mProviderManagers) {
            AbstractLocationProvider.State managerState = manager.getState();
            if (managerState.identity == null) {
                continue;
            }
            if (managerState.identity.getUid() != uid) {
                continue;
            }

            builder.add(managerState.identity.getPackageName(), managerState.extraAttributionTags);
            if (managerState.extraAttributionTags.isEmpty()
                    || managerState.identity.getAttributionTag() != null) {
                builder.add(managerState.identity.getPackageName(),
                        managerState.identity.getAttributionTag());
            } else {
                Log.e(TAG, manager.getName() + " provider has specified a null attribution tag and "
                        + "a non-empty set of extra attribution tags - dropping the null "
                        + "attribution tag");
            }
        }
        return builder.build();
    }

    private class LocalService extends LocationManagerInternal {

        LocalService() {}
@@ -1506,10 +1528,29 @@ public class LocationManagerService extends ILocationManager.Stub implements
        }

        @Override
        public void setOnProviderLocationTagsChangeListener(
                @Nullable OnProviderLocationTagsChangeListener listener) {
        public void setLocationPackageTagsListener(
                @Nullable LocationPackageTagsListener listener) {
            synchronized (mLock) {
                mOnProviderLocationTagsChangeListener = listener;
                mLocationTagsChangedListener = listener;

                // calculate initial tag list and send to listener
                if (listener != null) {
                    ArraySet<Integer> uids = new ArraySet<>(mProviderManagers.size());
                    for (LocationProviderManager manager : mProviderManagers) {
                        CallerIdentity identity = manager.getIdentity();
                        if (identity != null) {
                            uids.add(identity.getUid());
                        }
                    }

                    for (int uid : uids) {
                        PackageTagsList tags = calculateAppOpsLocationSourceTags(uid);
                        if (!tags.isEmpty()) {
                            FgThread.getHandler().post(
                                    () -> listener.onLocationPackageTagsChanged(uid, tags));
                        }
                    }
                }
            }
        }
    }
+3 −17
Original line number Diff line number Diff line
@@ -263,10 +263,10 @@ public abstract class AbstractLocationProvider {
    }

    /**
     * The current allowed state of this provider.
     * The current state of the provider.
     */
    public final boolean isAllowed() {
        return mInternalState.get().state.allowed;
    public final State getState() {
        return mInternalState.get().state;
    }

    /**
@@ -276,13 +276,6 @@ public abstract class AbstractLocationProvider {
        setState(state -> state.withAllowed(allowed));
    }

    /**
     * The current provider properties of this provider.
     */
    public final @Nullable ProviderProperties getProperties() {
        return mInternalState.get().state.properties;
    }

    /**
     * Call this method to report a change in provider properties.
     */
@@ -290,13 +283,6 @@ public abstract class AbstractLocationProvider {
        setState(state -> state.withProperties(properties));
    }

    /**
     * The current identity of this provider.
     */
    public final @Nullable CallerIdentity getIdentity() {
        return mInternalState.get().state.identity;
    }

    /**
     * Call this method to report a change in the provider's identity.
     */
Loading