Segfault in hashlib in OpenSSL FIPS mode using non-FIPS-compliant hashes, if "ssl" imported before "hashlib" #53392
Comments
|
Having run: but the following segfaults: 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): (gdb) bt (gdb) p self->ctx 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 #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 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: Thoughts? |
davidmalcolm
added
stdlib
|
First, is it only with 2.7 or 2.6? |
|
Thanks
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): |
Nice, so at least that oddity is eliminated :) So I guess it's down to the A, B, and C approaches you suggested. |
|
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. |
|
Not quite ready yet: Named methods work: but lookup by name still fails: |
|
I'm attaching an updated patch which:
Note that in this mode:
>>> _hashlib.new('md5')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
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. |
|
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. |
|
Thanks. The relevant code in setup.py is all wrapped with --pydebug: 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 "<stdin>", line 1, in <module>
File "/home/david/coding/python-svn/2.7-fips/Lib/md5.py", line 10, in <module>
from hashlib import md5
File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 136, in <module>
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 "<stdin>", line 1, in <module>
File "/home/david/coding/python-svn/2.7-fips/Lib/hashlib.py", line 136, in <module>
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 _md5Changing: --- 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 <module>
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 "<stdin>", line 1, in <module>
File "/home/david/coding/python-svn/2.7-fips/Lib/md5.py", line 10, in <module>
from hashlib import md5
ImportError: cannot import name md5 |
|
I've filed bpo-9216 to discuss this at a higher level, with an API proposal |
|
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? |
|
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 bpo-9216 (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 bpo-9216, 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. |
|
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. |
|
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). |
|
Looks like this is complete and can be closed. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: