2010/2/18 Balazs Lecz <[email protected]>:
> --- a/lib/serializer.py
> +++ b/lib/serializer.py
> +def LoadSignedJson(txt, key=None, keydict=None):
Don't include type names in variable names. If you later have to
change the type it makes things more complicated.
> + if not key and not keydict:
> + raise errors.ProgrammerError("A key or keydict must be supplied")
You would have to use the XOR operator (^) here. If both are passed in
it will use keydict.
> signed_dict = LoadJson(txt)
> if not isinstance(signed_dict, dict):
> raise errors.SignatureError('Invalid external message')
> @@ -140,6 +147,17 @@ def LoadSignedJson(txt, key):
> except KeyError:
> raise errors.SignatureError('Invalid external message')
>
> + if keydict:
> + try:
> + key_selector = signed_dict["key_selector"]
> + except KeyError:
> + raise errors.SignatureError("No key_selector found in external
> message")
> + try:
> + key = keydict[key_selector]
> + except KeyError:
> + raise errors.SignatureError("No key with key selector '%s' found" %
> + key_selector)
Instead of this rather complicated code I suggest you drop the
“keydict” parameter and allow “key” to be either a string (as it was)
or a callable. If it's callable, you call it with None (no key
selector in message) or the key selector and it'll either return None
if no key for the selector is available or the key if there is one.
This is more flexible as it doesn't require the caller to have a dict
(or a wrapper class providing __getitem__). You can simply call the
function like LoadSignedJson(…, mykeymap.get) and it still works.
> if hmac.new(key, salt + msg, sha1).hexdigest() != hmac_sign:
> raise errors.SignatureError('Invalid Signature')
>
> diff --git a/test/ganeti.serializer_unittest.py
> b/test/ganeti.serializer_unittest.py
> index 11a60e6..32b3744 100755
> --- a/test/ganeti.serializer_unittest.py
> +++ b/test/ganeti.serializer_unittest.py
> @@ -75,6 +75,12 @@ class TestSerializer(testutils.GanetiTestCase):
> "myprivatekey"),
> (data, "mysalt"))
>
> + self.assertEqualValues(
> + LoadSigned(DumpSigned(data, "myprivatekey",
> + "mysalt", "mykey_id"),
Don't pass keyword arguments as positional arguments (this might also
need fixing in other parts of this test).
> + keydict={"mykey_id": "myprivatekey"}),
> + (data, "mysalt"))
Regards,
Michael
