Skip to content

Commit af29c3f

Browse files
committed
fix(network): harden additional CA rotation
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
1 parent a7640a3 commit af29c3f

26 files changed

Lines changed: 1905 additions & 558 deletions

File tree

‎architecture/sandbox.md‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,13 +317,14 @@ than falling back to public roots. The bundle augments bundled, system, or
317317
native roots and preserves normal hostname verification.
318318

319319
Drivers deliver the normalized material at their runtime boundary. Docker and
320-
Podman bind-mount a gateway-owned artifact. Kubernetes server-side-applies one
321-
gateway-scoped ConfigMap per target namespace; combined topology mounts it only
322-
in the agent container that runs the supervisor, while sidecar topology mounts
323-
it only in `openshell-network`. VM writes it into the per-sandbox overlay. All
324-
paths converge on `/etc/openshell-tls/network-additional-ca.crt` and the
325-
operator-controlled `--network-additional-ca-bundle` argument. The supervisor
326-
also includes these roots in its child-process trust files.
320+
Podman bind-mount a gateway-owned artifact. Kubernetes creates immutable,
321+
content-addressed ConfigMap generations in each target namespace and validates
322+
an existing generation without mutation; combined topology mounts it only in
323+
the agent container that runs the supervisor, while sidecar topology mounts it
324+
only in `openshell-network`. VM writes it into the per-sandbox overlay. All paths
325+
converge on `/etc/openshell-tls/network-additional-ca.crt` and protected
326+
supervisor startup arguments. The supervisor verifies the expected generation
327+
before including these roots in destination and child-process trust files.
327328

328329
Destination trust remains separate from corporate-proxy CA material, gateway
329330
listener TLS, OIDC trust, and sandbox-to-gateway mTLS through

‎crates/openshell-core/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ tempfile = { version = "3", optional = true }
4444
[target.'cfg(unix)'.dependencies]
4545
nix = { workspace = true }
4646
rustix = { workspace = true }
47+
libc = "0.2"
4748

4849
[features]
4950
default = ["telemetry"]

‎crates/openshell-core/src/driver_utils.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ fn read_regular_file_bounded(path: &str, max_bytes: u64) -> Result<String, Bound
529529
use std::os::unix::fs::OpenOptionsExt as _;
530530
std::fs::OpenOptions::new()
531531
.read(true)
532-
.custom_flags(nix::libc::O_NONBLOCK)
532+
.custom_flags(libc::O_NONBLOCK)
533533
.open(path)
534534
};
535535
#[cfg(not(unix))]

‎crates/openshell-core/src/network_trust.rs‎

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,21 @@ use std::sync::Arc;
2020
/// supported driver on the same deployable boundary.
2121
pub const MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES: usize = 1024 * 1024 - "ca.crt".len();
2222

23+
/// Protected resource metadata key recording the destination-trust generation
24+
/// used to build a sandbox startup artifact.
25+
///
26+
/// Kubernetes stores this value in an annotation because a `sha256:<hex>`
27+
/// digest exceeds the 63-character label-value limit. Local container drivers
28+
/// may store the same protected key in a container label.
29+
pub const NETWORK_SUPERVISOR_TRUST_GENERATION_KEY: &str =
30+
"openshell.ai/network-additional-ca-generation";
31+
32+
/// Explicit generation marker for an omitted destination-trust configuration.
33+
///
34+
/// An explicit value, rather than an absent marker, lets a stopped sandbox
35+
/// distinguish a removal from a resource created by an older gateway.
36+
pub const NETWORK_SUPERVISOR_TRUST_GENERATION_NONE: &str = "none";
37+
2338
/// Normalized, gateway-owned trust material for sandbox destination TLS.
2439
///
2540
/// The PEM bytes contain only canonical X.509 certificate blocks. The type is
@@ -89,6 +104,148 @@ impl NetworkSupervisorTrustBundle {
89104
pub fn is_empty(&self) -> bool {
90105
self.normalized_pem.is_empty()
91106
}
107+
108+
/// Verify that the staged artifact still exactly matches this startup
109+
/// snapshot.
110+
///
111+
/// Local drivers call this before building or replacing a stopped sandbox.
112+
/// The check is deliberately bounded and follows no symlinks on Unix so a
113+
/// changed, missing, or non-regular artifact fails closed before a runtime
114+
/// can consume it. Diagnostics identify only the artifact path and digest;
115+
/// certificate contents are never included.
116+
pub fn verify_artifact(&self) -> Result<(), NetworkSupervisorTrustArtifactError> {
117+
use std::fs::{self, OpenOptions};
118+
use std::io::Read as _;
119+
120+
let path = self.artifact_path();
121+
let metadata = fs::symlink_metadata(path).map_err(|error| {
122+
NetworkSupervisorTrustArtifactError::Read {
123+
path: path.to_path_buf(),
124+
error,
125+
}
126+
})?;
127+
if !metadata.file_type().is_file() {
128+
return Err(NetworkSupervisorTrustArtifactError::NotRegularFile {
129+
path: path.to_path_buf(),
130+
});
131+
}
132+
if !has_expected_artifact_permissions(&metadata) {
133+
return Err(NetworkSupervisorTrustArtifactError::InsecurePermissions {
134+
path: path.to_path_buf(),
135+
});
136+
}
137+
if metadata.len() > MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES as u64 {
138+
return Err(NetworkSupervisorTrustArtifactError::TooLarge {
139+
path: path.to_path_buf(),
140+
limit: MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES,
141+
});
142+
}
143+
144+
let mut options = OpenOptions::new();
145+
options.read(true);
146+
#[cfg(unix)]
147+
{
148+
use std::os::unix::fs::OpenOptionsExt as _;
149+
options.custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW | libc::O_NONBLOCK);
150+
}
151+
let mut file =
152+
options
153+
.open(path)
154+
.map_err(|error| NetworkSupervisorTrustArtifactError::Read {
155+
path: path.to_path_buf(),
156+
error,
157+
})?;
158+
let opened_metadata =
159+
file.metadata()
160+
.map_err(|error| NetworkSupervisorTrustArtifactError::Read {
161+
path: path.to_path_buf(),
162+
error,
163+
})?;
164+
if !opened_metadata.file_type().is_file() {
165+
return Err(NetworkSupervisorTrustArtifactError::NotRegularFile {
166+
path: path.to_path_buf(),
167+
});
168+
}
169+
if !has_expected_artifact_permissions(&opened_metadata) {
170+
return Err(NetworkSupervisorTrustArtifactError::InsecurePermissions {
171+
path: path.to_path_buf(),
172+
});
173+
}
174+
if opened_metadata.len() > MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES as u64 {
175+
return Err(NetworkSupervisorTrustArtifactError::TooLarge {
176+
path: path.to_path_buf(),
177+
limit: MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES,
178+
});
179+
}
180+
181+
let capacity = usize::try_from(opened_metadata.len()).map_err(|_| {
182+
NetworkSupervisorTrustArtifactError::TooLarge {
183+
path: path.to_path_buf(),
184+
limit: MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES,
185+
}
186+
})?;
187+
let mut actual = Vec::with_capacity(capacity);
188+
file.by_ref()
189+
.take((MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES + 1) as u64)
190+
.read_to_end(&mut actual)
191+
.map_err(|error| NetworkSupervisorTrustArtifactError::Read {
192+
path: path.to_path_buf(),
193+
error,
194+
})?;
195+
if actual.len() > MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES {
196+
return Err(NetworkSupervisorTrustArtifactError::TooLarge {
197+
path: path.to_path_buf(),
198+
limit: MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES,
199+
});
200+
}
201+
if actual != self.normalized_pem() {
202+
return Err(NetworkSupervisorTrustArtifactError::GenerationMismatch {
203+
path: path.to_path_buf(),
204+
digest: self.digest.clone(),
205+
});
206+
}
207+
Ok(())
208+
}
209+
}
210+
211+
/// Return whether metadata has the read-only artifact permissions created by
212+
/// the gateway. The containing `0700` directory limits access to its owner;
213+
/// `0444` allows both root Docker and rootless Podman consumers to read a bind
214+
/// mounted generation without allowing any process to modify it.
215+
fn has_expected_artifact_permissions(metadata: &std::fs::Metadata) -> bool {
216+
#[cfg(unix)]
217+
{
218+
use std::os::unix::fs::PermissionsExt as _;
219+
metadata.permissions().mode() & 0o7777 == 0o444
220+
}
221+
#[cfg(not(unix))]
222+
{
223+
let _ = metadata;
224+
true
225+
}
226+
}
227+
228+
/// Non-secret failures while re-reading a gateway-owned staged trust artifact.
229+
#[derive(Debug, thiserror::Error)]
230+
pub enum NetworkSupervisorTrustArtifactError {
231+
#[error("network additional CA artifact '{path}' could not be read: {error}")]
232+
Read {
233+
path: PathBuf,
234+
#[source]
235+
error: std::io::Error,
236+
},
237+
#[error("network additional CA artifact '{path}' is not a regular file")]
238+
NotRegularFile { path: PathBuf },
239+
#[error(
240+
"network additional CA artifact '{path}' does not have the required read-only permissions"
241+
)]
242+
InsecurePermissions { path: PathBuf },
243+
#[error("network additional CA artifact '{path}' exceeds the shared {limit}-byte limit")]
244+
TooLarge { path: PathBuf, limit: usize },
245+
#[error(
246+
"network additional CA artifact '{path}' does not match the gateway startup trust generation {digest}"
247+
)]
248+
GenerationMismatch { path: PathBuf, digest: String },
92249
}
93250

94251
impl fmt::Debug for NetworkSupervisorTrustBundle {
@@ -142,4 +299,130 @@ mod tests {
142299
);
143300
assert_eq!(bundle.clone(), bundle);
144301
}
302+
303+
fn test_bundle(path: PathBuf, contents: &[u8]) -> NetworkSupervisorTrustBundle {
304+
NetworkSupervisorTrustBundle::new(contents.to_vec(), 1, "sha256:test-generation", path)
305+
}
306+
307+
fn set_read_only(path: &Path) {
308+
#[cfg(unix)]
309+
{
310+
use std::os::unix::fs::PermissionsExt as _;
311+
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o444)).unwrap();
312+
}
313+
#[cfg(not(unix))]
314+
let _ = path;
315+
}
316+
317+
#[cfg(unix)]
318+
#[test]
319+
fn artifact_verifier_rejects_writable_files() {
320+
use std::os::unix::fs::PermissionsExt as _;
321+
322+
let temp = tempfile::tempdir().unwrap();
323+
let path = temp.path().join("additional-ca.crt");
324+
std::fs::write(&path, b"normalized test CA\n").unwrap();
325+
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
326+
327+
let error = test_bundle(path, b"normalized test CA\n")
328+
.verify_artifact()
329+
.unwrap_err();
330+
assert!(matches!(
331+
error,
332+
NetworkSupervisorTrustArtifactError::InsecurePermissions { .. }
333+
));
334+
assert!(!error.to_string().contains("normalized test CA"));
335+
}
336+
337+
#[test]
338+
fn artifact_verifier_accepts_exact_file_contents() {
339+
let temp = tempfile::tempdir().unwrap();
340+
let path = temp.path().join("additional-ca.crt");
341+
let contents = b"normalized test CA\n";
342+
std::fs::write(&path, contents).unwrap();
343+
set_read_only(&path);
344+
345+
test_bundle(path, contents).verify_artifact().unwrap();
346+
}
347+
348+
#[test]
349+
fn artifact_verifier_rejects_replaced_contents_without_disclosing_them() {
350+
let temp = tempfile::tempdir().unwrap();
351+
let path = temp.path().join("additional-ca.crt");
352+
let expected = b"expected normalized CA\n";
353+
let replacement = b"replacement private CA must not be disclosed\n";
354+
std::fs::write(&path, replacement).unwrap();
355+
set_read_only(&path);
356+
357+
let error = test_bundle(path, expected).verify_artifact().unwrap_err();
358+
assert!(matches!(
359+
error,
360+
NetworkSupervisorTrustArtifactError::GenerationMismatch { .. }
361+
));
362+
let diagnostic = error.to_string();
363+
assert!(diagnostic.contains("sha256:test-generation"));
364+
assert!(!diagnostic.contains("expected normalized CA"));
365+
assert!(!diagnostic.contains("replacement private CA"));
366+
}
367+
368+
#[test]
369+
fn artifact_verifier_rejects_non_regular_files() {
370+
let temp = tempfile::tempdir().unwrap();
371+
let path = temp.path().join("directory");
372+
std::fs::create_dir(&path).unwrap();
373+
374+
let error = test_bundle(path, b"contents")
375+
.verify_artifact()
376+
.unwrap_err();
377+
assert!(matches!(
378+
error,
379+
NetworkSupervisorTrustArtifactError::NotRegularFile { .. }
380+
));
381+
}
382+
383+
#[cfg(unix)]
384+
#[test]
385+
fn artifact_verifier_rejects_symlinks() {
386+
use std::os::unix::fs::symlink;
387+
388+
let temp = tempfile::tempdir().unwrap();
389+
let target = temp.path().join("target");
390+
let link = temp.path().join("additional-ca.crt");
391+
std::fs::write(&target, b"normalized test CA\n").unwrap();
392+
symlink(&target, &link).unwrap();
393+
394+
let error = test_bundle(link, b"normalized test CA\n")
395+
.verify_artifact()
396+
.unwrap_err();
397+
assert!(matches!(
398+
error,
399+
NetworkSupervisorTrustArtifactError::NotRegularFile { .. }
400+
));
401+
}
402+
403+
#[test]
404+
fn artifact_verifier_enforces_shared_size_bound() {
405+
let temp = tempfile::tempdir().unwrap();
406+
let path = temp.path().join("additional-ca.crt");
407+
let at_limit = vec![b'x'; MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES];
408+
std::fs::write(&path, &at_limit).unwrap();
409+
set_read_only(&path);
410+
test_bundle(path.clone(), &at_limit)
411+
.verify_artifact()
412+
.unwrap();
413+
414+
let oversized = vec![b'x'; MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES + 1];
415+
std::fs::remove_file(&path).unwrap();
416+
std::fs::write(&path, &oversized).unwrap();
417+
set_read_only(&path);
418+
let error = test_bundle(path, b"expected")
419+
.verify_artifact()
420+
.unwrap_err();
421+
assert!(matches!(
422+
error,
423+
NetworkSupervisorTrustArtifactError::TooLarge { limit, .. }
424+
if limit == MAX_NETWORK_SUPERVISOR_TRUST_BUNDLE_BYTES
425+
));
426+
assert!(!error.to_string().contains(&"x".repeat(64)));
427+
}
145428
}

‎crates/openshell-driver-kubernetes/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ kube-runtime = { workspace = true }
3333
k8s-openapi = { workspace = true }
3434
serde = { workspace = true }
3535
serde_json = { workspace = true }
36+
sha2 = { workspace = true }
3637
clap = { workspace = true }
3738
tracing = { workspace = true }
3839
tracing-subscriber = { workspace = true }

0 commit comments

Comments
 (0)