Skip to content

Commit 8183e11

Browse files
tirantiran
authored
[3.9] bpo-40791: Use CRYPTO_memcmp() for compare_digest (GH-20456) (GH-20461)
hashlib.compare_digest uses OpenSSL's CRYPTO_memcmp() function when OpenSSL is available. Note: The _operator module is a builtin module. I don't want to add libcrypto dependency to libpython. Therefore I duplicated the wrapper function and added a copy to _hashopenssl.c.. (cherry picked from commit db5aed9) Co-authored-by: Christian Heimes <christian@python.org>
1 parent d23ee5d commit 8183e11

File tree

7 files changed

+221
-37
lines changed

7 files changed

+221
-37
lines changed

‎Doc/library/hmac.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ This module also provides the following helper function:
138138

139139
.. versionadded:: 3.3
140140

141+
.. versionchanged:: 3.9
142+
143+
The function uses OpenSSL's ``CRYPTO_memcmp()`` internally when
144+
available.
145+
141146

142147
.. seealso::
143148

‎Lib/hmac.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@
44
"""
55

66
import warnings as _warnings
7-
from _operator import _compare_digest as compare_digest
87
try:
98
import _hashlib as _hashopenssl
109
except ImportError:
1110
_hashopenssl = None
1211
_openssl_md_meths = None
12+
from _operator import _compare_digest as compare_digest
1313
else:
1414
_openssl_md_meths = frozenset(_hashopenssl.openssl_md_meth_names)
15+
compare_digest = _hashopenssl.compare_digest
1516
import hashlib as _hashlib
1617

1718
trans_5C = bytes((x ^ 0x5C) for x in range(256))

‎Lib/test/test_hmac.py

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@
88

99
from test.support import hashlib_helper
1010

11+
from _operator import _compare_digest as operator_compare_digest
12+
1113
try:
1214
from _hashlib import HMAC as C_HMAC
1315
from _hashlib import hmac_new as c_hmac_new
16+
from _hashlib import compare_digest as openssl_compare_digest
1417
except ImportError:
1518
C_HMAC = None
1619
c_hmac_new = None
20+
openssl_compare_digest = None
1721

1822

1923
def ignore_warning(func):
@@ -505,110 +509,124 @@ def test_equality_new(self):
505509

506510
class CompareDigestTestCase(unittest.TestCase):
507511

508-
def test_compare_digest(self):
512+
def test_hmac_compare_digest(self):
513+
self._test_compare_digest(hmac.compare_digest)
514+
if openssl_compare_digest is not None:
515+
self.assertIs(hmac.compare_digest, openssl_compare_digest)
516+
else:
517+
self.assertIs(hmac.compare_digest, operator_compare_digest)
518+
519+
def test_operator_compare_digest(self):
520+
self._test_compare_digest(operator_compare_digest)
521+
522+
@unittest.skipIf(openssl_compare_digest is None, "test requires _hashlib")
523+
def test_openssl_compare_digest(self):
524+
self._test_compare_digest(openssl_compare_digest)
525+
526+
def _test_compare_digest(self, compare_digest):
509527
# Testing input type exception handling
510528
a, b = 100, 200
511-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
529+
self.assertRaises(TypeError, compare_digest, a, b)
512530
a, b = 100, b"foobar"
513-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
531+
self.assertRaises(TypeError, compare_digest, a, b)
514532
a, b = b"foobar", 200
515-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
533+
self.assertRaises(TypeError, compare_digest, a, b)
516534
a, b = "foobar", b"foobar"
517-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
535+
self.assertRaises(TypeError, compare_digest, a, b)
518536
a, b = b"foobar", "foobar"
519-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
537+
self.assertRaises(TypeError, compare_digest, a, b)
520538

521539
# Testing bytes of different lengths
522540
a, b = b"foobar", b"foo"
523-
self.assertFalse(hmac.compare_digest(a, b))
541+
self.assertFalse(compare_digest(a, b))
524542
a, b = b"\xde\xad\xbe\xef", b"\xde\xad"
525-
self.assertFalse(hmac.compare_digest(a, b))
543+
self.assertFalse(compare_digest(a, b))
526544

527545
# Testing bytes of same lengths, different values
528546
a, b = b"foobar", b"foobaz"
529-
self.assertFalse(hmac.compare_digest(a, b))
547+
self.assertFalse(compare_digest(a, b))
530548
a, b = b"\xde\xad\xbe\xef", b"\xab\xad\x1d\xea"
531-
self.assertFalse(hmac.compare_digest(a, b))
549+
self.assertFalse(compare_digest(a, b))
532550

533551
# Testing bytes of same lengths, same values
534552
a, b = b"foobar", b"foobar"
535-
self.assertTrue(hmac.compare_digest(a, b))
553+
self.assertTrue(compare_digest(a, b))
536554
a, b = b"\xde\xad\xbe\xef", b"\xde\xad\xbe\xef"
537-
self.assertTrue(hmac.compare_digest(a, b))
555+
self.assertTrue(compare_digest(a, b))
538556

539557
# Testing bytearrays of same lengths, same values
540558
a, b = bytearray(b"foobar"), bytearray(b"foobar")
541-
self.assertTrue(hmac.compare_digest(a, b))
559+
self.assertTrue(compare_digest(a, b))
542560

543561
# Testing bytearrays of different lengths
544562
a, b = bytearray(b"foobar"), bytearray(b"foo")
545-
self.assertFalse(hmac.compare_digest(a, b))
563+
self.assertFalse(compare_digest(a, b))
546564

547565
# Testing bytearrays of same lengths, different values
548566
a, b = bytearray(b"foobar"), bytearray(b"foobaz")
549-
self.assertFalse(hmac.compare_digest(a, b))
567+
self.assertFalse(compare_digest(a, b))
550568

551569
# Testing byte and bytearray of same lengths, same values
552570
a, b = bytearray(b"foobar"), b"foobar"
553-
self.assertTrue(hmac.compare_digest(a, b))
554-
self.assertTrue(hmac.compare_digest(b, a))
571+
self.assertTrue(compare_digest(a, b))
572+
self.assertTrue(compare_digest(b, a))
555573

556574
# Testing byte bytearray of different lengths
557575
a, b = bytearray(b"foobar"), b"foo"
558-
self.assertFalse(hmac.compare_digest(a, b))
559-
self.assertFalse(hmac.compare_digest(b, a))
576+
self.assertFalse(compare_digest(a, b))
577+
self.assertFalse(compare_digest(b, a))
560578

561579
# Testing byte and bytearray of same lengths, different values
562580
a, b = bytearray(b"foobar"), b"foobaz"
563-
self.assertFalse(hmac.compare_digest(a, b))
564-
self.assertFalse(hmac.compare_digest(b, a))
581+
self.assertFalse(compare_digest(a, b))
582+
self.assertFalse(compare_digest(b, a))
565583

566584
# Testing str of same lengths
567585
a, b = "foobar", "foobar"
568-
self.assertTrue(hmac.compare_digest(a, b))
586+
self.assertTrue(compare_digest(a, b))
569587

570588
# Testing str of different lengths
571589
a, b = "foo", "foobar"
572-
self.assertFalse(hmac.compare_digest(a, b))
590+
self.assertFalse(compare_digest(a, b))
573591

574592
# Testing bytes of same lengths, different values
575593
a, b = "foobar", "foobaz"
576-
self.assertFalse(hmac.compare_digest(a, b))
594+
self.assertFalse(compare_digest(a, b))
577595

578596
# Testing error cases
579597
a, b = "foobar", b"foobar"
580-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
598+
self.assertRaises(TypeError, compare_digest, a, b)
581599
a, b = b"foobar", "foobar"
582-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
600+
self.assertRaises(TypeError, compare_digest, a, b)
583601
a, b = b"foobar", 1
584-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
602+
self.assertRaises(TypeError, compare_digest, a, b)
585603
a, b = 100, 200
586-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
604+
self.assertRaises(TypeError, compare_digest, a, b)
587605
a, b = "fooä", "fooä"
588-
self.assertRaises(TypeError, hmac.compare_digest, a, b)
606+
self.assertRaises(TypeError, compare_digest, a, b)
589607

590608
# subclasses are supported by ignore __eq__
591609
class mystr(str):
592610
def __eq__(self, other):
593611
return False
594612

595613
a, b = mystr("foobar"), mystr("foobar")
596-
self.assertTrue(hmac.compare_digest(a, b))
614+
self.assertTrue(compare_digest(a, b))
597615
a, b = mystr("foobar"), "foobar"
598-
self.assertTrue(hmac.compare_digest(a, b))
616+
self.assertTrue(compare_digest(a, b))
599617
a, b = mystr("foobar"), mystr("foobaz")
600-
self.assertFalse(hmac.compare_digest(a, b))
618+
self.assertFalse(compare_digest(a, b))
601619

602620
class mybytes(bytes):
603621
def __eq__(self, other):
604622
return False
605623

606624
a, b = mybytes(b"foobar"), mybytes(b"foobar")
607-
self.assertTrue(hmac.compare_digest(a, b))
625+
self.assertTrue(compare_digest(a, b))
608626
a, b = mybytes(b"foobar"), b"foobar"
609-
self.assertTrue(hmac.compare_digest(a, b))
627+
self.assertTrue(compare_digest(a, b))
610628
a, b = mybytes(b"foobar"), mybytes(b"foobaz")
611-
self.assertFalse(hmac.compare_digest(a, b))
629+
self.assertFalse(compare_digest(a, b))
612630

613631

614632
if __name__ == "__main__":

‎Misc/NEWS.d/next/Library/2020-05-27-18-04-52.bpo-40791.IzpNor.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:func:`hashlib.compare_digest` uses OpenSSL's ``CRYPTO_memcmp()`` function
2+
when OpenSSL is available.

‎Modules/_hashopenssl.c

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
/* EVP is the preferred interface to hashing in OpenSSL */
2222
#include <openssl/evp.h>
2323
#include <openssl/hmac.h>
24+
#include <openssl/crypto.h>
2425
/* We use the object interface to discover what hashes OpenSSL supports. */
2526
#include <openssl/objects.h>
2627
#include "openssl/err.h"
@@ -1833,13 +1834,128 @@ _hashlib_get_fips_mode_impl(PyObject *module)
18331834
#endif // !LIBRESSL_VERSION_NUMBER
18341835

18351836

1837+
static int
1838+
_tscmp(const unsigned char *a, const unsigned char *b,
1839+
Py_ssize_t len_a, Py_ssize_t len_b)
1840+
{
1841+
/* loop count depends on length of b. Might leak very little timing
1842+
* information if sizes are different.
1843+
*/
1844+
Py_ssize_t length = len_b;
1845+
const void *left = a;
1846+
const void *right = b;
1847+
int result = 0;
1848+
1849+
if (len_a != length) {
1850+
left = b;
1851+
result = 1;
1852+
}
1853+
1854+
result |= CRYPTO_memcmp(left, right, length);
1855+
1856+
return (result == 0);
1857+
}
1858+
1859+
/* NOTE: Keep in sync with _operator.c implementation. */
1860+
1861+
/*[clinic input]
1862+
_hashlib.compare_digest
1863+
1864+
a: object
1865+
b: object
1866+
/
1867+
1868+
Return 'a == b'.
1869+
1870+
This function uses an approach designed to prevent
1871+
timing analysis, making it appropriate for cryptography.
1872+
1873+
a and b must both be of the same type: either str (ASCII only),
1874+
or any bytes-like object.
1875+
1876+
Note: If a and b are of different lengths, or if an error occurs,
1877+
a timing attack could theoretically reveal information about the
1878+
types and lengths of a and b--but not their values.
1879+
[clinic start generated code]*/
1880+
1881+
static PyObject *
1882+
_hashlib_compare_digest_impl(PyObject *module, PyObject *a, PyObject *b)
1883+
/*[clinic end generated code: output=6f1c13927480aed9 input=9c40c6e566ca12f5]*/
1884+
{
1885+
int rc;
1886+
1887+
/* ASCII unicode string */
1888+
if(PyUnicode_Check(a) && PyUnicode_Check(b)) {
1889+
if (PyUnicode_READY(a) == -1 || PyUnicode_READY(b) == -1) {
1890+
return NULL;
1891+
}
1892+
if (!PyUnicode_IS_ASCII(a) || !PyUnicode_IS_ASCII(b)) {
1893+
PyErr_SetString(PyExc_TypeError,
1894+
"comparing strings with non-ASCII characters is "
1895+
"not supported");
1896+
return NULL;
1897+
}
1898+
1899+
rc = _tscmp(PyUnicode_DATA(a),
1900+
PyUnicode_DATA(b),
1901+
PyUnicode_GET_LENGTH(a),
1902+
PyUnicode_GET_LENGTH(b));
1903+
}
1904+
/* fallback to buffer interface for bytes, bytesarray and other */
1905+
else {
1906+
Py_buffer view_a;
1907+
Py_buffer view_b;
1908+
1909+
if (PyObject_CheckBuffer(a) == 0 && PyObject_CheckBuffer(b) == 0) {
1910+
PyErr_Format(PyExc_TypeError,
1911+
"unsupported operand types(s) or combination of types: "
1912+
"'%.100s' and '%.100s'",
1913+
Py_TYPE(a)->tp_name, Py_TYPE(b)->tp_name);
1914+
return NULL;
1915+
}
1916+
1917+
if (PyObject_GetBuffer(a, &view_a, PyBUF_SIMPLE) == -1) {
1918+
return NULL;
1919+
}
1920+
if (view_a.ndim > 1) {
1921+
PyErr_SetString(PyExc_BufferError,
1922+
"Buffer must be single dimension");
1923+
PyBuffer_Release(&view_a);
1924+
return NULL;
1925+
}
1926+
1927+
if (PyObject_GetBuffer(b, &view_b, PyBUF_SIMPLE) == -1) {
1928+
PyBuffer_Release(&view_a);
1929+
return NULL;
1930+
}
1931+
if (view_b.ndim > 1) {
1932+
PyErr_SetString(PyExc_BufferError,
1933+
"Buffer must be single dimension");
1934+
PyBuffer_Release(&view_a);
1935+
PyBuffer_Release(&view_b);
1936+
return NULL;
1937+
}
1938+
1939+
rc = _tscmp((const unsigned char*)view_a.buf,
1940+
(const unsigned char*)view_b.buf,
1941+
view_a.len,
1942+
view_b.len);
1943+
1944+
PyBuffer_Release(&view_a);
1945+
PyBuffer_Release(&view_b);
1946+
}
1947+
1948+
return PyBool_FromLong(rc);
1949+
}
1950+
18361951
/* List of functions exported by this module */
18371952

18381953
static struct PyMethodDef EVP_functions[] = {
18391954
EVP_NEW_METHODDEF
18401955
PBKDF2_HMAC_METHODDEF
18411956
_HASHLIB_SCRYPT_METHODDEF
18421957
_HASHLIB_GET_FIPS_MODE_METHODDEF
1958+
_HASHLIB_COMPARE_DIGEST_METHODDEF
18431959
_HASHLIB_HMAC_SINGLESHOT_METHODDEF
18441960
_HASHLIB_HMAC_NEW_METHODDEF
18451961
_HASHLIB_OPENSSL_MD5_METHODDEF

‎Modules/_operator.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,8 @@ _operator_length_hint_impl(PyObject *module, PyObject *obj,
785785
return PyObject_LengthHint(obj, default_value);
786786
}
787787

788+
/* NOTE: Keep in sync with _hashopenssl.c implementation. */
789+
788790
/*[clinic input]
789791
_operator._compare_digest = _operator.eq
790792

0 commit comments

Comments
 (0)