Re: Fix hibernation in FIPS mode?

From
Simo Sorce <hidden>
Date
2021-04-01 18:46:49
Also in
linux-pm, lkml
Reply to
Rafael J. Wysocki, 2021-04-01
On Thu, 2021-04-01 at 18:02 +0200, Rafael J. Wysocki wrote:
On Thu, Apr 1, 2021 at 3:54 PM Ard Biesheuvel [off-list ref] wrote:
quoted
On Thu, 1 Apr 2021 at 15:38, Rafael J. Wysocki [off-list ref] 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.
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.
There are two possibilities:
- we care about an adversary attempting to forge a collision, in which
case you need a cryptographic hash which is not broken;
- we only care about integrity, in which case crypto is overkill, and
CRC32 is sufficient. (Note that the likelihood of an honest,
inadvertent modification not being caught by CRC32 is 1 in 4 billion)
That depends on how you count.

Surely, there are modifications caught by MD5 that will not be caught by CRC32.
This is a technically correct statement, but does it matter in this
context? (Hint, probably not)
quoted
MD5 does not meet either requirement, given that it is known to be
broken, and overkill for simple integrity checks. MD5 should be phased
out and removed, and moving this code onto the correct abstraction
would be a reasonable step towards that goal.
This clearly is a matter of opinion.
Sorry, but this is not a matter of opinion.
The only reason to use a cryptographic hash is that you want to protect
from active tampering, rather than from accidental changes. And if you
need to protect from active tampering then you cannot use a known
broken hash, there is no point.

OTOH if you do not care for active tampering but only to catch
transmission/storage errors then all you care for is error checking. In
that case a cryptographic hash is overkill because it entails a lot
more computation than is needed.
I'm not religious about it though.  If there is a general consensus
that CRC32 is sufficient for error detection in hibernation files,
then it can be used.  So is there such a consensus and if so, can you
give me a pointer to some research that it is based on?
CRC32 is an industry standard to check for accidental modifications of
a bit stream. The chances of missing an accidental change are 1 in 4
billion.

Does your application require a higher threshold? If so you should
justify why you think your case is not fulfilled by an industry
standard.

HTH,
Simo.

-- 
Simo Sorce
RHEL Crypto Team
Red Hat, Inc



Thread (16 messages) 16 messages, 6 authors, last reply 2021-04-01
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help