Skip to content

Commit a9d1299

Browse files
committed
linux: never chown devices
Closes: #1845 Signed-off-by: Giuseppe Scrivano <[email protected]>
1 parent eecadda commit a9d1299

File tree

3 files changed

+97
-5
lines changed

3 files changed

+97
-5
lines changed

src/libcrun/container.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,15 +1017,27 @@ send_sync_cb (void *data, libcrun_error_t *err)
10171017
}
10181018

10191019
static int
1020-
maybe_chown_std_streams (uid_t container_uid, gid_t container_gid,
1021-
libcrun_error_t *err)
1020+
maybe_chown_std_streams (uid_t container_uid, gid_t container_gid, libcrun_error_t *err)
10221021
{
10231022
int ret, i;
10241023

10251024
for (i = 0; i < 3; i++)
10261025
{
10271026
if (! isatty (i))
10281027
{
1028+
struct stat statbuf;
1029+
ret = fstat (i, &statbuf);
1030+
if (UNLIKELY (ret < 0))
1031+
{
1032+
if (errno == EBADF)
1033+
continue;
1034+
return crun_make_error (err, errno, "fstat fd `%d`", i);
1035+
}
1036+
1037+
/* Skip chown for device files */
1038+
if (S_ISCHR (statbuf.st_mode) || S_ISBLK (statbuf.st_mode))
1039+
continue;
1040+
10291041
ret = fchown (i, container_uid, container_gid);
10301042
if (UNLIKELY (ret < 0))
10311043
{

tests/test_uid_gid.py

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/env python3
22
# crun - OCI runtime written in C
33
#
4-
# Copyright (C) 2017, 2018, 2019 Giuseppe Scrivano <[email protected]>
4+
# Copyright (C) 2017, 2018, 2019, 2025 Giuseppe Scrivano <[email protected]>
55
# crun is free software; you can redistribute it and/or modify
66
# it under the terms of the GNU General Public License as published by
77
# the Free Software Foundation; either version 2 of the License, or
@@ -16,6 +16,7 @@
1616
# along with crun. If not, see <http://www.gnu.org/licenses/>.
1717

1818
import os
19+
import sys
1920
from tests_utils import *
2021

2122
def test_userns_full_mapping():
@@ -168,6 +169,81 @@ def test_umask():
168169

169170
return 0
170171

172+
def test_dev_null_no_chown():
173+
"""Test that /dev/null file descriptors are not chowned to container user."""
174+
if is_rootless():
175+
return 77
176+
177+
# Get current owner of /dev/null and use owner + 1 as container user
178+
dev_null_stat = os.stat('/dev/null')
179+
container_uid = dev_null_stat.st_uid + 1
180+
container_gid = dev_null_stat.st_gid + 1
181+
182+
conf = base_config()
183+
conf['process']['user'] = {"uid": container_uid, "gid": container_gid}
184+
add_all_namespaces(conf)
185+
186+
# Check ownership of stdin fd which should be /dev/null
187+
conf['process']['args'] = ['/init', 'owner', '/proc/self/fd/0']
188+
189+
try:
190+
out, container_id = run_and_get_output(conf, stdin_dev_null=True)
191+
sys.stderr.write("# Container ran successfully, output: %s\n" % repr(out))
192+
if ':' in out:
193+
uid_str, gid_str = out.strip().split(':')
194+
uid, gid = int(uid_str), int(gid_str)
195+
# Should NOT be owned by container user
196+
if uid == container_uid or gid == container_gid:
197+
sys.stderr.write("# dev-null-no-chown test failed: /dev/null fd owned by container user %d:%d\n" % (uid, gid))
198+
sys.stderr.write("# stdout: %s\n" % repr(out))
199+
return -1
200+
sys.stderr.write("# dev-null-no-chown test passed: /dev/null fd owned by %d:%d (not container user %d:%d)\n" % (uid, gid, container_uid, container_gid))
201+
else:
202+
sys.stderr.write("# dev-null-no-chown test failed: unexpected owner output format\n")
203+
sys.stderr.write("# stdout: %s\n" % repr(out))
204+
return -1
205+
return 0
206+
except Exception as e:
207+
sys.stderr.write("# dev-null-no-chown test failed with exception: %s\n" % str(e))
208+
if hasattr(e, 'output'):
209+
sys.stderr.write("# command output: %s\n" % repr(e.output))
210+
return -1
211+
212+
def test_regular_files_chowned():
213+
"""Test that regular file descriptors are chowned to container user."""
214+
if is_rootless():
215+
return 77
216+
217+
# Get current owner of /dev/null and use owner + 1 as container user
218+
dev_null_stat = os.stat('/dev/null')
219+
container_uid = dev_null_stat.st_uid + 1
220+
container_gid = dev_null_stat.st_gid + 1
221+
222+
conf = base_config()
223+
conf['process']['user'] = {"uid": container_uid, "gid": container_gid}
224+
add_all_namespaces(conf)
225+
226+
# Check ownership of regular stdout (not /dev/null)
227+
conf['process']['args'] = ['/init', 'owner', '/proc/self/fd/1']
228+
229+
try:
230+
out, _ = run_and_get_output(conf)
231+
if ':' in out:
232+
uid_str, gid_str = out.strip().split(':')
233+
uid, gid = int(uid_str), int(gid_str)
234+
# Should be owned by container user
235+
if uid != container_uid or gid != container_gid:
236+
sys.stderr.write("# regular-files-chowned test failed: regular fd owned by %d:%d (expected %d:%d)\n" % (uid, gid, container_uid, container_gid))
237+
return -1
238+
sys.stderr.write("# regular-files-chowned test passed: regular fd owned by %d:%d (container user)\n" % (uid, gid))
239+
else:
240+
sys.stderr.write("# regular-files-chowned test failed: unexpected output format: %s\n" % repr(out))
241+
return -1
242+
return 0
243+
except Exception as e:
244+
sys.stderr.write("# regular-files-chowned test failed with exception: %s\n" % str(e))
245+
return -1
246+
171247
all_tests = {
172248
"uid" : test_uid,
173249
"gid" : test_gid,
@@ -176,6 +252,8 @@ def test_umask():
176252
"keep-groups" : test_keep_groups,
177253
"additional-gids": test_additional_gids,
178254
"umask": test_umask,
255+
"dev-null-no-chown": test_dev_null_no_chown,
256+
"regular-files-chowned": test_regular_files_chowned,
179257
}
180258

181259
if __name__ == "__main__":

tests/tests_utils.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ def get_crun_path():
224224
def run_and_get_output(config, detach=False, preserve_fds=None, pid_file=None,
225225
keep=False,
226226
command='run', env=None, use_popen=False, hide_stderr=False, cgroup_manager='cgroupfs',
227-
all_dev_null=False, id_container=None, relative_config_path="config.json",
227+
all_dev_null=False, stdin_dev_null=False, id_container=None, relative_config_path="config.json",
228228
chown_rootfs_to=None, callback_prepare_rootfs=None, debug_on_error=None):
229229

230230
# Some tests require that the container user, which might not be the
@@ -300,6 +300,8 @@ def run_and_get_output(config, detach=False, preserve_fds=None, pid_file=None,
300300
stdin = subprocess.DEVNULL
301301
stdout = subprocess.DEVNULL
302302
stderr = subprocess.DEVNULL
303+
elif stdin_dev_null:
304+
stdin = subprocess.DEVNULL
303305

304306
if use_popen:
305307
if not stdout:
@@ -311,7 +313,7 @@ def run_and_get_output(config, detach=False, preserve_fds=None, pid_file=None,
311313
close_fds=False), id_container
312314
else:
313315
try:
314-
return subprocess.check_output(args, cwd=temp_dir, stderr=stderr, env=env, close_fds=False, umask=default_umask).decode(), id_container
316+
return subprocess.check_output(args, cwd=temp_dir, stdin=stdin, stderr=stderr, env=env, close_fds=False, umask=default_umask).decode(), id_container
315317
except subprocess.CalledProcessError as e:
316318
sys.stderr.write("# Command failed: %s\n" % ' '.join(args))
317319
sys.stderr.write("# Working directory: %s\n" % temp_dir)

0 commit comments

Comments
 (0)