On Wed, Feb 5, 2014 at 4:54 PM, Jose A. Lopes <[email protected]> wrote:
>> + if field in constants.PRIVATE_PARAMETERS_BLACKLIST:
>> + if not field.endswith("_cluster"):
>
> This will break as soon as someone changes the parameters list.
> This should be documented in the constant, I guess.
There kind of is no good place to document this. The parameter names
are defined in OpParams.hs, which is very long. Maybe some sort of
Template Haskell wizardry, later on, can assert that if Private is
involved in the type of the variable then the resulting parameter name
must be protected?
>> + data[field] = PrivateDict(value)
>> + else:
>> + for os in data[field]:
>> + value[os] = PrivateDict(value[os])
>> + else:
>> + todo.append(value)
>> + else: # Values
>> + pass
>
> This is a strange algorithm. Have you considered doing a recursive
> traversal of the object tree?
Yes, this function was originally that and was then turned into this
purely procedural style for performance given that this is such a
high-traffic zone.
>> signed_dict = LoadJson(txt)
>> +
>> + WrapPrivateValues(signed_dict)
>> +
>> if not isinstance(signed_dict, dict):
>> raise errors.SignatureError("Invalid external message")
>
> Can't the line below fail if the WrapPrivateValues above returns an
> instance of PrivateDict?
Yes. Luckily, PrivateDict subclasses dict, so the check passes.
>> +def EncodeWithoutPrivate(obj):
>> + if isinstance(obj, Private):
>> + return None
>> + raise TypeError(repr(obj) + " is not JSON serializable")
>
> The message should mention the Private class.
These functions get passed to the JSON library and they are used
whenever they don't know what to do with an object; in this case, they
instruct the library to use None for Private values. For things that
are not Private, we still don't know how to deal with them. That's why
the generic exception is raised.
Here's the interdiff to be split across the relevant commits:
diff --git a/lib/config.py b/lib/config.py
index 8db03c2..2542e5b 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -2490,7 +2490,7 @@ class ConfigWriter(object):
self._BumpSerialNo()
txt = serializer.DumpJson(
self._config_data.ToDict(_with_private=True),
- private_decoder=serializer.EncodeWithPrivateFields
+ private_encoder=serializer.EncodeWithPrivateFields
)
getents = self._getents()
diff --git a/lib/rpc/client.py b/lib/rpc/client.py
index 47ce583..5e2fb93 100644
--- a/lib/rpc/client.py
+++ b/lib/rpc/client.py
@@ -121,7 +121,7 @@ def FormatRequest(method, args, version=None):
# Serialize the request
return serializer.DumpJson(request,
-
private_decoder=serializer.EncodeWithPrivateFields)
+
private_encoder=serializer.EncodeWithPrivateFields)
def CallRPCMethod(transport_cb, method, args, version=None):
diff --git a/lib/rpc/node.py b/lib/rpc/node.py
index e584956..dd50817 100644
--- a/lib/rpc/node.py
+++ b/lib/rpc/node.py
@@ -504,7 +504,7 @@ class _RpcClientBase:
pnbody = dict(
(n,
serializer.DumpJson(prep_fn(n, encode_args_fn(n)),
- private_decoder=serializer.EncodeWithPrivateFields))
+ private_encoder=serializer.EncodeWithPrivateFields))
for n in node_list
)
diff --git a/lib/serializer.py b/lib/serializer.py
index b5b6353..5230c21 100644
--- a/lib/serializer.py
+++ b/lib/serializer.py
@@ -45,17 +45,20 @@ from ganeti import constants
_RE_EOLSP = re.compile("[ \t]+$", re.MULTILINE)
-def DumpJson(data, private_decoder=None):
+def DumpJson(data, private_encoder=None):
"""Serialize a given object.
@param data: the data to serialize
@return: the string representation of data
+ @param private_encoder: specify L{serializer.EncodeWithPrivateFields} if you
+ require the produced JSON to also contain private
+ parameters. Otherwise, they will encode to null.
"""
- if private_decoder is None:
+ if private_encoder is None:
# Do not leak private fields by default.
- private_decoder = EncodeWithoutPrivate
- encoded = simplejson.dumps(data, default=private_decoder)
+ private_encoder = EncodeWithoutPrivateFields
+ encoded = simplejson.dumps(data, default=private_encoder)
txt = _RE_EOLSP.sub("", encoded)
if not txt.endswith("\n"):
@@ -81,6 +84,11 @@ def LoadJson(txt):
def WrapPrivateValues(json):
+ """Crawl a JSON decoded structure for private values and wrap them.
+
+ @param json: the json-decoded value to protect.
+ """
+
todo = [json]
while todo:
@@ -111,17 +119,18 @@ def WrapPrivateValues(json):
def DumpSignedJson(data, key, salt=None, key_selector=None,
- private_decoder=None):
+ private_encoder=None):
"""Serialize a given object and authenticate it.
@param data: the data to serialize
@param key: shared hmac key
@param key_selector: name/id that identifies the key (in case there are
multiple keys in use, e.g. in a multi-cluster environment)
+ @param private_encoder: see L{DumpJson}
@return: the string representation of data signed by the hmac key
"""
- txt = DumpJson(data, private_decoder=private_decoder)
+ txt = DumpJson(data, private_encoder=private_encoder)
if salt is None:
salt = ""
signed_dict = {
@@ -364,7 +373,7 @@ class PrivateDict(dict):
return returndict
-def EncodeWithoutPrivate(obj):
+def EncodeWithoutPrivateFields(obj):
if isinstance(obj, Private):
return None
raise TypeError(repr(obj) + " is not JSON serializable")
diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
index 961e3ba..f300981 100644
--- a/src/Ganeti/Constants.hs
+++ b/src/Ganeti/Constants.hs
@@ -4754,7 +4754,8 @@ glusterPortDefault = 24007
privateParametersBlacklist :: [String]
privateParametersBlacklist = [ "osparams_private"
, "osparams_secret"
- , "osparams_private_cluster" ]
+ , "osparams_private_cluster"
+ ]
-- | Warn the user that the logging level is too low for production use.
debugModeConfidentialityWarning :: String
diff --git a/test/hs/Test/Ganeti/OpCodes.hs b/test/hs/Test/Ganeti/OpCodes.hs
index 714d39a..3591480 100644
--- a/test/hs/Test/Ganeti/OpCodes.hs
+++ b/test/hs/Test/Ganeti/OpCodes.hs
@@ -590,7 +590,7 @@ case_py_compat_types = do
\ for op in decoded]\n\
\print serializer.Dump(\
\ encoded,\
- \ private_decoder = serializer.EncodeWithPrivateFields)"
+ \ private_encoder=serializer.EncodeWithPrivateFields)"
serialized
>>= checkPythonResult
let deserialised =
diff --git a/test/py/ganeti.serializer_unittest.py
b/test/py/ganeti.serializer_unittest.py
index 4a7874c..20ee4f1 100755
--- a/test/py/ganeti.serializer_unittest.py
+++ b/test/py/ganeti.serializer_unittest.py
@@ -57,7 +57,7 @@ class TestSerializer(testutils.GanetiTestCase):
def _TestSerializer(self, dump_fn, load_fn):
_dump_fn = lambda data: dump_fn(
data,
- private_decoder=serializer.EncodeWithPrivateFields
+ private_encoder=serializer.EncodeWithPrivateFields
)
for data in self._TESTDATA:
self.failUnless(_dump_fn(data).endswith("\n"))
@@ -78,7 +78,7 @@ class TestSerializer(testutils.GanetiTestCase):
def _TestSigned(self, dump_fn, load_fn):
_dump_fn = lambda *args, **kwargs: dump_fn(
*args,
- private_decoder=serializer.EncodeWithPrivateFields,
+ private_encoder=serializer.EncodeWithPrivateFields,
**kwargs
)
for data in self._TESTDATA:
@@ -201,7 +201,7 @@ class TestPrivate(unittest.TestCase):
self.assertIn(
"egg",
serializer.Dump(pDict,
- private_decoder=serializer.EncodeWithPrivateFields)
+ private_encoder=serializer.EncodeWithPrivateFields)
)
def testDictGet(self):
--
Raffa Santi
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
