Skip to content

Commit b4890f5

Browse files
authored
fix(network): restore SDK gvproxy integration tests (#874)
Fix SDK network/secrets integration failures by avoiding the current VMM seccomp/gvproxy conflict and making MITM TLS validation work inside the jail. Test plan: - Remote e2e runner: Python #155 non-detach failure subset, 25 passed in 116.10s - Remote e2e runner: Node tests/network-secrets.integration.test.ts, 5 passed - A/B check without SDK Cargo gvproxy feature changes: Python subset 25 passed, Node network-secrets 5 passed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved secure upstream TLS validation by extending sandbox trust to include existing system CA bundles and cert files (read-only). * Prevented syscall filtering from disrupting network-enabled runs by skipping the VMM syscall filter when networking is active. * Updated network interception integration coverage to use a new upstream endpoint, keeping secret-substitution checks stable. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 36b228d commit b4890f5

5 files changed

Lines changed: 110 additions & 14 deletions

File tree

sdks/node/tests/network-secrets.integration.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,13 @@ describe("SimpleBox network and secrets", { timeout: 180_000 }, () => {
6767
image: "python:slim",
6868
network: {
6969
mode: "enabled",
70-
allowNet: ["httpbin.org"],
70+
allowNet: ["httpbingo.org"],
7171
},
7272
secrets: [
7373
{
7474
name: "testkey",
7575
value: "super-secret-value",
76-
hosts: ["httpbin.org"],
76+
hosts: ["httpbingo.org"],
7777
},
7878
],
7979
autoRemove: true,
@@ -85,7 +85,7 @@ describe("SimpleBox network and secrets", { timeout: 180_000 }, () => {
8585
[
8686
"import os, urllib.request",
8787
"req = urllib.request.Request(",
88-
" 'https://httpbin.org/headers',",
88+
" 'https://httpbingo.org/headers',",
8989
" headers={'Authorization': 'Bearer ' + os.environ['BOXLITE_SECRET_TESTKEY']},",
9090
")",
9191
"print(urllib.request.urlopen(req, timeout=20).read().decode())",

sdks/python/tests/test_secret_substitution.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -240,29 +240,29 @@ def test_secret_substitution_reaches_upstream(self, runtime):
240240
secret = boxlite.Secret(
241241
name="testkey",
242242
value=real_value,
243-
hosts=["httpbin.org"],
243+
hosts=["httpbingo.org"],
244244
)
245245
sandbox = runtime.create(
246246
boxlite.BoxOptions(
247247
image="alpine:latest",
248248
network=boxlite.NetworkSpec(
249249
mode="enabled",
250-
allow_net=["httpbin.org"],
250+
allow_net=["httpbingo.org"],
251251
),
252252
secrets=[secret],
253253
)
254254
)
255255
try:
256256
# Guest sends placeholder in header; MITM substitutes real value;
257-
# httpbin.org echoes it back in JSON response.
257+
# httpbingo.org echoes it back in JSON response.
258258
execution = sandbox.exec(
259259
"wget",
260260
[
261261
"-q",
262262
"-O-",
263263
"--header",
264264
"Authorization: Bearer <BOXLITE_SECRET:testkey>",
265-
"https://httpbin.org/headers",
265+
"https://httpbingo.org/headers",
266266
],
267267
)
268268
stdout = "".join(list(execution.stdout()))
@@ -297,16 +297,16 @@ def test_non_secret_host_not_intercepted(self, runtime):
297297
image="alpine:latest",
298298
network=boxlite.NetworkSpec(
299299
mode="enabled",
300-
allow_net=["httpbin.org"],
300+
allow_net=["httpbingo.org"],
301301
),
302302
secrets=[secret],
303303
)
304304
)
305305
try:
306-
# httpbin.org is NOT in secret hosts — should work normally
306+
# httpbingo.org is NOT in secret hosts — should work normally
307307
execution = sandbox.exec(
308308
"wget",
309-
["-q", "-O-", "http://httpbin.org/ip"],
309+
["-q", "-O-", "http://httpbingo.org/ip"],
310310
)
311311
stdout = "".join(list(execution.stdout()))
312312
result = execution.wait()

src/boxlite/src/jailer/mod.rs

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,18 @@ fn build_path_access(layout: &BoxFilesystemLayout, volumes: &[VolumeSpec]) -> Ve
286286
});
287287
}
288288

289+
// The in-shim network backend may validate upstream TLS certificates
290+
// (for example secret-substitution MITM forwarding). Keep host trust
291+
// stores readable inside the sandbox without granting broader /etc access.
292+
for path in system_ca_paths() {
293+
if path.exists() {
294+
paths.push(PathAccess {
295+
path,
296+
writable: false,
297+
});
298+
}
299+
}
300+
289301
// User volumes
290302
for vol in volumes {
291303
let p = PathBuf::from(&vol.host_path);
@@ -300,6 +312,18 @@ fn build_path_access(layout: &BoxFilesystemLayout, volumes: &[VolumeSpec]) -> Ve
300312
paths
301313
}
302314

315+
fn system_ca_paths() -> [PathBuf; 7] {
316+
[
317+
PathBuf::from("/etc/ssl/certs"),
318+
PathBuf::from("/etc/pki/tls/certs"),
319+
PathBuf::from("/etc/ca-certificates"),
320+
PathBuf::from("/etc/ssl/cert.pem"),
321+
PathBuf::from("/etc/ssl/certs/ca-certificates.crt"),
322+
PathBuf::from("/etc/pki/tls/certs/ca-bundle.crt"),
323+
PathBuf::from("/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem"),
324+
]
325+
}
326+
303327
/// Jailer provides process isolation for boxlite-shim.
304328
///
305329
/// Encapsulates security configuration and delegates to a [`Sandbox`]
@@ -532,8 +556,27 @@ mod tests {
532556

533557
let paths = build_path_access(&layout, &[]);
534558

535-
// Empty box dir: no subdirectories exist yet, so no paths
536-
assert!(paths.is_empty(), "No paths for empty box dir");
559+
let existing_ca_paths: Vec<_> = system_ca_paths()
560+
.into_iter()
561+
.filter(|p| p.exists())
562+
.collect();
563+
564+
assert_eq!(
565+
paths.len(),
566+
existing_ca_paths.len(),
567+
"empty box dir should only include existing system CA paths"
568+
);
569+
for ca_path in existing_ca_paths {
570+
let entry = paths
571+
.iter()
572+
.find(|p| p.path == ca_path)
573+
.unwrap_or_else(|| panic!("missing CA path {}", ca_path.display()));
574+
assert!(
575+
!entry.writable,
576+
"CA path must be read-only: {}",
577+
ca_path.display()
578+
);
579+
}
537580
}
538581

539582
#[test]
@@ -662,6 +705,34 @@ mod tests {
662705
assert!(!bases_paths[0].writable);
663706
}
664707

708+
#[test]
709+
fn test_build_path_access_includes_system_ca_paths_readonly() {
710+
let dir = tempdir().unwrap();
711+
let layout = test_layout(dir.path().to_path_buf());
712+
let existing_ca_paths: Vec<_> = system_ca_paths()
713+
.into_iter()
714+
.filter(|p| p.exists())
715+
.collect();
716+
717+
if existing_ca_paths.is_empty() {
718+
return;
719+
}
720+
721+
let paths = build_path_access(&layout, &[]);
722+
723+
for ca_path in existing_ca_paths {
724+
let entry = paths
725+
.iter()
726+
.find(|p| p.path == ca_path)
727+
.unwrap_or_else(|| panic!("missing CA path {}", ca_path.display()));
728+
assert!(
729+
!entry.writable,
730+
"CA path must be read-only: {}",
731+
ca_path.display()
732+
);
733+
}
734+
}
735+
665736
#[test]
666737
fn test_build_path_access_includes_qcow2_backing_file() {
667738
use crate::disk::{BackingFormat, Qcow2Helper};

src/deps/libgvproxy-sys/gvproxy-bridge/mitm.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ type BoxCA struct {
2424
certCache sync.Map // hostname -> *tls.Certificate
2525
}
2626

27+
var (
28+
upstreamRootCAs *x509.CertPool
29+
upstreamRootCAsOnce sync.Once
30+
upstreamRootCAsErr error
31+
)
32+
2733
// NewBoxCAFromPEM reconstructs a BoxCA from PEM-encoded cert and key.
2834
// The cert/key are generated by the Rust side and passed via GvproxyConfig JSON.
2935
func NewBoxCAFromPEM(certPEM, keyPEM []byte) (*BoxCA, error) {
@@ -185,7 +191,18 @@ func resolveUpstreamTLS(hostname string, overrides ...*tls.Config) *tls.Config {
185191
if len(overrides) > 0 && overrides[0] != nil {
186192
return overrides[0]
187193
}
188-
return &tls.Config{ServerName: hostname}
194+
cfg := &tls.Config{ServerName: hostname}
195+
if roots, err := loadUpstreamRootCAs(); err == nil {
196+
cfg.RootCAs = roots
197+
}
198+
return cfg
199+
}
200+
201+
func loadUpstreamRootCAs() (*x509.CertPool, error) {
202+
upstreamRootCAsOnce.Do(func() {
203+
upstreamRootCAs, upstreamRootCAsErr = x509.SystemCertPool()
204+
})
205+
return upstreamRootCAs, upstreamRootCAsErr
189206
}
190207

191208
// SecretHostMatcher provides O(1) lookup for whether a hostname has secrets.

src/shim/src/main.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,10 @@ fn run_shim(mut config: InstanceSpec, timing: impl Fn(&str)) -> BoxliteResult<()
153153
{
154154
use boxlite::jailer::seccomp;
155155

156-
if config.security.jailer_enabled && config.security.seccomp_enabled {
156+
if config.security.jailer_enabled
157+
&& config.security.seccomp_enabled
158+
&& config.network_config.is_none()
159+
{
157160
tracing::info!(
158161
box_id = %config.box_id,
159162
"Applying VMM seccomp filter (TSYNC)"
@@ -165,6 +168,11 @@ fn run_shim(mut config: InstanceSpec, timing: impl Fn(&str)) -> BoxliteResult<()
165168
box_id = %config.box_id,
166169
"Seccomp isolation complete"
167170
);
171+
} else if config.security.jailer_enabled && config.security.seccomp_enabled {
172+
tracing::warn!(
173+
box_id = %config.box_id,
174+
"Seccomp disabled for network-enabled VM; gvproxy runs in-process and is not covered by the VMM syscall profile"
175+
);
168176
} else if config.security.jailer_enabled {
169177
tracing::warn!(
170178
box_id = %config.box_id,

0 commit comments

Comments
 (0)