From 125430e7b15e8f7611da1a61dcaf163a9a44abab Mon Sep 17 00:00:00 2001 From: Marcel Plch Date: Mon, 29 Jul 2019 12:45:11 +0200 Subject: [PATCH 1/4] FIPS review * Port _hmacopenssl to multiphase init. * Make _hmacopenssl.HMAC.copy create same type as self. * hmac.py cosmetic nitpick --- Lib/hmac.py | 2 +- Modules/_hmacopenssl.c | 112 +++++++++++++++++++++++++---------------- 2 files changed, 70 insertions(+), 44 deletions(-) diff --git a/Lib/hmac.py b/Lib/hmac.py index ed98406bd2e182..b9bf16b84ca0ce 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -42,7 +42,7 @@ def __init__(self, key, msg = None, digestmod = None): if _hashlib.get_fips_mode(): raise ValueError( 'hmac.HMAC is not available in FIPS mode. ' - + 'Use hmac.new().' + 'Use hmac.new().' ) if not isinstance(key, (bytes, bytearray)): diff --git a/Modules/_hmacopenssl.c b/Modules/_hmacopenssl.c index ca95d725f019aa..216ed04f23603d 100644 --- a/Modules/_hmacopenssl.c +++ b/Modules/_hmacopenssl.c @@ -24,7 +24,10 @@ #include -static PyTypeObject HmacType; +typedef struct hmacopenssl_state { + PyTypeObject *HmacType; +} hmacopenssl_state; + typedef struct { PyObject_HEAD @@ -36,9 +39,9 @@ typedef struct { #include "clinic/_hmacopenssl.c.h" /*[clinic input] module _hmacopenssl -class _hmacopenssl.HMAC "HmacObject *" "&HmacType" +class _hmacopenssl.HMAC "HmacObject *" "PyModule_GetState(module)->HmacType" [clinic start generated code]*/ -/*[clinic end generated code: output=da39a3ee5e6b4b0d input=c98d3f2af591c085]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=204b7f45847f57b4]*/ /*[clinic input] @@ -56,11 +59,18 @@ _hmacopenssl_new_impl(PyObject *module, Py_buffer *key, const char *digestmod) /*[clinic end generated code: output=46f1cb4e02921922 input=be8c0c2e4fad508c]*/ { + hmacopenssl_state *state; + if (digestmod == NULL) { PyErr_SetString(PyExc_ValueError, "digestmod must be specified"); return NULL; } + state = PyModule_GetState(module); + if (state == NULL) { + return NULL; + } + /* name mut be lowercase */ for (int i=0; digestmod[i]; i++) { if ( @@ -105,7 +115,7 @@ _hmacopenssl_new_impl(PyObject *module, Py_buffer *key, goto error; } - retval = (HmacObject *)PyObject_New(HmacObject, &HmacType); + retval = (HmacObject *)PyObject_New(HmacObject, state->HmacType); if (retval == NULL) { goto error; } @@ -133,7 +143,9 @@ static PyObject * _hmacopenssl_HMAC_copy_impl(HmacObject *self) /*[clinic end generated code: output=fe5ee41faf30dcf0 input=f5ed20feec42d8d0]*/ { - HmacObject *retval = (HmacObject *)PyObject_New(HmacObject, &HmacType); + HmacObject *retval; + + retval = (HmacObject *)PyObject_New(HmacObject, (PyTypeObject *)PyObject_Type((PyObject *)self)); if (retval == NULL) { return NULL; } @@ -147,7 +159,7 @@ _hmacopenssl_HMAC_copy_impl(HmacObject *self) return _setException(PyExc_ValueError); } - return (PyObject*)retval; + return (PyObject *)retval; } static void @@ -338,19 +350,24 @@ Attributes:\n\ name -- the name, including the hash algorithm used by this object\n\ digest_size -- number of bytes in digest() output\n"); -static PyTypeObject HmacType = { - PyVarObject_HEAD_INIT(NULL, 0) - "_hmacopenssl.HMAC", /*tp_name*/ - sizeof(HmacObject), /*tp_basicsize*/ - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - .tp_doc = hmactype_doc, - .tp_repr = (reprfunc)_hmac_repr, - .tp_dealloc = (destructor)_hmac_dealloc, - .tp_methods = Hmac_methods, - .tp_getset = Hmac_getset, - .tp_members = Hmac_members, +static PyType_Slot HmacType_slots[] = { + {Py_tp_doc, hmactype_doc}, + {Py_tp_repr, (reprfunc)_hmac_repr}, + {Py_tp_dealloc,(destructor)_hmac_dealloc}, + {Py_tp_methods, Hmac_methods}, + {Py_tp_getset, Hmac_getset}, + {Py_tp_members, Hmac_members}, + {0, NULL} +}; + +PyType_Spec HmacType_spec = { + "_hmacopenssl.HMAC", /* name */ + sizeof(HmacObject), /* basicsize */ + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .slots = HmacType_slots, }; + static struct PyMethodDef hmacopenssl_functions[] = { _HMACOPENSSL_NEW_METHODDEF {NULL, NULL} /* Sentinel */ @@ -360,37 +377,46 @@ static struct PyMethodDef hmacopenssl_functions[] = { /* Initialize this module. */ - -static struct PyModuleDef _hmacopenssl_module = { - PyModuleDef_HEAD_INIT, - "_hmacopenssl", - NULL, - -1, - hmacopenssl_functions, - NULL, - NULL, - NULL, - NULL -}; - -PyMODINIT_FUNC -PyInit__hmacopenssl(void) -{ +static int +hmacopenssl_exec(PyObject *m) { /* TODO build EVP_functions openssl_* entries dynamically based * on what hashes are supported rather than listing many - * but having some be unsupported. Only init appropriate + * and having some unsupported. Only init appropriate * constants. */ + PyObject *temp; - Py_TYPE(&HmacType) = &PyType_Type; - if (PyType_Ready(&HmacType) < 0) - return NULL; + temp = PyType_FromSpec(&HmacType_spec); + if (temp == NULL) { + goto fail; + } - PyObject *m = PyModule_Create(&_hmacopenssl_module); - if (m == NULL) - return NULL; + if (PyModule_AddObject(m, "HMAC", temp) == -1) { + goto fail; + } + + return 0; - Py_INCREF((PyObject *)&HmacType); - PyModule_AddObject(m, "HMAC", (PyObject *)&HmacType); +fail: + Py_XDECREF(temp); + return -1; +} - return m; +static PyModuleDef_Slot hmacopenssl_slots[] = { + {Py_mod_exec, hmacopenssl_exec}, + {0, NULL}, +}; + +static struct PyModuleDef _hmacopenssl_def = { + PyModuleDef_HEAD_INIT, /* m_base */ + .m_name = "_hmacopenssl", + .m_methods = hmacopenssl_functions, + .m_slots = hmacopenssl_slots, + .m_size = sizeof(hmacopenssl_state) +}; + + +PyMODINIT_FUNC +PyInit__hmacopenssl(void) +{ + return PyModuleDef_Init(&_hmacopenssl_def); } From a7996ce2dcddaa90959ecfb52b42cd94414320a3 Mon Sep 17 00:00:00 2001 From: Marcel Plch Date: Mon, 29 Jul 2019 13:05:04 +0200 Subject: [PATCH 2/4] revert cosmetic nitpick and remove trailing whitespace --- Lib/hmac.py | 2 +- Modules/_hmacopenssl.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/hmac.py b/Lib/hmac.py index b9bf16b84ca0ce..ed98406bd2e182 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -42,7 +42,7 @@ def __init__(self, key, msg = None, digestmod = None): if _hashlib.get_fips_mode(): raise ValueError( 'hmac.HMAC is not available in FIPS mode. ' - 'Use hmac.new().' + + 'Use hmac.new().' ) if not isinstance(key, (bytes, bytearray)): diff --git a/Modules/_hmacopenssl.c b/Modules/_hmacopenssl.c index 216ed04f23603d..221714ca434913 100644 --- a/Modules/_hmacopenssl.c +++ b/Modules/_hmacopenssl.c @@ -363,7 +363,7 @@ static PyType_Slot HmacType_slots[] = { PyType_Spec HmacType_spec = { "_hmacopenssl.HMAC", /* name */ sizeof(HmacObject), /* basicsize */ - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, .slots = HmacType_slots, }; @@ -407,7 +407,7 @@ static PyModuleDef_Slot hmacopenssl_slots[] = { }; static struct PyModuleDef _hmacopenssl_def = { - PyModuleDef_HEAD_INIT, /* m_base */ + PyModuleDef_HEAD_INIT, /* m_base */ .m_name = "_hmacopenssl", .m_methods = hmacopenssl_functions, .m_slots = hmacopenssl_slots, From 6b74d9ea9ced573d72e4d34ad0c08cc2ee703b30 Mon Sep 17 00:00:00 2001 From: Charalampos Stratakis Date: Wed, 31 Jul 2019 15:43:43 +0200 Subject: [PATCH 3/4] Add initial tests for various hashes under FIPS mode --- Lib/test/test_fips.py | 64 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 Lib/test/test_fips.py diff --git a/Lib/test/test_fips.py b/Lib/test/test_fips.py new file mode 100644 index 00000000000000..bee911ef405aba --- /dev/null +++ b/Lib/test/test_fips.py @@ -0,0 +1,64 @@ +import unittest +import hmac, _hmacopenssl +import hashlib, _hashlib + + + +class HashlibFipsTests(unittest.TestCase): + + @unittest.skipUnless(hashlib.get_fips_mode(), "Test only when FIPS is enabled") + def test_fips_imports(self): + """blake2s and blake2b should fail to import in FIPS mode + """ + with self.assertRaises(ValueError, msg='blake2s not available in FIPS'): + m = hashlib.blake2s() + with self.assertRaises(ValueError, msg='blake2b not available in FIPS'): + m = hashlib.blake2b() + + def compare_hashes(self, python_hash, openssl_hash): + """ + Compare between the python implementation and the openssl one that the digests + are the same + """ + if python_hash.name.startswith('shake_128'): + m = python_hash.hexdigest(16) + elif python_hash.name.startswith('shake_256'): + m = python_hash.hexdigest(32) + else: + m = python_hash.hexdigest() + h = openssl_hash.hexdigest() + + self.assertEqual(m, h) + + @unittest.skipIf(hashlib.get_fips_mode(), "blake2 hashes are not available under FIPS") + def test_blake2_hashes(self): + self.compare_hashes(hashlib.blake2b(b'abc'), _hashlib.openssl_blake2b(b'abc')) + self.compare_hashes(hashlib.blake2s(b'abc'), _hashlib.openssl_blake2s(b'abc')) + + def test_sha3_hashes(self): + self.compare_hashes(hashlib.sha3_224(b'abc'), _hashlib.openssl_sha3_224(b'abc')) + self.compare_hashes(hashlib.sha3_256(b'abc'), _hashlib.openssl_sha3_256(b'abc')) + self.compare_hashes(hashlib.sha3_384(b'abc'), _hashlib.openssl_sha3_384(b'abc')) + self.compare_hashes(hashlib.sha3_512(b'abc'), _hashlib.openssl_sha3_512(b'abc')) + + @unittest.skipIf(hashlib.get_fips_mode(), "shake hashes are not available under FIPS") + def test_shake_hashes(self): + self.compare_hashes(hashlib.shake_128(b'abc'), _hashlib.openssl_shake_128(b'abc')) + self.compare_hashes(hashlib.shake_256(b'abc'), _hashlib.openssl_shake_256(b'abc')) + + def test_sha(self): + self.compare_hashes(hashlib.sha1(b'abc'), _hashlib.openssl_sha1(b'abc')) + self.compare_hashes(hashlib.sha224(b'abc'), _hashlib.openssl_sha224(b'abc')) + self.compare_hashes(hashlib.sha256(b'abc'), _hashlib.openssl_sha256(b'abc')) + self.compare_hashes(hashlib.sha384(b'abc'), _hashlib.openssl_sha384(b'abc')) + self.compare_hashes(hashlib.sha512(b'abc'), _hashlib.openssl_sha512(b'abc')) + + def test_hmac_digests(self): + self.compare_hashes(_hmacopenssl.new(b'My hovercraft is full of eels', digestmod='sha384'), + hmac.new(b'My hovercraft is full of eels', digestmod='sha384')) + + + + +if __name__ == "__main__": + unittest.main() From 64863fb86a34d12b103d18c3a0ae639b4193fa82 Mon Sep 17 00:00:00 2001 From: Marcel Plch Date: Thu, 1 Aug 2019 16:39:37 +0200 Subject: [PATCH 4/4] Initialize HMAC type. --- Modules/_hmacopenssl.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Modules/_hmacopenssl.c b/Modules/_hmacopenssl.c index 221714ca434913..239445a0831b24 100644 --- a/Modules/_hmacopenssl.c +++ b/Modules/_hmacopenssl.c @@ -22,12 +22,12 @@ #include "_hashopenssl.h" -#include typedef struct hmacopenssl_state { PyTypeObject *HmacType; } hmacopenssl_state; +#include typedef struct { PyObject_HEAD @@ -39,7 +39,7 @@ typedef struct { #include "clinic/_hmacopenssl.c.h" /*[clinic input] module _hmacopenssl -class _hmacopenssl.HMAC "HmacObject *" "PyModule_GetState(module)->HmacType" +class _hmacopenssl.HMAC "HmacObject *" "((hmacopenssl_state *)PyModule_GetState(module))->HmacType" [clinic start generated code]*/ /*[clinic end generated code: output=da39a3ee5e6b4b0d input=204b7f45847f57b4]*/ @@ -71,7 +71,7 @@ _hmacopenssl_new_impl(PyObject *module, Py_buffer *key, return NULL; } - /* name mut be lowercase */ + /* name must be lowercase */ for (int i=0; digestmod[i]; i++) { if ( ((digestmod[i] < 'a') || (digestmod[i] > 'z')) @@ -383,7 +383,8 @@ hmacopenssl_exec(PyObject *m) { * on what hashes are supported rather than listing many * and having some unsupported. Only init appropriate * constants. */ - PyObject *temp; + PyObject *temp = NULL; + hmacopenssl_state *state; temp = PyType_FromSpec(&HmacType_spec); if (temp == NULL) { @@ -394,6 +395,9 @@ hmacopenssl_exec(PyObject *m) { goto fail; } + state = PyModule_GetState(m); + state->HmacType = (PyTypeObject *)temp; + return 0; fail: