From 753b2f35731423597d7f51f33427b6d619fcd0b5 Mon Sep 17 00:00:00 2001 From: grembo Date: Wed, 22 Jan 2020 07:00:51 +0100 Subject: [PATCH] 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 https://github.com/iocage/iocage/issues/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 | 35 ++++++++++++--------- iocage_lib/ioc_json.py | 26 +++++++++++++-- iocage_lib/ioc_start.py | 68 ++++++++++++++++++++++++++++------------ 3 files changed, 93 insertions(+), 36 deletions(-) diff --git a/iocage_lib/ioc_common.py b/iocage_lib/ioc_common.py index 210b032fe..421d0d367 100644 --- a/iocage_lib/ioc_common.py +++ b/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,7 +745,7 @@ 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'], @@ -750,22 +753,26 @@ def generate_devfs_ruleset(conf, paths=None, includes=None, callback=None, ) 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): diff --git a/iocage_lib/ioc_json.py b/iocage_lib/ioc_json.py index 820eaff4c..28249543f 100644 --- a/iocage_lib/ioc_json.py +++ b/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: diff --git a/iocage_lib/ioc_start.py b/iocage_lib/ioc_start.py index e57f56a88..f8fc250fb 100644 --- a/iocage_lib/ioc_start.py +++ b/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)