Issue 9146: Segfault in hashlib in OpenSSL FIPS mode using non-FIPS-compliant hashes, if "ssl" imported before "hashlib" (original) (raw)
Created on 2010-07-02 19:10 by dmalcolm, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Messages (18)
Author: Dave Malcolm (dmalcolm) 
Date: 2010-07-02 19:10
Having run: prelink --undo --all the following works OK: OPENSSL_FORCE_FIPS_MODE=1 python -c "import hashlib; m = m = hashlib.md5(); m.update('abc')"
but the following segfaults: OPENSSL_FORCE_FIPS_MODE=1 python -c "import ssl; import hashlib; m = m = hashlib.md5(); m.update('abc')"
and the following gives the same segfault, in a simpler way (strictly speaking one shouldn't directly import _ssl, but this most directly reproduces the crash): OPENSSL_FORCE_FIPS_MODE=1 python -c "import _ssl; import hashlib; m = m = hashlib.md5(); m.update('abc')"
(gdb) bt #0 0x0000000000000000 in ?? () #1 0x00007fffee978736 in EVP_hash (self=0xaed3c0, vp=0x95b6a4, len=3) at /home/david/coding/python-svn/trunk-fips/Modules/_hashopenssl.c:112 #2 0x00007fffee978cb5 in EVP_update (self=0xaed3c0, args=('abc',)) at /home/david/coding/python-svn/trunk-fips/Modules/_hashopenssl.c:247 #3 0x0000000000567faa in PyCFunction_Call (func=<built-in method update of _hashlib.HASH object at remote 0xaed3c0>, arg=('abc',), kw=0x0) at Objects/methodobject.c:81 and the segfault is due to EVP_DigestUpdate calling through the ctx->update function pointer, which in this case is NULL.
(gdb) p self->ctx $2 = {digest = 0x7ffff0c955a0, engine = 0x0, flags = 0, md_data = 0x0, pctx = 0x0, update = 0}
self->ctx->update == NULL and self->ctx->digest == &bad_md, as set up by:
The setup is here: (different run of gdb, so different addresses):
(gdb) bt #0 EVP_DigestInit_ex (ctx=0x7fffecdc7b80, type=0x7fffefc6fc60, impl=0x0) at digest.c:249 #1 0x00007fffecbc53ce in init_hashlib () at /usr/src/debug/Python-2.6.2/Modules/_hashopenssl.c:513
#ifdef OPENSSL_FIPS if (FIPS_mode()) { if (!(type->flags & EVP_MD_FLAG_FIPS) && !(ctx->flags & EVP_MD_CTX_FLAG_NON_FIPS_ALLOW)) { EVPerr(EVP_F_EVP_DIGESTINIT_EX, EVP_R_DISABLED_FOR_FIPS); => ctx->digest = &bad_md; return 0; } } #endif
(seen on x86_64 Linux; Fedora 13, with openssl-1.0.0-1.fc13.x86_64 on SVN trunk, and on other builds)
Clearly, the Python process should not segfault or abort.
I don't think it's clear what the behavior should be here, though - how is the Python standard library to be used in conjunction with OpenSSL's FIPS mode?
From page 18 of the OpenSSL's FIPS guide: http://ftp.openssl.org/docs/fips/UserGuide.pdf "By design, the OpenSSL API attempts to disable nonFIPS algorithms, when in FIPS mode, at the EVP level and via most low level function calls. Failure to check the return code from low level functions could result in unexpected behavior. Note also that sufficiently creative or unusual use of the API may still allow the use of nonFIPS algorithms. The nonFIPS algorithm disabling is intended as a aid to the developer in preventing the accidental use of nonFIPS algorithms in FIPS mode, and not as an absolute guarantee. It is the responsibility of the application developer to ensure that no nonFIPS algorithms are used when in FIPS mode."
It seems odd that the behavior of "md5" within "hashlib" can vary depending on whether "_ssl" was imported first. Reducing surprises for the programmer suggests to me that the behavior for these two cases should be harmonized.
It also seems odd that SSL can refuse the usage of MD5 whilst other implementations exist and are available; my understanding of FIPS mode is that it's intended for locked-down environments that wish to ensure that only known-good crypto is used (and e.g. MD5 is known to be be broken for some uses, see: http://www.kb.cert.org/vuls/id/836068)
Possible approaches: (A) change hashlib so that it continues to support md5 if ssl/_ssl is imported first, even in FIPS mode (B) change hashlib so that in FIPS mode, it fails to support md5 even if ssl/_ssl is not imported first (B.1) by not recognizing "md5" as a digest, leading to NameError exceptions and similar (I dislike this approach, as it leads to obscure failures) (B.2) by raising a new exception e.g. "ProhibitedInFIPSMode" or somesuch (so that the failure is obvious and searchable) (C.) as per (B.), but support an explicit override: hashlib.md5(non_fips_allow=True)
Thoughts?
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-02 19:27
First, is it only with 2.7 or 2.6? Second, I don't really get the point of the FIPS mode. The PDF you linked to seems full of bureaucratic jargon. Third, I can't reproduce under Mandriva, but perhaps it's because it's using OpenSSL 1.0.0 (which the PDF says isn't supported). Fourth, if MD5 is insecure and FIPS disables insecure algorithm, then why should hashlib allow MD5 hashing when FIPS mode is enabled? Fifth, please take a look at the OpenSSL initialization routine in _sslmodule.c and try to transplant it to the hashlib initialization routine:
/* Init OpenSSL */
SSL_load_error_strings();
SSL_library_init();
OpenSSL_add_all_algorithms();Author: Dave Malcolm (dmalcolm)
Date: 2010-07-02 19:46
Thanks
First, is it only with 2.7 or 2.6? I've seen this with both 2.6 tarball builds and SVN trunk; in both cases against openssl-1.0.0-1.[
Second, I don't really get the point of the FIPS mode. The PDF you linked to seems full of bureaucratic jargon. Agreed.
Third, I can't reproduce under Mandriva, but perhaps it's because it's using OpenSSL 1.0.0 (which the PDF says isn't supported). Thanks for the info. This is my fault, sorry. I'm using openssl-1.0.0-1 on Fedora; on doublechecking I see that we appear to be carrying various patches in our openssl packages, which I assume are a backport from a more recent version of the FIPS support: (see http://cvs.fedoraproject.org/viewvc/devel/openssl/ ); I'll see if I can get more information from other Fedorans to verify this, and on what version of openssl upstream this is equivalent to.
Fourth, if MD5 is insecure and FIPS disables insecure algorithm, then why should hashlib allow MD5 hashing when FIPS mode is enabled? My understanding is that some sites wish to prohibit the use of MD5 as an authentication mechanism, but don't need to prohibit it where the use is not "cryptography", e.g. to compute an index into a hash table where intentionally induced hash collisions can not break the application's security.
Fifth, please take a look at the OpenSSL initialization routine in _sslmodule.c and try to transplant it to the hashlib initialization routine: [snip]
Done; with the attached patch to SVN trunk, I don't need the initial "import ssl" to reproduce the segfault; the following will segfault (in the same place): OPENSSL_FORCE_FIPS_MODE=1 gdb --args ./python -c "import hashlib; m = m = hashlib.md5(); m.update('abc')"
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-02 19:54
with the attached patch to SVN trunk, I don't need the initial "import ssl" to reproduce the segfault
Nice, so at least that oddity is eliminated :)
So I guess it's down to the A, B, and C approaches you suggested. Of course, if we choose to allow MD5, we must find a way for OpenSSL to accept running an MD5 hash...
Author: Dave Malcolm (dmalcolm)
Date: 2010-07-02 23:57
Attached patch checks for errors in the initialization of _hashlib, and only registers the names that are actually available.
It also contains the ssl init from the first patch.
I added a _hashlib._errors dict, containing errors, so that you can examine them at runtime:
$ OPENSSL_FORCE_FIPS_MODE=1 ./python Python 2.7rc2+ (trunk:82445, Jul 2 2010, 14:00:30) [GCC 4.4.3 20100422 (Red Hat 4.4.3-18)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import _hashlib [35786 refs] _hashlib._errors {'md5': '_hashopenssl.c:541: error:060800A0:digital envelope routines:EVP_DigestInit_ex:unknown cipher'} [35825 refs] dir (_hashlib) ['doc', 'file', 'name', 'package', '_errors', 'new', 'openssl_sha1', 'openssl_sha224', 'openssl_sha256', 'openssl_sha384', 'openssl_sha512'] [35838 refs] (note the absence of openssl_md5)
Note that hashlib (as opposed to _hashlib) seems to gracefully fall back to Python's _md5 module when in this state:
import hashlib [36107 refs] m = m = hashlib.md5(); m.update('abc\n'); print m.hexdigest() 0bee89b07a248e27c83fc3d5951213c1 [36109 refs]
This seems to be option (A) from my initial message.
Author: Dave Malcolm (dmalcolm)
Date: 2010-07-03 00:03
Not quite ready yet:
Named methods work: $ OPENSSL_FORCE_FIPS_MODE=1 ./python -c "import hashlib; m = hashlib.md5(); m.update('abc\n'); print m.hexdigest()"0bee89b07a248e27c83fc3d5951213c1 [15741 refs]
but lookup by name still fails: OPENSSL_FORCE_FIPS_MODE=1 ./python -c "import hashlib; m = hashlib.new('md5'); m.update('abc\n'); print m.hexdigest()" Segmentation fault (core dumped)
Author: Dave Malcolm (dmalcolm)
Date: 2010-07-06 17:44
I'm attaching an updated patch which:
- adds error checking to the various places where EVP_DigestInit is called
- adds a test to test_hashlib to ensure that hashlib still works gracefully when OPENSSL_FORCE_FIPS_MODE=1 is set in the environment
Note that in this mode:
_hashlib.new('md5') Traceback (most recent call last): File "", line 1, in ValueError: error:060800A0:digital envelope routines:EVP_DigestInit_ex:unknown cipher [57670 refs]
but hashlib falls back to using the "md5" module instead.
I started writing a test for _hashlib (as opposed to hashlib), but it's too hard to express a runtime conditional on whether OPENSSL_FORCE_FIPS_MODE will actually affect the behavior of EVP_DigestInit across the versions of openssl that might be installed on the system.
I'm still waiting to hear back from the Fedora OpenSSL packager for info on how to reproduce this on a vanilla OpenSSL.
Author: Gregory P. Smith (gregory.p.smith) *
Date: 2010-07-07 04:35
I'm pretty sure Python setup.py does not build the non-openssl md5, sha1, sha256 and sha512 extension modules at all when openssl is present. So falling back on them is not likely to work unless anyone who wants this crazy force fips mode thing to not prevent them from existing in Python goes to the extra effort to compile them.
Author: Dave Malcolm (dmalcolm)
Date: 2010-07-07 17:19
Thanks.
The relevant code in setup.py is all wrapped with --pydebug: if COMPILED_WITH_PYDEBUG or not have_usable_openssl:
All of my testing had been --with-pydebug.
Rebuilding without --with-pydebug leads to some interesting failures; as you say, if OpenSSL has md5 support disabled, this isn't going to work unless the _md5 module is manually enabled.
Notes on some of the failures seen:
$ OPENSSL_FORCE_FIPS_MODE=1 ./python Python 2.7.0+ (trunk:82622M, Jul 7 2010, 12:08:16) [GCC 4.4.3 20100422 (Red Hat 4.4.3-18)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import md5 Traceback (most recent call last): File "", line 1, in File "/home/david/coding/python-svn/2.7-fips/Lib/md5.py", line 10, in from hashlib import md5 File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 136, in globals()[__func_name] = __get_hash(__func_name) File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 100, in __get_openssl_constructor return __get_builtin_constructor(name) File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 71, in __get_builtin_constructor import _md5 ImportError: No module named _md5
import hashlib Traceback (most recent call last): File "", line 1, in File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 136, in globals()[__func_name] = __get_hash(__func_name) File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 100, in __get_openssl_constructor return __get_builtin_constructor(name) File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 71, in __get_builtin_constructor import _md5 ImportError: No module named _md5
Changing: Index: Lib/hashlib.py
--- Lib/hashlib.py (revision 82622) +++ Lib/hashlib.py (working copy) @@ -134,7 +134,7 @@ # version not supporting that algorithm. try: globals()[__func_name] = __get_hash(__func_name)
- except ValueError:
- except (ValueError, ImportError): import logging logging.exception('code for hash %s was not found.', __func_name)
indicates that it's just md5 that's not found, and enables "import hashlib" to work, albeit with a traceback upon import: ERROR:root:code for hash md5 was not found. Traceback (most recent call last): File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 136, in globals()[__func_name] = __get_hash(__func_name) File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 100, in __get_openssl_constructor return __get_builtin_constructor(name) File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 71, in __get_builtin_constructor import _md5 ImportError: No module named _md5
"import md5" also then fails in an obscure way:
import md5 Traceback (most recent call last): File "", line 1, in File "/home/david/coding/python-svn/2.7-fips/Lib/md5.py", line 10, in from hashlib import md5 ImportError: cannot import name md5
Author: Dave Malcolm (dmalcolm)
Date: 2010-07-10 00:23
I've filed issue 9216 to discuss this at a higher level, with an API proposal
Author: Christian Heimes (christian.heimes) *
Date: 2016-09-08 14:50
I can no longer reproduce the crash with Python 2.7 and 3.5 (Fedora 24 with OpenSSL 1.0.2h). Is this still a problem for you?
Author: Kevin Christopher (kscguru) *
Date: 2017-03-30 01:26
I tripped over this exact issue a few months ago, while working on a FIPSified OpenSSL library (custom platform).
Attached a different (more minimal) patch; this one focuses more narrowly on the exact failure mode. It's based on 'master' (~3.7), applies cleanly / tests_hashlib passes. My employer has been using this patch successfully (in a FIPS environment) for ~3 months now, and I'm looking to upstream.
Earlier, dmalcolm identified that OpenSSL's EVP_DigestUpdate dereferences a NULL in FIPS mode on non-FIPS algorithms. OpenSSL is not quite that bad ... turns out, EVP_DigestInit returns an error if FIPS disallows an algorithm, and ignoring the error (which _hashopenssl.c currently does) results in later dereferencing NULL. It's straightforward to add the right error checks and convert them to Python exceptions, which seems the most Pythonic way to handle the error - and it also minimizes the FIPS-only codepaths.
There's one catch, _hashopenssl.c likes to pre-construct several algorithms at module import and raising an exception during import leads hashlib.py to silently drop the exception (hashlib.py:158-161). So I made the pre-constructors lazy: the first use of any constructor will attempt to initialize it, and raise the exception on first use if FIPS mode makes it unusable. The reason for choosing this approach is Lib/hashlib.py and __get_openssl_constructor(), which already has logic to handle exactly this ValueError by falling back to __get_builtin_constructor() (and the less-optimized _md5/_sha1/_sha256 modules).
In the context of (a usedforsecurity flag which can be passed to OpenSSL to allow non-FIPS algorithms), this change has the net effect of silently falling back from OpenSSL's MD5 implementation to python's _md5 C code when in FIPS mode. That's not exactly the intent of FIPS mode (python's _md5 module is certainly not FIPS-compliant), but converting a crash to a catchable exception is a major improvement. I like the discussion in , and see this change as complementary: it avoids crashing and moves policy handling of how to handle FIPS mode _hashopenssl.c to hashlib.py, and my gut feeling is that policy logic is better placed in a .py library than in a .c library.
The advantage of this patch is that the new exceptions are obviously correct regardless of FIPS considerations, and the lazy constructors maintain the spirit of amortizing EVP_DigestInit calls while also reporting any error at a more useful point.
Author: Gregory P. Smith (gregory.p.smith) *
Date: 2017-03-30 02:21
I like your patch, raising an exception is indeed the right thing to do. i'll get this patch in.
whether or not the built-in non-openssl based _md5 and _sha1 module exist in "fips" mode is a separate build time issue - lets keep this one just dealing with the always undesirable segfault.
Author: Gregory P. Smith (gregory.p.smith) *
Date: 2017-05-24 07:04
New changeset 07244a83014fad42da937c17d98474b47a570bf7 by Gregory P. Smith in branch 'master': bpo-9146: Raise a ValueError if OpenSSL fails to init a hash func. (#1777) https://github.com/python/cpython/commit/07244a83014fad42da937c17d98474b47a570bf7
Author: Gregory P. Smith (gregory.p.smith) *
Date: 2017-05-24 17:44
Resolved for 3.7, assigning to christian to deal with the backports as I believe he has employer motivation to see those in (should be trivial).
Author: Gregory P. Smith (gregory.p.smith) *
Date: 2017-09-03 21:08
New changeset 4f013881cb0ca7d29620ddb0594dde09bc5d24ca by Gregory P. Smith in branch 'master': bpo-9146: add the missing NEWS entry. (#3275) https://github.com/python/cpython/commit/4f013881cb0ca7d29620ddb0594dde09bc5d24ca
Author: Gregory P. Smith (gregory.p.smith) *
Date: 2017-09-03 21:35
New changeset 31b8efeaa893e95358b71eb2b8365552d3966b4a by Gregory P. Smith in branch '3.6': [3.6] bpo-9146: Raise a ValueError if OpenSSL fails to init a hash func (#3274) https://github.com/python/cpython/commit/31b8efeaa893e95358b71eb2b8365552d3966b4a
Author: Irit Katriel (iritkatriel) *
Date: 2020-09-19 11:53
Looks like this is complete and can be closed.
History
Date
User
Action
Args
2022-04-11 14:57:03
admin
set
github: 53392
2020-09-19 23:56:13
gregory.p.smith
set
status: open -> closed
stage: backport needed -> resolved
2020-09-19 11:53:54
iritkatriel
set
nosy: + iritkatriel
messages: +
2017-09-03 21:35:21
gregory.p.smith
set
messages: +
2017-09-03 21:08:50
gregory.p.smith
set
messages: +
2017-09-03 20:28:34
gregory.p.smith
set
pull_requests: + <pull%5Frequest3318>
2017-09-03 20:11:41
gregory.p.smith
set
pull_requests: + <pull%5Frequest3317>
2017-05-25 13:26:23
cstratak
set
nosy: + cstratak
2017-05-24 17:44:38
gregory.p.smith
set
versions: + Python 3.5, - Python 3.7
messages: +
assignee: gregory.p.smith -> christian.heimes
resolution: fixed
stage: patch review -> backport needed
2017-05-24 07:04:40
gregory.p.smith
set
messages: +
2017-05-23 23:09:32
gregory.p.smith
set
pull_requests: + <pull%5Frequest1859>
2017-03-30 02:21:47
gregory.p.smith
set
assignee: gregory.p.smith
messages: +
versions: + Python 3.6, Python 3.7
2017-03-30 01:26:02
kscguru
set
files: + hashopenssl-fips-exceptions.patch
status: pending -> open
nosy: + kscguru
messages: +
2016-09-08 14:50:01
christian.heimes
set
status: open -> pending
messages: +
2013-08-24 22:36:08
dstufft
set
nosy: + dstufft
2013-07-20 17:27:27
jpokorny
set
nosy: + jpokorny
2012-10-06 23:45:18
christian.heimes
set
nosy: + christian.heimes
2010-07-10 00:23:32
dmalcolm
set
messages: +
2010-07-07 17:19:01
dmalcolm
set
messages: +
2010-07-07 04:36:00
gregory.p.smith
set
nosy: - gps
2010-07-07 04:35:31
gregory.p.smith
set
nosy: + gregory.p.smith
messages: +
2010-07-06 17:44:55
dmalcolm
set
files: + hashopenssl-fips-mode-errors-v3.patch
messages: +
2010-07-03 00:03:34
dmalcolm
set
messages: +
2010-07-02 23:57:07
dmalcolm
set
files: + remove-unusable-hashes-from-hashopenssl.patch
messages: +
stage: patch review
2010-07-02 19:54:57
pitrou
set
messages: +
2010-07-02 19:46:45
dmalcolm
set
files: + add_ssl_init_to_hashlib.patch
keywords: + patch
messages: +
2010-07-02 19:27:45
pitrou
set
nosy: + pitrou, gps
messages: +
2010-07-02 19:10:08
dmalcolm
create