NAS-104777 / 12.0 / Change devfs ruleset handling so that configured …

…rulesets != 4 are (#1106)

* Change devfs ruleset handling so that configured rulesets != 4 are
cloned/copied into dynamic ones.  This makes devfs rule handling more
symmetrical to what is done when the default ruleset 4 is configured (which
in fact never applies devfs ruleset 4, but creates an iocage specific
dynamic ruleset instead - which can be quite confusing).

As a result, this addresses the problem of non-dynamic rulesets being
removed on `iocage stop` raised in
#952.

This also makes iocage fail to start a jail if the configured devfs ruleset
is not available - beforehand it would start with a default ruleset in this
case, which can have severe unwanted side effects.

Finally, this sets the minimum dynamically assigned devfs rule id to 1000 to
reserve the lower ids for static configuration by the admin in devfs.rules
(this is mostly for convenience).

* Change devfs_ruleset config parsing on jail start, so that:
- Users get a helpful error message in case a configured devfs_ruleset
  doesn't exist (also shows the configured ruleset and not
  the dynamically created one, which was not helpful).
- Users learn on jail start about how the devfs_ruleset is created
  (show id it was cloned from or that it is based on
  iocage's default).
- Avoid leaking devfs_rulesets on starting plugins that define
  devfs_paths/devfs_includes (would lose one ruleset
  everytime otherwise).
- Show a warning in case a plugin with devfs_paths/devfs_includes
  is started with a manually configured devfs_ruleset (as
  this won't - and never did - apply those.
- Move magic numbers to constants in ioc_common.py.

This doesn't fix the iocage man page, which shows (and had shown)
inaccurate information on this for a while.

* Move "min_dyn_devfs_ruleset" to jail property, change jail start devfs_ruleset
message to be one-liner containing information on configured devfs ruleset
(configured or iocage generated).

* Validate 'devfs_ruleset' and 'min_dyn_devfs_ruleset' while setting.

iocage_lib/ioc_common.py

@@ -46,6 +46,9 @@
 from iocage_lib.dataset import Dataset
 
 INTERACTIVE = False
+# 4 is a magic number for default and doesn't refer
+# to the actual ruleset 4 in devfs.rules(!)
+IOCAGE_DEVFS_RULESET = 4
 
 
 def callback(_log, callback_exception):
@@ -742,30 +745,34 @@ def generate_devfs_ruleset(conf, paths=None, includes=None, callback=None,
     Will add a per jail devfs ruleset with the specified rules,
     specifying defaults that equal devfs_ruleset 4
     """
-    ruleset = conf['devfs_ruleset']
+    configured_ruleset = conf['devfs_ruleset']
     devfs_includes = []
     devfs_rulesets = su.run(
         ['devfs', 'rule', 'showsets'],
         stdout=su.PIPE, universal_newlines=True
     )
     ruleset_list = [int(i) for i in devfs_rulesets.stdout.splitlines()]
 
-    if ruleset != '4':
-        if int(ruleset) in ruleset_list:
-            return str(ruleset)
-
-        logit({
-            "level": "INFO",
-            "message": f'* Ruleset {ruleset} does not exist, using defaults'
-        },
-            _callback=callback,
-            silent=silent)
-
-    ruleset = 5  # 0-4 is always reserved
+    ruleset = int(conf["min_dyn_devfs_ruleset"])
     while ruleset in ruleset_list:
         ruleset += 1
     ruleset = str(ruleset)
 
+    # Custom devfs_ruleset configured, clone to dynamic ruleset
+    if int(configured_ruleset) != IOCAGE_DEVFS_RULESET:
+        if int(configured_ruleset) not in ruleset_list:
+            return (True, configured_ruleset, '0')
+        rules = su.run(
+            ['devfs', 'rule', '-s', configured_ruleset, 'show'],
+            stdout=su.PIPE, universal_newlines=True
+        )
+        for rule in rules.stdout.splitlines():
+            su.run(['devfs', 'rule', '-s', ruleset, 'add'] +
+                   rule.split(' ')[1:], stdout=su.PIPE)
+
+        return (True, configured_ruleset, ruleset)
+
+    # Create default ruleset
     devfs_dict = dict((dev, None) for dev in (
         'hide', 'null', 'zero', 'crypto', 'random', 'urandom', 'ptyp*',
         'ptyq*', 'ptyr*', 'ptys*', 'ptyP*', 'ptyQ*', 'ptyR*', 'ptyS*', 'ptyl*',
@@ -817,7 +824,7 @@ def generate_devfs_ruleset(conf, paths=None, includes=None, callback=None,
 
         su.run(['devfs', 'rule', '-s', ruleset] + path, stdout=su.PIPE)
 
-    return ruleset
+    return (False, configured_ruleset, ruleset)
 
 
 def runscript(script):

iocage_lib/ioc_json.py

@@ -433,7 +433,7 @@ def __init__(self, location, checking_datasets, silent, callback):
     @staticmethod
     def get_version():
         """Sets the iocage configuration version."""
-        version = '26'
+        version = '27'
 
         return version
 
@@ -864,6 +864,10 @@ def check_config(self, conf, default=False):
             if conf.get(option) == 'none':
                 conf[option] = 'auto'
 
+        # Version 27 key
+        if not conf.get('min_dyn_devfs_ruleset'):
+            conf['min_dyn_devfs_ruleset'] = '1000'
+
         if not default:
             conf.update(jail_conf)
 
@@ -1090,7 +1094,7 @@ def retrieve_default_props():
             'vnet2_mac': 'none',
             'vnet3_mac': 'none',
             'vnet_default_interface': 'auto',
-            'devfs_ruleset': '4',
+            'devfs_ruleset': str(iocage_lib.ioc_common.IOCAGE_DEVFS_RULESET),
             'exec_start': '/bin/sh /etc/rc',
             'exec_stop': '/bin/sh /etc/rc.shutdown',
             'exec_prestart': '/usr/bin/true',
@@ -1201,6 +1205,7 @@ def retrieve_default_props():
             'nat_forwards': 'none',
             'plugin_name': 'none',
             'plugin_repository': 'none',
+            'min_dyn_devfs_ruleset': '1000',
         }
 
     def check_default_config(self):
@@ -2110,6 +2115,7 @@ def json_check_prop(self, key, value, conf, default=False):
             'nat_forwards': ('string', ),
             'plugin_name': ('string', ),
             'plugin_repository': ('string', ),
+            'min_dyn_devfs_ruleset': ('string', ),
         }
 
         zfs_props = {
@@ -2390,6 +2396,22 @@ def json_check_prop(self, key, value, conf, default=False):
                                 silent=self.silent,
                                 exception=ioc_exceptions.ValidationFailed
                             )
+                elif key in ('devfs_ruleset', 'min_dyn_devfs_ruleset'):
+                    try:
+                        intval = int(value)
+                        if intval <= 0:
+                            raise ValueError()
+                        conf[key] = str(intval)
+                    except ValueError:
+                        iocage_lib.ioc_common.logit(
+                            {
+                                'level': 'EXCEPTION',
+                                'message': f'Invalid {key} value: {value}'
+                            },
+                            _callback=self.callback,
+                            silent=self.silent,
+                            exception=ioc_exceptions.ValidationFailed
+                        )
 
                 return value, conf
             else:

iocage_lib/ioc_start.py

@@ -145,7 +145,6 @@ def __start_jail__(self):
         allow_quotas = self.conf["allow_quotas"]
         allow_socket_af = self.conf["allow_socket_af"]
         allow_vmm = self.conf["allow_vmm"]
-        devfs_ruleset = iocage_lib.ioc_common.generate_devfs_ruleset(self.conf)
         exec_prestart = self.conf["exec_prestart"]
         exec_poststart = self.conf["exec_poststart"]
         exec_clean = self.conf["exec_clean"]
@@ -489,16 +488,8 @@ def __start_jail__(self):
             _callback=self.callback,
             silent=self.silent)
 
-        if wants_dhcp and self.conf['type'] != 'pluginv2' \
-                and self.conf['devfs_ruleset'] != '4':
-            iocage_lib.ioc_common.logit({
-                "level": "WARNING",
-                "message": f"  {self.uuid} is not using the devfs_ruleset"
-                           f" of 4, not generating a ruleset for the jail,"
-                           " DHCP may not work."
-            },
-                _callback=self.callback,
-                silent=self.silent)
+        devfs_paths = None
+        devfs_includes = None
 
         if self.conf['type'] == 'pluginv2' and os.path.isfile(
             os.path.join(self.path, f'{self.conf["plugin_name"]}.json')
@@ -512,17 +503,51 @@ def __start_jail__(self):
                     plugin_name = self.conf['plugin_name']
                     plugin_devfs = devfs_json[
                         "devfs_ruleset"][f"plugin_{plugin_name}"]
-                    plugin_devfs_paths = plugin_devfs['paths']
-
-                    plugin_devfs_includes = None if 'includes' not in \
+                    devfs_paths = plugin_devfs['paths']
+                    devfs_includes = None if 'includes' not in \
                         plugin_devfs else plugin_devfs['includes']
 
-                    devfs_ruleset = \
-                        iocage_lib.ioc_common.generate_devfs_ruleset(
-                            self.conf,
-                            paths=plugin_devfs_paths,
-                            includes=plugin_devfs_includes
-                        )
+        # Generate dynamic devfs ruleset from configured one
+        (manual_devfs_config, configured_devfs_ruleset, devfs_ruleset) \
+            = iocage_lib.ioc_common.generate_devfs_ruleset(
+                self.conf, devfs_paths, devfs_includes)
+
+        if int(devfs_ruleset) <= 0:
+            iocage_lib.ioc_common.logit({
+                "level": "ERROR",
+                "message": f"{self.uuid} devfs_ruleset"
+                           f" {configured_devfs_ruleset} does not exist!"
+                           " - Not starting jail"
+            },
+                _callback=self.callback,
+                silent=self.silent)
+            return
+
+        # Manually configured devfs_ruleset doesn't support all iocage features
+        if manual_devfs_config:
+            if devfs_paths is not None or devfs_includes is not None:
+                iocage_lib.ioc_common.logit({
+                    "level": "WARNING",
+                    "message": f"  {self.uuid} is not using the devfs_ruleset"
+                               " of "
+                               f"{iocage_lib.ioc_common.IOCAGE_DEVFS_RULESET}"
+                               ", devices and includes from plugin not added"
+                               ", some features of the plugin may not work."
+                },
+                    _callback=self.callback,
+                    silent=self.silent)
+
+            if wants_dhcp and self.conf['type'] != 'pluginv2':
+                iocage_lib.ioc_common.logit({
+                    "level": "WARNING",
+                    "message": f"  {self.uuid} is not using the devfs_ruleset"
+                               " of "
+                               f"{iocage_lib.ioc_common.IOCAGE_DEVFS_RULESET}"
+                               ", not generating a ruleset for the jail,"
+                               " DHCP may not work."
+                },
+                    _callback=self.callback,
+                    silent=self.silent)
 
         parameters = [
             fdescfs, _allow_mlock, tmpfs,
@@ -624,6 +649,9 @@ def __start_jail__(self):
         iocage_lib.ioc_common.logit({
             'level': 'INFO',
             'message': f'  + Using devfs_ruleset: {devfs_ruleset}'
+                       + (' (cloned from devfs_ruleset '
+                          f'{configured_devfs_ruleset})' if manual_devfs_config
+                          else ' (iocage generated default)')
         },
             _callback=self.callback,
             silent=self.silent)