Restrict showing of DRBD secret using types
authorHrvoje Ribicic <riba@google.com>
Tue, 1 Dec 2015 16:11:38 +0000 (16:11 +0000)
committerHrvoje Ribicic <riba@google.com>
Tue, 1 Dec 2015 16:50:53 +0000 (16:50 +0000)
While the Python changes from 2.9 do prevent Ganeti from accidentally
revealing the Haskell secret, they may not do so forever. The queries
are planned to switch from Python to Haskell at some point, and should
someone want to use the DRBD secret, they can do so easily.

As a more elegant way of hiding the secret, wrap it in a Private
wrapper, preventing it from leaking out unless explicitly requested.

Signed-off-by: Hrvoje Ribicic <riba@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

lib/objects.py
lib/storage/drbd.py
src/Ganeti/Config.hs
src/Ganeti/Objects.hs
test/hs/Test/Ganeti/Objects.hs
test/py/ganeti.storage.drbd_unittest.py

index fd28f5d..915bec1 100644 (file)
@@ -814,10 +814,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:
@@ -838,6 +843,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 7381c01..215a0f4 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 df576b2..18e7366 100644 (file)
@@ -176,7 +176,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.
@@ -385,7 +385,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
@@ -397,7 +397,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 c99c1c2..85cb6bd 100644 (file)
@@ -397,7 +397,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
@@ -426,7 +426,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]
@@ -458,7 +458,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 db0252a..363dbc0 100644 (file)
@@ -638,7 +638,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,