Skip to content

Commit

Permalink
linux: reject sysctl kernel.domainname when OCI knob domainname is set
Browse files Browse the repository at this point in the history
Setting sysctl `kernel.domainname` directly by user is not environment
agnostic, it shows either incorrect ( on non-working ) behaviour in
`rootless` environment.

It was decided to make this part of `runtime-spec` so the OCI runtime
can itself handle this behaviour correctly. As a result a new field
`domainname` was added to `runtime-spec`. Since crun already implementes
this field therefore `sysctl` configured by user conflicts with the
behaviour expected by the OCI runtime.

Runtime-spec PR: opencontainers/runtime-spec#1156

Furthermore a similar `sysctl` `kernal.hostname` is blocked by crun explicitly
to prevent this conflicting behaviour. https://github.com/containers/crun/blob/main/src/libcrun/linux.c#L3203

Following commit ensures that crun rejects sysctl `kernel.domainname`
when OCI field `domainname` is already set.

Signed-off-by: Aditya R <arajan@redhat.com>
  • Loading branch information
flouthoc committed Oct 3, 2022
1 parent a73a1d4 commit 391df45
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 6 deletions.
17 changes: 12 additions & 5 deletions src/libcrun/linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -3161,7 +3161,7 @@ const char *sysctlRequiringIPC[] = {
};

static int
validate_sysctl (const char *original_value, const char *name, unsigned long namespaces_created, libcrun_error_t *err)
validate_sysctl (const char *original_key, const char *original_value, const char *name, unsigned long namespaces_created, runtime_spec_schema_config_schema *def, libcrun_error_t *err)
{
const char *namespace = "";

Expand Down Expand Up @@ -3192,6 +3192,13 @@ validate_sysctl (const char *original_value, const char *name, unsigned long nam

if (strcmp (name, "kernel/domainname") == 0)
{
// Value of sysctl `kernel/domainname` is going to
// conflict with already set field `domainname` in
// OCI spec, in such scenario crun will fail to prevent
// unexpected behaviour for end user.
if (! is_empty_string (def->domainname) && (strcmp (original_value, def->domainname) != 0))
return crun_make_error (err, 0, "the sysctl `%s` conflicts with OCI field `domainname`", original_key);

if (namespaces_created & CLONE_NEWUTS)
return 0;

Expand All @@ -3200,7 +3207,7 @@ validate_sysctl (const char *original_value, const char *name, unsigned long nam
}

if (strcmp (name, "kernel/hostname") == 0)
return crun_make_error (err, 0, "the sysctl `%s` conflicts with OCI field `hostname`", original_value);
return crun_make_error (err, 0, "the sysctl `%s` conflicts with OCI field `hostname`", original_key);
}
if (has_prefix (name, "net/"))
{
Expand All @@ -3211,10 +3218,10 @@ validate_sysctl (const char *original_value, const char *name, unsigned long nam
goto fail;
}

return crun_make_error (err, 0, "the sysctl `%s` is not namespaced", original_value);
return crun_make_error (err, 0, "the sysctl `%s` is not namespaced", original_key);

fail:
return crun_make_error (err, 0, "the sysctl `%s` requires a new %s namespace", original_value, namespace);
return crun_make_error (err, 0, "the sysctl `%s` requires a new %s namespace", original_key, namespace);
}

int
Expand Down Expand Up @@ -3256,7 +3263,7 @@ libcrun_set_sysctl (libcrun_container_t *container, libcrun_error_t *err)
if (*it == '.')
*it = '/';

ret = validate_sysctl (def->linux->sysctl->keys[i], name, namespaces_created, err);
ret = validate_sysctl (def->linux->sysctl->keys[i], def->linux->sysctl->values[i], name, namespaces_created, def, err);
if (UNLIKELY (ret < 0))
return ret;

Expand Down
46 changes: 45 additions & 1 deletion tests/test_domainname.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,53 @@ def test_domainname():
if cid is not None:
run_crun_command(["delete", "-f", cid])


def test_domainname_conflict_sysctl():
# Setting sysctl `kernel.domainname` and OCI field `domainname` must fail
# since it produces unexpected behaviour for the end-users.
conf = base_config()
conf['process']['args'] = ['/init', 'getdomainname']
conf['domainname'] = "foomachine"
add_all_namespaces(conf)
conf['linux']['sysctl'] = {'kernel.domainname' : 'foo'}
cid = None
try:
out, cid = run_and_get_output(conf)
if out == "(none)\n":
return 0
sys.stderr.write("unexpected success\n")
return -1
except:
return 0
finally:
if cid is not None:
run_crun_command(["delete", "-f", cid])
return 0

def test_domainname_with_sysctl():
# Setting sysctl `kernel.domainname` and OCI field `domainname` must pass
# when both have exact same value
conf = base_config()
conf['process']['args'] = ['/init', 'getdomainname']
conf['domainname'] = "foo"
add_all_namespaces(conf)
conf['linux']['sysctl'] = {'kernel.domainname' : 'foo'}
cid = None
try:
out, cid = run_and_get_output(conf)
if out == "(none)\n":
return 0
return 0
except:
return -1
finally:
if cid is not None:
run_crun_command(["delete", "-f", cid])
return 0

all_tests = {
"domainname" : test_domainname,
"domainname conflict with syctl" : test_domainname_conflict_sysctl,
"domainname with syctl" : test_domainname_with_sysctl,
}

if __name__ == "__main__":
Expand Down

0 comments on commit 391df45

Please sign in to comment.