Merge branch 'stable-2.12' into stable-2.13
authorHrvoje Ribicic <riba@google.com>
Thu, 3 Dec 2015 21:13:39 +0000 (21:13 +0000)
committerHrvoje Ribicic <riba@google.com>
Thu, 3 Dec 2015 21:13:39 +0000 (21:13 +0000)
* stable-2.12
  Restrict showing of DRBD secret using types
  Calculate correct affected nodes set in InstanceChangeGroup

* stable-2.11
  (no changes)

* stable-2.10
  (no changes)

* stable-2.9
  QA: Ensure the DRBD secret is not retrievable via RAPI
  Redact the DRBD secret in instance queries
  Do not attempt to use the DRBD secret in gnt-instance info

Signed-off-by: Hrvoje Ribicic <riba@google.com>
Reviewed-by: Oleg Ponomarev <oponomarev@google.com>

lib/client/gnt_instance.py
lib/cmdlib/instance.py
lib/cmdlib/instance_query.py
lib/objects.py
lib/storage/drbd.py
qa/ganeti-qa.py
qa/qa_rapi.py
src/Ganeti/Config.hs
src/Ganeti/Objects.hs
test/hs/Test/Ganeti/Objects.hs
test/py/ganeti.storage.drbd_unittest.py

index 76ab88e..1beb3f3 100644 (file)
@@ -960,8 +960,7 @@ def _FormatDiskDetails(dev_type, dev, roman):
                 (drbd_info["secondary_node"],
                  compat.TryToRoman(drbd_info["secondary_minor"],
                                    convert=roman))),
-      ("port", str(compat.TryToRoman(drbd_info["port"], roman))),
-      ("auth key", str(drbd_info["secret"])),
+      ("port", str(compat.TryToRoman(drbd_info["port"], convert=roman))),
       ]
   elif dev_type == constants.DT_PLAIN:
     vg_name, lv_name = dev["logical_id"]
index 7ce4dd9..e6c5866 100644 (file)
@@ -4358,7 +4358,7 @@ class LUInstanceChangeGroup(LogicalUnit):
         self._LockInstancesNodes()
 
         # Lock all nodes in all potential target groups
-        lock_groups = (frozenset(self.owned_locks(locking.LEVEL_NODEGROUP)) -
+        lock_groups = (frozenset(self.owned_locks(locking.LEVEL_NODEGROUP)) |
                        self.cfg.GetInstanceNodeGroups(self.op.instance_uuid))
         member_nodes = [node_uuid
                         for group in lock_groups
index 30a5c19..fbf4362 100644 (file)
@@ -167,6 +167,7 @@ class LUInstanceQueryData(NoHooksLU):
 
     """
     drbd_info = None
+    output_logical_id = dev.logical_id
     if dev.dev_type in constants.DTS_DRBD:
       # we change the snode then (otherwise we use the one passed in)
       if dev.logical_id[0] == instance.primary_node:
@@ -183,8 +184,9 @@ class LUInstanceQueryData(NoHooksLU):
         "secondary_node": node_uuid2name_fn(snode_uuid),
         "secondary_minor": snode_minor,
         "port": dev.logical_id[2],
-        "secret": dev.logical_id[5],
       }
+      # replace the secret present at the end of the ids with None
+      output_logical_id = dev.logical_id[:-1] + (None,)
 
     dev_pstatus = self._ComputeBlockdevStatus(instance.primary_node,
                                               instance, dev)
@@ -201,7 +203,7 @@ class LUInstanceQueryData(NoHooksLU):
     return {
       "iv_name": dev.iv_name,
       "dev_type": dev.dev_type,
-      "logical_id": dev.logical_id,
+      "logical_id": output_logical_id,
       "drbd_info": drbd_info,
       "pstatus": dev_pstatus,
       "sstatus": dev_sstatus,
index f212543..0b21523 100644 (file)
@@ -826,10 +826,15 @@ class Disk(ConfigObject):
     standard python types.
 
     """
-    bo = super(Disk, self).ToDict()
+    bo = super(Disk, self).ToDict(_with_private=_with_private)
     if not include_dynamic_params and "dynamic_params" in bo:
       del bo["dynamic_params"]
 
+    if _with_private and "logical_id" in bo:
+      mutable_id = list(bo["logical_id"])
+      mutable_id[5] = mutable_id[5].Get()
+      bo["logical_id"] = tuple(mutable_id)
+
     for attr in ("children",):
       alist = bo.get(attr, None)
       if alist:
@@ -850,6 +855,12 @@ class Disk(ConfigObject):
       # we need a tuple of length six here
       if len(obj.logical_id) < 6:
         obj.logical_id += (None,) * (6 - len(obj.logical_id))
+      # If we do have a tuple of length 6, make the last entry (secret key)
+      # private
+      elif (len(obj.logical_id) == 6 and
+            not isinstance(obj.logical_id[-1], serializer.Private)):
+        obj.logical_id = obj.logical_id[:-1] + \
+                         (serializer.Private(obj.logical_id[-1]),)
     return obj
 
   def __str__(self):
index 558508c..3d0c8c0 100644 (file)
@@ -201,7 +201,9 @@ class DRBD8Dev(base.BlockDev):
     self._rhost = dyn_params[constants.DDP_REMOTE_IP]
     self._rport = unique_id[2]
     self._aminor = dyn_params[constants.DDP_LOCAL_MINOR]
-    self._secret = unique_id[5]
+    # The secret is wrapped in the Private data type, and it has to be extracted
+    # before use
+    self._secret = unique_id[5].Get()
 
     if children:
       if not _CanReadDevice(children[1].dev_path):
index ff78348..3be0614 100755 (executable)
@@ -871,6 +871,8 @@ def RunInstanceTests():
           RunExportImportTests(instance, inodes)
           RunHardwareFailureTests(instance, inodes)
           RunRepairDiskSizes()
+          RunTestIf(["rapi", "instance-data-censorship"],
+                    qa_rapi.TestInstanceDataCensorship, instance, inodes)
           RunTest(qa_instance.TestInstanceRemove, instance)
         finally:
           instance.Release()
index d997c24..a008247 100644 (file)
@@ -63,6 +63,7 @@ import qa_error
 import qa_logging
 import qa_utils
 
+from qa_instance import GetInstanceInfo
 from qa_instance import IsDiskReplacingSupported
 from qa_instance import IsFailoverSupported
 from qa_instance import IsMigrationSupported
@@ -1306,3 +1307,57 @@ def TestFilters():
              "GET", None)])
   for u in uuids:
     _DoTests([("/2/filters/%s" % u, lambda r: r is None, "DELETE", None)])
+
+
+_DRBD_SECRET_RE = re.compile('shared-secret.*"([0-9A-Fa-f]+)"')
+
+
+def _RetrieveSecret(instance, pnode):
+  """Retrieves the DRBD secret given an instance object and the primary node.
+
+  @type instance: L{qa_config._QaInstance}
+  @type pnode: L{qa_config._QaNode}
+
+  @rtype: string
+
+  """
+  instance_info = GetInstanceInfo(instance.name)
+
+  # We are interested in only the first disk on the primary
+  drbd_minor = instance_info["drbd-minors"][pnode.primary][0]
+
+  # This form should work for all DRBD versions
+  drbd_command = ("drbdsetup show %d; drbdsetup %d show || true" %
+                  (drbd_minor, drbd_minor))
+  instance_drbd_info = \
+    qa_utils.GetCommandOutput(pnode.primary, drbd_command)
+
+  match_obj = _DRBD_SECRET_RE.search(instance_drbd_info)
+  if match_obj is None:
+    raise qa_error.Error("Could not retrieve DRBD secret for instance %s from"
+                         " node %s." % (instance.name, pnode.primary))
+
+  return match_obj.groups(0)[0]
+
+
+def TestInstanceDataCensorship(instance, inodes):
+  """Test protection of sensitive instance data."""
+
+  if instance.disk_template != constants.DT_DRBD8:
+    print qa_utils.FormatInfo("Only the DRBD secret is a sensitive parameter"
+                              " right now, skipping for non-DRBD instance.")
+    return
+
+  drbd_secret = _RetrieveSecret(instance, inodes[0])
+
+  job_id = _rapi_client.GetInstanceInfo(instance.name)
+  if not _rapi_client.WaitForJobCompletion(job_id):
+    raise qa_error.Error("Could not fetch instance info for instance %s" %
+                         instance.name)
+  info_dict = _rapi_client.GetJobStatus(job_id)
+
+  if drbd_secret in str(info_dict):
+    print qa_utils.FormatInfo("DRBD secret: %s" % drbd_secret)
+    print qa_utils.FormatInfo("Retrieved data\n%s" % str(info_dict))
+    raise qa_error.Error("Found DRBD secret in contents of RAPI instance info"
+                         " call; see above.")
index 1076881..0e1cfc2 100644 (file)
@@ -177,7 +177,7 @@ getMasterNodes cfg =
 
 -- | Get the list of master candidates, /not including/ the master itself.
 getMasterCandidates :: ConfigData -> [Node]
-getMasterCandidates cfg = 
+getMasterCandidates cfg =
   filter ((==) NRCandidate . getNodeRole cfg) . F.toList . configNodes $ cfg
 
 -- | Get the list of master candidates, /including/ the master.
@@ -392,7 +392,7 @@ getInstDisksFromObj cfg =
 -- | Collects a value for all DRBD disks
 collectFromDrbdDisks
   :: (Monoid a)
-  => (String -> String -> Int -> Int -> Int -> DRBDSecret -> a)
+  => (String -> String -> Int -> Int -> Int -> Private DRBDSecret -> a)
   -- ^ NodeA, NodeB, Port, MinorA, MinorB, Secret
   -> Disk -> a
 collectFromDrbdDisks f = col
@@ -404,7 +404,8 @@ collectFromDrbdDisks f = col
 
 -- | Returns the DRBD secrets of a given 'Disk'
 getDrbdSecretsForDisk :: Disk -> [DRBDSecret]
-getDrbdSecretsForDisk = collectFromDrbdDisks (\_ _ _ _ _ secret -> [secret])
+getDrbdSecretsForDisk = collectFromDrbdDisks
+                          (\_ _ _ _ _ (Private secret) -> [secret])
 
 -- | Returns the DRBD minors of a given 'Disk'
 getDrbdMinorsForDisk :: Disk -> [(Int, String)]
index 4bbf5b6..23060c5 100644 (file)
@@ -424,7 +424,7 @@ instance J.JSON LogicalVolume where
 -- logical id, this is probably a good reference point.
 data DiskLogicalId
   = LIDPlain LogicalVolume  -- ^ Volume group, logical volume
-  | LIDDrbd8 String String Int Int Int DRBDSecret
+  | LIDDrbd8 String String Int Int Int (Private DRBDSecret)
   -- ^ NodeA, NodeB, Port, MinorA, MinorB, Secret
   | LIDFile FileDriver String -- ^ Driver, path
   | LIDSharedFile FileDriver String -- ^ Driver, path
@@ -453,7 +453,7 @@ lidEncodeType v = [(devType, showJSON . lidDiskType $ v)]
 encodeDLId :: DiskLogicalId -> JSValue
 encodeDLId (LIDPlain (LogicalVolume vg lv)) =
   JSArray [showJSON vg, showJSON lv]
-encodeDLId (LIDDrbd8 nodeA nodeB port minorA minorB key) =
+encodeDLId (LIDDrbd8 nodeA nodeB port minorA minorB (Private key)) =
   JSArray [ showJSON nodeA, showJSON nodeB, showJSON port
           , showJSON minorA, showJSON minorB, showJSON key ]
 encodeDLId (LIDRados pool name) = JSArray [showJSON pool, showJSON name]
@@ -485,7 +485,7 @@ decodeDLId obj lid = do
           mA' <- readJSON mA
           mB' <- readJSON mB
           k'  <- readJSON k
-          return $ LIDDrbd8 nA' nB' p' mA' mB' k'
+          return . LIDDrbd8 nA' nB' p' mA' mB' $ Private k'
         _ -> fail "Can't read logical_id for DRBD8 type"
     DTPlain ->
       case lid of
index 6edab31..06f15c4 100644 (file)
@@ -683,7 +683,8 @@ caseIncludeLogicalIdDrbd =
       time = TOD 0 0
       d =
         Disk
-          (LIDDrbd8 "node1.example.com" "node2.example.com" 2000 1 5 "secret")
+          (LIDDrbd8 "node1.example.com" "node2.example.com" 2000 1 5
+           (Private "secret"))
           [ Disk (mkLIDPlain "onevg" "onelv") [] "disk1" 1000 DiskRdWr Nothing
               Nothing Nothing "145145-asdf-sdf2-2134-asfd-534g2x" 0 time time
           , Disk (mkLIDPlain vg_name lv_name) [] "disk2" 1000 DiskRdWr Nothing
index 9553e47..9a1894f 100755 (executable)
@@ -35,6 +35,7 @@ import os
 
 from ganeti import constants
 from ganeti import errors
+from ganeti import serializer
 from ganeti.storage import drbd
 from ganeti.storage import drbd_info
 from ganeti.storage import drbd_cmdgen
@@ -436,7 +437,8 @@ class TestDRBD8Construction(testutils.GanetiTestCase):
       drbd_info.DRBD8Info.CreateFromFile(
         filename=testutils.TestDataFilename("proc_drbd84.txt"))
 
-    self.test_unique_id = ("hosta.com", 123, "host2.com", 123, 0, "secret")
+    self.test_unique_id = ("hosta.com", 123, "host2.com", 123, 0,
+                           serializer.Private("secret"))
     self.test_dyn_params = {
       constants.DDP_LOCAL_IP: "192.0.2.1",
       constants.DDP_LOCAL_MINOR: 0,