Skip to content

Commit e451bb3

Browse files
authored
fix(mxc): keep host supervisor proxy CA files in sandbox state (#5059)
Keep each MXC supervisor's proxy CA certificate and trust bundle in its sandbox host state directory, avoiding cross-sandbox overwrites and removing the files on sandbox deletion. Resolve Windows TLS output directories to extended-length paths before atomic writes, with the existing fallback when canonicalization is unavailable. Closes #5058 Signed-off-by: Jeffrey Quesnelle <emozilla@nousresearch.com> Signed-off-by: Shailendra Singh <shailendras@nvidia.com>
1 parent cec319b commit e451bb3

3 files changed

Lines changed: 133 additions & 3 deletions

File tree

‎crates/openshell-driver-mxc/README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ etw_audit = false
127127
Only `process_container` supports this architecture. `isolation_session` is
128128
rejected during sandbox validation.
129129

130+
The host supervisor writes its proxy CA certificate and trust bundle to
131+
`proxy-tls\` in the sandbox's host state directory,
132+
`<state_dir>\<sandbox-id>\<generation>`, and deleting the sandbox removes them.
133+
130134
MXC 1.0.0 is required. Pre-1.0 executors and the retired
131135
`default_configuration_id` setting are not supported; remove that setting
132136
before upgrading. Both live launches and standalone mapper output use the

‎crates/openshell-driver-mxc/src/driver.rs‎

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ const DRIVER_NAME: &str = "mxc";
4040
const DRIVER_VERSION: &str = env!("CARGO_PKG_VERSION");
4141
const DEFAULT_IMAGE_SENTINEL: &str = "mxc:process-container";
4242
const HOST_AUTH_BUNDLE_FILE: &str = "supervisor-auth.json";
43+
/// Directory in the sandbox host state directory for the proxy CA files.
44+
const HOST_PROXY_TLS_DIR: &str = "proxy-tls";
4345
const HOST_RUNTIME_DESCRIPTOR_FILE: &str = "runtime-descriptor.json";
4446
const BOUNDARY_CONFIG_FILE: &str = "boundary.json";
4547
const BOUNDARY_TLS_CERT_FILE: &str = "sandbox.crt";
@@ -858,6 +860,17 @@ fn spawn_supervisor(
858860
auth_bundle_path: &Path,
859861
main: &openshell_core::sandbox_env::MainProcessConfig,
860862
) -> Result<Child, String> {
863+
supervisor_command(context, descriptor_path, auth_bundle_path, main)?
864+
.spawn()
865+
.map_err(|error| format!("start host openshell-supervisor: {error}"))
866+
}
867+
868+
fn supervisor_command(
869+
context: &LifecycleContext,
870+
descriptor_path: &Path,
871+
auth_bundle_path: &Path,
872+
main: &openshell_core::sandbox_env::MainProcessConfig,
873+
) -> Result<Command, String> {
861874
let main_process_spec = mxc_main_process_spec(main)?;
862875
let mut command = Command::new(&context.config.supervisor_binary_path);
863876
command
@@ -887,6 +900,14 @@ fn spawn_supervisor(
887900
)
888901
.env(openshell_core::sandbox_env::SANDBOX_ID, &context.sandbox.id)
889902
.env(openshell_core::sandbox_env::SANDBOX, &context.sandbox.name)
903+
// The supervisor's default `/etc/openshell-tls` resolves to
904+
// `\etc\openshell-tls` on the current drive, where every sandbox on
905+
// the host would write and read the same CA files. Keep them with the
906+
// sandbox's host state, which `delete_sandbox` removes.
907+
.env(
908+
openshell_core::sandbox_env::PROXY_TLS_DIR,
909+
context.host_state_dir.join(HOST_PROXY_TLS_DIR),
910+
)
890911
.env(
891912
openshell_core::sandbox_env::LOG_LEVEL,
892913
openshell_core::driver_utils::sandbox_log_level(&context.sandbox, "info"),
@@ -900,9 +921,7 @@ fn spawn_supervisor(
900921
server_name,
901922
);
902923
}
903-
command
904-
.spawn()
905-
.map_err(|error| format!("start host openshell-supervisor: {error}"))
924+
Ok(command)
906925
}
907926

908927
fn mxc_main_process_spec(
@@ -1848,6 +1867,50 @@ mod tests {
18481867
assert!(!backend.config.state_dir.exists());
18491868
}
18501869

1870+
#[test]
1871+
fn host_supervisor_proxy_tls_dir_is_in_sandbox_host_state() {
1872+
let (backend, sandbox, dir) = preflight_fixture();
1873+
let host_state_dir = backend
1874+
.config
1875+
.state_dir
1876+
.join(&sandbox.id)
1877+
.join("generation");
1878+
let context = LifecycleContext {
1879+
invoker: backend.invoker.clone(),
1880+
config: backend.config.clone(),
1881+
gateway: backend.gateway.clone(),
1882+
registry: backend.registry.clone(),
1883+
watch_tx: backend.watch_tx.clone(),
1884+
attribution: backend.attribution.clone(),
1885+
sandbox_config: sandbox_config(&sandbox).unwrap(),
1886+
sandbox,
1887+
generation: "generation".into(),
1888+
host_state_dir: host_state_dir.clone(),
1889+
boundary_state_dir: dir.path().join("boundary"),
1890+
policy_mapper: backend.policy_mapper.clone(),
1891+
};
1892+
let main = mxc_main_process_config(
1893+
context.sandbox.spec.as_ref(),
1894+
&context.sandbox_config.command,
1895+
);
1896+
let command = supervisor_command(
1897+
&context,
1898+
&host_state_dir.join(HOST_RUNTIME_DESCRIPTOR_FILE),
1899+
&host_state_dir.join(HOST_AUTH_BUNDLE_FILE),
1900+
&main,
1901+
)
1902+
.unwrap();
1903+
let tls_dir = command
1904+
.as_std()
1905+
.get_envs()
1906+
.find(|(key, _)| *key == openshell_core::sandbox_env::PROXY_TLS_DIR)
1907+
.and_then(|(_, value)| value);
1908+
assert_eq!(
1909+
tls_dir.map(Path::new),
1910+
Some(host_state_dir.join(HOST_PROXY_TLS_DIR).as_path())
1911+
);
1912+
}
1913+
18511914
fn with_ui_policy(
18521915
mut sandbox: DriverSandbox,
18531916
ui: openshell_core::proto::UiPolicy,

‎crates/openshell-supervisor-network/src/l7/tls.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,15 @@ fn write_tls_output(path: &Path, contents: &[u8]) -> Result<()> {
393393
Err(error) => return Err(error).into_diagnostic(),
394394
}
395395

396+
// `NamedTempFile::persist` passes the temporary file's path to
397+
// `SetFileAttributesW` and both paths to `MoveFileExW` unchanged, and those
398+
// calls fail for paths longer than `MAX_PATH` unless they carry the `\\?\`
399+
// prefix. A sandbox's host state directory can be that deep.
400+
#[cfg(windows)]
401+
let extended = extended_length_path(path);
402+
#[cfg(windows)]
403+
let path = extended.as_path();
404+
396405
let parent = path
397406
.parent()
398407
.ok_or_else(|| miette!("TLS output has no parent: {}", path.display()))?;
@@ -418,6 +427,19 @@ fn write_tls_output(path: &Path, contents: &[u8]) -> Result<()> {
418427
Ok(())
419428
}
420429

430+
/// `path` with its directory in the extended-length (`\\?\`) form that
431+
/// `canonicalize` returns on Windows, or `path` unchanged if `canonicalize`
432+
/// fails. It can fail on volumes that the Mount Manager does not track, where
433+
/// paths shorter than `MAX_PATH` still work as given.
434+
#[cfg(windows)]
435+
fn extended_length_path(path: &Path) -> PathBuf {
436+
let (Some(parent), Some(file_name)) = (path.parent(), path.file_name()) else {
437+
return path.to_path_buf();
438+
};
439+
std::fs::canonicalize(parent)
440+
.map_or_else(|_| path.to_path_buf(), |parent| parent.join(file_name))
441+
}
442+
421443
/// Load PEM-encoded certificates from a string into a root certificate store.
422444
///
423445
/// Returns `(added, ignored)` counts. Invalid or unparseable certificates
@@ -882,6 +904,47 @@ mod tests {
882904
}
883905
}
884906

907+
#[cfg(windows)]
908+
#[test]
909+
fn write_ca_files_accepts_paths_longer_than_max_path() {
910+
use std::os::windows::ffi::OsStrExt;
911+
912+
let ca = SandboxCa::generate().unwrap();
913+
let dir = tempfile::tempdir().unwrap();
914+
let mut output_dir = dir.path().to_path_buf();
915+
// `MAX_PATH` counts UTF-16 code units.
916+
while output_dir
917+
.join("openshell-ca.pem")
918+
.as_os_str()
919+
.encode_wide()
920+
.count()
921+
<= 260
922+
{
923+
output_dir.push("0123456789abcdef0123456789abcdef");
924+
}
925+
926+
let (ca_path, bundle_path) = write_ca_files(&ca, &output_dir, "").unwrap();
927+
928+
assert_eq!(std::fs::read_to_string(&ca_path).unwrap(), ca.cert_pem());
929+
assert!(
930+
std::fs::read_to_string(&bundle_path)
931+
.unwrap()
932+
.contains(ca.cert_pem())
933+
);
934+
}
935+
936+
#[cfg(windows)]
937+
#[test]
938+
fn extended_length_path_keeps_the_path_when_canonicalize_fails() {
939+
let dir = tempfile::tempdir().unwrap();
940+
let resolved = extended_length_path(&dir.path().join("openshell-ca.pem"));
941+
assert!(resolved.as_os_str().to_string_lossy().starts_with(r"\\?\"));
942+
943+
// The directory does not exist, so `canonicalize` fails.
944+
let unresolved = dir.path().join("missing").join("openshell-ca.pem");
945+
assert_eq!(extended_length_path(&unresolved), unresolved);
946+
}
947+
885948
#[test]
886949
fn durable_ca_round_trip_preserves_certificate_bytes() {
887950
let generated = SandboxCa::generate().unwrap();

0 commit comments

Comments
 (0)