Disabling HMAC in FIPS mode is not a solution we tend to pursue, all it does is make any python applications depending on that interface broken instead of not compliant in FIPS mode.

Hashes are FIPS relevant only when used for protecting user data, in case of HMAC that is always the case, in other cases it is less clear. The best solution, of course, is to also back hashes onto OpenSSL (which usually gives also a performace boost compared to pure python implementations).
It also helps with the fact that openssl will prevent use of unapproved hashes in FIPS mode and will be appropriately updated over time as FIPS approved algorithms change.

Is there any reason why the HMAC interface can't be made to use OpenSSL behind it ?

As for disabling stuff my recommendation is in order:
1) Use OpenSSL so that OpenSSL itself will care about what to allow/disallow in FIPS mode
2) Introduce crypto-policies backends for python so that we can control centrally and update over time the DEFAULT and the FIPS policies as time passes.
3) remove algorithms
4) detect FIPS mode and disable unapproved algorithms

Detecting fips mode and disabling on your own is last resort because it requires someone to then track changes in FIPS policies and change code in your project specifically. This is a brittle process and leads to inconsistent behaviour down the line when we change policies for the system but forget to reach to all custom things done in various random packages.

HTH.

> The best solution, of course, is to also back hashes onto OpenSSL.

Python uses OpenSSL by default, except for SHA3/BLAKE2, where OpenSSL doesn't have the required features.
But currently, users are free to use any other hash. (Plus there are fallbacks if OpenSSL is unavailable/not working; we will, of course, disable those.)


> Is there any reason why the HMAC interface can't be made to use OpenSSL behind it ?

Python's current HMAC interface allows *any* hashing function -- even a user-written one. (Technically, people can use any object implementing the [PEP 247] interface.)
Commonly, people use wrappers over OpenSSL functions, but it seems that determining if something is such a wrapper would be fragile.
We could limit HMAC to accept only strings that name OpenSSL hashes, but that would mean we don't keep the current API.


> Detecting fips mode and disabling on your own is last resort because it requires someone to then track changes in FIPS policies and change code in your project specifically. This is a brittle process and leads to inconsistent behaviour down the line when we change policies for the system but forget to reach to all custom things done in various random packages.

I meant something slightly different:

- In FIPS mode, only allow OpenSSL hashes. (For HMAC, possibly only allow specifying them by name.)
- Outside FIPS mode, allow any hash. SHA3/BLAKE2 would use Python's implementation and extended API.


[PEP 247]: https://www.python.org/dev/peps/pep-0247/

(In reply to Petr Viktorin from comment #5)
> > The best solution, of course, is to also back hashes onto OpenSSL.
> 
> Python uses OpenSSL by default, except for SHA3/BLAKE2, where OpenSSL
> doesn't have the required features.
> But currently, users are free to use any other hash. (Plus there are
> fallbacks if OpenSSL is unavailable/not working; we will, of course, disable
> those.)

This is fine.

> 
> > Is there any reason why the HMAC interface can't be made to use OpenSSL behind it ?
> 
> Python's current HMAC interface allows *any* hashing function -- even a
> user-written one. (Technically, people can use any object implementing the
> [PEP 247] interface.)
> Commonly, people use wrappers over OpenSSL functions, but it seems that
> determining if something is such a wrapper would be fragile.
> We could limit HMAC to accept only strings that name OpenSSL hashes, but
> that would mean we don't keep the current API.

Could you expose a method or just a variable in the object that exists only if the object is an openssl wrapper ?
This way you just need to ensure the openssl wrappers expose it.

If this variable/method is present you always use the openssl hmac interface, if not you fall back to the current code.

You refuse to operate with any object without the right marker in FIPS mode (still requires FIPS mode detection but not the need to keep around lists of hashes or breaking the API). 

> > Detecting fips mode and disabling on your own is last resort because it requires someone to then track changes in FIPS policies and change code in your project specifically. This is a brittle process and leads to inconsistent behaviour down the line when we change policies for the system but forget to reach to all custom things done in various random packages.
> 
> I meant something slightly different:
> 
> - In FIPS mode, only allow OpenSSL hashes. (For HMAC, possibly only allow
> specifying them by name.)

You should be able to maintain the HMAC interface and still make sure only approved hashes and interfaces will be used without the need to maintain and update lists of strings outside of the wrappers if you can implement something as described above. Less fragile IMHO, the only gotcha is to be able to bubble up the proper exception when FIPS mode denies the use of one of the hashes in the openssl library.

> - Outside FIPS mode, allow any hash. SHA3/BLAKE2 would use Python's
> implementation and extended API.

Sure, see above for how it would be safer to do it so that you do not need to have extra lists of FIPS approved algorithms or break the API (hopefully).
 
> 
> [PEP 247]: https://www.python.org/dev/peps/pep-0247/

> Could you expose a method or just a variable in the object that exists only if the object is an openssl wrapper ?
> This way you just need to ensure the openssl wrappers expose it.

That could work!
Do we need to worry about the user adding this marker on their own hasher, and passing it to HMAC?

(In reply to Petr Viktorin from comment #7)
> > Could you expose a method or just a variable in the object that exists only if the object is an openssl wrapper ?
> > This way you just need to ensure the openssl wrappers expose it.
> 
> That could work!
> Do we need to worry about the user adding this marker on their own hasher,
> and passing it to HMAC?

I do not think so, as soon as they do that they should get a nice and explanatory segfault as soon as the openssl HMAC is called anyway.
Ideally this variable holds the openssl Hash context anyway and is clearly named something like openssl_hash_ctx ...

Here's a summary (mini design document). Simo & Christian, could you check this makes sense?

## Issues & Upstream status quo
1. Python re-implements hash algorithms (md5, sha1, sha256, sha512)
   - Only used as fallbacks if OpenSSL is not available 
2. Python extends hash algorithms (shake, blake)
   - extra features: blake2 – key/tree hashing; shake – custom hash size
   - OpenSSL is never used for these
3. Pure-Python HMAC class
   - Implements own crypto
   - Underlying hash is a parameter; can be any object with a “hasher interface” (PEP 452)

## Guiding principles for RHEL8 changes
- FIPS mode off: Functionality is same as upstream (except details like function names/types)
- FIPS mode on: Functionality is *either* same as upstream *or* an exception is raised
- FIPS mode on: All hashes and HMAC are provided by OpenSSL

## Changes in RHEL8
1. Fallback hashes: remove implementations
   - Fallback implementations md5, sha1, sha256, sha512 are removed
2. Extended hashes:  force OpenSSL in FIPS
   - OpenSSL wrappers for the hashes blake2{b512,s256}, sha3_{224,256,384,512}, shake_{128,256} are now exported from _hashlib 
   - Under FIPS mode, the _blake2 and _sha3 modules raise ImportError; their hashes (including shake*) are unusable
   - Hashlib falls back to OpenSSL if _blake2 or _sha3 are not importable
3. HMAC:  new OpenSSL wrapper forced in FIPS mode
   - New _hmacopenssl  module exports an HMAC class with hmac.HMAC API
     except it can only be instantiated with an OpenSSL hash name as string
   - Under FIPS mode, hmac.new instantiates this wrapper.
     It extracts hash name from _hashlib’s wrappers of OpenSSL hashes; fails for custom hasher objects

## WIP Implementation
Git: https://github.com/encukou/cpython -- branch fips
Web: https://github.com/python/cpython/compare/3.6...encukou:fips


If you prefer, a Google doc version (with additional volatile implementation issues and upstreaming notes) is at: https://docs.google.com/document/d/1Y1nxEKQCSaf1Eq8pq1KBbH2zM7-fDGtLKOUL0i9YaRk/edit

Looks mostly good except for a coupls of things I would like more input on.

Why can't hmac.HMAC be switched to use openssl hmac in FIPS mode ?
It sounds like this will break applications that use hmac.HMAC then run in FIPS mode done this way. Or is it hmac.HMAC a relatively uncommon interface usage ?

Why error out on import for non openssl implemented hashes (like blake2), generally in crypto libraries we fail the actual operation rather than reporting the hash does not exist as many applications just import all but then often simply do not use the unapproved hashes. Erroring on import makes the app completely broken at load time.
Returning exception on usage breaks the app only if it actually ends up using the function.
The latter is often preferable.

> Why can't hmac.HMAC be switched to use openssl hmac in FIPS mode ?

AFAICS, hmac.HMAC is an undocumented detail; the public constructor is hmac.new.
There are two complications:
- hmac.new has logic to extract the OpenSSL name from wrapper objects. I'd like to keep this written in Python.
- hmac.HMAC is a class, so `isinstance(foo, hmac.HMAC)` needs to work

But they're not unsolvable, we'll think about it more.


> Why error out on import for non openssl implemented hashes (like blake2)

The ImportError is from _blake2 (note the underscore). That is an internal implementation module, exposed by functions like hashlib.blake2s.
The hashlib module will fall back to OpenSSL if _blake2 is unavailable, making the operations fail if unsupported.

Thanks for the explanation sounds like we are on the right path.

Details from the discussion on not replacing hmac.HMAC with the OpenSSL wrapper:

The user-facing story is:
- The hmac.HMAC is the Python implementation of a HMAC object. It is unacceptable in FIPS mode.
- The _hmacopenssl.HMAC is the OpenSSL wrapper
- hmac.new() is the documented public constructor, which will give you the best implementation available.
- We consider constructing hmac.HMAC directly a *bug* with an easy fix. (The error message under FIPS will even suggest using hmac.new() instead.)

We will want to upstream _hmacopenssl.HMAC, and make hmac.new() return it for known cases where it’ll be more *efficient*.
I expect upstream hmac.new() will use the logic in Python 3.7+ hmac.digest() -- detect OpenSSL hash names as strings only, rather than try to extract them from other objects (as we want to do in RHEL FIPS mode, where there's no fallback).
But the C part should be usable as-is.

- It's pretty common to use hmac.HMAC() directly. Although hmac.new() is the documented interface, even I prefer to call hmac.HMAC() directly. The proposed approach may introduce additional and unnecessary work.
- hmac.HMAC object also exposes the inner and outer hashing object as public attributes HMAC.inner and HMAC.outer. I hope nobody uses these. It's not possible to retrieve the inner and outer EVP objects from a HMAC struct.
- The hmac.new() constructor and hmac.HMAC() take a string, a callable, or a module with a new() function as argument. For the callable and module.new case, it is possibe to get the name from callable().name or module.new().name.

> It's pretty common to use hmac.HMAC() directly. Although hmac.new() is the documented interface, even I prefer to call hmac.HMAC() directly. The proposed approach may introduce additional and unnecessary work.

If you use it, could you document the expected semantics?
- Is `isinstance(hmac.HMAC(...), hmac.HMAC)` expected to always work?
- Is `hmac.HMAC(...)` expected to be equivalent to `hmac.new(...)`?

Do you know how common it is to use hmac.HMAC with non-openSSL hashes? That would need additional work need as well.


> hmac.HMAC object also exposes the inner and outer hashing object as public attributes HMAC.inner and HMAC.outer. I hope nobody uses these. It's not possible to retrieve the inner and outer EVP objects from a HMAC struct.

There's not much we can do about that under FIPS mode.


> The hmac.new() constructor and hmac.HMAC() take a string, a callable, or a module with a new() function as argument. For the callable and module.new case, it is possibe to get the name from callable().name or module.new().name.

Are you suggesting that if some arbitrary hasher object is passed in, we instead use an OpenSSL hash that happens to have the same name?
That would not be correct.
I'd much rather only do that is we *know* the object is an OpenSSL wrapper.

> It's pretty common to use hmac.HMAC() directly.

Alright, I'll bite the bullet: in FIPS mode, hmac.HMAC will be replaced by a subclass of _hmacopenssl.HMAC.
This keeps determining the hash name in Python, and should have both of the properties above.

There is some "collateral damage" in FIPS mode:

- The distutils upload command omits the md5 checksum
- The multiprocessing challenge now uses sha256 rather than md5 (https://bugs.python.org/issue17258)
- smtplib’s CRAM-MD5 authentication will fail
- The md5-based uuid.uuid3 function will fail
- The md5sum demo/script will fail

Also, the "_ssl" module seems fail with "[SSL] malloc failure" when asked to do a handshake in FIPS mode. This breaks lots of network-related tests.

Failure to use md5 is expected, and we should move everything we can off of md5.
UUID creation is possibly the only exception *if* md5 is part of the specification, if not we should move uuid3 also off of md5, if not we may want to use an independent implementation for that one.

Failure of _ssl module is not expected, and should be investigated.

As far as I can see, md5 is part of the uuid3 specification.
What do you mean by "independent implementation" here?

Specifically for UUID uses.
As far as I can tell in uuid3 the hash algorithm is not used as a protection mechanism of any sort, but just as a way to get a uniform uuid value from arbitrary data (called name space in the RFC).
This means this use is not FIPS relevant.

So we have 2 options here:
1) Use a python implementation of md5 to generate uuid3 (and soon a python implementation of SHA-1 to generate uuid5).

2) Wihitn uuid3 only call the OpenSSL hashing function after calling this on the context:
EVP_MD_CTX_set_flags(ctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW);

HTH.

The uuid module is written in Python. With the current approach, no Python code has access to md5.

1) Using the Python implementation of md5 means we'd build it, so it would be available for any Python code. Isn't that what we're trying to avoid?

2) The RHEL7 patch uses EVP_MD_CTX_FLAG_NON_FIPS_ALLOW.
   Should this be exposed to Python with the "usedforsecurity" flag again?


Is shelling out to /usr/bin/uuid an option?

(In reply to Petr Viktorin from comment #21)
> The uuid module is written in Python. With the current approach, no Python
> code has access to md5.
> 
> 1) Using the Python implementation of md5 means we'd build it, so it would
> be available for any Python code. Isn't that what we're trying to avoid?

I was thinking about a private interface for uuid.

> 2) The RHEL7 patch uses EVP_MD_CTX_FLAG_NON_FIPS_ALLOW.
>    Should this be exposed to Python with the "usedforsecurity" flag again?

I am not sure I would make it available everywhere, but I am not necessarily against it, not too happy with the name, people have a hard time understanding when something is "used for security"
 
> Is shelling out to /usr/bin/uuid an option?

I do not know what are the policy in python on shelling out from code, it sounds dirty though, and may fail in various environments I guess.

Python's uuid module already has optional bindings to libuuid. I would prefer to hook up uuid_generate_md5 and uuid_generate_sha1 from libuuid instead of adding private interfaces for NON_FIPS_ALLOWED variants of MD5 and SHA1.

(In reply to Christian Heimes from comment #23)
> Python's uuid module already has optional bindings to libuuid. I would
> prefer to hook up uuid_generate_md5 and uuid_generate_sha1 from libuuid
> instead of adding private interfaces for NON_FIPS_ALLOWED variants of MD5
> and SHA1.

I agree this is probably a preferable option.

*** Bug 1724745 has been marked as a duplicate of this bug. ***

We released an erratum to get some early testing.

Meanwhile, I have patches ready for:
- Using uuid_generate_md5 from libuuid
- Always using sha256 in distutils upload, and adding md5 in non-FIPS mode

Meanwhile, Marcel is investigating OpenSSL failures. A similar OpenSSL failure was reported in Bug #1728361.

We'll eventually open a new bug for a new erratum.

Note that the doc text I set includes changes for the follow-up Bug #1744670.
Follow-up Bug #1745685 and Bug #1745499 don't need docs (they fix regressions in this one).

(In reply to Petr Viktorin from comment #17)
> Also, the "_ssl" module seems fail with "[SSL] malloc failure" when asked to
> do a handshake in FIPS mode. This breaks lots of network-related tests.

please note that in FIPS mode (especially when talking to Crypto-Policy enabled servers, like the RHEL version of OpenSSL), both ciphersuites that use RSA key exchange (use `openssl ciphers -ciphersuites '' -V kRSA` for a list) and connections that would use SHA-1 signatures over DHE or ECDHE parameters are expected to fail.

(In reply to Hubert Kario from comment #29)
> (In reply to Petr Viktorin from comment #17)
> > Also, the "_ssl" module seems fail with "[SSL] malloc failure" when asked to
> > do a handshake in FIPS mode. This breaks lots of network-related tests.
> 
> please note that in FIPS mode (especially when talking to Crypto-Policy
> enabled servers, like the RHEL version of OpenSSL), both ciphersuites that
> use RSA key exchange (use `openssl ciphers -ciphersuites '' -V kRSA` for a
> list) and connections that would use SHA-1 signatures over DHE or ECDHE
> parameters are expected to fail.

/me wearing his CPython upstream hat:

A while ago I started to update certificate, private keys, and DH param files to be compatible with stricter crypto policies. Most certificates should use  3072 bit RSA keys, SHA256 signatures, or finite field DH with 3072bit group params. Tests may still use RSA key exchange instead of ECDHE key agreement. I need to address though and mark/remove tests that explicitly verify kRSA. IIRC I landed all improvements in Python 3.6, too. 3.6.8 has at least dhff3072 and updated keycert3.pem

Could you please give me an example of a failing TLS/SSL test?

AFAIK, the "[SSL] malloc failure" was caused by an OpenSSL bug hat has since been fixed in RHEL.

On my Fedora 30, these tests still fail with these patches after FIPS_mode_set(1):

8 tests failed:
    test_asyncio test_ftplib test_httplib test_imaplib test_nntplib
    test_poplib test_ssl test_urllib2_localnet

For example:

ERROR: test_connect (test.test_ssl.SimpleBackgroundTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pviktori/dev/cpython/Lib/test/test_ssl.py", line 1550, in test_connect
    s.connect(self.server_addr)
  File "/home/pviktori/dev/cpython/Lib/ssl.py", line 1109, in connect
    self._real_connect(addr, False)
  File "/home/pviktori/dev/cpython/Lib/ssl.py", line 1100, in _real_connect
    self.do_handshake()
  File "/home/pviktori/dev/cpython/Lib/ssl.py", line 1077, in do_handshake
    self._sslobj.do_handshake()
  File "/home/pviktori/dev/cpython/Lib/ssl.py", line 689, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLError: [SSL] malloc failure (_ssl.c:852)

> On my Fedora 30, these tests still fail with these patches after FIPS_mode_set(1):

FIPS mode in Fedora isn't really supported

(In reply to Petr Viktorin from comment #31)
> ERROR: test_connect (test.test_ssl.SimpleBackgroundTests)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/pviktori/dev/cpython/Lib/test/test_ssl.py", line 1550, in
> test_connect
>     s.connect(self.server_addr)
> ...
> ssl.SSLError: [SSL] malloc failure (_ssl.c:852)

On Python 3.6 line 1550 refers to a test with an unverified connection (ssl.CERT_NONE). Does FIPS mode block any connection attempts that do not verify certificate?

rpm -q openssl ?

The latest (openssl-1.1.1c-6.fc30) should have this issue fixed already.

Christian,
no FIPS is not in the business of policing whether authentication is used in various protocols.
It only restricts what crypto primitives can be used in FIPS compliant mode.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2019:3520