On 4/12/21 3:20 PM, Eric Biggers wrote:
On Mon, Apr 12, 2021 at 03:04:58PM -0400, Chris von Recklinghausen wrote:
On 4/12/21 1:45 PM, Eric Biggers wrote:
On Mon, Apr 12, 2021 at 10:09:32AM -0400, Chris von Recklinghausen wrote:
Suspend fails on a system in fips mode because md5 is used for the e820
integrity check and is not available. Use crc32 instead.

This patch changes the integrity check algorithm from md5 to crc32.

The purpose of the integrity check is to detect possible differences
between the memory map used at the time when the hibernation image is
about to be loaded into memory and the memory map used at the image
creation time, because it is generally unsafe to load the image if the
current memory map doesn't match the one used when it was created. so
it is not intended as a cryptographic integrity check.
This still doesn't actually explain why a non-cryptographic checksum is
sufficient.  "Detection of possible differences" could very well require
cryptographic authentication; it depends on whether malicious changes need to be
detected or not.
Hi Eric,

The cases that the commit comments for 62a03defeabd mention are the same as
for this patch, e.g.

     1. Without this patch applied, it is possible that BIOS has
        provided an inconsistent memory map, but the resume kernel is still
        able to restore the image anyway(e.g, E820_RAM region is the superset
        of the previous one), although the system might be unstable. So this
        patch tries to treat any inconsistent e820 as illegal.

     2. Another case is, this patch replies on comparing the e820_saved, but
        currently the e820_save might not be strictly the same across
        hibernation, even if BIOS has provided consistent e820 map - In
        theory mptable might modify the BIOS-provided e820_saved dynamically
        in early_reserve_e820_mpc_new, which would allocate a buffer from
        E820_RAM, and marks it from E820_RAM to E820_RESERVED).
        This is a potential and rare case we need to deal with in OS in
        the future.

Maybe they should be added to the comments with this patch as well? In any
case, the above comments only mention detecting consequences of BIOS
issues/actions on the e820 map and not intrusions from attackers requiring
cryptographic protection. Does that seem to be a reasonable explanation to
you? If so I can add these to the commit comments.

I'll make the other changes you suggest below.

Thanks,

Those details are still missing the high-level point.  Is this just meant to
detect non-malicious changes (presumably caused by BIOS bugs), or is it meant to
detect malicious changes?  That's all that really needs to be mentioned.


Ok, I'll say the intent is to detect non-malicious changes presumably from BIOS issues.

Thanks,

Chris


- Eric


Reply via email to