diff --git a/NEWS b/NEWS index 6516412b8b7c2758de9b9f66df9a6ac2d00d802d..ec983769bee4482cc66cdf7c74a10ef5d3395cf1 100644 --- a/NEWS +++ b/NEWS @@ -101,6 +101,15 @@ Details - Improved burnin +Version 2.0.5 +------------- + +- Fix security issue due to missing validation of iallocator names; this + allows local and remote execution of arbitrary executables +- Fix failure of gnt-node list during instance removal +- Ship the RAPI documentation in the archive + + Version 2.0.4 ------------- diff --git a/configure.ac b/configure.ac index 97de447b6d014b40c7b42f81f46a78e79c101295..2a2ac0e0d689ed06cfe16fb7b945f736a39b9b05 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ m4_define([gnt_version_major], [2]) m4_define([gnt_version_minor], [1]) m4_define([gnt_version_revision], [0]) -m4_define([gnt_version_suffix], [~rc1]) +m4_define([gnt_version_suffix], [~rc2]) m4_define([gnt_version_full], m4_format([%d.%d.%d%s], gnt_version_major, gnt_version_minor, diff --git a/lib/backend.py b/lib/backend.py index cce99aba726755574160655942abb0865f53409f..bd9f5fa54b6639c6f77916a5af2a99e27f6dd450 100644 --- a/lib/backend.py +++ b/lib/backend.py @@ -1735,10 +1735,11 @@ def _TryOSFromDisk(name, base_dir=None): """ if base_dir is None: os_dir = utils.FindFile(name, constants.OS_SEARCH_PATH, os.path.isdir) - if os_dir is None: - return False, "Directory for OS %s not found in search path" % name else: - os_dir = os.path.sep.join([base_dir, name]) + os_dir = utils.FindFile(name, [base_dir], os.path.isdir) + + if os_dir is None: + return False, "Directory for OS %s not found in search path" % name status, api_versions = _OSOndiskAPIVersion(name, os_dir) if not status: @@ -2616,8 +2617,6 @@ class HooksRunner(object): on the master side. """ - RE_MASK = re.compile("^[a-zA-Z0-9_-]+$") - def __init__(self, hooks_base_dir=None): """Constructor for hooks runner. @@ -2725,7 +2724,7 @@ class HooksRunner(object): for relname in dir_contents: fname = os.path.join(dir_name, relname) if not (os.path.isfile(fname) and os.access(fname, os.X_OK) and - self.RE_MASK.match(relname) is not None): + constants.EXT_PLUGIN_MASK.match(relname) is not None): rrval = constants.HKR_SKIP output = "" else: diff --git a/lib/cli.py b/lib/cli.py index 752546b172129664c5e863d6b98ad82254c5c7f3..17d173e466993fc446b3e97c8f352b1add393c31 100644 --- a/lib/cli.py +++ b/lib/cli.py @@ -82,6 +82,7 @@ __all__ = [ "NODE_PLACEMENT_OPT", "NOHDR_OPT", "NOIPCHECK_OPT", + "NONAMECHECK_OPT", "NOLVM_STORAGE_OPT", "NOMODIFY_ETCHOSTS_OPT", "NOMODIFY_SSH_SETUP_OPT", @@ -597,6 +598,11 @@ NOIPCHECK_OPT = cli_option("--no-ip-check", dest="ip_check", default=True, help="Don't check that the instance's IP" " is alive") +NONAMECHECK_OPT = cli_option("--no-name-check", dest="name_check", + default=True, action="store_false", + help="Don't check that the instance's name" + " is resolvable") + NET_OPT = cli_option("--net", help="NIC parameters", default=[], dest="nics", action="append", type="identkeyval") @@ -1467,6 +1473,7 @@ def GenericInstanceCreate(mode, opts, args): nics=nics, pnode=pnode, snode=snode, ip_check=opts.ip_check, + name_check=opts.name_check, wait_for_sync=opts.wait_for_sync, file_storage_dir=opts.file_storage_dir, file_driver=opts.file_driver, diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 7260a463a403907208479f5820f1d862be735fdc..0770523ce8a1411fa726f2e0bc10b8bd3be6cb29 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -2579,10 +2579,9 @@ class LUQueryNodes(NoHooksLU): inst_fields = frozenset(("pinst_cnt", "pinst_list", "sinst_cnt", "sinst_list")) if inst_fields & frozenset(self.op.output_fields): - instancelist = self.cfg.GetInstanceList() + inst_data = self.cfg.GetAllInstancesInfo() - for instance_name in instancelist: - inst = self.cfg.GetInstanceInfo(instance_name) + for instance_name, inst in inst_data.items(): if inst.primary_node in node_to_primary: node_to_primary[inst.primary_node].add(inst.name) for secnode in inst.secondary_nodes: @@ -5619,6 +5618,19 @@ class LUCreateInstance(LogicalUnit): "hvparams", "beparams"] REQ_BGL = False + def CheckArguments(self): + """Check arguments. + + """ + # do not require name_check to ease forward/backward compatibility + # for tools + if not hasattr(self.op, "name_check"): + self.op.name_check = True + if self.op.ip_check and not self.op.name_check: + # TODO: make the ip check more flexible and not depend on the name check + raise errors.OpPrereqError("Cannot do ip checks without a name check", + errors.ECODE_INVAL) + def _ExpandNode(self, node): """Expands and checks one node name. @@ -5683,8 +5695,14 @@ class LUCreateInstance(LogicalUnit): #### instance parameters check # instance name verification - hostname1 = utils.GetHostInfo(self.op.instance_name) - self.op.instance_name = instance_name = hostname1.name + if self.op.name_check: + hostname1 = utils.GetHostInfo(self.op.instance_name) + self.op.instance_name = instance_name = hostname1.name + # used in CheckPrereq for ip ping check + self.check_ip = hostname1.ip + else: + instance_name = self.op.instance_name + self.check_ip = None # this is just a preventive check, but someone might still add this # instance in the meantime, and creation will fail at lock-add time @@ -5713,6 +5731,10 @@ class LUCreateInstance(LogicalUnit): if ip is None or ip.lower() == constants.VALUE_NONE: nic_ip = None elif ip.lower() == constants.VALUE_AUTO: + if not self.op.name_check: + raise errors.OpPrereqError("IP address set to auto but name checks" + " have been skipped. Aborting.", + errors.ECODE_INVAL) nic_ip = hostname1.ip else: if not utils.IsValidIP(ip): @@ -5780,9 +5802,6 @@ class LUCreateInstance(LogicalUnit): errors.ECODE_INVAL) self.disks.append({"size": size, "mode": mode}) - # used in CheckPrereq for ip ping check - self.check_ip = hostname1.ip - # file storage checks if (self.op.file_driver and not self.op.file_driver in constants.FILE_DRIVER): @@ -5993,12 +6012,8 @@ class LUCreateInstance(LogicalUnit): nic.mac = export_info.get(constants.INISECT_INS, nic_mac_ini) # ENDIF: self.op.mode == constants.INSTANCE_IMPORT - # ip ping checks (we use the same ip that was resolved in ExpandNames) - if self.op.start and not self.op.ip_check: - raise errors.OpPrereqError("Cannot ignore IP address conflicts when" - " adding an instance in start mode", - errors.ECODE_INVAL) + # ip ping checks (we use the same ip that was resolved in ExpandNames) if self.op.ip_check: if utils.TcpPing(self.check_ip, constants.DEFAULT_NODED_PORT): raise errors.OpPrereqError("IP %s of instance %s already in use" % diff --git a/lib/constants.py b/lib/constants.py index c353878ed83ce66d21c237da5e709dedd7b6f26b..81302575487a44ed192e61aa7b21888a215ef215 100644 --- a/lib/constants.py +++ b/lib/constants.py @@ -21,6 +21,8 @@ """Module holding different constants.""" +import re + from ganeti import _autoconf # various versions @@ -168,6 +170,9 @@ VALUE_NONE = "none" VALUE_TRUE = "true" VALUE_FALSE = "false" +# External script validation mask +EXT_PLUGIN_MASK = re.compile("^[a-zA-Z0-9_-]+$") + # hooks-related constants HOOKS_BASE_DIR = CONF_DIR + "/hooks" HOOKS_PHASE_PRE = "pre" diff --git a/lib/opcodes.py b/lib/opcodes.py index 0244144810554d5c021b5d1274b6e134f1288847..80c802df001f23bf680ef333d7bd525c2f0d81c5 100644 --- a/lib/opcodes.py +++ b/lib/opcodes.py @@ -447,7 +447,7 @@ class OpCreateInstance(OpCode): "pnode", "disk_template", "snode", "mode", "disks", "nics", "src_node", "src_path", "start", - "wait_for_sync", "ip_check", + "wait_for_sync", "ip_check", "name_check", "file_storage_dir", "file_driver", "iallocator", "hypervisor", "hvparams", "beparams", diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py index 669bae4c4086d23860dcbe5812d54d3a169d064f..48a52c68da920e6eb0eeccef88ee7b3eaa6ebf2f 100644 --- a/lib/rapi/rlib2.py +++ b/lib/rapi/rlib2.py @@ -464,6 +464,7 @@ class R_2_instances(baserlib.R_Generic): nics=nics, start=fn('start', True), ip_check=fn('ip_check', True), + name_check=fn('name_check', True), wait_for_sync=True, hypervisor=fn('hypervisor', None), hvparams=hvparams, diff --git a/lib/ssh.py b/lib/ssh.py index 87c25c929af9420d7f85a0fa735f62e9e0c83f4e..31fbdbbf69fdf175da4fbd95cb0a1dbf6baa5ad1 100644 --- a/lib/ssh.py +++ b/lib/ssh.py @@ -212,7 +212,7 @@ class SshRunner: - detail: string with details """ - retval = self.Run(node, 'root', 'hostname') + retval = self.Run(node, 'root', 'hostname --fqdn') if retval.failed: msg = "ssh problem" diff --git a/lib/utils.py b/lib/utils.py index f08e75c65a60319bc7f80219d57de8f6c42275ba..105364e96ba1452779c4710fbbb3f434b6fb4779 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -1653,9 +1653,17 @@ def FindFile(name, search_path, test=os.path.exists): @return: full path to the object if found, None otherwise """ + # validate the filename mask + if constants.EXT_PLUGIN_MASK.match(name) is None: + logging.critical("Invalid value passed for external script name: '%s'", + name) + return None + for dir_name in search_path: item_name = os.path.sep.join([dir_name, name]) - if test(item_name): + # check the user test and that we're indeed resolving to the given + # basename + if test(item_name) and os.path.basename(item_name) == name: return item_name return None diff --git a/man/gnt-instance.sgml b/man/gnt-instance.sgml index ea92bf1b73714396f9f0085e43dba90978035021..d56a242d05fcd6fa391e280cb0d80d48b64af734 100644 --- a/man/gnt-instance.sgml +++ b/man/gnt-instance.sgml @@ -78,6 +78,10 @@ <arg>-s <replaceable>SIZE</replaceable></arg> </group> <sbr> + <arg>--no-ip-check</arg> + <arg>--no-name-check</arg> + <arg>--no-start</arg> + <sbr> <group> <arg rep="repeat">--net=<replaceable>N</replaceable><arg rep="repeat">:options</arg></arg> <arg>--no-nics</arg> @@ -146,6 +150,28 @@ 2:size=100G</userinput>. </para> + <para> + The <option>--no-ip-check</option> skips the checks that are + done to see if the instance's IP is not already alive + (i.e. reachable from the master node). + </para> + + <para> + The <option>--no-name-check</option> skips the check for the + instance name via the resolver (e.g. in DNS or /etc/hosts, + depending on your setup). Since the name check is used to + compute the IP address, if you pass this option you must + also pass the <option>--no-ip-check</option> option. + </para> + + <para> + If you don't wat the instance to automatically start after + creation, this is possible via the + <option>--no-start</option> option. This will leave the + instance down until a subsequent <command>gnt-instance + start</command> command. + </para> + <para> The NICs of the instances can be specified via the <option>--net</option> option. By default, one NIC is @@ -769,6 +795,14 @@ command for details.</simpara> </listitem> </varlistentry> + <varlistentry> + <term>name_check</term> + <listitem> + <simpara>Skip the name check for instances; + see the description in the <command>add</command> + command for details.</simpara> + </listitem> + </varlistentry> <varlistentry> <term>file_storage_dir, file_driver</term> <listitem> diff --git a/scripts/gnt-backup b/scripts/gnt-backup index a07335651a668ebb3cc019df11a41626580f35fc..986b449c2388297c46b7e058660eec63d6bc24bb 100755 --- a/scripts/gnt-backup +++ b/scripts/gnt-backup @@ -137,6 +137,7 @@ import_opts = [ SRC_DIR_OPT, SRC_NODE_OPT, NOIPCHECK_OPT, + NONAMECHECK_OPT, IALLOCATOR_OPT, FILESTORE_DIR_OPT, FILESTORE_DRIVER_OPT, diff --git a/scripts/gnt-instance b/scripts/gnt-instance index 44b84101b973c5bcb16825d1cea86062c5de0b4d..d32b254078a65741f8183b41dd7c6d5f42d18d47 100755 --- a/scripts/gnt-instance +++ b/scripts/gnt-instance @@ -318,7 +318,6 @@ def AddInstance(opts, args): """ return GenericInstanceCreate(constants.INSTANCE_CREATE, opts, args) - return 0 def BatchCreate(opts, args): @@ -357,6 +356,7 @@ def BatchCreate(opts, args): "nics": None, "start": True, "ip_check": True, + "name_check": True, "hypervisor": None, "hvparams": {}, "file_storage_dir": None, @@ -453,6 +453,7 @@ def BatchCreate(opts, args): nics=tmp_nics, start=specs['start'], ip_check=specs['ip_check'], + name_check=specs['name_check'], wait_for_sync=True, iallocator=specs['iallocator'], hypervisor=hypervisor, @@ -1280,6 +1281,7 @@ add_opts = [ NET_OPT, NODE_PLACEMENT_OPT, NOIPCHECK_OPT, + NONAMECHECK_OPT, NONICS_OPT, NOSTART_OPT, NWSYNC_OPT, diff --git a/tools/burnin b/tools/burnin index c442957a0bae80a870397aeb39a96b1b67414611..8b350585c76dd28ac143a1212e0b6dc5221d768c 100755 --- a/tools/burnin +++ b/tools/burnin @@ -115,6 +115,8 @@ OPTIONS = [ completion_suggest=("128M 256M 512M 1G 4G 8G" " 12G 16G").split()), cli.VERBOSE_OPT, + cli.NOIPCHECK_OPT, + cli.NONAMECHECK_OPT, cli.cli_option("--no-replace1", dest="do_replace1", help="Skip disk replacement with the same secondary", action="store_false", default=True), @@ -422,6 +424,9 @@ class Burner(object): if options.nodes and options.iallocator: Err("Give either the nodes option or the iallocator option, not both") + if options.http_check and not options.name_check: + Err("Can't enable HTTP checks without name checks") + self.opts = options self.instances = args self.bep = { @@ -498,7 +503,8 @@ class Burner(object): pnode=pnode, snode=snode, start=True, - ip_check=True, + ip_check=self.opts.ip_check, + name_check=self.opts.name_check, wait_for_sync=True, file_driver="loop", file_storage_dir=None, @@ -648,7 +654,8 @@ class Burner(object): pnode=pnode, snode=snode, start=True, - ip_check=True, + ip_check=self.opts.ip_check, + name_check=self.opts.name_check, wait_for_sync=True, file_storage_dir=None, file_driver="loop",