Headline
CVE-2022-48566: Issue 40791: hmac.compare_digest could try harder to be constant-time.
An issue was discovered in compare_digest in Lib/hmac.py in Python through 3.9.1. Constant-time-defeating optimisations were possible in the accumulator variable in hmac.compare_digest.
Created on 2020-05-27 07:41 by Devin Jeanpierre, last changed 2022-04-11 14:59 by admin. This issue is now closed.
Messages (15) msg370053 - (view) Author: Devin Jeanpierre (Devin Jeanpierre) * Date: 2020-05-27 07:41
`hmac.compare_digest` (via `_tscmp`) does not mark the accumulator variable `result` as volatile, which means that the compiler is allowed to short-circuit the comparison loop as long as it still reads from both strings.
In particular, when `result` is non-volatile, the compiler is allowed to change the loop from this:
```c for (i=0; i < length; i++) { result |= *left++ ^ *right++; } return (result == 0); ```
into (the moral equivalent of) this:
```c for (i=0; i < length; i++) { result |= *left++ ^ *right++; if (result) { for (; ++i < length;) { *left++; *right++; } return 1; } } return (result == 0); ```
(Code not tested.)
This might not seem like much, but it cuts out almost all of the data dependencies between `result`, `left`, and `right`, which in theory would free the CPU to race ahead using out of order execution – it could execute code that depends on the result of `_tscmp`, even while `_tscmp` is still performing the volatile reads. (I have not actually benchmarked this. :)) In other words, this weird short circuiting could still actually improve performance. That, in turn, means that it would break constant-time guarantees.
(This is different from saying that it _would_ increase performance, but marking it volatile removes the worry.)
(Prior art/discussion: https://github.com/google/tink/commit/335291c42eecf29fca3d85fed6179d11287d253e )
I propose two changes, one trivial, and one that’s more invasive:
Make `result` a `volatile unsigned char` instead of `unsigned char`.
When SSL is available, instead use `CRYPTO_memcmp` from OpenSSL/BoringSSL. We are, in effect, "rolling our own crypto". The SSL libraries are more strictly audited for timing issues, down to actually checking the generated machine code. As tools improve, those libraries will grow to use those tools. If we use their functions, we get the benefit of those audits and improvements.
msg370094 - (view) Author: Raymond Hettinger (rhettinger) * Date: 2020-05-27 15:26
+1 for both of these suggestions
msg370108 - (view) Author: Gregory P. Smith (gregory.p.smith) * Date: 2020-05-27 16:05
Christian - Devin could likely use some help with the build/ifdef plumbing required for (2) to use CRYPTO_memcmp from Modules/_operator.c when OpenSSL is available.
msg370109 - (view) Author: Christian Heimes (christian.heimes) * Date: 2020-05-27 16:14
GPS, I got you covered :)
CRYPTO_memcmp() was on my TODO list for a while. Thanks for nagging me.
_operator is a built-in module. I don’t want to add libcrypto dependency to libpython. I copied the code, made some adjustments and added it to _hashopenssl.c.
msg370121 - (view) Author: Christian Heimes (christian.heimes) * Date: 2020-05-27 19:18
Greg, is GH-20456 a bug fix / security enhancement or a new feature? I’m hesitant to backport it to 3.7 and 3.8. 3.9 might be ok.
msg370122 - (view) Author: Gregory P. Smith (gregory.p.smith) * Date: 2020-05-27 19:46
I’d feel fine doing that for 3.9 given 3.9.0 is only in beta and this changes no public APIs. For 3.8 and 3.7 i wouldn’t.
Be sure to update the versionchanged in the docs if you choose to do it for 3.9.
msg370124 - (view) Author: Christian Heimes (christian.heimes) * Date: 2020-05-27 19:50
New changeset db5aed931f8a617f7b63e773f62db468fe9c5ca1 by Christian Heimes in branch 'master’: bpo-40791: Use CRYPTO_memcmp() for compare_digest (#20456) https://github.com/python/cpython/commit/db5aed931f8a617f7b63e773f62db468fe9c5ca1
msg370195 - (view) Author: miss-islington (miss-islington) Date: 2020-05-28 12:10
New changeset 8183e11d87388e4e44e3242c42085b87a878f781 by Christian Heimes in branch '3.9’: [3.9] bpo-40791: Use CRYPTO_memcmp() for compare_digest (GH-20456) (GH-20461) https://github.com/python/cpython/commit/8183e11d87388e4e44e3242c42085b87a878f781
msg381530 - (view) Author: Gregory P. Smith (gregory.p.smith) * Date: 2020-11-21 08:55
New changeset 31729366e2bc09632e78f3896dbce0ae64914f28 by Devin Jeanpierre in branch 'master’: bpo-40791: Make compare_digest more constant-time. (GH-20444) https://github.com/python/cpython/commit/31729366e2bc09632e78f3896dbce0ae64914f28
msg381531 - (view) Author: miss-islington (miss-islington) Date: 2020-11-21 09:12
New changeset 97136d71a78a4b6b816f7e14acc52be426efcb6f by Miss Islington (bot) in branch '3.8’: bpo-40791: Make compare_digest more constant-time. (GH-20444) https://github.com/python/cpython/commit/97136d71a78a4b6b816f7e14acc52be426efcb6f
msg381533 - (view) Author: miss-islington (miss-islington) Date: 2020-11-21 09:18
New changeset c1bbca5b004b3f74d240ef8a76ff445cc1a27efb by Miss Islington (bot) in branch '3.9’: bpo-40791: Make compare_digest more constant-time. (GH-20444) https://github.com/python/cpython/commit/c1bbca5b004b3f74d240ef8a76ff445cc1a27efb
msg381624 - (view) Author: Benjamin Peterson (benjamin.peterson) * Date: 2020-11-22 17:33
New changeset db95802bdfac4d13db3e2a391ec7b9e2f8d92dbe by Miss Islington (bot) in branch '3.7’: bpo-40791: Make compare_digest more constant-time. (GH-23438) https://github.com/python/cpython/commit/db95802bdfac4d13db3e2a391ec7b9e2f8d92dbe
msg382972 - (view) Author: Michał Górny (mgorny) * Date: 2020-12-14 10:12
Any reason this wasn’t backported to 3.6? FWICS it’s supposed to be security supported still.
msg382993 - (view) Author: Ned Deily (ned.deily) * Date: 2020-12-14 17:05
New changeset 8bef9ebb1b88cfa4b2a38b93fe4ea22015d8254a by Miss Islington (bot) in branch '3.6’: bpo-40791: Make compare_digest more constant-time. (GH-23438) (GH-23767) https://github.com/python/cpython/commit/8bef9ebb1b88cfa4b2a38b93fe4ea22015d8254a
msg382995 - (view) Author: Ned Deily (ned.deily) * Date: 2020-12-14 17:11
> Any reason this wasn’t backported to 3.6?
Just an oversight. Thanks for pointing it out.
History Date User Action Args 2022-04-11 14:59:31adminsetgithub: 84968 2020-12-14 17:11:27ned.deilysetmessages: + msg382995
versions: + Python 3.6 2020-12-14 17:05:09ned.deilysetnosy: + ned.deily
messages: + msg382993
2020-12-14 16:41:09miss-islingtonsetpull_requests: + pull_request22623 2020-12-14 10:12:35mgornysetnosy: + mgorny
messages: + msg382972
2020-11-22 17:34:28benjamin.petersonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved 2020-11-22 17:33:22benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg381624
2020-11-21 09:18:44miss-islingtonsetmessages: + msg381533 2020-11-21 09:12:24miss-islingtonsetmessages: + msg381531 2020-11-21 08:56:06miss-islingtonsetpull_requests: + pull_request22330 2020-11-21 08:55:58miss-islingtonsetpull_requests: + pull_request22329 2020-11-21 08:55:51miss-islingtonsetpull_requests: + pull_request22328 2020-11-21 08:55:27gregory.p.smithsetmessages: + msg381530 2020-05-28 12:10:04miss-islingtonsetnosy: + miss-islington
messages: + msg370195
2020-05-27 19:51:51christian.heimessetpull_requests: + pull_request19714 2020-05-27 19:50:11christian.heimessetmessages: + msg370124 2020-05-27 19:46:01gregory.p.smithsetmessages: + msg370122 2020-05-27 19:18:24christian.heimessetmessages: + msg370121 2020-05-27 16:14:21christian.heimessetmessages: + msg370109 2020-05-27 16:12:11christian.heimessetpull_requests: + pull_request19711 2020-05-27 16:05:20gregory.p.smithsetassignee: christian.heimes
messages: + msg370108 2020-05-27 15:26:51rhettingersetversions: - Python 3.5, Python 3.6
nosy: + rhettinger
messages: + msg370094
type: security
2020-05-27 15:25:47zach.waresetnosy: + gregory.p.smith, christian.heimes
2020-05-27 09:00:48Devin Jeanpierresetkeywords: + patch
stage: patch review
pull_requests: + pull_request19700 2020-05-27 07:41:07Devin Jeanpierrecreate
Related news
Ubuntu Security Notice 6891-1 - It was discovered that Python incorrectly handled certain inputs. An attacker could possibly use this issue to execute arbitrary code. This issue only affected Ubuntu 14.04 LTS and Ubuntu 18.04 LTS. It was discovered that Python incorrectly used regular expressions vulnerable to catastrophic backtracking. A remote attacker could possibly use this issue to cause a denial of service. This issue only affected Ubuntu 14.04 LTS.
Dell vApp Manger, versions prior to 9.2.4.x contain an arbitrary file read vulnerability. A remote attacker could potentially exploit this vulnerability to read arbitrary files from the target system.
Ubuntu Security Notice 6400-1 - It was discovered that Python did not properly provide constant-time processing for a crypto operation. An attacker could possibly use this issue to perform a timing attack and recover sensitive information.