classification
Title: Use OpenSSL's HMAC API
Type: enhancement Stage: patch review
Components: Extension Modules Versions: Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: christian.heimes, erlendaasland, gregory.p.smith, miss-islington, pablogsal, petr.viktorin, vstinner
Priority: critical Keywords: patch

Created on 2020-05-16 15:01 by christian.heimes, last changed 2021-05-12 18:45 by miss-islington.

Pull Requests
URL Status Linked Edit
PR 20129 merged christian.heimes, 2020-05-16 15:06
PR 20132 merged christian.heimes, 2020-05-16 17:36
PR 20238 merged christian.heimes, 2020-05-19 20:44
PR 20245 merged miss-islington, 2020-05-19 22:36
PR 24920 merged christian.heimes, 2021-03-18 17:22
PR 25063 merged pablogsal, 2021-03-29 10:55
PR 26072 merged petr.viktorin, 2021-05-12 15:40
PR 26079 merged erlendaasland, 2021-05-12 17:53
PR 26082 merged miss-islington, 2021-05-12 18:20
Messages (14)
msg369050 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-05-16 15:01
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.
msg369079 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-05-16 23:05
New changeset 837f9e42e3a1ad03b340661afe85e67d2719334f by Christian Heimes in branch 'master':
bpo-40645: Deprecated internal details of hmac.HMAC (GH-20132)
https://github.com/python/cpython/commit/837f9e42e3a1ad03b340661afe85e67d2719334f
msg369104 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-05-17 08:46
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)
msg369114 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-05-17 11:49
New changeset 54f2898fe7e4ca1f239e96284af3cc5b34d2ae02 by Christian Heimes in branch 'master':
bpo-40645: Implement HMAC in C (GH-20129)
https://github.com/python/cpython/commit/54f2898fe7e4ca1f239e96284af3cc5b34d2ae02
msg369375 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-05-19 16:47
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
msg369388 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-05-19 20:46
Thanks, Victor!
msg369403 - (view) Author: miss-islington (miss-islington) Date: 2020-05-19 22:35
New changeset aca4670ad695d4b01c7880fe3d0af817421945bd by Christian Heimes in branch 'master':
bpo-40645: restrict HMAC key len to INT_MAX (GH-20238)
https://github.com/python/cpython/commit/aca4670ad695d4b01c7880fe3d0af817421945bd
msg369404 - (view) Author: miss-islington (miss-islington) Date: 2020-05-19 22:53
New changeset 6ed37430d31e915103ab5decd14d757eb2d159d5 by Miss Islington (bot) in branch '3.9':
bpo-40645: restrict HMAC key len to INT_MAX (GH-20238)
https://github.com/python/cpython/commit/6ed37430d31e915103ab5decd14d757eb2d159d5
msg389027 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-03-18 16:14
memo to me: switch to new C implementation of HMAC.
msg389601 - (view) Author: miss-islington (miss-islington) Date: 2021-03-27 13:55
New changeset 933dfd7504e521a27fd8b94d02b79f9ed08f4631 by Christian Heimes in branch 'master':
bpo-40645: use C implementation of HMAC (GH-24920)
https://github.com/python/cpython/commit/933dfd7504e521a27fd8b94d02b79f9ed08f4631
msg389682 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-03-29 10:28
Seems that commit 933dfd7504e521a27fd8b94d02b79f9ed08f4631 introduced some reference leaks:


commit 933dfd7504e521a27fd8b94d02b79f9ed08f4631
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 bpo-40645.5pXhb-.rst">Misc/NEWS.d/next/Library/2021-03-19-10-22-17.bpo-40645.5pXhb-.rst
msg389684 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-03-29 10:36
See for instance: https://buildbot.python.org/all/#/builders/75/builds/224
msg389701 - (view) Author: miss-islington (miss-islington) Date: 2021-03-29 13:17
New changeset 70cdf1812cf479c6b1cd7435a6fc0679ec1fb0da by Pablo Galindo in branch 'master':
bpo-40645: Fix reference leak in the _hashopenssl extension (GH-25063)
https://github.com/python/cpython/commit/70cdf1812cf479c6b1cd7435a6fc0679ec1fb0da
msg393542 - (view) Author: miss-islington (miss-islington) Date: 2021-05-12 18:45
New changeset 1ee58f252454072a1c9da77999db8e6417a307a0 by Miss Islington (bot) in branch '3.10':
bpo-40645: Fix ref leaks in _hashopenssl (GH-26079)
https://github.com/python/cpython/commit/1ee58f252454072a1c9da77999db8e6417a307a0
History
Date User Action Args
2021-05-12 18:45:32miss-islingtonsetmessages: + msg393542
2021-05-12 18:20:50miss-islingtonsetpull_requests: + pull_request24721
2021-05-12 17:53:21erlendaaslandsetnosy: + erlendaasland
pull_requests: + pull_request24718
2021-05-12 15:40:49petr.viktorinsetnosy: + petr.viktorin
pull_requests: + pull_request24712
2021-03-29 13:17:44miss-islingtonsetmessages: + msg389701
2021-03-29 10:55:32pablogsalsetpull_requests: + pull_request23813
2021-03-29 10:36:49pablogsalsetmessages: + msg389684
2021-03-29 10:28:07pablogsalsetnosy: + pablogsal
messages: + msg389682
2021-03-27 13:55:10miss-islingtonsetmessages: + msg389601
2021-03-18 17:22:46christian.heimessetpull_requests: + pull_request23684
2021-03-18 16:14:58christian.heimessetpriority: normal -> critical

messages: + msg389027
versions: + Python 3.10
2020-05-19 22:53:00miss-islingtonsetmessages: + msg369404
2020-05-19 22:36:05miss-islingtonsetpull_requests: + pull_request19533
2020-05-19 22:35:58miss-islingtonsetnosy: + miss-islington
messages: + msg369403
2020-05-19 20:46:25christian.heimessetmessages: + msg369388
2020-05-19 20:44:05christian.heimessetpull_requests: + pull_request19526
2020-05-19 16:47:52vstinnersetnosy: + vstinner
messages: + msg369375
2020-05-17 11:49:13christian.heimessetmessages: + msg369114
2020-05-17 08:46:02christian.heimessetmessages: + msg369104
2020-05-16 23:05:43christian.heimessetmessages: + msg369079
2020-05-16 17:36:12christian.heimessetpull_requests: + pull_request19437
2020-05-16 15:06:40christian.heimessetkeywords: + patch
pull_requests: + pull_request19434
2020-05-16 15:01:31christian.heimescreate