Skip to content

Commit 8a4f67b

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

27 files changed

Lines changed: 1908 additions & 561 deletions

File tree

‎architecture/sandbox.md‎

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

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

224225
Destination trust remains separate from corporate-proxy CA material, gateway
225226
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
@@ -581,7 +581,7 @@ fn read_regular_file_bounded(path: &str, max_bytes: u64) -> Result<String, Bound
581581
use std::os::unix::fs::OpenOptionsExt as _;
582582
std::fs::OpenOptions::new()
583583
.read(true)
584-
.custom_flags(nix::libc::O_NONBLOCK)
584+
.custom_flags(libc::O_NONBLOCK)
585585
.open(path)
586586
};
587587
#[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)