Skip to content

Commit 76279f5

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

53 files changed

Lines changed: 9240 additions & 1227 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

architecture/sandbox.md

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

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

8384
Destination trust remains separate from corporate-proxy CA material, gateway
8485
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
@@ -38,6 +38,7 @@ tempfile = { version = "3", optional = true }
3838
[target.'cfg(unix)'.dependencies]
3939
nix = { workspace = true }
4040
rustix = { workspace = true }
41+
libc = "0.2"
4142

4243
[features]
4344
default = ["telemetry"]

crates/openshell-core/src/driver_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ fn read_regular_file_bounded(path: &str, max_bytes: u64) -> Result<String, Bound
578578
use std::os::unix::fs::OpenOptionsExt as _;
579579
std::fs::OpenOptions::new()
580580
.read(true)
581-
.custom_flags(nix::libc::O_NONBLOCK)
581+
.custom_flags(libc::O_NONBLOCK)
582582
.open(path)
583583
};
584584
#[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
}

0 commit comments

Comments
 (0)