Check for remote address in allow-list update should ignore underlay

[bnaecker]

When attempting to update the allow-list for user-facing services, Nexus ensures that the address of the peer sending the request is contained in the allow-list itself. This is a safety mechanism to prevent locking oneself out. However, there are still ways one can get locked out. As an example, if a user sets the allow-list to enable traffic from some external proxy, but that proxy is issued a new IP address without notice, they won't be able to get back into the rack to change the list.

To address this, we could make use of the Nexus external API proxy offered by wicketd. This works at the transport layer, and runs over the underlay network. That means it's not subject to the allow-list checks at all, since it's over a completely different interface. However, clients will never be able to actually update the allow-list from the proxy, because the containment mentioned at the beginning will fail! Users don't know (and can't know) the underlay address, so Nexus will always reject the list.

We should relax this check in Nexus. The remote address must either be on the allow-list, or on the underlay network. It's not yet clear to me if that means "same rack subnet" or "any rack subnet", or if we want to do an explicit lookup in the database to make sure it's in a known-valid rack.

[jgallagher]

An idle thought - the nexus techport proxy is an entirely separate dropshot server:

omicron/nexus/src/lib.rs

Lines 179 to 192 in c472464

	let http_server_techport_external = {
	let server_starter_external_techport =
	dropshot::HttpServerStarter::new_with_tls(
	&techport_server_config,
	external_api(),
	Arc::clone(&apictx),
	&log.new(o!("component" => "dropshot_external_techport")),
	tls_config.map(dropshot::ConfigTls::Dynamic),
	)
	.map_err(|error| {
	format!("initializing external techport server: {}", error)
	})?;
	server_starter_external_techport.start()
	};

I wonder if it would be easier to stash something in the respective server contexts to indicate which was which, and this disable the check of the request IP based on which dropshot server is handling the request?

[bnaecker]

That's an interesting idea, I like it. I'm looking at the server contexts now, and they're all currently shared between the three servers (internal, external, and techport), through an Arc. I think we'd need a deeper copy to make this work, or split the internal fields up in a more structured way so that we can add a new field like this. The ServerContext has clearly grown organically, it looks like from before we even had the internal server, since it includes the latency tracking for the internal API endpoints. So this might be a good time to make such a change in any case!