From fc466763192738b98566d21a46bfd6046d65e020 Mon Sep 17 00:00:00 2001
From: Aditya R <arajan@redhat.com>
Date: Sat, 1 Oct 2022 17:05:01 +0530
Subject: [PATCH] linux: reject sysctl kernel.domainname when OCI knob
 domainname is set

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: https://github.com/opencontainers/runtime-spec/pull/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>
---
 src/libcrun/linux.c      | 21 +++++++++++++++------
 tests/test_domainname.py | 24 +++++++++++++++++++++++-
 tests/test_start.py      |  1 +
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/libcrun/linux.c b/src/libcrun/linux.c
index f413f46445..741aaca00f 100644
--- a/src/libcrun/linux.c
+++ b/src/libcrun/linux.c
@@ -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_value, const char *name, unsigned long namespaces_created, runtime_spec_schema_config_schema *def, libcrun_error_t *err)
 {
   const char *namespace = "";
 
@@ -3192,11 +3192,20 @@ validate_sysctl (const char *original_value, const char *name, unsigned long nam
 
       if (strcmp (name, "kernel/domainname") == 0)
         {
-          if (namespaces_created & CLONE_NEWUTS)
-            return 0;
+          if (! is_empty_string (def->domainname))
+            {
+              // 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.
+              return crun_make_error (err, 0, "the sysctl `%s` conflicts with OCI field `domainname`", original_value);
+            }
+          else
+            {
+              if (namespaces_created & CLONE_NEWUTS)
+                return 0;
 
-          namespace = "UTS";
-          goto fail;
+              namespace = "UTS";
+              goto fail;
+            }
         }
 
       if (strcmp (name, "kernel/hostname") == 0)
@@ -3256,7 +3265,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], name, namespaces_created, def, err);
       if (UNLIKELY (ret < 0))
         return ret;
 
diff --git a/tests/test_domainname.py b/tests/test_domainname.py
index f204a9c9eb..bc631d4190 100644
--- a/tests/test_domainname.py
+++ b/tests/test_domainname.py
@@ -43,9 +43,31 @@ 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
+
 all_tests = {
     "domainname" : test_domainname,
+    "domainname conflict with syctl" : test_domainname_conflict_sysctl,
 }
 
 if __name__ == "__main__":
diff --git a/tests/test_start.py b/tests/test_start.py
index c587adc8bc..4fe1ca2d61 100755
--- a/tests/test_start.py
+++ b/tests/test_start.py
@@ -486,3 +486,4 @@ def test_listen_pid_env():
 
 if __name__ == "__main__":
     tests_main(all_tests)
+