From acd65a16f46384639126076acdb8aa8e616ab86c Mon Sep 17 00:00:00 2001 From: Michael Hanselmann <hansmi@google.com> Date: Fri, 11 Jun 2010 14:07:23 +0200 Subject: [PATCH] import/export: Validate remote host/port The hostname and port received from the remote cluster should be validated, just in case. Signed-off-by: Michael Hanselmann <hansmi@google.com> Reviewed-by: Guido Trotter <ultrotter@google.com> --- daemons/import-export | 11 +++++++++++ lib/cmdlib.py | 12 ++++++++++-- lib/masterd/instance.py | 7 ++++--- test/ganeti.masterd.instance_unittest.py | 14 ++++++++++++++ test/import-export_unittest.bash | 17 +++++++++++++++-- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/daemons/import-export b/daemons/import-export index f7ebd4dee..f41fa70c0 100755 --- a/daemons/import-export +++ b/daemons/import-export @@ -40,6 +40,7 @@ import math from ganeti import constants from ganeti import cli from ganeti import utils +from ganeti import errors from ganeti import serializer from ganeti import objects from ganeti import locking @@ -401,6 +402,16 @@ def ParseOptions(): # Won't return parser.error("Invalid mode: %s" % mode) + # Normalize and check parameters + if options.host is not None: + try: + options.host = utils.HostInfo.NormalizeName(options.host) + except errors.OpPrereqError, err: + parser.error("Invalid hostname '%s': %s" % (options.host, err)) + + if options.port is not None: + options.port = utils.ValidateServiceName(options.port) + if (options.exp_size is not None and options.exp_size != constants.IE_CUSTOM_SIZE): try: diff --git a/lib/cmdlib.py b/lib/cmdlib.py index ab1adb220..8dbb1de26 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -9100,6 +9100,7 @@ class LUExportInstance(LogicalUnit): _CheckNodeNotDrained(self, self.dst_node.name) self._cds = None + self.dest_disk_info = None self.dest_x509_ca = None elif self.export_mode == constants.EXPORT_MODE_REMOTE: @@ -9139,13 +9140,20 @@ class LUExportInstance(LogicalUnit): self.dest_x509_ca = cert # Verify target information + disk_info = [] for idx, disk_data in enumerate(self.op.target_node): try: - masterd.instance.CheckRemoteExportDiskInfo(cds, idx, disk_data) + (host, port) = masterd.instance.CheckRemoteExportDiskInfo(cds, idx, + disk_data) except errors.GenericError, err: raise errors.OpPrereqError("Target info for disk %s: %s" % (idx, err), errors.ECODE_INVAL) + disk_info.append((host, port)) + + assert len(disk_info) == len(self.op.target_node) + self.dest_disk_info = disk_info + else: raise errors.ProgrammerError("Unhandled export mode %r" % self.export_mode) @@ -9236,7 +9244,7 @@ class LUExportInstance(LogicalUnit): opts = objects.ImportExportOptions(key_name=key_name, ca_pem=dest_ca_pem) - (fin_resu, dresults) = helper.RemoteExport(opts, self.op.target_node, + (fin_resu, dresults) = helper.RemoteExport(opts, self.dest_disk_info, timeouts) finally: helper.Cleanup() diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py index 04806dad6..6eb01e16d 100644 --- a/lib/masterd/instance.py +++ b/lib/masterd/instance.py @@ -1239,8 +1239,8 @@ class ExportInstanceHelper: ieloop = ImportExportLoop(self._lu) try: - for idx, (dev, (host, port, _, _)) in enumerate(zip(instance.disks, - disk_info)): + for idx, (dev, (host, port)) in enumerate(zip(instance.disks, + disk_info)): self._feedback_fn("Sending disk %s to %s:%s" % (idx, host, port)) finished_fn = compat.partial(self._TransferFinished, idx) ieloop.Add(DiskExport(self._lu, instance.primary_node, @@ -1514,7 +1514,8 @@ def CheckRemoteExportDiskInfo(cds, disk_index, disk_info): if not utils.VerifySha1Hmac(cds, msg, hmac_digest, salt=hmac_salt): raise errors.GenericError("HMAC is wrong") - return (host, port) + return (utils.HostInfo.NormalizeName(host), + utils.ValidateServiceName(port)) def ComputeRemoteImportDiskInfo(cds, salt, disk_index, host, port): diff --git a/test/ganeti.masterd.instance_unittest.py b/test/ganeti.masterd.instance_unittest.py index 066da7d76..2daa1a4fc 100755 --- a/test/ganeti.masterd.instance_unittest.py +++ b/test/ganeti.masterd.instance_unittest.py @@ -111,6 +111,20 @@ class TestRieDiskInfo(unittest.TestCase): self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo, cds, i, di) + def testInvalidHostPort(self): + cds = "3ZoJY8KtGJ" + salt = "drK5oYiHWD" + + for host in [",", "...", "Hello World", "`", "!", "#", "\\"]: + di = ComputeRemoteImportDiskInfo(cds, salt, 0, host, 1234) + self.assertRaises(errors.OpPrereqError, + CheckRemoteExportDiskInfo, cds, 0, di) + + for port in [-1, 792825908, "HelloWorld!", "`#", "\\\"", "_?_"]: + di = ComputeRemoteImportDiskInfo(cds, salt, 0, "localhost", port) + self.assertRaises(errors.OpPrereqError, + CheckRemoteExportDiskInfo, cds, 0, di) + def testCheckErrors(self): cds = "0776450535a" self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo, diff --git a/test/import-export_unittest.bash b/test/import-export_unittest.bash index 96a055f3d..bbb5e026f 100755 --- a/test/import-export_unittest.bash +++ b/test/import-export_unittest.bash @@ -108,8 +108,21 @@ $impexpd $src_statusfile >/dev/null 2>&1 && $impexpd $src_statusfile invalidmode >/dev/null 2>&1 && err "daemon-util succeeded with invalid mode" -$impexpd $src_statusfile import --compression=rot13 >/dev/null 2>&1 && - err "daemon-util succeeded with invalid compression" +for mode in import export; do + $impexpd $src_statusfile $mode --compression=rot13 >/dev/null 2>&1 && + err "daemon-util succeeded with invalid compression" + + for host in '' ' ' ' s p a c e' ... , foo.example.net... \ + 'some"evil"name' 'x\ny\tmoo'; do + $impexpd $src_statusfile $mode --host="$host" >/dev/null 2>&1 && + err "daemon-util succeeded with invalid host '$host'" + done + + for port in '' ' ' -1234 'some ` port " here'; do + $impexpd $src_statusfile $mode --port="$port" >/dev/null 2>&1 && + err "daemon-util succeeded with invalid port '$port'" + done +done upto 'Generate test data' cat $(get_testfile proc_drbd8.txt) $(get_testfile cert1.pem) > $testdata -- GitLab