Tunnel sled initialization requests through sprockets sessions #1128
Conversation
| // running plain HTTP and blindly trusting all connections from | ||
| // localhost. We have a similar sprockets proxy on the client side, | ||
| // where the proxy blindly trusts all connections from localhost | ||
| // (although the client-side proxy only runs while is being made, |
There was a problem hiding this comment.
typo (missing word?): while is being made
| let proxy_addr = proxy.local_addr(); | ||
|
|
||
| let proxy_task = tokio::spawn(async move { | ||
| // TODO-robustness `proxy.run()` only fails if `accept()`ing on our |
There was a problem hiding this comment.
Panicking seems totally reasonable to me.
| // sessions, so we'll have the bootstrap dropshot server listen on | ||
| // `bootstrap_address` (and no sprockets proxy). | ||
| // | ||
| // TODO-security: With this configuration, dropshot itself is |
There was a problem hiding this comment.
We probably should lift this to a Github issue as you mentioned in the PR description; I don't think we want to lose track of this.
There was a problem hiding this comment.
👍 I'll open an issue as a part of merging this PR.
There was a problem hiding this comment.
Merged and opened #1161
| // something other than TCP that we can lock down, or abandoning | ||
| // dropshot and using a bespoke protocol over a raw | ||
| // sprockets-encrypted TCP connection. | ||
| let (bootstrap_dropshot_addr, sprockets_proxy_bind_addr) = |
There was a problem hiding this comment.
Nit: These two values seem really tightly coupled; I wonder if they should be a distinct type.
That being said, I understand passing things through the ConfigDropshot doesn't make that trivial, so we can punt on this.
sled-agent/src/sp.rs
Outdated
| // Ensure we have the global zone IP address we need for the SP. | ||
| let data_link = if let Some(link) = sled_config.data_link.clone() { | ||
| link | ||
| } else { | ||
| Dladm::find_physical()? | ||
| }; | ||
| Zones::ensure_has_global_zone_v6_address(data_link, *sp_addr, "simsp") | ||
| .map_err(|err| SpError::EnsureGlobalZoneAddressError { | ||
| addr: *sp_addr, | ||
| err, | ||
| })?; |
There was a problem hiding this comment.
As of #1066 , I've moved most of the VNIC / address allocation to happen over an etherstub instead of a physical data link. motivation here.
As a result, most of our addresses are being allocated over these etherstubs; there's an example here:
omicron/sled-agent/src/bootstrap/agent.rs
Lines 157 to 177 in 813a859
Could this sp_addr also be allocated on the etherstub link?
There was a problem hiding this comment.
I expect so; I'll rebase to confirm. For now I haven't been testing the management network side of the simulated SPs at all (and am running a test config without any networking), since we only need the simulated RoT to do sprockets things. This will be revisited as we move back to MGS and start integrating it into the control plane, at which point it will need to access the (simulated) SPs over UDP.
There was a problem hiding this comment.
Changed in 02c2f57; we not only could change this allocation, but had to: Zones::ensure_has_global_zone_v6_address() now takes an EthernetVnic as its first arg, so the previous code allocating on the physical link no longer compiled.
jgallagher
force-pushed
the
sled-agent-sprockets
branch
from
55b5d1b to
02c2f57
Compare
jgallagher
force-pushed
the
sled-agent-sprockets
branch
from
02c2f57 to
a326f3f
Compare
Crucible changes: Remove unused fields in IOop (#1149) New downstairs clone subcommand. (#1129) Simplify the do_work_task loop (#1150) Move `Guest` stuff into a module (#1125) Bump nix to 0.27.1 and use new safer Fd APIs (#1110) Move `FramedWrite` work to a separate task (#1145) Use fewer borrows in ExtentInner API (#1147) Update Rust crate reedline to 0.28.0 (#1141) Update Rust crate tokio to 1.36 (#1143) Update Rust crate slog-bunyan to 2.5.0 (#1139) Update Rust crate rayon to 1.8.1 (#1138) Update Rust crate itertools to 0.12.1 (#1137) Update Rust crate byte-unit to 5.1.4 (#1136) Update Rust crate base64 to 0.21.7 (#1135) Update Rust crate async-trait to 0.1.77 (#1134) Discard deferred msgs (#1131) Minor Downstairs cleanup (#1127) Update test_fail_live_repair to support pstop (#1128) Ignore client messages after stopping the IO task (#1126) Move client IO task into a struct (#1124) Bump Rust to 1.75 and fix new Clippy lints (#1123) Propolis changes: PHD: convert to async (#633) PHD: assume specialized Windows images (#636) propolis-standalone-config needn't be a crate standalone: Use tar for snapshot/restore phd: use latest "lab-2.0-opte" target, not a specific version (#637) PHD: add tests for migration of running processes (#623) PHD: fix `cargo xtask phd` tidy not doing anything (#630) PHD: add documentation for `cargo xtask phd` (#629) standalone: improve virtual device creation errors (#632) phd: add Windows Server 2019 guest adapter (#627) PHD: add `cargo xtask phd` to make using PHD nicer (#619)
Crucible changes: Remove unused fields in IOop (#1149) New downstairs clone subcommand. (#1129) Simplify the do_work_task loop (#1150) Move `Guest` stuff into a module (#1125) Bump nix to 0.27.1 and use new safer Fd APIs (#1110) Move `FramedWrite` work to a separate task (#1145) Use fewer borrows in ExtentInner API (#1147) Update Rust crate reedline to 0.28.0 (#1141) Update Rust crate tokio to 1.36 (#1143) Update Rust crate slog-bunyan to 2.5.0 (#1139) Update Rust crate rayon to 1.8.1 (#1138) Update Rust crate itertools to 0.12.1 (#1137) Update Rust crate byte-unit to 5.1.4 (#1136) Update Rust crate base64 to 0.21.7 (#1135) Update Rust crate async-trait to 0.1.77 (#1134) Discard deferred msgs (#1131) Minor Downstairs cleanup (#1127) Update test_fail_live_repair to support pstop (#1128) Ignore client messages after stopping the IO task (#1126) Move client IO task into a struct (#1124) Bump Rust to 1.75 and fix new Clippy lints (#1123) Propolis changes: PHD: convert to async (#633) PHD: assume specialized Windows images (#636) propolis-standalone-config needn't be a crate standalone: Use tar for snapshot/restore phd: use latest "lab-2.0-opte" target, not a specific version (#637) PHD: add tests for migration of running processes (#623) PHD: fix `cargo xtask phd` tidy not doing anything (#630) PHD: add documentation for `cargo xtask phd` (#629) standalone: improve virtual device creation errors (#632) phd: add Windows Server 2019 guest adapter (#627) PHD: add `cargo xtask phd` to make using PHD nicer (#619) Co-authored-by: Alan Hanson <[email protected]>
Changes included in this PR:
config-sp.tomlfile (similarly to how it looks forconfig-rss.toml) that defines the configuration for a simulated SP/RoT. If found, it will tweak the bootstrap config to set up a sprockets proxy (more on this below).Without a handle to an SP (or prior to this PR), connections from bootstrap-agent-to-bootstrap-agent to init sleds were direct:
graph TD A[bootstrap-agent] -->|bootstrap network| B[bootstrap-agent dropshot server]After this PR and with a handle to an SP, connections between bootstrap agents now flow through sprockets proxies on localhost:
graph TD A[bootstrap-agent] -->|loopback| B(sprockets-proxy client) B -->|bootstrap network| C(sprockets-proxy server) C -->|loopback| D[bootstrap-agent dropshot server]The use of TCP over localhost has (potentially severe) security implications; they're noted in a
TODO-securitycomment insled-agent/src/bin/sled-agent.rswhere we set up the listen address for dropshot, but I'd be happy to lift them to a github issue if that would be better.I tried to include sufficient logging to follow the proxy connections, but it still leaves something to be desired since dropshot logs only include the connection coming from the proxy in front of it (on localhost). Here's a snippet from starting up the control plane for the first time on two VMs,
helios1andhelios2, between when RSS determines the plan and when the two bootstrap agents receive the sled agent requests and begin starting them:helios1 (where RSS runs):
helios2:
Setting up the above requires using
thing-flingerto (among other things) distributeconfig-sp.tomlfiles so all nodes know to simulate SP/RoTs, which are required to establish sprockets sessions. That happens insled-agent-overlay-filesand is included in this PR.My setup is having trouble starting nexus, but I believe that to be a VM networking issue and not related to the changes in this PR, given the above logs and the lack of changes to nexus/service-related code. I'm going to continue to debug that but believe this code is ready to review even without tracking that all the way down.Edit: This was fixed by #1132.