Avoid calling noteOp while enqueueing the broadcast. (11eb3d4c) · Commits · e / os / android_frameworks_base

core/java/android/app/AppOpsManager.java

+9 −0

Original line number	Diff line number	Diff line
		@@ -3300,6 +3300,15 @@ public class AppOpsManager {
		return sAppOpInfos[op].name;
		}

		/**
		* Returns whether the provided {@code op} is a valid op code or not.
		*
		* @hide
		*/
		public static boolean isValidOp(int op) {
		return op >= 0 && op < sAppOpInfos.length;
		}

		/**
		* @hide
		*/

services/core/java/com/android/server/am/BroadcastQueueImpl.java

+3 −1

Original line number	Diff line number	Diff line
		@@ -832,7 +832,9 @@ class BroadcastQueueImpl extends BroadcastQueue {

		// If this receiver is going to be skipped, skip it now itself and don't even enqueue
		// it.
		final String skipReason = mSkipPolicy.shouldSkipMessage(r, receiver);
		final String skipReason = Flags.avoidNoteOpAtEnqueue()
		? mSkipPolicy.shouldSkipAtEnqueueMessage(r, receiver)
		: mSkipPolicy.shouldSkipMessage(r, receiver);
		if (skipReason != null) {
		setDeliveryState(null, null, r, i, receiver, BroadcastRecord.DELIVERY_SKIPPED,
		"skipped by policy at enqueue: " + skipReason);

services/core/java/com/android/server/am/BroadcastSkipPolicy.java

+138 −64

Original line number	Diff line number	Diff line
		@@ -71,10 +71,20 @@ public class BroadcastSkipPolicy {
		* {@code null} if it can proceed.
		*/
		public @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r, @NonNull Object target) {
		return shouldSkipMessage(r, target, false /* preflight */);
		}

		public @Nullable String shouldSkipAtEnqueueMessage(@NonNull BroadcastRecord r,
		@NonNull Object target) {
		return shouldSkipMessage(r, target, true /* preflight */);
		}

		private @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r, @NonNull Object target,
		boolean preflight) {
		if (target instanceof BroadcastFilter) {
		return shouldSkipMessage(r, (BroadcastFilter) target);
		return shouldSkipMessage(r, (BroadcastFilter) target, preflight);
		} else {
		return shouldSkipMessage(r, (ResolveInfo) target);
		return shouldSkipMessage(r, (ResolveInfo) target, preflight);
		}
		}

		@@ -86,7 +96,7 @@ public class BroadcastSkipPolicy {
		* {@code null} if it can proceed.
		*/
		private @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r,
		@NonNull ResolveInfo info) {
		@NonNull ResolveInfo info, boolean preflight) {
		final BroadcastOptions brOptions = r.options;
		final ComponentName component = new ComponentName(
		info.activityInfo.applicationInfo.packageName,
		@@ -134,17 +144,25 @@ public class BroadcastSkipPolicy {
		+ " requires " + info.activityInfo.permission;
		}
		} else if (info.activityInfo.permission != null) {
		final int opCode = AppOpsManager.permissionToOpCode(info.activityInfo.permission);
		if (opCode != AppOpsManager.OP_NONE && mService.getAppOpsManager().noteOpNoThrow(opCode,
		final String op = AppOpsManager.permissionToOp(info.activityInfo.permission);
		if (op != null) {
		final int mode;
		if (preflight) {
		mode = mService.getAppOpsManager().checkOpNoThrow(op,
		r.callingUid, r.callerPackage, r.callerFeatureId);
		} else {
		mode = mService.getAppOpsManager().noteOpNoThrow(op,
		r.callingUid, r.callerPackage, r.callerFeatureId,
		"Broadcast delivered to " + info.activityInfo.name)
		!= AppOpsManager.MODE_ALLOWED) {
		"Broadcast delivered to " + info.activityInfo.name);
		}
		if (mode != AppOpsManager.MODE_ALLOWED) {
		return "Appop Denial: broadcasting "
		+ broadcastDescription(r, component)
		+ " requires appop " + AppOpsManager.permissionToOp(
		info.activityInfo.permission);
		}
		}
		}

		if ((info.activityInfo.flags&ActivityInfo.FLAG_SINGLE_USER) != 0) {
		if (ActivityManager.checkUidPermission(
		@@ -250,8 +268,8 @@ public class BroadcastSkipPolicy {
		perm = PackageManager.PERMISSION_DENIED;
		}

		int appOp = AppOpsManager.permissionToOpCode(excludedPermission);
		if (appOp != AppOpsManager.OP_NONE) {
		final String appOp = AppOpsManager.permissionToOp(excludedPermission);
		if (appOp != null) {
		// When there is an app op associated with the permission,
		// skip when both the permission and the app op are
		// granted.
		@@ -292,9 +310,10 @@ public class BroadcastSkipPolicy {
		createAttributionSourcesForResolveInfo(info);
		for (int i = 0; i < r.requiredPermissions.length; i++) {
		String requiredPermission = r.requiredPermissions[i];
		perm = hasPermissionForDataDelivery(
		perm = hasPermission(
		requiredPermission,
		"Broadcast delivered to " + info.activityInfo.name,
		preflight,
		attributionSources)
		? PackageManager.PERMISSION_GRANTED
		: PackageManager.PERMISSION_DENIED;
		@@ -308,10 +327,14 @@ public class BroadcastSkipPolicy {
		}
		}
		}
		if (r.appOp != AppOpsManager.OP_NONE) {
		if (!noteOpForManifestReceiver(r.appOp, r, info, component)) {
		if (r.appOp != AppOpsManager.OP_NONE && AppOpsManager.isValidOp(r.appOp)) {
		final String op = AppOpsManager.opToPublicName(r.appOp);
		final boolean appOpAllowed = preflight
		? checkOpForManifestReceiver(r.appOp, op, r, info, component)
		: noteOpForManifestReceiver(r.appOp, op, r, info, component);
		if (!appOpAllowed) {
		return "Skipping delivery to " + info.activityInfo.packageName
		+ " due to required appop " + r.appOp;
		+ " due to required appop " + AppOpsManager.opToName(r.appOp);
		}
		}

		@@ -338,7 +361,7 @@ public class BroadcastSkipPolicy {
		* {@code null} if it can proceed.
		*/
		private @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r,
		@NonNull BroadcastFilter filter) {
		@NonNull BroadcastFilter filter, boolean preflight) {
		if (r.options != null && !r.options.testRequireCompatChange(filter.owningUid)) {
		return "Compat change filtered: broadcasting " + r.intent.toString()
		+ " to uid " + filter.owningUid + " due to compat change "
		@@ -372,21 +395,28 @@ public class BroadcastSkipPolicy {
		+ " requires " + filter.requiredPermission
		+ " due to registered receiver " + filter;
		} else {
		final int opCode = AppOpsManager.permissionToOpCode(filter.requiredPermission);
		if (opCode != AppOpsManager.OP_NONE
		&& mService.getAppOpsManager().noteOpNoThrow(opCode, r.callingUid,
		r.callerPackage, r.callerFeatureId, "Broadcast sent to protected receiver")
		!= AppOpsManager.MODE_ALLOWED) {
		final String op = AppOpsManager.permissionToOp(filter.requiredPermission);
		if (op != null) {
		final int mode;
		if (preflight) {
		mode = mService.getAppOpsManager().checkOpNoThrow(op,
		r.callingUid, r.callerPackage, r.callerFeatureId);
		} else {
		mode = mService.getAppOpsManager().noteOpNoThrow(op, r.callingUid,
		r.callerPackage, r.callerFeatureId,
		"Broadcast sent to protected receiver");
		}
		if (mode != AppOpsManager.MODE_ALLOWED) {
		return "Appop Denial: broadcasting "
		+ r.intent.toString()
		+ r.intent
		+ " from " + r.callerPackage + " (pid="
		+ r.callingPid + ", uid=" + r.callingUid + ")"
		+ " requires appop " + AppOpsManager.permissionToOp(
		filter.requiredPermission)
		+ " requires appop " + op
		+ " due to registered receiver " + filter;
		}
		}
		}
		}

		if ((filter.receiverList.app == null \|\| filter.receiverList.app.isKilled()
		\|\| filter.receiverList.app.mErrorState.isCrashing())) {
		@@ -433,9 +463,10 @@ public class BroadcastSkipPolicy {
		.build();
		for (int i = 0; i < r.requiredPermissions.length; i++) {
		String requiredPermission = r.requiredPermissions[i];
		final int perm = hasPermissionForDataDelivery(
		final int perm = hasPermission(
		requiredPermission,
		"Broadcast delivered to registered receiver " + filter.receiverId,
		preflight,
		attributionSource)
		? PackageManager.PERMISSION_GRANTED
		: PackageManager.PERMISSION_DENIED;
		@@ -471,8 +502,8 @@ public class BroadcastSkipPolicy {
		final int perm = checkComponentPermission(excludedPermission,
		filter.receiverList.pid, filter.receiverList.uid, -1, true);

		int appOp = AppOpsManager.permissionToOpCode(excludedPermission);
		if (appOp != AppOpsManager.OP_NONE) {
		final String appOp = AppOpsManager.permissionToOp(excludedPermission);
		if (appOp != null) {
		// When there is an app op associated with the permission,
		// skip when both the permission and the app op are
		// granted.
		@@ -482,12 +513,11 @@ public class BroadcastSkipPolicy {
		filter.packageName)
		== AppOpsManager.MODE_ALLOWED)) {
		return "Appop Denial: receiving "
		+ r.intent.toString()
		+ r.intent
		+ " to " + filter.receiverList.app
		+ " (pid=" + filter.receiverList.pid
		+ ", uid=" + filter.receiverList.uid + ")"
		+ " excludes appop " + AppOpsManager.permissionToOp(
		excludedPermission)
		+ " excludes appop " + appOp
		+ " due to sender " + r.callerPackage
		+ " (uid " + r.callingUid + ")";
		}
		@@ -496,7 +526,7 @@ public class BroadcastSkipPolicy {
		// skip when permission is granted.
		if (perm == PackageManager.PERMISSION_GRANTED) {
		return "Permission Denial: receiving "
		+ r.intent.toString()
		+ r.intent
		+ " to " + filter.receiverList.app
		+ " (pid=" + filter.receiverList.pid
		+ ", uid=" + filter.receiverList.uid + ")"
		@@ -523,13 +553,20 @@ public class BroadcastSkipPolicy {
		}

		// If the broadcast also requires an app op check that as well.
		if (r.appOp != AppOpsManager.OP_NONE
		&& mService.getAppOpsManager().noteOpNoThrow(r.appOp,
		if (r.appOp != AppOpsManager.OP_NONE && AppOpsManager.isValidOp(r.appOp)) {
		final String op = AppOpsManager.opToPublicName(r.appOp);
		final int mode;
		if (preflight) {
		mode = mService.getAppOpsManager().checkOpNoThrow(op,
		filter.receiverList.uid, filter.packageName, filter.featureId);
		} else {
		mode = mService.getAppOpsManager().noteOpNoThrow(op,
		filter.receiverList.uid, filter.packageName, filter.featureId,
		"Broadcast delivered to registered receiver " + filter.receiverId)
		!= AppOpsManager.MODE_ALLOWED) {
		"Broadcast delivered to registered receiver " + filter.receiverId);
		}
		if (mode != AppOpsManager.MODE_ALLOWED) {
		return "Appop Denial: receiving "
		+ r.intent.toString()
		+ r.intent
		+ " to " + filter.receiverList.app
		+ " (pid=" + filter.receiverList.pid
		+ ", uid=" + filter.receiverList.uid + ")"
		@@ -537,6 +574,7 @@ public class BroadcastSkipPolicy {
		+ " due to sender " + r.callerPackage
		+ " (uid " + r.callingUid + ")";
		}
		}

		// Ensure that broadcasts are only sent to other apps if they are explicitly marked as
		// exported, or are System level broadcasts
		@@ -572,14 +610,14 @@ public class BroadcastSkipPolicy {
		+ ", uid=" + r.callingUid + ") to " + component.flattenToShortString();
		}

		private boolean noteOpForManifestReceiver(int appOp, BroadcastRecord r, ResolveInfo info,
		ComponentName component) {
		private boolean noteOpForManifestReceiver(int opCode, String appOp, BroadcastRecord r,
		ResolveInfo info, ComponentName component) {
		if (ArrayUtils.isEmpty(info.activityInfo.attributionTags)) {
		return noteOpForManifestReceiverInner(appOp, r, info, component, null);
		return noteOpForManifestReceiverInner(opCode, appOp, r, info, component, null);
		} else {
		// Attribution tags provided, noteOp each tag
		for (String tag : info.activityInfo.attributionTags) {
		if (!noteOpForManifestReceiverInner(appOp, r, info, component, tag)) {
		if (!noteOpForManifestReceiverInner(opCode, appOp, r, info, component, tag)) {
		return false;
		}
		}
		@@ -587,8 +625,8 @@ public class BroadcastSkipPolicy {
		}
		}

		private boolean noteOpForManifestReceiverInner(int appOp, BroadcastRecord r, ResolveInfo info,
		ComponentName component, String tag) {
		private boolean noteOpForManifestReceiverInner(int opCode, String appOp, BroadcastRecord r,
		ResolveInfo info, ComponentName component, String tag) {
		if (mService.getAppOpsManager().noteOpNoThrow(appOp,
		info.activityInfo.applicationInfo.uid,
		info.activityInfo.packageName,
		@@ -598,7 +636,37 @@ public class BroadcastSkipPolicy {
		Slog.w(TAG, "Appop Denial: receiving "
		+ r.intent + " to "
		+ component.flattenToShortString()
		+ " requires appop " + AppOpsManager.opToName(appOp)
		+ " requires appop " + AppOpsManager.opToName(opCode)
		+ " due to sender " + r.callerPackage
		+ " (uid " + r.callingUid + ")");
		return false;
		}
		return true;
		}

		private boolean checkOpForManifestReceiver(int opCode, String appOp, BroadcastRecord r,
		ResolveInfo info, ComponentName component) {
		if (ArrayUtils.isEmpty(info.activityInfo.attributionTags)) {
		return checkOpForManifestReceiverInner(opCode, appOp, r, info, component, null);
		} else {
		// Attribution tags provided, noteOp each tag
		for (String tag : info.activityInfo.attributionTags) {
		if (!checkOpForManifestReceiverInner(opCode, appOp, r, info, component, tag)) {
		return false;
		}
		}
		return true;
		}
		}

		private boolean checkOpForManifestReceiverInner(int opCode, String appOp, BroadcastRecord r,
		ResolveInfo info, ComponentName component, String tag) {
		if (mService.getAppOpsManager().checkOpNoThrow(appOp, info.activityInfo.applicationInfo.uid,
		info.activityInfo.packageName, tag) != AppOpsManager.MODE_ALLOWED) {
		Slog.w(TAG, "Appop Denial: receiving "
		+ r.intent + " to "
		+ component.flattenToShortString()
		+ " requires appop " + AppOpsManager.opToName(opCode)
		+ " due to sender " + r.callerPackage
		+ " (uid " + r.callingUid + ")");
		return false;
		@@ -694,9 +762,10 @@ public class BroadcastSkipPolicy {
		return mPermissionManager;
		}

		private boolean hasPermissionForDataDelivery(
		private boolean hasPermission(
		@NonNull String permission,
		@NonNull String message,
		boolean preflight,
		@NonNull AttributionSource... attributionSources) {
		final PermissionManager permissionManager = getPermissionManager();
		if (permissionManager == null) {
		@@ -704,9 +773,14 @@ public class BroadcastSkipPolicy {
		}

		for (AttributionSource attributionSource : attributionSources) {
		final int permissionCheckResult =
		permissionManager.checkPermissionForDataDelivery(
		final int permissionCheckResult;
		if (preflight) {
		permissionCheckResult = permissionManager.checkPermissionForPreflight(
		permission, attributionSource);
		} else {
		permissionCheckResult = permissionManager.checkPermissionForDataDelivery(
		permission, attributionSource, message);
		}
		if (permissionCheckResult != PackageManager.PERMISSION_GRANTED) {
		return false;
		}

services/core/java/com/android/server/am/broadcasts_flags.aconfig

+11 −0

Original line number	Diff line number	Diff line
		@@ -16,3 +16,14 @@ flag {
		is_fixed_read_only: true
		bug: "369487976"
		}

		flag {
		name: "avoid_note_op_at_enqueue"
		namespace: "backstage_power"
		description: "Avoid triggering noteOp while enqueueing a broadcast"
		is_fixed_read_only: true
		bug: "268016162"
		metadata {
		purpose: PURPOSE_BUGFIX
		}
		}
		No newline at end of file

services/tests/mockingservicestests/src/com/android/server/am/BaseBroadcastQueueTest.java

+122 −11

File changed.

Preview size limit exceeded, changes collapsed.