More test coverage for hmac

[packetslave]

BPO	14322
Nosy	@eamanu, @iritkatriel, @CCLDArjun
PRs	bpo-14322: added test case for invalid update to hmac #26636
Files	test_hmac.patch: Old test_hmac.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2012-03-15.17:13:28.755>
labels = ['easy', '3.8', '3.9', '3.10', 'library', 'type-feature', 'tests']
title = 'More test coverage for hmac'
updated_at = <Date 2021-06-17.11:49:39.078>
user = 'https://bugs.python.org/packetslave'

bugs.python.org fields:

activity = <Date 2021-06-17.11:49:39.078>
actor = 'eamanu'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Tests']
creation = <Date 2012-03-15.17:13:28.755>
creator = 'packetslave'
dependencies = []
files = ['24865', '24896']
hgrepos = []
issue_num = 14322
keywords = ['patch', 'easy']
message_count = 9.0
messages = ['155913', '156041', '156134', '156758', '172265', '221970', '379374', '395484', '395593']
nosy_count = 3.0
nosy_names = ['eamanu', 'iritkatriel', 'CCLDArjun']
pr_nums = ['26636']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14322'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[packetslave]

Adding some tests to non-default code paths in hmac.py to get to 100% coverage.

[merwok]

Thanks for the patch. Could you rewrite it using self.assert* methods instead of asserts?

I’m not an hmac expert, so you may have to wait a bit for another core developer to see and commit the patch.

[packetslave]

Updated to use the correct assert* methods.

[pitrou]

Some style comments:

• don't use "except: self.fail()", just let the exception pass through

• when monkeypatching hmac.new, use a try...finally block so that the mock doesn't stay in place if the test fails for whatever reason

• it's a bit of a nit, but when using "with warnings.catch_warnings", I would put the asserts on the context manager result outside of the "with" block

• I get a warning in test_noncallable_digestmod, it would be nice to silence it:

/home/antoine/cpython/default/Lib/test/test_hmac.py:254: RuntimeWarning: No block_size attribute on given digest object; Assuming 64.
h = hmac.HMAC(b"key", b"", hmac)

[tiran]

IIRC we also need a signed contributor agreement in order to commit your patch.

[BreamoreBoy]

If there isn't a signed contributor agreement I'll put up a new version of the patch.

In msg156758 Antoine said 'don't use "except: self.fail()", just let the exception pass through'. There are several of these in the existing code. Should they all be removed or must it be done on a case by case basis?

[iritkatriel]

The patch is out of date and needs to be refreshed.

[CCLDArjun]

The only things I think we should add to the current hmac tests are test_update_error_handling and test_with_invalid_msg.

For test_withnoncallable_digestmod(), hmac itself seems it can no longer be used:

>>> hmac.HMAC(b"gggg", None, "hmac")
<module '_hashlib' from '/Users/arjun/Python/Sources/cpython/build/lib.macosx-11.4-x86_64-3.11-pydebug/_hashlib.cpython-311d-darwin.so'>
using new
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/arjun/Python/Sources/cpython/Lib/hmac.py", line 61, in __init__
    self._init_hmac(key, msg, digestmod)
  File "/Users/arjun/Python/Sources/cpython/Lib/hmac.py", line 69, in _init_hmac
    self._hmac = _hashopenssl.hmac_new(key, msg, digestmod=digestmod)
ValueError: unsupported hash type hmac

For tests test_small_block_size and test_no_block_size, a custom .blocksize cannot be set (changing .block_size has no difference on digest()):

>>> hmac.HMAC(b"gggg", None, "md5").blocksize = 15
<module '_hashlib' from '/Users/arjun/Python/Sources/cpython/build/lib.macosx-11.4-x86_64-3.11-pydebug/_hashlib.cpython-311d-darwin.so'>
using new
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'HMAC' object attribute 'blocksize' is read-only

class myHMAC(hmac.HMAC):
    blocksize = 1
    def __init__(self, key, msg, digestmod_):
        super().__init__(key, msg, digestmod=digestmod_)

h = myHMAC(b"key", b"", digestmod_="md5")
h.digest() # <- works perfectly fine.

Does this sound okay? I can go ahead an implement it if so.

[CCLDArjun]

would the update invalid test belong in the TestVectorsTestCase class or should I make a new one?