From d51ae04cc5b370a6c7a304d7e8b872cfc6af1491 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Fri, 11 Jun 2010 19:04:51 +0200
Subject: [PATCH] Use import/export magic for backup/import and inter-cluster
 moves

This should prevent bugs in our code from accidentally overwriting
disks.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
 lib/cmdlib.py                            |  12 ++-
 lib/masterd/instance.py                  | 108 +++++++++++++++--------
 test/ganeti.masterd.instance_unittest.py |  16 ++--
 3 files changed, 88 insertions(+), 48 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 8dbb1de26..b9983e4c6 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -9143,13 +9143,13 @@ class LUExportInstance(LogicalUnit):
       disk_info = []
       for idx, disk_data in enumerate(self.op.target_node):
         try:
-          (host, port) = masterd.instance.CheckRemoteExportDiskInfo(cds, idx,
-                                                                    disk_data)
+          (host, port, magic) = \
+            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))
+        disk_info.append((host, port, magic))
 
       assert len(disk_info) == len(self.op.target_node)
       self.dest_disk_info = disk_info
@@ -9241,10 +9241,8 @@ class LUExportInstance(LogicalUnit):
             OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM,
                                             self.dest_x509_ca)
 
-          opts = objects.ImportExportOptions(key_name=key_name,
-                                             ca_pem=dest_ca_pem)
-
-          (fin_resu, dresults) = helper.RemoteExport(opts, self.dest_disk_info,
+          (fin_resu, dresults) = helper.RemoteExport(self.dest_disk_info,
+                                                     key_name, dest_ca_pem,
                                                      timeouts)
       finally:
         helper.Cleanup()
diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py
index df0b18754..490ffce0c 100644
--- a/lib/masterd/instance.py
+++ b/lib/masterd/instance.py
@@ -815,7 +815,7 @@ class ImportExportLoop:
 
 class _TransferInstCbBase(ImportExportCbBase):
   def __init__(self, lu, feedback_fn, instance, timeouts, src_node, src_cbs,
-               dest_node, dest_ip, export_opts):
+               dest_node, dest_ip):
     """Initializes this class.
 
     """
@@ -829,7 +829,6 @@ class _TransferInstCbBase(ImportExportCbBase):
     self.src_cbs = src_cbs
     self.dest_node = dest_node
     self.dest_ip = dest_ip
-    self.export_opts = export_opts
 
 
 class _TransferInstSourceCb(_TransferInstCbBase):
@@ -888,11 +887,12 @@ class _TransferInstDestCb(_TransferInstCbBase):
     assert self.src_cbs
     assert dtp.src_export is None
     assert dtp.dest_import
+    assert dtp.export_opts
 
     self.feedback_fn("%s is now listening, starting export" % dtp.data.name)
 
     # Start export on source node
-    de = DiskExport(self.lu, self.src_node, self.export_opts,
+    de = DiskExport(self.lu, self.src_node, dtp.export_opts,
                     self.dest_ip, ie.listen_port,
                     self.instance, dtp.data.src_io, dtp.data.src_ioargs,
                     self.timeouts, self.src_cbs, private=dtp)
@@ -952,7 +952,7 @@ class DiskTransfer(object):
 
 
 class _DiskTransferPrivate(object):
-  def __init__(self, data, success):
+  def __init__(self, data, success, export_opts):
     """Initializes this class.
 
     @type data: L{DiskTransfer}
@@ -960,12 +960,12 @@ class _DiskTransferPrivate(object):
 
     """
     self.data = data
+    self.success = success
+    self.export_opts = export_opts
 
     self.src_export = None
     self.dest_import = None
 
-    self.success = success
-
   def RecordResult(self, success):
     """Updates the status.
 
@@ -975,6 +975,25 @@ class _DiskTransferPrivate(object):
     self.success = self.success and success
 
 
+def _GetInstDiskMagic(base, instance_name, index):
+  """Computes the magic value for a disk export or import.
+
+  @type base: string
+  @param base: Random seed value (can be the same for all disks of a transfer)
+  @type instance_name: string
+  @param instance_name: Name of instance
+  @type index: number
+  @param index: Disk index
+
+  """
+  h = compat.sha1_hash()
+  h.update(str(constants.RIE_VERSION))
+  h.update(base)
+  h.update(instance_name)
+  h.update(str(index))
+  return h.hexdigest()
+
+
 def TransferInstanceData(lu, feedback_fn, src_node, dest_node, dest_ip,
                          instance, all_transfers):
   """Transfers an instance's data from one node to another.
@@ -1002,25 +1021,28 @@ def TransferInstanceData(lu, feedback_fn, src_node, dest_node, dest_ip,
   logging.debug("Source node %s, destination node %s, compression '%s'",
                 src_node, dest_node, compress)
 
-  opts = objects.ImportExportOptions(key_name=None, ca_pem=None,
-                                     compress=compress)
-
   timeouts = ImportExportTimeouts(constants.DISK_TRANSFER_CONNECT_TIMEOUT)
   src_cbs = _TransferInstSourceCb(lu, feedback_fn, instance, timeouts,
-                                  src_node, None, dest_node, dest_ip, opts)
+                                  src_node, None, dest_node, dest_ip)
   dest_cbs = _TransferInstDestCb(lu, feedback_fn, instance, timeouts,
-                                 src_node, src_cbs, dest_node, dest_ip, opts)
+                                 src_node, src_cbs, dest_node, dest_ip)
 
   all_dtp = []
 
+  base_magic = utils.GenerateSecret(6)
+
   ieloop = ImportExportLoop(lu)
   try:
-    for transfer in all_transfers:
+    for idx, transfer in enumerate(all_transfers):
       if transfer:
         feedback_fn("Exporting %s from %s to %s" %
                     (transfer.name, src_node, dest_node))
 
-        dtp = _DiskTransferPrivate(transfer, True)
+        magic = _GetInstDiskMagic(base_magic, instance.name, idx)
+        opts = objects.ImportExportOptions(key_name=None, ca_pem=None,
+                                           compress=compress, magic=magic)
+
+        dtp = _DiskTransferPrivate(transfer, True, opts)
 
         di = DiskImport(lu, dest_node, opts, instance,
                         transfer.dest_io, transfer.dest_ioargs,
@@ -1224,13 +1246,15 @@ class ExportInstanceHelper:
 
     return (fin_resu, dresults)
 
-  def RemoteExport(self, opts, disk_info, timeouts):
+  def RemoteExport(self, disk_info, key_name, dest_ca_pem, timeouts):
     """Inter-cluster instance export.
 
-    @type opts: L{objects.ImportExportOptions}
-    @param opts: Import/export daemon options
     @type disk_info: list
     @param disk_info: Per-disk destination information
+    @type key_name: string
+    @param key_name: Name of X509 key to use
+    @type dest_ca_pem: string
+    @param dest_ca_pem: Destination X509 CA in PEM format
     @type timeouts: L{ImportExportTimeouts}
     @param timeouts: Timeouts for this import
 
@@ -1243,8 +1267,12 @@ class ExportInstanceHelper:
 
     ieloop = ImportExportLoop(self._lu)
     try:
-      for idx, (dev, (host, port)) in enumerate(zip(instance.disks,
-                                                    disk_info)):
+      for idx, (dev, (host, port, magic)) in enumerate(zip(instance.disks,
+                                                           disk_info)):
+        opts = objects.ImportExportOptions(key_name=key_name,
+                                           ca_pem=dest_ca_pem,
+                                           magic=magic)
+
         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,
@@ -1323,9 +1351,9 @@ class _RemoteImportCb(ImportExportCbBase):
     host = self._external_address
 
     disks = []
-    for idx, port in enumerate(self._daemon_port):
+    for idx, (port, magic) in enumerate(self._daemon_port):
       disks.append(ComputeRemoteImportDiskInfo(self._cds, self._salt,
-                                               idx, host, port))
+                                               idx, host, port, magic))
 
     assert len(disks) == self._disk_count
 
@@ -1344,7 +1372,7 @@ class _RemoteImportCb(ImportExportCbBase):
 
     assert self._daemon_port[idx] is None
 
-    self._daemon_port[idx] = ie.listen_port
+    self._daemon_port[idx] = (ie.listen_port, ie.magic)
 
     self._CheckAllListening()
 
@@ -1393,6 +1421,8 @@ def RemoteImport(lu, feedback_fn, instance, source_x509_ca, cds, timeouts):
   source_ca_pem = OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM,
                                                   source_x509_ca)
 
+  magic_base = utils.GenerateSecret(6)
+
   # Create crypto key
   result = lu.rpc.call_x509_cert_create(instance.primary_node,
                                         constants.RIE_CERT_VALIDITY)
@@ -1404,10 +1434,6 @@ def RemoteImport(lu, feedback_fn, instance, source_x509_ca, cds, timeouts):
     x509_cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM,
                                                 x509_cert_pem)
 
-    # Import daemon options
-    opts = objects.ImportExportOptions(key_name=x509_key_name,
-                                       ca_pem=source_ca_pem)
-
     # Sign certificate
     signed_x509_cert_pem = \
       utils.SignX509Certificate(x509_cert, cds, utils.GenerateSecret(8))
@@ -1418,6 +1444,13 @@ def RemoteImport(lu, feedback_fn, instance, source_x509_ca, cds, timeouts):
     ieloop = ImportExportLoop(lu)
     try:
       for idx, dev in enumerate(instance.disks):
+        magic = _GetInstDiskMagic(magic_base, instance.name, idx)
+
+        # Import daemon options
+        opts = objects.ImportExportOptions(key_name=x509_key_name,
+                                           ca_pem=source_ca_pem,
+                                           magic=magic)
+
         ieloop.Add(DiskImport(lu, instance.primary_node, opts, instance,
                               constants.IEIO_SCRIPT, (dev, idx),
                               timeouts, cbs, private=(idx, )))
@@ -1480,7 +1513,7 @@ def CheckRemoteExportHandshake(cds, handshake):
   return None
 
 
-def _GetRieDiskInfoMessage(disk_index, host, port):
+def _GetRieDiskInfoMessage(disk_index, host, port, magic):
   """Returns the hashed text for import/export disk information.
 
   @type disk_index: number
@@ -1489,9 +1522,11 @@ def _GetRieDiskInfoMessage(disk_index, host, port):
   @param host: Hostname
   @type port: number
   @param port: Daemon port
+  @type magic: string
+  @param magic: Magic value
 
   """
-  return "%s:%s:%s" % (disk_index, host, port)
+  return "%s:%s:%s:%s" % (disk_index, host, port, magic)
 
 
 def CheckRemoteExportDiskInfo(cds, disk_index, disk_info):
@@ -1506,23 +1541,24 @@ def CheckRemoteExportDiskInfo(cds, disk_index, disk_info):
 
   """
   try:
-    (host, port, hmac_digest, hmac_salt) = disk_info
+    (host, port, magic, hmac_digest, hmac_salt) = disk_info
   except (TypeError, ValueError), err:
     raise errors.GenericError("Invalid data: %s" % err)
 
-  if not (host and port):
-    raise errors.GenericError("Missing destination host or port")
+  if not (host and port and magic):
+    raise errors.GenericError("Missing destination host, port or magic")
 
-  msg = _GetRieDiskInfoMessage(disk_index, host, port)
+  msg = _GetRieDiskInfoMessage(disk_index, host, port, magic)
 
   if not utils.VerifySha1Hmac(cds, msg, hmac_digest, salt=hmac_salt):
     raise errors.GenericError("HMAC is wrong")
 
   return (utils.HostInfo.NormalizeName(host),
-          utils.ValidateServiceName(port))
+          utils.ValidateServiceName(port),
+          magic)
 
 
-def ComputeRemoteImportDiskInfo(cds, salt, disk_index, host, port):
+def ComputeRemoteImportDiskInfo(cds, salt, disk_index, host, port, magic):
   """Computes the signed disk information for a remote import.
 
   @type cds: string
@@ -1535,8 +1571,10 @@ def ComputeRemoteImportDiskInfo(cds, salt, disk_index, host, port):
   @param host: Hostname
   @type port: number
   @param port: Daemon port
+  @type magic: string
+  @param magic: Magic value
 
   """
-  msg = _GetRieDiskInfoMessage(disk_index, host, port)
+  msg = _GetRieDiskInfoMessage(disk_index, host, port, magic)
   hmac_digest = utils.Sha1Hmac(cds, msg, salt=salt)
-  return (host, port, hmac_digest, salt)
+  return (host, port, magic, hmac_digest, salt)
diff --git a/test/ganeti.masterd.instance_unittest.py b/test/ganeti.masterd.instance_unittest.py
index 2daa1a4fc..28a799ee3 100755
--- a/test/ganeti.masterd.instance_unittest.py
+++ b/test/ganeti.masterd.instance_unittest.py
@@ -102,9 +102,9 @@ class TestRieDiskInfo(unittest.TestCase):
   def test(self):
     cds = "bbf46ea9a"
     salt = "ee5ad9"
-    di = ComputeRemoteImportDiskInfo(cds, salt, 0, "node1", 1234)
+    di = ComputeRemoteImportDiskInfo(cds, salt, 0, "node1", 1234, "mag111")
     self.assertEqual(CheckRemoteExportDiskInfo(cds, 0, di),
-                     ("node1", 1234))
+                     ("node1", 1234, "mag111"))
 
     for i in range(1, 100):
       # Wrong disk index
@@ -116,12 +116,12 @@ class TestRieDiskInfo(unittest.TestCase):
     salt = "drK5oYiHWD"
 
     for host in [",", "...", "Hello World", "`", "!", "#", "\\"]:
-      di = ComputeRemoteImportDiskInfo(cds, salt, 0, host, 1234)
+      di = ComputeRemoteImportDiskInfo(cds, salt, 0, host, 1234, "magic")
       self.assertRaises(errors.OpPrereqError,
                         CheckRemoteExportDiskInfo, cds, 0, di)
 
     for port in [-1, 792825908, "HelloWorld!", "`#", "\\\"", "_?_"]:
-      di = ComputeRemoteImportDiskInfo(cds, salt, 0, "localhost", port)
+      di = ComputeRemoteImportDiskInfo(cds, salt, 0, "localhost", port, "magic")
       self.assertRaises(errors.OpPrereqError,
                         CheckRemoteExportDiskInfo, cds, 0, di)
 
@@ -136,11 +136,15 @@ class TestRieDiskInfo(unittest.TestCase):
 
     # No host/port
     self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo,
-                      cds, 0, ("", 0, "", ""))
+                      cds, 0, ("", 1234, "magic", "", ""))
+    self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo,
+                      cds, 0, ("host", 0, "magic", "", ""))
+    self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo,
+                      cds, 0, ("host", 1234, "", "", ""))
 
     # Wrong hash
     self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo,
-                      cds, 0, ("nodeX", 123, "fakehash", "xyz"))
+                      cds, 0, ("nodeX", 123, "magic", "fakehash", "xyz"))
 
 
 class TestFormatProgress(unittest.TestCase):
-- 
GitLab