diff --git a/core/java/com/android/internal/util/FunctionalUtils.java b/core/java/com/android/internal/util/FunctionalUtils.java index 05ecdf9056236..91cc4b93dafbf 100644 --- a/core/java/com/android/internal/util/FunctionalUtils.java +++ b/core/java/com/android/internal/util/FunctionalUtils.java @@ -247,6 +247,48 @@ public class FunctionalUtils { } } + /** + * A {@link Consumer} that allows the caller to specify a custom checked {@link Exception} that + * can be thrown by the implementer. This is usually used when proxying/wrapping calls between + * different classes. + * + * @param Method parameter type + * @param Checked exception type + */ + @FunctionalInterface + public interface ThrowingCheckedConsumer { + void accept(Input input) throws ExceptionType; + } + + /** + * A {@link Consumer} that allows the caller to specify 2 different custom checked + * {@link Exception}s that can be thrown by the implementer. This is usually used when + * proxying/wrapping calls between different classes. + * + * @param Method parameter type + * @param First checked exception type + * @param Second checked exception type + */ + @FunctionalInterface + public interface ThrowingChecked2Consumer { + void accept(Input input) throws ExceptionOne, ExceptionTwo; + } + + /** + * A {@link Function} that allows the caller to specify a custom checked {@link Exception} that + * can be thrown by the implementer. This is usually used when proxying/wrapping calls between + * different classes. + * + * @param Method parameter type + * @param Method return type + * @param Checked exception type + */ + @FunctionalInterface + public interface ThrowingCheckedFunction { + Output apply(Input input) throws ExceptionType; + } + // TODO: add unit test /** * Gets a user-friendly name for a lambda function. diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index b4fcb9cd3bb7b..816c50dde2a80 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -60,7 +60,7 @@ import java.util.function.Consumer; * * @hide Only for use within the system server. */ -public abstract class PackageManagerInternal { +public abstract class PackageManagerInternal implements PackageSettingsSnapshotProvider { @IntDef(prefix = "PACKAGE_", value = { PACKAGE_SYSTEM, PACKAGE_SETUP_WIZARD, @@ -795,6 +795,9 @@ public abstract class PackageManagerInternal { * Perform the given action for each package. * Note that packages lock will be held while performing the actions. * + * If the caller does not need all packages, prefer the potentially non-locking + * {@link #withPackageSettingsSnapshot(Consumer)}. + * * @param actionLocked action to be performed */ public abstract void forEachPackage(Consumer actionLocked); @@ -803,6 +806,9 @@ public abstract class PackageManagerInternal { * Perform the given action for each {@link PackageSetting}. * Note that packages lock will be held while performing the actions. * + * If the caller does not need all packages, prefer the potentially non-locking + * {@link #withPackageSettingsSnapshot(Consumer)}. + * * @param actionLocked action to be performed */ public abstract void forEachPackageSetting(Consumer actionLocked); diff --git a/services/core/java/android/content/pm/PackageSettingsSnapshotProvider.java b/services/core/java/android/content/pm/PackageSettingsSnapshotProvider.java new file mode 100644 index 0000000000000..b9130d76dbf44 --- /dev/null +++ b/services/core/java/android/content/pm/PackageSettingsSnapshotProvider.java @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content.pm; + +import android.annotation.NonNull; + +import com.android.internal.util.FunctionalUtils; +import com.android.server.pm.PackageManagerService; +import com.android.server.pm.PackageSetting; + +import java.util.function.Consumer; +import java.util.function.Function; + +/** @hide */ +public interface PackageSettingsSnapshotProvider { + + /** + * Run a function block that requires access to {@link PackageSetting} data. This will + * ensure the {@link PackageManagerService} lock is taken before any caller's internal lock + * to avoid deadlock. Note that this method may or may not lock. If a snapshot is available + * and valid, it will iterate the snapshot set of data. + */ + void withPackageSettingsSnapshot( + @NonNull Consumer> block); + + /** + * Variant which returns a value to the caller. + * @see #withPackageSettingsSnapshot(Consumer) + */ + Output withPackageSettingsSnapshotReturning( + @NonNull FunctionalUtils.ThrowingFunction, Output> + block); + + /** + * Variant which throws. + * @see #withPackageSettingsSnapshot(Consumer) + */ + void withPackageSettingsSnapshotThrowing( + @NonNull FunctionalUtils.ThrowingCheckedConsumer, + ExceptionType> block) throws ExceptionType; + + /** + * Variant which throws 2 exceptions. + * @see #withPackageSettingsSnapshot(Consumer) + */ + void + withPackageSettingsSnapshotThrowing2( + @NonNull FunctionalUtils.ThrowingChecked2Consumer< + Function, ExceptionOne, ExceptionTwo> block) + throws ExceptionOne, ExceptionTwo; + + /** + * Variant which returns a value to the caller and throws. + * @see #withPackageSettingsSnapshot(Consumer) + */ + Output + withPackageSettingsSnapshotReturningThrowing( + @NonNull FunctionalUtils.ThrowingCheckedFunction< + Function, Output, ExceptionType> block) + throws ExceptionType; +} diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 03153d375d337..30aa8fdc4aaf0 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -1731,91 +1731,6 @@ public class PackageManagerService extends IPackageManager.Stub return PackageManagerService.this.getPackage(packageName); } - @NonNull - @Override - public void withPackageSettings(@NonNull Consumer> block) { - final Computer snapshot = snapshotComputer(); - - // This method needs to either lock or not lock consistently throughout the method, - // so if the live computer is returned, force a wrapping sync block. - if (snapshot == mLiveComputer) { - synchronized (mLock) { - block.accept(snapshot::getPackageSetting); - } - } else { - block.accept(snapshot::getPackageSetting); - } - } - - @Override - public Output withPackageSettingsReturning( - @NonNull FunctionalUtils.ThrowingFunction, Output> - block) { - final Computer snapshot = snapshotComputer(); - - // This method needs to either lock or not lock consistently throughout the method, - // so if the live computer is returned, force a wrapping sync block. - if (snapshot == mLiveComputer) { - synchronized (mLock) { - return block.apply(snapshot::getPackageSetting); - } - } else { - return block.apply(snapshot::getPackageSetting); - } - } - - @Override - public void withPackageSettingsThrowing( - @NonNull ThrowingConsumer, ExceptionType> block) - throws ExceptionType { - final Computer snapshot = snapshotComputer(); - - // This method needs to either lock or not lock consistently throughout the method, - // so if the live computer is returned, force a wrapping sync block. - if (snapshot == mLiveComputer) { - synchronized (mLock) { - block.accept(snapshot::getPackageSetting); - } - } else { - block.accept(snapshot::getPackageSetting); - } - } - - @Override - public void - withPackageSettingsThrowing2( - @NonNull Throwing2Consumer, ExceptionOne, - ExceptionTwo> block) throws ExceptionOne, ExceptionTwo { - final Computer snapshot = snapshotComputer(); - - // This method needs to either lock or not lock consistently throughout the method, - // so if the live computer is returned, force a wrapping sync block. - if (snapshot == mLiveComputer) { - synchronized (mLock) { - block.accept(snapshot::getPackageSetting); - } - } else { - block.accept(snapshot::getPackageSetting); - } - } - - @Override - public Output - withPackageSettingsReturningThrowing(@NonNull ThrowingFunction, Output, ExceptionType> block) throws ExceptionType { - final Computer snapshot = snapshotComputer(); - - // This method needs to either lock or not lock consistently throughout the method, - // so if the live computer is returned, force a wrapping sync block. - if (snapshot == mLiveComputer) { - synchronized (mLock) { - return block.apply(snapshot::getPackageSetting); - } - } else { - return block.apply(snapshot::getPackageSetting); - } - } - @Override public boolean filterAppAccess(String packageName, int callingUid, int userId) { return mPmInternal.filterAppAccess(packageName, callingUid, userId); @@ -1830,6 +1745,44 @@ public class PackageManagerService extends IPackageManager.Stub public boolean doesUserExist(@UserIdInt int userId) { return mUserManager.exists(userId); } + + @Override + public void withPackageSettingsSnapshot( + @NonNull Consumer> block) { + mPmInternal.withPackageSettingsSnapshot(block); + } + + @Override + public Output withPackageSettingsSnapshotReturning( + @NonNull FunctionalUtils.ThrowingFunction, Output> + block) { + return mPmInternal.withPackageSettingsSnapshotReturning(block); + } + + @Override + public void withPackageSettingsSnapshotThrowing( + @NonNull FunctionalUtils.ThrowingCheckedConsumer, + ExceptionType> block) throws ExceptionType { + mPmInternal.withPackageSettingsSnapshotThrowing(block); + } + + @Override + public void + withPackageSettingsSnapshotThrowing2( + @NonNull FunctionalUtils.ThrowingChecked2Consumer< + Function, ExceptionOne, ExceptionTwo> block) + throws ExceptionOne, ExceptionTwo { + mPmInternal.withPackageSettingsSnapshotThrowing2(block); + } + + @Override + public Output + withPackageSettingsSnapshotReturningThrowing( + @NonNull FunctionalUtils.ThrowingCheckedFunction< + Function, Output, ExceptionType> block) + throws ExceptionType { + return mPmInternal.withPackageSettingsSnapshotReturningThrowing(block); + } } /** @@ -1843,7 +1796,7 @@ public class PackageManagerService extends IPackageManager.Stub private final Watcher mWatcher = new Watcher() { @Override - public void onChange(@Nullable Watchable what) { + public void onChange(@Nullable Watchable what) { PackageManagerService.this.onChange(what); } }; @@ -27290,6 +27243,94 @@ public class PackageManagerService extends IPackageManager.Stub public void deleteOatArtifactsOfPackage(String packageName) { PackageManagerService.this.deleteOatArtifactsOfPackage(packageName); } + + @Override + public void withPackageSettingsSnapshot( + @NonNull Consumer> block) { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + block.accept(snapshot::getPackageSetting); + } + } else { + block.accept(snapshot::getPackageSetting); + } + } + + @Override + public Output withPackageSettingsSnapshotReturning( + @NonNull FunctionalUtils.ThrowingFunction, Output> + block) { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + return block.apply(snapshot::getPackageSetting); + } + } else { + return block.apply(snapshot::getPackageSetting); + } + } + + @Override + public void withPackageSettingsSnapshotThrowing( + @NonNull FunctionalUtils.ThrowingCheckedConsumer, + ExceptionType> block) throws ExceptionType { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + block.accept(snapshot::getPackageSetting); + } + } else { + block.accept(snapshot::getPackageSetting); + } + } + + @Override + public void + withPackageSettingsSnapshotThrowing2( + @NonNull FunctionalUtils.ThrowingChecked2Consumer< + Function, ExceptionOne, ExceptionTwo> block) + throws ExceptionOne, ExceptionTwo { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + block.accept(snapshot::getPackageSetting); + } + } else { + block.accept(snapshot::getPackageSetting); + } + } + + @Override + public Output + withPackageSettingsSnapshotReturningThrowing( + @NonNull FunctionalUtils.ThrowingCheckedFunction< + Function, Output, ExceptionType> block) + throws ExceptionType { + final Computer snapshot = snapshotComputer(); + + // This method needs to either lock or not lock consistently throughout the method, + // so if the live computer is returned, force a wrapping sync block. + if (snapshot == mLiveComputer) { + synchronized (mLock) { + return block.apply(snapshot::getPackageSetting); + } + } else { + return block.apply(snapshot::getPackageSetting); + } + } } @GuardedBy("mLock") diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java index b54b1698a3f42..65e4e95759b8c 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java @@ -25,6 +25,7 @@ import android.content.Intent; import android.content.pm.IntentFilterVerificationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; +import android.content.pm.PackageSettingsSnapshotProvider; import android.content.pm.ResolveInfo; import android.content.pm.verify.domain.DomainVerificationInfo; import android.content.pm.verify.domain.DomainVerificationManager; @@ -36,7 +37,6 @@ import android.util.Pair; import android.util.TypedXmlPullParser; import android.util.TypedXmlSerializer; -import com.android.internal.util.FunctionalUtils; import com.android.server.pm.PackageManagerService; import com.android.server.pm.PackageSetting; import com.android.server.pm.Settings; @@ -49,7 +49,6 @@ import java.io.IOException; import java.util.List; import java.util.Set; import java.util.UUID; -import java.util.function.Consumer; import java.util.function.Function; public interface DomainVerificationManagerInternal { @@ -406,7 +405,8 @@ public interface DomainVerificationManagerInternal { @NonNull Set domains, int state) throws NameNotFoundException; - interface Connection extends DomainVerificationEnforcer.Callback { + interface Connection extends DomainVerificationEnforcer.Callback, + PackageSettingsSnapshotProvider { /** * Notify that a settings change has been made and that eventually @@ -430,60 +430,7 @@ public interface DomainVerificationManagerInternal { */ void schedule(int code, @Nullable Object object); - /** - * Run a function block that requires access to {@link PackageSetting} data. This will - * ensure the {@link PackageManagerService} is taken before - * {@link DomainVerificationManagerInternal}'s lock is taken to avoid deadlock. - */ - void withPackageSettings(@NonNull Consumer> block); - - /** - * Variant which returns a value to the caller. - * @see #withPackageSettings(Consumer) - */ - Output withPackageSettingsReturning( - @NonNull FunctionalUtils.ThrowingFunction, Output> - block); - - /** - * Variant which throws. - * @see #withPackageSettings(Consumer) - */ - void withPackageSettingsThrowing( - @NonNull ThrowingConsumer, ExceptionType> block) - throws ExceptionType; - - /** - * Variant which throws 2 exceptions. - * @see #withPackageSettings(Consumer) - */ - void - withPackageSettingsThrowing2( - @NonNull Throwing2Consumer, ExceptionOne, - ExceptionTwo> block) throws ExceptionOne, ExceptionTwo; - - /** - * Variant which returns a value to the caller and throws. - * @see #withPackageSettings(Consumer) - */ - Output withPackageSettingsReturningThrowing( - @NonNull ThrowingFunction, Output, ExceptionType> - block) throws ExceptionType; - @UserIdInt int[] getAllUserIds(); - - interface ThrowingConsumer { - void accept(Input input) throws ExceptionType; - } - - interface Throwing2Consumer { - void accept(Input input) throws ExceptionOne, ExceptionTwo; - } - - interface ThrowingFunction { - Output apply(Input input) throws ExceptionType; - } } } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java index f96eeb39e69e4..b1b4e2afeb497 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java @@ -260,7 +260,7 @@ public class DomainVerificationService extends SystemService public DomainVerificationInfo getDomainVerificationInfo(@NonNull String packageName) throws NameNotFoundException { mEnforcer.assertApprovedQuerent(mConnection.getCallingUid(), mProxy); - return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + return mConnection.withPackageSettingsSnapshotReturningThrowing(pkgSettings -> { synchronized (mLock) { PackageSetting pkgSetting = pkgSettings.apply(packageName); AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); @@ -320,7 +320,7 @@ public class DomainVerificationService extends SystemService @NonNull Set domains, int state) throws NameNotFoundException { mEnforcer.assertApprovedVerifier(callingUid, mProxy); - return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + return mConnection.withPackageSettingsSnapshotReturningThrowing(pkgSettings -> { synchronized (mLock) { List verifiedDomains = new ArrayList<>(); @@ -376,7 +376,7 @@ public class DomainVerificationService extends SystemService ArraySet verifiedDomains = new ArraySet<>(); if (packageName == null) { - mConnection.withPackageSettings(pkgSettings -> { + mConnection.withPackageSettingsSnapshot(pkgSettings -> { synchronized (mLock) { ArraySet validDomains = new ArraySet<>(); @@ -411,7 +411,7 @@ public class DomainVerificationService extends SystemService } }); } else { - mConnection.withPackageSettingsThrowing(pkgSettings -> { + mConnection.withPackageSettingsSnapshotThrowing(pkgSettings -> { synchronized (mLock) { DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); if (pkgState == null) { @@ -548,7 +548,7 @@ public class DomainVerificationService extends SystemService return DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID; } - return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + return mConnection.withPackageSettingsSnapshotReturningThrowing(pkgSettings -> { synchronized (mLock) { GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, false /* forAutoVerify */, callingUid, userId, pkgSettings); @@ -588,7 +588,7 @@ public class DomainVerificationService extends SystemService @NonNull String packageName, boolean enabled, @Nullable ArraySet domains) throws NameNotFoundException { mEnforcer.assertInternal(mConnection.getCallingUid()); - mConnection.withPackageSettingsThrowing(pkgSettings -> { + mConnection.withPackageSettingsSnapshotThrowing(pkgSettings -> { synchronized (mLock) { DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); if (pkgState == null) { @@ -694,7 +694,7 @@ public class DomainVerificationService extends SystemService throw DomainVerificationUtils.throwPackageUnavailable(packageName); } - return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + return mConnection.withPackageSettingsSnapshotReturningThrowing(pkgSettings -> { synchronized (mLock) { PackageSetting pkgSetting = pkgSettings.apply(packageName); AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); @@ -747,7 +747,7 @@ public class DomainVerificationService extends SystemService mEnforcer.assertOwnerQuerent(mConnection.getCallingUid(), mConnection.getCallingUserId(), userId); - return mConnection.withPackageSettingsReturningThrowing(pkgSettings -> { + return mConnection.withPackageSettingsSnapshotReturningThrowing(pkgSettings -> { SparseArray> levelToPackages = getOwnersForDomainInternal(domain, false, userId, pkgSettings); if (levelToPackages.size() == 0) { @@ -956,17 +956,25 @@ public class DomainVerificationService extends SystemService } AndroidPackage pkg = newPkgSetting.getPkg(); - ArraySet domains = mCollector.collectValidAutoVerifyDomains(pkg); - boolean hasAutoVerifyDomains = !domains.isEmpty(); + ArraySet autoVerifyDomains = mCollector.collectValidAutoVerifyDomains(pkg); + boolean hasAutoVerifyDomains = !autoVerifyDomains.isEmpty(); boolean isPendingOrRestored = pkgState != null; if (isPendingOrRestored) { pkgState = new DomainVerificationPkgState(pkgState, domainSetId, hasAutoVerifyDomains); + pkgState.getStateMap().retainAll(autoVerifyDomains); + + Set webDomains = mCollector.collectAllWebDomains(pkg); + SparseArray userStates = pkgState.getUserStates(); + int size = userStates.size(); + for (int index = 0; index < size; index++) { + userStates.valueAt(index).retainHosts(webDomains); + } } else { pkgState = new DomainVerificationPkgState(pkgName, domainSetId, hasAutoVerifyDomains); } - boolean needsBroadcast = - applyImmutableState(newPkgSetting, pkgState.getStateMap(), domains); + boolean needsBroadcast = applyImmutableState(newPkgSetting, pkgState.getStateMap(), + autoVerifyDomains); if (needsBroadcast && !isPendingOrRestored) { // TODO(b/159952358): Test this behavior // Attempt to preserve user experience by automatically verifying all domains from @@ -997,9 +1005,10 @@ public class DomainVerificationService extends SystemService && legacyInfo.getStatus() == PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS) { ArrayMap stateMap = pkgState.getStateMap(); - int domainsSize = domains.size(); + int domainsSize = autoVerifyDomains.size(); for (int index = 0; index < domainsSize; index++) { - stateMap.put(domains.valueAt(index), DomainVerificationState.STATE_MIGRATED); + stateMap.put(autoVerifyDomains.valueAt(index), + DomainVerificationState.STATE_MIGRATED); } } } @@ -1048,7 +1057,7 @@ public class DomainVerificationService extends SystemService public void writeSettings(@NonNull TypedXmlSerializer serializer, boolean includeSignatures, @UserIdInt int userId) throws IOException { - mConnection.withPackageSettingsThrowing(pkgSettings -> { + mConnection.withPackageSettingsSnapshotThrowing(pkgSettings -> { synchronized (mLock) { Function pkgNameToSignature = null; if (includeSignatures) { @@ -1077,7 +1086,7 @@ public class DomainVerificationService extends SystemService @Override public void readSettings(@NonNull TypedXmlPullParser parser) throws IOException, XmlPullParserException { - mConnection.withPackageSettingsThrowing2( + mConnection.withPackageSettingsSnapshotThrowing2( pkgSettings -> { synchronized (mLock) { mSettings.readSettings(parser, mAttachedPkgStates, pkgSettings); @@ -1094,7 +1103,7 @@ public class DomainVerificationService extends SystemService @Override public void restoreSettings(@NonNull TypedXmlPullParser parser) throws IOException, XmlPullParserException { - mConnection.withPackageSettingsThrowing2( + mConnection.withPackageSettingsSnapshotThrowing2( pkgSettings -> { synchronized (mLock) { mSettings.restoreSettings(parser, mAttachedPkgStates, pkgSettings); @@ -1175,7 +1184,7 @@ public class DomainVerificationService extends SystemService @Override public void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable Integer userId) throws NameNotFoundException { - mConnection.withPackageSettingsThrowing( + mConnection.withPackageSettingsSnapshotThrowing( pkgSettings -> printState(writer, packageName, userId, pkgSettings)); } @@ -1193,7 +1202,7 @@ public class DomainVerificationService extends SystemService public void printOwnersForPackage(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable @UserIdInt Integer userId) throws NameNotFoundException { - mConnection.withPackageSettingsThrowing(pkgSettings -> { + mConnection.withPackageSettingsSnapshotThrowing(pkgSettings -> { synchronized (mLock) { if (packageName == null) { int size = mAttachedPkgStates.size(); @@ -1242,7 +1251,7 @@ public class DomainVerificationService extends SystemService @Override public void printOwnersForDomains(@NonNull IndentingPrintWriter writer, @NonNull List domains, @Nullable @UserIdInt Integer userId) { - mConnection.withPackageSettings(pkgSettings -> { + mConnection.withPackageSettingsSnapshot(pkgSettings -> { synchronized (mLock) { int size = domains.size(); for (int index = 0; index < size; index++) { @@ -1421,7 +1430,7 @@ public class DomainVerificationService extends SystemService @Override public void clearDomainVerificationState(@Nullable List packageNames) { mEnforcer.assertInternal(mConnection.getCallingUid()); - mConnection.withPackageSettings(pkgSettings -> { + mConnection.withPackageSettingsSnapshot(pkgSettings -> { synchronized (mLock) { if (packageNames == null) { int size = mAttachedPkgStates.size(); @@ -2022,43 +2031,46 @@ public class DomainVerificationService extends SystemService } @Override - public void withPackageSettings( + public void withPackageSettingsSnapshot( @NonNull Consumer> block) { enforceLocking(); - mConnection.withPackageSettings(block); + mConnection.withPackageSettingsSnapshot(block); } @Override - public Output withPackageSettingsReturning( + public Output withPackageSettingsSnapshotReturning( @NonNull FunctionalUtils.ThrowingFunction, Output> block) { enforceLocking(); - return mConnection.withPackageSettingsReturning(block); + return mConnection.withPackageSettingsSnapshotReturning(block); } @Override - public void withPackageSettingsThrowing( - @NonNull ThrowingConsumer, ExceptionType> block) - throws ExceptionType { + public void withPackageSettingsSnapshotThrowing( + @NonNull FunctionalUtils.ThrowingCheckedConsumer, + ExceptionType> block) throws ExceptionType { enforceLocking(); - mConnection.withPackageSettingsThrowing(block); + mConnection.withPackageSettingsSnapshotThrowing(block); } @Override public void - withPackageSettingsThrowing2( - @NonNull Throwing2Consumer, ExceptionOne, - ExceptionTwo> block) throws ExceptionOne, ExceptionTwo { + withPackageSettingsSnapshotThrowing2( + @NonNull FunctionalUtils.ThrowingChecked2Consumer< + Function, ExceptionOne, ExceptionTwo> block) + throws ExceptionOne, ExceptionTwo { enforceLocking(); - mConnection.withPackageSettingsThrowing2(block); + mConnection.withPackageSettingsSnapshotThrowing2(block); } @Override public Output - withPackageSettingsReturningThrowing(@NonNull ThrowingFunction, Output, ExceptionType> block) throws ExceptionType { + withPackageSettingsSnapshotReturningThrowing( + @NonNull FunctionalUtils.ThrowingCheckedFunction< + Function, Output, ExceptionType> block) + throws ExceptionType { enforceLocking(); - return mConnection.withPackageSettingsReturningThrowing(block); + return mConnection.withPackageSettingsSnapshotReturningThrowing(block); } @Override diff --git a/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java b/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java index aa7407ce3fe84..41de3fc06d47f 100644 --- a/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java +++ b/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java @@ -73,6 +73,11 @@ public class DomainVerificationInternalUserState { return this; } + public DomainVerificationInternalUserState retainHosts(@NonNull Set hosts) { + mEnabledHosts.retainAll(hosts); + return this; + } + // Code below generated by codegen v1.0.22. diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt index 83c126842213b..6c2a8916617b2 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt @@ -22,6 +22,7 @@ import android.content.pm.PackageUserState import android.content.pm.Signature import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainOwner import android.content.pm.verify.domain.DomainVerificationInfo.STATE_MODIFIABLE_VERIFIED import android.content.pm.verify.domain.DomainVerificationInfo.STATE_NO_RESPONSE import android.content.pm.verify.domain.DomainVerificationInfo.STATE_SUCCESS @@ -297,11 +298,95 @@ class DomainVerificationPackageTest { service.addPackage(pkg1) + assertAddPackageActivePendingRestoredState(service) + } + + @Test + fun addPackagePendingStripInvalidDomains() { + val xml = addPackagePendingOrRestoredWithInvalidDomains() + val service = makeService(pkg1, pkg2) + xml.byteInputStream().use { + service.readSettings(Xml.resolvePullParser(it)) + } + + service.addPackage(pkg1) + + val userState = service.getUserState(pkg1.getName()) + assertThat(userState.packageName).isEqualTo(pkg1.getName()) + assertThat(userState.identifier).isEqualTo(pkg1.domainSetId) + assertThat(userState.isLinkHandlingAllowed).isEqualTo(false) + assertThat(userState.user.identifier).isEqualTo(USER_ID) + assertThat(userState.hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_SELECTED, + )) + + assertAddPackageActivePendingRestoredState(service) + } + + @Test + fun addPackageRestoredStripInvalidDomains() { + val xml = addPackagePendingOrRestoredWithInvalidDomains() + val service = makeService(pkg1, pkg2) + xml.byteInputStream().use { + service.restoreSettings(Xml.resolvePullParser(it)) + } + + service.addPackage(pkg1) + + assertAddPackageActivePendingRestoredState(service, expectRestore = true) + } + + /** + * Shared string that contains invalid [DOMAIN_3] and [DOMAIN_4] which should be stripped from + * the final state. + */ + private fun addPackagePendingOrRestoredWithInvalidDomains(): String = + // language=XML + """ + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent() + + /** + * Shared method to assert the same output when testing adding pkg1. + */ + private fun assertAddPackageActivePendingRestoredState( + service: DomainVerificationService, + expectRestore: Boolean = false + ) { val info = service.getInfo(pkg1.getName()) assertThat(info.packageName).isEqualTo(pkg1.getName()) assertThat(info.identifier).isEqualTo(pkg1.domainSetId) assertThat(info.hostToStateMap).containsExactlyEntriesIn(mapOf( - DOMAIN_1 to STATE_SUCCESS, + // To share the majority of code, special case restoration to check a different int + DOMAIN_1 to if (expectRestore) STATE_MODIFIABLE_VERIFIED else STATE_SUCCESS, DOMAIN_2 to STATE_NO_RESPONSE, )) @@ -317,6 +402,23 @@ class DomainVerificationPackageTest { assertThat(service.queryValidVerificationPackageNames()) .containsExactly(pkg1.getName()) + + // Re-enable link handling to check that the 3/4 domains were stripped + service.setDomainVerificationLinkHandlingAllowed(pkg1.getName(), true, USER_ID) + + assertThat(service.getOwnersForDomain(DOMAIN_1, USER_ID)) + .containsExactly(DomainOwner(PKG_ONE, false)) + + assertThat(service.getOwnersForDomain(DOMAIN_2, USER_ID)) + .containsExactly(DomainOwner(PKG_ONE, true)) + + assertThat(service.getOwnersForDomain(DOMAIN_2, USER_ID + 10)).isEmpty() + + listOf(DOMAIN_3, DOMAIN_4).forEach { domain -> + listOf(USER_ID, USER_ID + 10).forEach { userId -> + assertThat(service.getOwnersForDomain(domain, userId)).isEmpty() + } + } } @Test @@ -528,9 +630,9 @@ class DomainVerificationPackageTest { serviceBefore.addPackage(pkg2) serviceBefore.setStatus(pkg1.domainSetId, setOf(DOMAIN_1), STATE_SUCCESS) - serviceBefore.setDomainVerificationLinkHandlingAllowed(pkg1.getName(), false, 1) + serviceBefore.setDomainVerificationLinkHandlingAllowed(pkg1.getName(), false, 10) serviceBefore.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_2), true, 0) - serviceBefore.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_3), true, 1) + serviceBefore.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_3), true, 10) fun assertExpectedState(service: DomainVerificationService) { service.assertState( @@ -541,7 +643,7 @@ class DomainVerificationPackageTest { ) service.assertState( - pkg1, userId = 1, linkHandingAllowed = false, hostToStateMap = mapOf( + pkg1, userId = 10, linkHandingAllowed = false, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_VERIFIED, DOMAIN_2 to DOMAIN_STATE_NONE, ) @@ -556,7 +658,7 @@ class DomainVerificationPackageTest { ) service.assertState( - pkg2, userId = 1, hostToStateMap = mapOf( + pkg2, userId = 10, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, DOMAIN_2 to DOMAIN_STATE_NONE, DOMAIN_3 to DOMAIN_STATE_SELECTED, @@ -572,7 +674,7 @@ class DomainVerificationPackageTest { } val backupUser1 = ByteArrayOutputStream().use { - serviceBefore.writeSettings(Xml.resolveSerializer(it), true, 1) + serviceBefore.writeSettings(Xml.resolveSerializer(it), true, 10) it.toByteArray() } @@ -581,7 +683,7 @@ class DomainVerificationPackageTest { serviceAfter.addPackage(pkg2) // Check the state is default before the restoration applies - listOf(0, 1).forEach { + listOf(0, 10).forEach { serviceAfter.assertState( pkg1, userId = it, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, @@ -590,7 +692,7 @@ class DomainVerificationPackageTest { ) } - listOf(0, 1).forEach { + listOf(0, 10).forEach { serviceAfter.assertState( pkg2, userId = it, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, @@ -606,14 +708,14 @@ class DomainVerificationPackageTest { // Assert user 1 was restored serviceAfter.assertState( - pkg1, userId = 1, linkHandingAllowed = false, hostToStateMap = mapOf( + pkg1, userId = 10, linkHandingAllowed = false, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_VERIFIED, DOMAIN_2 to DOMAIN_STATE_NONE, ) ) serviceAfter.assertState( - pkg2, userId = 1, hostToStateMap = mapOf( + pkg2, userId = 10, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, DOMAIN_2 to DOMAIN_STATE_NONE, DOMAIN_3 to DOMAIN_STATE_SELECTED, @@ -679,7 +781,7 @@ class DomainVerificationPackageTest { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } whenever(doesUserExist(0)) { true } - whenever(doesUserExist(1)) { true } + whenever(doesUserExist(10)) { true } whenever(scheduleWriteSettings()) // Need to provide an internal UID so some permission checks are ignored @@ -732,7 +834,7 @@ class DomainVerificationPackageTest { whenever(getInstantApp(anyInt())) { false } whenever(firstInstallTime) { 0L } whenever(readUserState(0)) { PackageUserState() } - whenever(readUserState(1)) { PackageUserState() } + whenever(readUserState(10)) { PackageUserState() } whenever(signatures) { arrayOf(Signature(signature)) } whenever(isSystem) { isSystemApp } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt index 00986f741eea4..48845be693feb 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationTestUtils.kt @@ -30,25 +30,25 @@ internal object DomainVerificationTestUtils { fun DomainVerificationManagerInternal.Connection.mockPackageSettings( block: (String) -> PackageSetting? ) { - whenever(withPackageSettings(any())) { + whenever(withPackageSettingsSnapshot(any())) { (arguments[0] as Consumer>).accept { block(it) } } - whenever(withPackageSettingsReturning(any())) { + whenever(withPackageSettingsSnapshotReturning(any())) { (arguments[0] as FunctionalUtils.ThrowingFunction, *>) .apply { block(it) } } - whenever(withPackageSettingsThrowing(any())) { - (arguments[0] as DomainVerificationManagerInternal.Connection.ThrowingConsumer< + whenever(withPackageSettingsSnapshotThrowing(any())) { + (arguments[0] as FunctionalUtils.ThrowingCheckedConsumer< Function, *>) .accept { block(it) } } - whenever(withPackageSettingsThrowing2(any())) { - (arguments[0] as DomainVerificationManagerInternal.Connection.Throwing2Consumer< + whenever(withPackageSettingsSnapshotThrowing2(any())) { + (arguments[0] as FunctionalUtils.ThrowingChecked2Consumer< Function, *, *>) .accept { block(it) } } - whenever(withPackageSettingsReturningThrowing(any())) { - (arguments[0] as DomainVerificationManagerInternal.Connection.ThrowingFunction< + whenever(withPackageSettingsSnapshotReturningThrowing(any())) { + (arguments[0] as FunctionalUtils.ThrowingCheckedFunction< Function, *, *>) .apply { block(it) } }