Add permissions to FeatureFlagService and override api.

This will allow flag flipper to change flags and prevent others
from doing so.

Bug: 279054964
Test: atest FrameworksServicesTests:com.android.server.flag
Change-Id: I6f2e2acecc202d3e24f94ef0801b98c2fd0aa456
This commit is contained in:
Dave Mankoff
2023-05-30 20:51:52 +00:00
parent 7c3cae6b71
commit c25fd5aefe
13 changed files with 499 additions and 27 deletions

View File

@@ -24,4 +24,8 @@ package android.flags;
* @hide
*/
public interface DynamicFlag<T> extends Flag<T> {
@Override
default boolean isDynamic() {
return true;
}
}

View File

@@ -250,7 +250,13 @@ public class FeatureFlags {
return flag;
}
protected void sync() {
/**
* Sync any known flags that have not yet been synced.
*
* This is called implicitly when any flag is read, and is not generally needed except in
* exceptional circumstances.
*/
public void sync() {
synchronized (FeatureFlags.class) {
if (mDirtyFlags.isEmpty()) {
return;

View File

@@ -37,4 +37,9 @@ public interface Flag<T> {
/** The value of this flag if no override has been set. Null values are not supported. */
@NonNull
T getDefault();
/** Returns true if the value of this flag can change at runtime. */
default boolean isDynamic() {
return false;
}
}

View File

@@ -62,4 +62,28 @@ interface IFeatureFlags {
* {@link #registerCallback}.
*/
void unregisterCallback(IFeatureFlagsCallback callback);
/**
* Query the {@link com.android.server.flags.FeatureFlagsService} for flags, but don't
* cache them. See {@link #syncFlags}.
*
* You almost certainly don't want this method. This is intended for the Flag Flipper
* application that needs to query the state of system but doesn't want to affect it by
* doing so. All other clients should use {@link syncFlags}.
*/
List<SyncableFlag> queryFlags(in List<SyncableFlag> flagList);
/**
* Change a flags value in the system.
*
* This is intended for use by the Flag Flipper application.
*/
void overrideFlag(in SyncableFlag flag);
/**
* Restore a flag to its default value.
*
* This is intended for use by the Flag Flipper application.
*/
void resetFlag(in SyncableFlag flag);
}

View File

@@ -28,16 +28,28 @@ public final class SyncableFlag implements Parcelable {
private final String mName;
private final String mValue;
private final boolean mDynamic;
private final boolean mOverridden;
public SyncableFlag(
@NonNull String namespace,
@NonNull String name,
@NonNull String value,
boolean dynamic) {
this(namespace, name, value, dynamic, false);
}
public SyncableFlag(
@NonNull String namespace,
@NonNull String name,
@NonNull String value,
boolean dynamic,
boolean overridden
) {
mNamespace = namespace;
mName = name;
mValue = value;
mDynamic = dynamic;
mOverridden = overridden;
}
@NonNull
@@ -55,16 +67,23 @@ public final class SyncableFlag implements Parcelable {
return mValue;
}
@NonNull
public boolean isDynamic() {
return mDynamic;
}
public boolean isOverridden() {
return mOverridden;
}
@NonNull
public static final Parcelable.Creator<SyncableFlag> CREATOR = new Parcelable.Creator<>() {
public SyncableFlag createFromParcel(Parcel in) {
return new SyncableFlag(
in.readString(), in.readString(), in.readString(), in.readBoolean());
in.readString(),
in.readString(),
in.readString(),
in.readBoolean(),
in.readBoolean());
}
public SyncableFlag[] newArray(int size) {
@@ -83,6 +102,7 @@ public final class SyncableFlag implements Parcelable {
dest.writeString(mName);
dest.writeString(mValue);
dest.writeBoolean(mDynamic);
dest.writeBoolean(mOverridden);
}
@Override

View File

@@ -0,0 +1,93 @@
/*
* Copyright (C) 2023 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 com.android.internal.flags;
import android.flags.BooleanFlag;
import android.flags.DynamicBooleanFlag;
import android.flags.FeatureFlags;
import android.flags.FusedOffFlag;
import android.flags.FusedOnFlag;
import android.flags.SyncableFlag;
import java.util.ArrayList;
import java.util.List;
/**
* Flags defined here are can be read by code in core.
*
* Flags not defined here will throw a security exception if third-party processes attempts to read
* them.
*
* DO NOT define a flag here unless you explicitly intend for that flag to be readable by code that
* runs inside a third party process.
*/
public abstract class CoreFlags {
private static final List<SyncableFlag> sKnownFlags = new ArrayList<>();
public static BooleanFlag BOOL_FLAG = booleanFlag("core", "bool_flag", false);
public static FusedOffFlag OFF_FLAG = fusedOffFlag("core", "off_flag");
public static FusedOnFlag ON_FLAG = fusedOnFlag("core", "on_flag");
public static DynamicBooleanFlag DYN_FLAG = dynamicBooleanFlag("core", "dyn_flag", true);
/** Returns true if the passed in flag matches a flag in this class. */
public static boolean isCoreFlag(SyncableFlag flag) {
for (SyncableFlag knownFlag : sKnownFlags) {
if (knownFlag.getName().equals(flag.getName())
&& knownFlag.getNamespace().equals(flag.getNamespace())) {
return true;
}
}
return false;
}
public static List<SyncableFlag> getCoreFlags() {
return sKnownFlags;
}
private static BooleanFlag booleanFlag(String namespace, String name, boolean defaultValue) {
BooleanFlag f = FeatureFlags.booleanFlag(namespace, name, defaultValue);
sKnownFlags.add(new SyncableFlag(namespace, name, Boolean.toString(defaultValue), false));
return f;
}
private static FusedOffFlag fusedOffFlag(String namespace, String name) {
FusedOffFlag f = FeatureFlags.fusedOffFlag(namespace, name);
sKnownFlags.add(new SyncableFlag(namespace, name, "false", false));
return f;
}
private static FusedOnFlag fusedOnFlag(String namespace, String name) {
FusedOnFlag f = FeatureFlags.fusedOnFlag(namespace, name);
sKnownFlags.add(new SyncableFlag(namespace, name, "true", false));
return f;
}
private static DynamicBooleanFlag dynamicBooleanFlag(
String namespace, String name, boolean defaultValue) {
DynamicBooleanFlag f = FeatureFlags.dynamicBooleanFlag(namespace, name, defaultValue);
sKnownFlags.add(new SyncableFlag(namespace, name, Boolean.toString(defaultValue), true));
return f;
}
}

View File

@@ -7655,6 +7655,24 @@
<permission android:name="android.permission.GET_ANY_PROVIDER_TYPE"
android:protectionLevel="signature" />
<!-- @hide Allows internal applications to read and synchronize non-core flags.
Apps without this permission can only read a subset of flags specifically intended
for use in "core", (i.e. third party apps). Apps with this permission can define their
own flags, and federate those values with other system-level apps.
<p>Not for use by third-party applications.
<p>Protection level: signature
-->
<permission android:name="android.permission.SYNC_FLAGS"
android:protectionLevel="signature" />
<!-- @hide Allows internal applications to override flags in the FeatureFlags service.
<p>Not for use by third-party applications.
<p>Protection level: signature
-->
<permission android:name="android.permission.WRITE_FLAGS"
android:protectionLevel="signature" />
<!-- Attribution for Geofencing service. -->
<attribution android:tag="GeofencingService" android:label="@string/geofencing_service"/>
<!-- Attribution for Country Detector. -->

View File

@@ -38,6 +38,56 @@ class IFeatureFlagsFake implements IFeatureFlags {
return mOverrides == null ? flagList : mOverrides;
}
@Override
public List<SyncableFlag> queryFlags(List<SyncableFlag> flagList) {
return mOverrides == null ? flagList : mOverrides; }
@Override
public void overrideFlag(SyncableFlag syncableFlag) {
SyncableFlag match = findFlag(syncableFlag);
if (match != null) {
mOverrides.remove(match);
}
mOverrides.add(syncableFlag);
for (IFeatureFlagsCallback cb : mCallbacks) {
try {
cb.onFlagChange(syncableFlag);
} catch (RemoteException e) {
// does not happen in fakes.
}
}
}
@Override
public void resetFlag(SyncableFlag syncableFlag) {
SyncableFlag match = findFlag(syncableFlag);
if (match != null) {
mOverrides.remove(match);
}
for (IFeatureFlagsCallback cb : mCallbacks) {
try {
cb.onFlagChange(syncableFlag);
} catch (RemoteException e) {
// does not happen in fakes.
}
}
}
private SyncableFlag findFlag(SyncableFlag syncableFlag) {
SyncableFlag match = null;
for (SyncableFlag sf : mOverrides) {
if (sf.getName().equals(syncableFlag.getName())
&& sf.getNamespace().equals(syncableFlag.getNamespace())) {
match = sf;
break;
}
}
return match;
}
@Override
public void registerCallback(IFeatureFlagsCallback callback) {
mCallbacks.add(callback);

View File

@@ -132,25 +132,8 @@ class DynamicFlagBinderDelegate {
// about changes coming in from adb, DeviceConfig, or other sources.
// And also so that we can keep flags relatively consistent across processes.
// If we already have a value cached, just use that.
String value = null;
DynamicFlagData data = mDynamicFlags.getOrNull(ns, name);
if (data != null) {
value = data.getValue();
} else {
// Put the value in the cache for future reference.
data = new DynamicFlagData(ns, name);
mDynamicFlags.setIfChanged(ns, name, data);
}
// If we're not in a release build, flags can be overridden locally on device.
if (!Build.IS_USER && value == null) {
value = mFlagStore.get(ns, name);
}
// If we still don't have a value, maybe DeviceConfig does?
// Fallback to sf.getValue() here as well.
if (value == null) {
value = DeviceConfig.getString(ns, name, sf.getValue());
}
String value = getFlagValue(ns, name, sf.getValue());
// DeviceConfig listeners are per-namespace.
if (!mDynamicFlags.containsNamespace(ns)) {
DeviceConfig.addOnPropertiesChangedListener(
@@ -200,6 +183,30 @@ class DynamicFlagBinderDelegate {
}
}
String getFlagValue(String namespace, String name, String defaultValue) {
// If we already have a value cached, just use that.
String value = null;
DynamicFlagData data = mDynamicFlags.getOrNull(namespace, name);
if (data != null) {
value = data.getValue();
} else {
// Put the value in the cache for future reference.
data = new DynamicFlagData(namespace, name);
mDynamicFlags.setIfChanged(namespace, name, data);
}
// If we're not in a release build, flags can be overridden locally on device.
if (!Build.IS_USER && value == null) {
value = mFlagStore.get(namespace, name);
}
// If we still don't have a value, maybe DeviceConfig does?
// Fallback to sf.getValue() here as well.
if (value == null) {
value = DeviceConfig.getString(namespace, name, defaultValue);
}
return value;
}
private static class DynamicFlagData {
private final String mNamespace;
private final String mName;

View File

@@ -24,6 +24,9 @@ import android.flags.SyncableFlag;
import android.os.Build;
import android.os.ParcelFileDescriptor;
import com.android.internal.flags.CoreFlags;
import com.android.server.flags.FeatureFlagsService.PermissionsChecker;
import java.io.FileOutputStream;
import java.util.ArrayList;
import java.util.List;
@@ -33,11 +36,16 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub {
private final FlagsShellCommand mShellCommand;
private final FlagCache<String> mFlagCache = new FlagCache<>();
private final DynamicFlagBinderDelegate mDynamicFlagDelegate;
private final PermissionsChecker mPermissionsChecker;
FeatureFlagsBinder(FlagOverrideStore flagStore, FlagsShellCommand shellCommand) {
FeatureFlagsBinder(
FlagOverrideStore flagStore,
FlagsShellCommand shellCommand,
PermissionsChecker permissionsChecker) {
mFlagStore = flagStore;
mShellCommand = shellCommand;
mDynamicFlagDelegate = new DynamicFlagBinderDelegate(flagStore);
mPermissionsChecker = permissionsChecker;
}
@Override
@@ -50,11 +58,29 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub {
mDynamicFlagDelegate.unregisterCallback(getCallingPid(), callback);
}
// Note: The internals of this method should be kept in sync with queryFlags
// as they both should return identical results. The difference is that this method
// caches any values it receives and/or reads, whereas queryFlags does not.
@Override
public List<SyncableFlag> syncFlags(List<SyncableFlag> incomingFlags) {
int pid = getCallingPid();
List<SyncableFlag> outputFlags = new ArrayList<>();
boolean hasFullSyncPrivileges = false;
SecurityException permissionFailureException = null;
try {
assertSyncPermission();
hasFullSyncPrivileges = true;
} catch (SecurityException e) {
permissionFailureException = e;
}
for (SyncableFlag sf : incomingFlags) {
if (!hasFullSyncPrivileges && !CoreFlags.isCoreFlag(sf)) {
throw permissionFailureException;
}
String ns = sf.getNamespace();
String name = sf.getName();
SyncableFlag outFlag;
@@ -76,6 +102,58 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub {
return outputFlags;
}
@Override
public void overrideFlag(SyncableFlag flag) {
assertWritePermission();
mFlagStore.set(flag.getNamespace(), flag.getName(), flag.getValue());
}
@Override
public void resetFlag(SyncableFlag flag) {
assertWritePermission();
mFlagStore.erase(flag.getNamespace(), flag.getName());
}
@Override
public List<SyncableFlag> queryFlags(List<SyncableFlag> incomingFlags) {
assertSyncPermission();
List<SyncableFlag> outputFlags = new ArrayList<>();
for (SyncableFlag sf : incomingFlags) {
String ns = sf.getNamespace();
String name = sf.getName();
String value;
String storeValue = mFlagStore.get(ns, name);
boolean overridden = storeValue != null;
if (sf.isDynamic()) {
value = mDynamicFlagDelegate.getFlagValue(ns, name, sf.getValue());
} else {
value = mFlagCache.getOrNull(ns, name);
if (value == null) {
value = Build.IS_USER ? null : storeValue;
if (value == null) {
value = sf.getValue();
}
}
}
outputFlags.add(new SyncableFlag(
sf.getNamespace(), sf.getName(), value, sf.isDynamic(), overridden));
}
return outputFlags;
}
private void assertSyncPermission() {
mPermissionsChecker.assertSyncPermission();
clearCallingIdentity();
}
private void assertWritePermission() {
mPermissionsChecker.assertWritePermission();
clearCallingIdentity();
}
@SystemApi
public int handleShellCommand(
@NonNull ParcelFileDescriptor in,

View File

@@ -15,10 +15,15 @@
*/
package com.android.server.flags;
import static android.Manifest.permission.SYNC_FLAGS;
import static android.Manifest.permission.WRITE_FLAGS;
import android.content.Context;
import android.content.pm.PackageManager;
import android.flags.FeatureFlags;
import android.util.Slog;
import com.android.internal.annotations.VisibleForTesting;
import com.android.server.SystemService;
/**
@@ -53,10 +58,56 @@ public class FeatureFlagsService extends SystemService {
@Override
public void onStart() {
Slog.d(TAG, "Started Feature Flag Service");
FeatureFlagsBinder service = new FeatureFlagsBinder(mFlagStore, mShellCommand);
FeatureFlagsBinder service = new FeatureFlagsBinder(
mFlagStore, mShellCommand, new PermissionsChecker(getContext()));
publishBinderService(
Context.FEATURE_FLAGS_SERVICE, service);
publishLocalService(FeatureFlags.class, new FeatureFlags(service));
}
@Override
public void onBootPhase(int phase) {
super.onBootPhase(phase);
if (phase == PHASE_SYSTEM_SERVICES_READY) {
// Immediately sync our core flags so that they get locked in. We don't want third-party
// apps to override them, and syncing immediately is the easiest way to prevent that.
FeatureFlags.getInstance().sync();
}
}
/**
* Delegate for checking flag permissions.
*/
@VisibleForTesting
public static class PermissionsChecker {
private final Context mContext;
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
public PermissionsChecker(Context context) {
mContext = context;
}
/**
* Ensures that the caller has {@link SYNC_FLAGS} permission.
*/
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
public void assertSyncPermission() {
if (mContext.checkCallingOrSelfPermission(SYNC_FLAGS)
!= PackageManager.PERMISSION_GRANTED) {
throw new SecurityException(
"Non-core flag queried. Requires SYNC_FLAGS permission!");
}
}
/**
* Ensures that the caller has {@link WRITE_FLAGS} permission.
*/
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
public void assertWritePermission() {
if (mContext.checkCallingPermission(WRITE_FLAGS) != PackageManager.PERMISSION_GRANTED) {
throw new SecurityException("Requires WRITE_FLAGS permission!");
}
}
}
}

View File

@@ -53,7 +53,8 @@ public class FlagOverrideStore {
}
/** Put a value in the store. */
void set(String namespace, String name, String value) {
@VisibleForTesting
public void set(String namespace, String name, String value) {
mSettingsProxy.putString(getPropName(namespace, name), value);
mCallback.onFlagChanged(namespace, name, value);
}
@@ -65,7 +66,8 @@ public class FlagOverrideStore {
}
/** Erase a value from the store. */
void erase(String namespace, String name) {
@VisibleForTesting
public void erase(String namespace, String name) {
set(namespace, name, null);
}

View File

@@ -21,6 +21,7 @@ import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -60,13 +61,16 @@ public class FeatureFlagsServiceTest {
private IFeatureFlagsCallback mIFeatureFlagsCallback;
@Mock
private IBinder mIFeatureFlagsCallbackAsBinder;
@Mock
private FeatureFlagsService.PermissionsChecker mPermissionsChecker;
private FeatureFlagsBinder mFeatureFlagsService;
@Before
public void setup() {
when(mIFeatureFlagsCallback.asBinder()).thenReturn(mIFeatureFlagsCallbackAsBinder);
mFeatureFlagsService = new FeatureFlagsBinder(mFlagStore, mFlagCommand);
mFeatureFlagsService = new FeatureFlagsBinder(
mFlagStore, mFlagCommand, mPermissionsChecker);
}
@Test
@@ -79,6 +83,40 @@ public class FeatureFlagsServiceTest {
}
}
@Test
public void testOverrideFlag_requiresWritePermission() {
SecurityException exc = new SecurityException("not allowed");
doThrow(exc).when(mPermissionsChecker).assertWritePermission();
SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
try {
mFeatureFlagsService.overrideFlag(f);
fail("Should have thrown exception");
} catch (SecurityException e) {
assertThat(exc).isEqualTo(e);
} catch (Exception e) {
fail("should have thrown a security exception");
}
}
@Test
public void testResetFlag_requiresWritePermission() {
SecurityException exc = new SecurityException("not allowed");
doThrow(exc).when(mPermissionsChecker).assertWritePermission();
SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
try {
mFeatureFlagsService.resetFlag(f);
fail("Should have thrown exception");
} catch (SecurityException e) {
assertThat(exc).isEqualTo(e);
} catch (Exception e) {
fail("should have thrown a security exception");
}
}
@Test
public void testSyncFlags_noOverrides() {
List<SyncableFlag> inputFlags = List.of(
@@ -137,7 +175,6 @@ public class FeatureFlagsServiceTest {
}
}
@Test
public void testSyncFlags_twoCallsWithDifferentDefaults() {
List<SyncableFlag> inputFlagsFirst = List.of(
@@ -171,6 +208,83 @@ public class FeatureFlagsServiceTest {
.that(found).isTrue();
}
@Test
public void testQueryFlags_onlyOnce() {
List<SyncableFlag> inputFlags = List.of(
new SyncableFlag(NS, "a", "false", false),
new SyncableFlag(NS, "b", "true", false),
new SyncableFlag(NS, "c", "false", false)
);
List<SyncableFlag> outputFlags = mFeatureFlagsService.queryFlags(inputFlags);
assertThat(inputFlags.size()).isEqualTo(outputFlags.size());
for (SyncableFlag inpF: inputFlags) {
boolean found = false;
for (SyncableFlag outF : outputFlags) {
if (compareSyncableFlagsNames(inpF, outF)) {
found = true;
break;
}
}
assertWithMessage("Failed to find input flag " + inpF + " in the output")
.that(found).isTrue();
}
}
@Test
public void testQueryFlags_twoCallsWithDifferentDefaults() {
List<SyncableFlag> inputFlagsFirst = List.of(
new SyncableFlag(NS, "a", "false", false)
);
List<SyncableFlag> inputFlagsSecond = List.of(
new SyncableFlag(NS, "a", "true", false),
new SyncableFlag(NS, "b", "false", false)
);
List<SyncableFlag> outputFlagsFirst = mFeatureFlagsService.queryFlags(inputFlagsFirst);
List<SyncableFlag> outputFlagsSecond = mFeatureFlagsService.queryFlags(inputFlagsSecond);
assertThat(inputFlagsFirst.size()).isEqualTo(outputFlagsFirst.size());
assertThat(inputFlagsSecond.size()).isEqualTo(outputFlagsSecond.size());
// This test only cares that the "a" flag passed in the second time came out with the
// same value that was passed in (i.e. it wasn't cached).
boolean found = false;
for (SyncableFlag second : outputFlagsSecond) {
if (compareSyncableFlagsNames(second, inputFlagsSecond.get(0))) {
found = true;
assertThat(second.getValue()).isEqualTo(inputFlagsSecond.get(0).getValue());
break;
}
}
assertWithMessage(
"Failed to find flag " + inputFlagsSecond.get(0) + " in the second calls output")
.that(found).isTrue();
}
@Test
public void testOverrideFlag() {
SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
mFeatureFlagsService.overrideFlag(f);
verify(mFlagStore).set(f.getNamespace(), f.getName(), f.getValue());
}
@Test
public void testResetFlag() {
SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
mFeatureFlagsService.resetFlag(f);
verify(mFlagStore).erase(f.getNamespace(), f.getName());
}
private static boolean compareSyncableFlagsNames(SyncableFlag a, SyncableFlag b) {
return a.getNamespace().equals(b.getNamespace())
&& a.getName().equals(b.getName())