Kevin Christopher added the comment:

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 issue9216 (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 issue9216, 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.

----------
nosy: +kscguru
status: pending -> open
Added file: http://bugs.python.org/file46765/hashopenssl-fips-exceptions.patch

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue9146>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to