[Patches] [ python-Patches-1182394 ] HMAC hexdigest and general review

[SourceForge.net]

Patches item #1182394, was opened at 2005-04-13 13:08
Message generated for change (Comment added) made by akuchling
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1182394&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Submitted By: Shane Holloway (shane_holloway)
Assigned to: Nobody/Anonymous (nobody)
Summary: HMAC hexdigest and general review

Initial Comment:
This patch is primarily aimed at speeding up the
HMAC.hexdigest method.  The current implementation is::

        return "".join([hex(ord(x))[2:].zfill(2)
                        for x in tuple(self.digest())])

Which is correct, but a little too clever.  Especially
when other tools are available.  The attached patch
adds HMAC._current that returns the hash object
combined from self.inner and self.outter to be used by
HMAC.digest and HMAC.hexdigest.  This results in an 11
fold speed improvement for md5 and sha digestmod's
while also making the code a little easier.


Other review points for the HMAC module include
removing "_secret_backdoor_key" in favor of using
"None" as the sentinel.  In this case, it makes sense
to use None because it is an invalid key, avoids a
global lookup in py2.4 and beyond, and is just as
readable.  

I also moved blocksize to a class level constant.  This
allows implementations to create different blocksize
hashes by inheriting and changing this value.  This
also involved a change to copy() to use
self.__class__(None) instead of directly calling
HMAC(None).

The final comment is that it would be nice to have a
C-level implementation of _strxor in binascii for this
module and possibly other uses.  However, I don't
really know a significantly faster way of implementing
this method in python.

Thanks,
-Shane Holloway

----------------------------------------------------------------------

>Comment By: A.M. Kuchling (akuchling)
Date: 2005-06-16 14:08

Message:
Logged In: YES 
user_id=11375

The HMAC._current() change and adding self.blocksize both
look reasonable.  I don't see the point of the
_secret_backdoor_key change, though; accidentally using None
or calling HMAC() will get an object that fails to work, and
the speedup seems too small to compensate for this.  Do you
have a use case that requires making lots of HMAC.copy() calls?

If the backdoor_key gets taken out, I don't think we need to
keep the name around for backward compatibility; no callers
of the module should have been using it.


----------------------------------------------------------------------

Comment By: Josiah Carlson (josiahcarlson)
Date: 2005-04-26 12:32

Message:
Logged In: YES 
user_id=341410

Obviously "always have a length of 4" should have been
"always have a length of zero modulo 4".

----------------------------------------------------------------------

Comment By: Josiah Carlson (josiahcarlson)
Date: 2005-04-26 03:10

Message:
Logged In: YES 
user_id=341410

I think the patch looks good.  Though perhaps leaving a...
_secret_backdoor_key = None
... for backwards compatibility is a good idea.

The fastest method I've found (in pure Python) for xoring
strings is to use an array.array, with the largest typecode
that is a divisor of your sequence (cooking your strings to
always have a length of 4 and to use signed 4 byte longs
seems to work well on PII/PIII based systems).

Alternatively, writing it in C and compiling it with the new
GCC 4 will offer automatic vectorization, and save
allocating Python integers.

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2005-04-14 03:54

Message:
Logged In: YES 
user_id=6656

Attempting to add the ^ for strings is a non-starter.

No opinion on the rest of the patch.

----------------------------------------------------------------------

Comment By: Shane Holloway (shane_holloway)
Date: 2005-04-13 17:21

Message:
Logged In: YES 
user_id=283742

I would prefer to keep str's interface lean.  I think
binascii would be a nice home for strxor though.

----------------------------------------------------------------------

Comment By: Antti Louko (alouko)
Date: 2005-04-13 14:01

Message:
Logged In: YES 
user_id=116402

This seems good.

I would also need strxor.  Should we add it to the string
type? ^ operator is not currently defined for strings.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1182394&group_id=5470