Use OpenSSL's HMAC API

[tiran]

BPO	40645
Nosy	@gpshead, @vstinner, @tiran, @encukou, @pablogsal, @miss-islington, @erlend-aasland
PRs	bpo-40645: Implement HMAC in C (GH-20129) #20129 bpo-40645: Deprecated internal details of hmac.HMAC (GH-20132) #20132 bpo-40645: restrict HMAC key len to INT_MAX #20238 [3.9] bpo-40645: restrict HMAC key len to INT_MAX (GH-20238) #20245 bpo-40645: use C implementation of HMAC #24920 bpo-40645: Fix reference leak in the _hashopenssl extension #25063 bpo-40645: Fix reference leak in the _hashopenssl extension #26072 bpo-40645: Fix ref leaks in _hashopenssl #26079 [3.10] bpo-40645: Fix ref leaks in _hashopenssl (GH-26079) #26082

^{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 = 'https://github.com/tiran'
closed_at = <Date 2021-10-20.14:44:16.885>
created_at = <Date 2020-05-16.15:01:31.309>
labels = ['extension-modules', 'type-feature', '3.9', '3.10']
title = "Use OpenSSL's HMAC API"
updated_at = <Date 2021-10-20.14:44:16.884>
user = 'https://github.com/tiran'

bugs.python.org fields:

activity = <Date 2021-10-20.14:44:16.884>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2021-10-20.14:44:16.885>
closer = 'christian.heimes'
components = ['Extension Modules']
creation = <Date 2020-05-16.15:01:31.309>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 40645
keywords = ['patch']
message_count = 14.0
messages = ['369050', '369079', '369104', '369114', '369375', '369388', '369403', '369404', '389027', '389601', '389682', '389684', '389701', '393542']
nosy_count = 7.0
nosy_names = ['gregory.p.smith', 'vstinner', 'christian.heimes', 'petr.viktorin', 'pablogsal', 'miss-islington', 'erlendaasland']
pr_nums = ['20129', '20132', '20238', '20245', '24920', '25063', '26072', '26079', '26082']
priority = 'critical'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40645'
versions = ['Python 3.9', 'Python 3.10']

[tiran]

Python's hmac module provides a pure Python based implementation on top of the hashlib module. OpenSSL offers a dedicated HMAC implementation that has a couple of benefits over pure Python implementation:

• OpenSSL HMAC is slightly faster and requires slightly less memory and allocations.
• Python's HMAC only works for RFC 2104 HMACs with digests like MD5, SHA1, SHA2, and SHA3. Other digests types like Blake2 use a completely different style of HMAC. OpenSSL's HMAC API works for all sorts of digests. OpenSSL 3.0.0 also supports Blake2 MAC with its standard API.
• OpenSSL HMAC is standard and compliance conform. Certain compliance restrictions require that MAC and keyed hashing is implemented in a certain way. Our HMAC code is considered a custom implementation of a crypto primitive and in violation of compliance rules.

For 3.9 I plan to deprecate hmac.HMAC.digest_con, hmac.HMAC.inner, and hmac.HMAC.outer attributes. They are implementation specific details any way. I'm also going to provide a _hashlib.hmac_new() function so we can test the new code.

For 3.10 I'll be switching over to _haslib.hmac_new() when the digestmod is a string or a callable that returns _hashlib.HASH code.

[tiran]

New changeset 837f9e4 by Christian Heimes in branch 'master':
bpo-40645: Deprecated internal details of hmac.HMAC (GH-20132)
837f9e4

[tiran]

def new(key, msg=None, digestmod=''):
    # use fast HMAC if OpenSSL bindings are available and digestmod is
    # either a string or a callable that returns an OpenSSL HASH object.
    if _hashopenssl is not None:
        if isinstance(digestmod, str):
            return _hashopenssl.hmac_new(key, msg, digestmod)
        if callable(digestmod):
            digest = digestmod(b'')
            if isinstance(digest, _hashopenssl.HASH):
                return _hashopenssl.hmac_new(key, msg, digest.name)
    return HMAC(key, msg, digestmod)

[tiran]

New changeset 54f2898 by Christian Heimes in branch 'master':
bpo-40645: Implement HMAC in C (GH-20129)
54f2898

[vstinner]

Compiler warning on 64-bit Windows:

c:\vstinner\python\3.9\modules\_hashopenssl.c(1427): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data

[tiran]

Thanks, Victor!

[miss-islington]

New changeset aca4670 by Christian Heimes in branch 'master':
bpo-40645: restrict HMAC key len to INT_MAX (GH-20238)
aca4670

[miss-islington]

New changeset 6ed3743 by Miss Islington (bot) in branch '3.9':
bpo-40645: restrict HMAC key len to INT_MAX (GH-20238)
6ed3743

[tiran]

memo to me: switch to new C implementation of HMAC.

[miss-islington]

New changeset 933dfd7 by Christian Heimes in branch 'master':
bpo-40645: use C implementation of HMAC (GH-24920)
933dfd7

[pablogsal]

Seems that commit 933dfd7 introduced some reference leaks:

commit 933dfd7
Author: Christian Heimes <christian@python.org>
Date: Sat Mar 27 14:55:03 2021 +0100

bpo-40645: use C implementation of HMAC (GH-24920)

𓋹 ./python.exe -m test test_hashlib -R 3:3
0:00:00 load avg: 5.67 Run tests sequentially
0:00:00 load avg: 5.67 [1/1] test_hashlib
beginning 6 repetitions
123456
......
test_hashlib leaked [1, 1, 1] references, sum=3
test_hashlib failed

== Tests result: FAILURE ==

1 test failed:
test_hashlib

Total duration: 8.7 sec
Tests result: FAILURE

- [x] fix tests
- [ ] add test scenarios for old/new code.

Signed-off-by: Christian Heimes <christian@python.org>

Lib/hashlib.py | 1 +
Lib/hmac.py | 86 +++++++-----
Lib/test/test_hmac.py | 114 +++++++++-------
.../2021-03-19-10-22-17.bpo-40645.5pXhb-.rst | 2 +
Modules/_hashopenssl.c | 150 +++++++++++++++++++--
Modules/clinic/_hashopenssl.c.h | 38 +-----
6 files changed, 267 insertions(+), 124 deletions(-)
create mode 100644 Misc/NEWS.d/next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst

[pablogsal]

See for instance: https://buildbot.python.org/all/#/builders/75/builds/224

[miss-islington]

New changeset 70cdf18 by Pablo Galindo in branch 'master':
bpo-40645: Fix reference leak in the _hashopenssl extension (GH-25063)
70cdf18

[miss-islington]

New changeset 1ee58f2 by Miss Islington (bot) in branch '3.10':
bpo-40645: Fix ref leaks in _hashopenssl (GH-26079)
1ee58f2