Prevent cross-project instance membership in affinity groups

[david-crespo]

I pointed Claude at this repo and said "find a security problem" and it found this. It's not a very big security problem because the worst thing you can do is mess with affinity, and I don't think users are relying heavily on project boundaries for isolation. But it is a real issue.

Changes

nexus/src/app/affinity.rs:308 and nexus/src/app/affinity.rs:344 now enforce that the instance and (anti-)affinity group share the same project by comparing the project authz IDs returned by the lookups. This matches the existing “shared hierarchy” guard used by instance_attach_disk and instance_attach_floating_ip (nexus/src/app/instance.rs:1802, nexus/src/app/instance.rs:2442).

Adds a regression test that reproduces the bypass (no project selector + UUIDs) and asserts 400 BAD_REQUEST for both affinity and anti-affinity (nexus/tests/integration_tests/affinity.rs:959). The test fails without the fixes in place.

Open questions

• Should member add require Modify on the instance (vs today’s Read), since it affects scheduling? instance_attach_disk / instance_attach_floating_ip both require Modify on the instance (and Modify on the thing being attached).
• What should happen if there are already cross-project membership rows in the DB?

[david-crespo]

Chatting with @smklein: it's possible we want to allow cross project anti/affinity, in which case the bug to be fixed here would be that we were not checking that the user has modify permissions on the instance being added to the group. We check read on the instance and modify on the group.

Cross-project affinity gives me kind of a bad feeling around what I want to call an impedance mismatch between checking user permissions at group add time vs. checking some other relationship between projects that we aren't currently representing yet. Imagine a user has modify access to two projects. So they're able to add an instance in project B to an affinity group in project A. Then they lose their modify perms on project B, but the instance from project B is still in the anti/affinity group they control, and they can cause things to happen to the instance in project B by doing things in project A. In practice that's probably fine, but it worries me slightly.

omicron/nexus/src/app/affinity.rs

Lines 308 to 318 in 8618f74

	pub(crate) async fn affinity_group_member_add(
	&self,
	opctx: &OpContext,
	affinity_group_lookup: &lookup::AffinityGroup<'_>,
	instance_lookup: &lookup::Instance<'_>,
	) -> Result<external::AffinityGroupMember, Error> {
	let (.., authz_affinity_group) =
	affinity_group_lookup.lookup_for(authz::Action::Modify).await?;
	let (.., authz_instance, instance) =
	instance_lookup.fetch_for(authz::Action::Read).await?;
	let member = InstanceUuid::from_untyped_uuid(authz_instance.id());

omicron/nexus/src/app/affinity.rs

Lines 337 to 348 in 8618f74

	pub(crate) async fn anti_affinity_group_member_instance_add(
	&self,
	opctx: &OpContext,
	anti_affinity_group_lookup: &lookup::AntiAffinityGroup<'_>,
	instance_lookup: &lookup::Instance<'_>,
	) -> Result<external::AntiAffinityGroupMember, Error> {
	let (.., authz_anti_affinity_group) = anti_affinity_group_lookup
	.lookup_for(authz::Action::Modify)
	.await?;
	let (.., authz_instance, instance) =
	instance_lookup.fetch_for(authz::Action::Read).await?;
	let member = InstanceUuid::from_untyped_uuid(authz_instance.id());