Skip to content

[nexus] add an ExternalHttpClient for sending requests outside the rack#10835

Open
hawkw wants to merge 21 commits into
mainfrom
eliza/external-client
Open

[nexus] add an ExternalHttpClient for sending requests outside the rack#10835
hawkw wants to merge 21 commits into
mainfrom
eliza/external-client

Conversation

@hawkw

@hawkw hawkw commented Jul 16, 2026

Copy link
Copy Markdown
Member

Depends on #10872.

There are various circumstances in which Nexus must make HTTP requests to endpoints external to the rack, at URLs which are often user-configurable. The primary example of this is for delivering alerts via webhooks, where the alert payload is sent in an HTTP POST request to the user-configured webhook receiver endpoint.

When making external requests, it is important to ensure that they are never sent to IP addresses on the underlay network, which are also reachable from a Nexus zone. This is necessarily primarily to defend against a class of attacks called server-side request forgery, or SSRF, in which an attacker manipulates the capability of a server that exposes a way to make user-controlled requests to send malicious requests to destinations that the attacker could not otherwise access. To be specific, an attacker who wished to influence the behavior of an underlay network service that they would otherwise not be able to access could exploit the ability of Nexus to communicate both on the underlay and externally by configuring a webhook receiver endpoint that points at an API exposed by an underlay network service, and then trigger a webhook liveness probe, sending a request to that underlay service. In the case of webhooks specifically, there are limited avenues to exploit such a vulnerability,1 but it must be protected against nonetheless.

Furthermore, it is also possible for a user-provided URL to point to an underlay address due to a non-malicious misconfiguration. The IPv6 ULA address space used for the underlay network is quite large, but because the address ranges to use are provided by the operator at RSS-time, it is entirely possible for someone to accidentally run a service (such as a webhook receiver) within their network but external to the rack on an address which collides with the Oxide underlay network. In such a case, the operator would be surprised and dismayed when their webhook is sent successfully...to an underlay network service that doesn't know what to do with it. Thus, we want to fail loudly rather than quietly doing the wrong thing, for the benefit of debugging legitimate configuration mistakes.

Currently, webhook requests protect against SSRF and accidental underlay network address collisions at the network level, by binding connections exclusively to the Nexus zone's OPTE interface, using the IP_BOUND_IF and IPV6_BOUND_IF sockopts. Binding only on this interface ensures that the connection cannot route to the underlay. The backstory for this branch is that @rcgoodfellow and I recently discovered that this doesn't always work due to a race condition in how control plane zone routes are configured (see #10829). Initially, I thought that once the race was fixed, the approach of binding directly to the OPTE interface would never work, so it was necessary to reimplement the SSRF-prevention mechanism at the application layer. It turns out that this was not actually the case, and Ry was able to make a fairly simple fix (#10837) that would allow this technique to continue working.

However, there are some advantages to rejecting underlay IPs at the application level in addition to relying on network isolation. One big one is defense in depth: while binding on an interface that can never route to underlay IPs is in many ways a more reliable defense than maintaining a list of denied subnets in the application,2 this also relies on the assumption that the routing rules for that interface won't change in a way that makes it fail open. Therefore, doing both seems better than doing either one. Furthermore, the application-level rejection has the advantage of improved visibility: trying to connect to an underlay address can be rejected with an explicit error message that makes it clear why it didn't work, rather than with a generic "address unreachable" error when we can't open a connection to that address. In future changes, we can expose this to the operator more directly, so they can tell if they have accidentally pointed a webhook or something at a colliding address. Therefore, I decided to keep working on the code I had started to implement application-layer filtering even though we have a fix for the network-level isolation.

This branch, therefore, introduces a new ExternalHttpClient type, which is intended as The Canonical Way for Nexus to construct clients to external endpoints. It configures a reqwest::Client which is bound to the OPTE interface only, and wraps it in a layer that rejects underlay addresses. It is necessary to check for such addresses in several places: first, we must check if the user-provided URL's host is an IP address literal which is on an underlay network or other disallowed subnet. If the host is a domain name, we must also check if the IP addresses resolved for that domain are underlay addresses (which could occur maliciously as part of a subdomain takeover or other form of DNS hijacking attack). Finally, we must also apply this policy when following redirects,3 checking whether the redirect URL is an underlay address, or if it is to a domain name that resolves to such an address. This ended up being rather a lot of code.

I've also changed the webhook delivery client to use the new ExternalHttpClient. Follow-up work will include:

  • Changing the client used for SAML IdP requests to also use ExternalHttpClient.
  • Recording the error message returned by a webhook delivery request in the database so that it may be reported to operators.
  • Checking the webhook receiver endpoint URL against this policy when a receiver is created or updated, as well as when actually sending requests to it.4

Footnotes

  1. Because webhook requests are POSTs, and any data a receiver endpoint sends in response to such a request is not exposed to the caller that triggers a probe, the capability to exploit this for data exfiltration is somewhat limited. And, the attacker cannot cause Nexus to send arbitrary request payloads through webhook requests, so their ability to request operations on underlay services depends on those services having endpoints which will accept a POST with an arbitrary JSON body.

  2. Since, well, it would be easy to forget to add a new underlay subnet to that list...

  3. This doesn't apply in the webhook delivery case, since webhook requests never follow redirects. But, since the intent is that all external requests should eventually use this code, I went ahead and implemented it.

  4. Note that this alone would not be sufficient to ensure that we defend against SSRF attacks, since, as I mentioned above, an attacker could compromise a DNS server to point a previously-okay domain name at a disallowed IP. Therefore, we must always check when we actually attempt to connect to a specific thing. But, eager validation would help in the case of legitimate accidental misconfigurations by returning an error to the user immediately, rather than waiting until we actually try to talk to that thing.

hawkw added a commit that referenced this pull request Jul 21, 2026
The `omicron_common::address` module contains a whole bunch of constants
whose names end with or otherwise contain the word "prefix" (or, in this
case, the word "PREFIX"). Unfortunately, this word is used somewhat
inconsistently. In some cases, "prefix" means "the _length_ of a prefix
of a subnet", such as [`BOOTSTRAP_SUBNET_PREFIX`], which describes a /40
network, and therefore unsurprisingly is the number 40, or
[`RACK_PREFIX`] and [`AZ_PREFIX`], which are 56 and 48, respectively.
These constants are typically used as const generic parameters to the
`Ipv6Subnet` type, which is generic over the length of a prefix. In
other cases, we use the word "prefix" to refer to the "first octets in a
subnet", such as [`IPV6_MULTICAST_PREFIX`] and
`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`, which are `ff00` and `ff04`, and
respectivefly describe the first octet or frist two octets in subnets
which are /8 and /16, respectively.

This was a bit confusing for me to discover that sometimes ending with
"_PREFIX" means a constant contains one thing, and sometimes it means
another thing. If one were to use one kind of `_PREFIX` constant in a
place where the other kind is expected, I can imagine things going quite
wrong, although this is helped a bit by the ones that represent prefix
lengths typically being `u8`s and the ones that represent IPv6 octet
pairs being, naturally, octet _pairs_, i.e. `u16`s. Luckily, one cannot
accidentally insert a `u16` into a `u8`-shaped hole, or vice versa, so
we haven't messed this up _yet_, at least as far as I can tell.

However, there is another annoying issue here, which is that I would
quite like to move the constant [`BOOTSTRAP_PREFIX`], which represents
the first two octets of the bootstrap network, from
`sled_hardware_types::underlay` to `omicron_common::address`.[^1] I'd
like to be able to reference it in Nexus in #10835. Sadly, as I
mentioned above, there is already a `BOOTSTRAP_SUBNET_PREFIX` here,
which represents the _length_ of a prefix, rather than the actual
octets. Having both `BOOTSTRAP_PREFIX` and `BOOTSTRAP_SUBNET_PREFIX`,
with different meanings, seems bad and unfortunate.

Therefore, this branch adopts a more consistent naming scheme, where
constants that represent the length of a prefix are always named
`FOO_PREFIX_LENGTH`, and constants which represent the first octet(s) of
a subnet, often beginning with the letter `f`, are named `FOO_PREFIX`.
Doing this touches a lot of files, but reviewers should really only need
to look closely at `common/src/address.rs`, as the rest are just places
where these various constants are referenced. I would love to have a
network guy or two fact-check whether I am using these terms correctly.
Thanks!

**AI Usage Disclosure**: This was a large, mechanical change, so I used
an AI tool called "the rust-analyzer `Rename Symbol` button" to assist
in making this change.

[^1]: Which it's only currently referenced in `sled-hardware`, which
depends on `omicron-common` as well as on `sled-hardware-types`.

[`BOOTSTRAP_SUBNET_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L22
[`AZ_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L23
[`RACK_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L24
[`IPV6_MULTICAST_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L120
[`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L130
[`BOOTSTRAP_PREFIX`]:
https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/sled-hardware/types/src/underlay.rs#L6
hawkw added a commit that referenced this pull request Jul 21, 2026
The `omicron_common::address` module contains a whole bunch of constants
whose names end with or otherwise contain the word "prefix" (or, in this
case, the word "PREFIX"). Unfortunately, this word is used somewhat
inconsistently. In some cases, "prefix" means "the _length_ of a prefix
of a subnet", such as [`BOOTSTRAP_SUBNET_PREFIX`], which describes a /40
network, and therefore unsurprisingly is the number 40, or
[`RACK_PREFIX`] and [`AZ_PREFIX`], which are 56 and 48, respectively.
These constants are typically used as const generic parameters to the
`Ipv6Subnet` type, which is generic over the length of a prefix. In
other cases, we use the word "prefix" to refer to the "first octets in a
subnet", such as [`IPV6_MULTICAST_PREFIX`] and
[`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`], which are `ff00` and `ff04`, and
respectivefly describe the first octet or first two octets in subnets
which are /8 and /16, respectively.

This was a bit confusing for me to discover that sometimes ending with
`_PREFIX` means a constant contains one thing, and sometimes it means
another thing. If one were to use one kind of `_PREFIX` constant in a
place where the other kind is expected, I can imagine things going quite
wrong, although this is helped a bit by the ones that represent prefix
lengths typically being `u8`s and the ones that represent IPv6 octet
pairs being, naturally, octet _pairs_, i.e. `u16`s. Luckily, one cannot
accidentally insert a `u16` into a `u8`-shaped hole, or vice versa, so
we haven't messed this up _yet_, at least as far as I can tell.

However, there is another annoying issue here, which is that I would
quite like to move the constant [`BOOTSTRAP_PREFIX`], which represents
the first two octets of the bootstrap network, from
`sled_hardware_types::underlay` to `omicron_common::address`.[^1] I'd
like to be able to reference it in Nexus in #10835. Sadly, as I
mentioned above, there is already a `BOOTSTRAP_SUBNET_PREFIX` here,
which represents the _length_ of a prefix, rather than the actual
octets. Having both `BOOTSTRAP_PREFIX` and `BOOTSTRAP_SUBNET_PREFIX`,
with different meanings, seems bad and unfortunate.

Therefore, this branch adopts a more consistent naming scheme, where
constants that represent the length of a prefix are always named
`FOO_PREFIX_LENGTH`, while constants which represent the first octet(s)
of a subnet, often beginning with the letter `f`, are named
`FOO_PREFIX`. Doing this touches a lot of files, but reviewers should
really only need to look closely at `common/src/address.rs`, as the rest
are just places where these various constants are referenced. I would
love to have a network guy or two fact-check whether I am using these
terms correctly. Thanks!

**AI Usage Disclosure**: This was a large, mechanical change, so I used
an AI tool called "the rust-analyzer `Rename Symbol` button" to assist
in making this change.

[^1]: Which it's only currently referenced in `sled-hardware`, which
    depends on `omicron-common` as well as on `sled-hardware-types`.

[`BOOTSTRAP_SUBNET_PREFIX`]:
    https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L22
[`RACK_PREFIX`]:
    https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L24
[`AZ_PREFIX`]:
    https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L23
[`IPV6_MULTICAST_PREFIX`]:
    https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L120
[`IPV6_ADMIN_SCOPED_MULTICAST_PREFIX`]:
    https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/common/src/address.rs#L130
[`BOOTSTRAP_PREFIX`]:
    https://github.com/oxidecomputer/omicron/blob/720bec205869c24ca34cc9c8d4b46345c8fcbafd/sled-hardware/types/src/underlay.rs#L6
@hawkw
hawkw force-pushed the eliza/external-client branch from 9fad25f to 7f5bf0a Compare July 21, 2026 18:36
@hawkw
hawkw force-pushed the eliza/external-client branch from 7f5bf0a to 03889a2 Compare July 21, 2026 19:02
@hawkw
hawkw marked this pull request as ready for review July 21, 2026 20:09
@hawkw
hawkw requested review from inickles and jgallagher July 21, 2026 20:09
@hawkw
hawkw requested a review from internet-diglett July 21, 2026 20:09
@hawkw hawkw added networking Related to the networking. nexus Related to nexus labels Jul 21, 2026
@hawkw hawkw changed the title [wip] new version of external http client that actually works [nexus] add an ExternalHttpClient for sending requests outside the rack Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

networking Related to the networking. nexus Related to nexus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant