Skip to content

Commit 635b471

Browse files
committed
kata: don't add storage for implicit VOLUME mounts
1 parent 02f3608 commit 635b471

File tree

2 files changed

+168
-0
lines changed

2 files changed

+168
-0
lines changed
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Markus Rudy <[email protected]>
3+
Date: Mon, 30 Jun 2025 11:18:47 +0200
4+
Subject: [PATCH] genpolicy: don't allow mount/storage for declared VOLUMEs
5+
6+
Signed-off-by: Markus Rudy <[email protected]>
7+
---
8+
src/tools/genpolicy/genpolicy-settings.json | 12 -----
9+
src/tools/genpolicy/src/mount_and_storage.rs | 55 --------------------
10+
src/tools/genpolicy/src/settings.rs | 1 -
11+
src/tools/genpolicy/src/yaml.rs | 19 +++----
12+
4 files changed, 8 insertions(+), 79 deletions(-)
13+
14+
diff --git a/src/tools/genpolicy/genpolicy-settings.json b/src/tools/genpolicy/genpolicy-settings.json
15+
index 4cf7504ff9d4ef42b1dec5fe39781c7ead2fb0b2..62db4a2a4d74c3593b0f5064a093a9f6de6920ea 100644
16+
--- a/src/tools/genpolicy/genpolicy-settings.json
17+
+++ b/src/tools/genpolicy/genpolicy-settings.json
18+
@@ -206,18 +206,6 @@
19+
"rprivate",
20+
"ro"
21+
]
22+
- },
23+
- "image_volume": {
24+
- "mount_type": "bind",
25+
- "mount_source": "$(sfprefix)",
26+
- "driver": "local",
27+
- "source": "local",
28+
- "fstype": "bind",
29+
- "options": [
30+
- "rbind",
31+
- "rprivate",
32+
- "rw"
33+
- ]
34+
}
35+
},
36+
"mount_destinations": [
37+
diff --git a/src/tools/genpolicy/src/mount_and_storage.rs b/src/tools/genpolicy/src/mount_and_storage.rs
38+
index 3e9376e77562f9e18d7160859377175e12ca783d..87de0fb6aa27284e43fa23746b379fb778005ee9 100644
39+
--- a/src/tools/genpolicy/src/mount_and_storage.rs
40+
+++ b/src/tools/genpolicy/src/mount_and_storage.rs
41+
@@ -377,58 +377,3 @@ fn get_downward_api_mount(yaml_mount: &pod::VolumeMount, p_mounts: &mut Vec<poli
42+
});
43+
}
44+
}
45+
-
46+
-pub fn get_image_mount_and_storage(
47+
- settings: &settings::Settings,
48+
- p_mounts: &mut Vec<policy::KataMount>,
49+
- storages: &mut Vec<agent::Storage>,
50+
- destination: &str,
51+
-) {
52+
- // https://github.com/kubernetes/examples/blob/master/cassandra/image/Dockerfile
53+
- // has a volume mount starting with two '/' characters:
54+
- //
55+
- // CASSANDRA_DATA=/cassandra_data
56+
- // VOLUME ["/$CASSANDRA_DATA"]
57+
- let mut destination_string = destination.to_string();
58+
- while destination_string.contains("//") {
59+
- destination_string = destination_string.replace("//", "/");
60+
- }
61+
- debug!("get_image_mount_and_storage: image dest = {destination}, dest = {destination_string}");
62+
-
63+
- for mount in &mut *p_mounts {
64+
- if mount.destination == destination_string {
65+
- debug!(
66+
- "get_image_mount_and_storage: mount {destination_string} already defined by YAML"
67+
- );
68+
- return;
69+
- }
70+
- }
71+
-
72+
- let settings_image = &settings.volumes.image_volume;
73+
- debug!(
74+
- "get_image_mount_and_storage: settings for container image volumes: {:?}",
75+
- settings_image
76+
- );
77+
-
78+
- storages.push(agent::Storage {
79+
- driver: settings_image.driver.clone(),
80+
- driver_options: Vec::new(),
81+
- source: settings_image.source.clone(),
82+
- fstype: settings_image.fstype.clone(),
83+
- options: settings_image.options.clone(),
84+
- mount_point: destination_string.clone(),
85+
- fs_group: protobuf::MessageField::none(),
86+
- special_fields: ::protobuf::SpecialFields::new(),
87+
- });
88+
-
89+
- let file_name = Path::new(&destination_string).file_name().unwrap();
90+
- let name = OsString::from(file_name).into_string().unwrap();
91+
- let source = format!("{}{name}$", &settings_image.mount_source);
92+
-
93+
- p_mounts.push(policy::KataMount {
94+
- destination: destination_string,
95+
- type_: settings_image.fstype.clone(),
96+
- source,
97+
- options: settings_image.options.clone(),
98+
- });
99+
-}
100+
diff --git a/src/tools/genpolicy/src/settings.rs b/src/tools/genpolicy/src/settings.rs
101+
index b7f0515d17c1607985676c178b72b48ae9e38ece..fdd69c70d05f137dcbee328e8ffe95ac9291df68 100644
102+
--- a/src/tools/genpolicy/src/settings.rs
103+
+++ b/src/tools/genpolicy/src/settings.rs
104+
@@ -35,7 +35,6 @@ pub struct Volumes {
105+
pub emptyDir_memory: EmptyDirVolume,
106+
pub configMap: ConfigMapVolume,
107+
pub confidential_configMap: ConfigMapVolume,
108+
- pub image_volume: ImageVolume,
109+
}
110+
111+
/// EmptyDir volume settings loaded from genpolicy-settings.json.
112+
diff --git a/src/tools/genpolicy/src/yaml.rs b/src/tools/genpolicy/src/yaml.rs
113+
index 938dc64437f4ff4a2b9daf969a01059f5155e50b..0c2ca5921151b385b5873c90dfe12021bde43c1f 100644
114+
--- a/src/tools/genpolicy/src/yaml.rs
115+
+++ b/src/tools/genpolicy/src/yaml.rs
116+
@@ -27,6 +27,7 @@ use crate::volume;
117+
118+
use async_trait::async_trait;
119+
use core::fmt::Debug;
120+
+use std::collections::BTreeSet;
121+
use log::debug;
122+
use protocols::agent;
123+
use serde::{Deserialize, Serialize};
124+
@@ -290,6 +291,7 @@ pub fn get_container_mounts_and_storages(
125+
settings: &settings::Settings,
126+
volumes_option: &Option<Vec<volume::Volume>>,
127+
) {
128+
+ let mut mountPaths = BTreeSet::new();
129+
if let Some(volumes) = volumes_option {
130+
if let Some(volume_mounts) = &container.volumeMounts {
131+
for volume in volumes {
132+
@@ -302,24 +304,19 @@ pub fn get_container_mounts_and_storages(
133+
volume,
134+
volume_mount,
135+
);
136+
+ mountPaths.insert(volume_mount.mountPath.clone());
137+
}
138+
}
139+
}
140+
}
141+
}
142+
143+
- // Add storage and mount for each volume defined in the docker container image
144+
- // configuration layer.
145+
+ // Check that all VOLUME declarations have corresponding volume mounts.
146+
if let Some(volumes) = &container.registry.config_layer.config.Volumes {
147+
- for volume in volumes {
148+
- debug!("get_container_mounts_and_storages: {:?}", &volume);
149+
-
150+
- mount_and_storage::get_image_mount_and_storage(
151+
- settings,
152+
- policy_mounts,
153+
- storages,
154+
- volume.0,
155+
- );
156+
+ let volumePaths: BTreeSet<_> = volumes.keys().cloned().collect();
157+
+ let uncoveredPaths: Vec<String> = volumePaths.difference(&mountPaths).cloned().collect();
158+
+ if uncoveredPaths.len() > 0 {
159+
+ panic!("The following volumes declared in image config don't have corresponding Kubernetes mounts: {uncoveredPaths:?}");
160+
}
161+
}
162+
}

packages/by-name/kata/kata-runtime/package.nix

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,12 @@ buildGoModule (finalAttrs: {
166166
# This patch makes genpolicy conform to a bug in containerd that leads to ignored additional GIDs.
167167
# Upstream PR: https://github.com/kata-containers/kata-containers/pull/11358.
168168
./0025-genpolicy-ignore-groups-with-same-name-as-user.patch
169+
170+
# Don't add storages for volumes declared in the image config.
171+
# This fixes a security issue where the host is able to write untrusted content to paths
172+
# under these volumes, by failing the policy generation if volumes without mounts are found.
173+
# TODO(burgerdev): open upstream issue after disclosure.
174+
./0026-genpolicy-don-t-allow-mount-storage-for-declared-VOL.patch
169175
];
170176
};
171177

0 commit comments

Comments
 (0)