Re: Fix hibernation in FIPS mode?

On Thu, Apr 01, 2021 at 09:54:21AM -0400, Chris von Recklinghausen wrote:

On 4/1/21 9:38 AM, Rafael J. Wysocki wrote:

quoted

On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel [off-list ref] wrote:

quoted

On Tue, 30 Mar 2021 at 21:56, Simo Sorce [off-list ref] wrote:

quoted

On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote:

quoted

On Tue, 30 Mar 2021 at 20:05, Simo Sorce [off-list ref] wrote:

quoted

On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote:

quoted

On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui [off-list ref] wrote:

quoted

Hi,
MD5 was marked incompliant with FIPS in 2009:
a3bef3a31a19 ("crypto: testmgr - Skip algs not flagged fips_allowed in fips mode")
a1915d51e8e7 ("crypto: testmgr - Mark algs allowed in fips mode")

But hibernation_e820_save() is still using MD5, and fails in FIPS mode
due to the 2018 patch:
749fa17093ff ("PM / hibernate: Check the success of generating md5 digest before hibernation")

As a result, hibernation doesn't work when FIPS is on.

Do you think if hibernation_e820_save() should be changed to use a
FIPS-compliant algorithm like SHA-1?

I would say yes, it should.

quoted

PS, currently it looks like FIPS mode is broken in the mainline:
https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg49414.html

FYI, SHA-1 is not a good choice, it is only permitted in HMAC
constructions and only for specified uses. If you need to change
algorithm you should go straight to SHA-2 or SHA-3 based hashes.

What is the reason for using a [broken] cryptographic hash here? if
this is just an integrity check, better use CRC32

Not really.

CRC32 is not really sufficient for integrity checking here AFAICS.  It
might be made a fallback option if MD5 is not available, but making it
the default would be somewhat over the top IMO.

Would ghash be a better choice? It produces the same size digest as md5.

Does anyone have any other suggestions of algorithms to try?

Thanks,

Chris

quoted

quoted

quoted

If the integrity check is used exclusively to verify there were no
accidental changes and is not used as a security measure, by all means
I agree that using crc32 is a better idea.

Looking at 62a03defeabd58f74e07ca030d6c21e069d4d88e which introduced
this, it is only a best effort check which is simply omitted if md5
happens to be unavailable, so there is definitely no need for crypto
here.

Yes, it is about integrity checking only.  No, CRC32 is not equivalent
to MD5 in that respect AFAICS.

If you need to detect intentional changes (ensure authenticity, not just
integrity) then you need a cryptographic MAC, such as HMAC-SHA256.

If you only need to detect accidental changes (ensure integrity-only), then a
checksum such as CRC-32 or xxHash64 is sufficient.  A cryptographic hash
function such as SHA-256 would also be sufficient, though much slower.  Using a
broken cryptographic hash function such as MD5 doesn't make sense because it is
broken (so doesn't actually provide cryptographic security), and is much slower
than a checksum.

If the 1 in 4 billion collision rate of a CRC-32 isn't sufficient, then use
CRC-64 or xxHash64 for a 1 in 2^64 collision rate.

Don't use GHASH, as it is neither a checksum nor a cryptographic hash function.

- Eric