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: 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 <[email protected]>

Author: flouthoc

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;
 

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__":

tests/test_start.py

@@ -486,3 +486,4 @@ def test_listen_pid_env():
 
 if __name__ == "__main__":
     tests_main(all_tests)
+