* Fix hibernation in FIPS mode? @ 2021-03-29 22:13 Dexuan Cui 2021-03-30 14:46 ` Rafael J. Wysocki 0 siblings, 1 reply; 16+ messages in thread From: Dexuan Cui @ 2021-03-29 22:13 UTC (permalink / raw) To: 'linux-pm@vger.kernel.org' Cc: crecklin, 'linux-crypto@vger.kernel.org', 'linux-kernel@vger.kernel.org' 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? PS, currently it looks like FIPS mode is broken in the mainline: https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg49414.html Thanks, -- Dexuan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-03-29 22:13 Fix hibernation in FIPS mode? Dexuan Cui @ 2021-03-30 14:46 ` Rafael J. Wysocki 2021-03-30 14:51 ` Chris von Recklinghausen 2021-03-30 18:04 ` Simo Sorce 0 siblings, 2 replies; 16+ messages in thread From: Rafael J. Wysocki @ 2021-03-30 14:46 UTC (permalink / raw) To: Dexuan Cui; +Cc: linux-pm, crecklin, linux-crypto, linux-kernel On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > 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. > PS, currently it looks like FIPS mode is broken in the mainline: > https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg49414.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-03-30 14:46 ` Rafael J. Wysocki @ 2021-03-30 14:51 ` Chris von Recklinghausen 2021-03-30 18:04 ` Simo Sorce 1 sibling, 0 replies; 16+ messages in thread From: Chris von Recklinghausen @ 2021-03-30 14:51 UTC (permalink / raw) To: Rafael J. Wysocki, Dexuan Cui; +Cc: linux-pm, linux-crypto, linux-kernel On 3/30/21 10:46 AM, Rafael J. Wysocki wrote: > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: >> 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. If we're not actually encrypting anything, shouldn't something like ghash work as well? > >> PS, currently it looks like FIPS mode is broken in the mainline: >> https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg49414.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-03-30 14:46 ` Rafael J. Wysocki 2021-03-30 14:51 ` Chris von Recklinghausen @ 2021-03-30 18:04 ` Simo Sorce 2021-03-30 19:45 ` Ard Biesheuvel 1 sibling, 1 reply; 16+ messages in thread From: Simo Sorce @ 2021-03-30 18:04 UTC (permalink / raw) To: Rafael J. Wysocki, Dexuan Cui Cc: linux-pm, crecklin, linux-crypto, linux-kernel On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > 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. > > > 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. HTH, Simo. -- Simo Sorce RHEL Crypto Team Red Hat, Inc ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-03-30 18:04 ` Simo Sorce @ 2021-03-30 19:45 ` Ard Biesheuvel 2021-03-30 19:55 ` Simo Sorce 0 siblings, 1 reply; 16+ messages in thread From: Ard Biesheuvel @ 2021-03-30 19:45 UTC (permalink / raw) To: Simo Sorce Cc: Rafael J. Wysocki, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > 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. > > > > > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-03-30 19:45 ` Ard Biesheuvel @ 2021-03-30 19:55 ` Simo Sorce 2021-04-01 8:46 ` Ard Biesheuvel 0 siblings, 1 reply; 16+ messages in thread From: Simo Sorce @ 2021-03-30 19:55 UTC (permalink / raw) To: Ard Biesheuvel Cc: Rafael J. Wysocki, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > 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. > > > > > > > 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 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. Simo. -- Simo Sorce RHEL Crypto Team Red Hat, Inc ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-03-30 19:55 ` Simo Sorce @ 2021-04-01 8:46 ` Ard Biesheuvel 2021-04-01 13:38 ` Rafael J. Wysocki 0 siblings, 1 reply; 16+ messages in thread From: Ard Biesheuvel @ 2021-04-01 8:46 UTC (permalink / raw) To: Simo Sorce Cc: Rafael J. Wysocki, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > 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. > > > > > > > > > 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 > > 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 8:46 ` Ard Biesheuvel @ 2021-04-01 13:38 ` Rafael J. Wysocki 2021-04-01 13:54 ` Chris von Recklinghausen 2021-04-01 13:54 ` Ard Biesheuvel 0 siblings, 2 replies; 16+ messages in thread From: Rafael J. Wysocki @ 2021-04-01 13:38 UTC (permalink / raw) To: Ard Biesheuvel Cc: Simo Sorce, Rafael J. Wysocki, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > 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. > > > > > > > > > > > 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. > > 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. Thanks! ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 13:38 ` Rafael J. Wysocki @ 2021-04-01 13:54 ` Chris von Recklinghausen 2021-04-01 13:56 ` Ard Biesheuvel 2021-04-01 18:26 ` Eric Biggers 2021-04-01 13:54 ` Ard Biesheuvel 1 sibling, 2 replies; 16+ messages in thread From: Chris von Recklinghausen @ 2021-04-01 13:54 UTC (permalink / raw) To: Rafael J. Wysocki, Ard Biesheuvel Cc: Simo Sorce, Dexuan Cui, linux-pm, linux-crypto, linux-kernel On 4/1/21 9:38 AM, Rafael J. Wysocki wrote: > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: >> On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: >>> On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: >>>> On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: >>>>> On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: >>>>>> On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: >>>>>>> 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. >>>>>> >>>>>>> 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 > >>> 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. > > Thanks! > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 13:54 ` Chris von Recklinghausen @ 2021-04-01 13:56 ` Ard Biesheuvel 2021-04-01 18:26 ` Eric Biggers 1 sibling, 0 replies; 16+ messages in thread From: Ard Biesheuvel @ 2021-04-01 13:56 UTC (permalink / raw) To: crecklin Cc: Rafael J. Wysocki, Simo Sorce, Dexuan Cui, linux-pm, linux-crypto, linux-kernel On Thu, 1 Apr 2021 at 15:54, Chris von Recklinghausen <crecklin@redhat.com> wrote: > > On 4/1/21 9:38 AM, Rafael J. Wysocki wrote: > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > >> On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > >>> On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > >>>> On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > >>>>> On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > >>>>>> On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > >>>>>>> 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. > >>>>>> > >>>>>>> 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. > No, ghash is a MAC not a hash. It should only ever be used with GCM and nowhere else. > Does anyone have any other suggestions of algorithms to try? > Just use crc32 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 13:54 ` Chris von Recklinghausen 2021-04-01 13:56 ` Ard Biesheuvel @ 2021-04-01 18:26 ` Eric Biggers 1 sibling, 0 replies; 16+ messages in thread From: Eric Biggers @ 2021-04-01 18:26 UTC (permalink / raw) To: Chris von Recklinghausen Cc: Rafael J. Wysocki, Ard Biesheuvel, Simo Sorce, Dexuan Cui, linux-pm, linux-crypto, linux-kernel 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: > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > > > 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. > > > > > > > > > > > > > > > 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 > > > > > > > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 13:38 ` Rafael J. Wysocki 2021-04-01 13:54 ` Chris von Recklinghausen @ 2021-04-01 13:54 ` Ard Biesheuvel 2021-04-01 16:02 ` Rafael J. Wysocki 1 sibling, 1 reply; 16+ messages in thread From: Ard Biesheuvel @ 2021-04-01 13:54 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Simo Sorce, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Thu, 1 Apr 2021 at 15:38, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > > 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. > > > > > > > > > > > > > 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. > > > > 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) 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 13:54 ` Ard Biesheuvel @ 2021-04-01 16:02 ` Rafael J. Wysocki 2021-04-01 16:22 ` Simo Sorce 0 siblings, 1 reply; 16+ messages in thread From: Rafael J. Wysocki @ 2021-04-01 16:02 UTC (permalink / raw) To: Ard Biesheuvel Cc: Rafael J. Wysocki, Simo Sorce, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Thu, Apr 1, 2021 at 3:54 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Thu, 1 Apr 2021 at 15:38, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > > > 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. > > > > > > > > > > > > > > > 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. > > > > > > 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. > 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. 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? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 16:02 ` Rafael J. Wysocki @ 2021-04-01 16:22 ` Simo Sorce 2021-04-01 16:31 ` Rafael J. Wysocki 0 siblings, 1 reply; 16+ messages in thread From: Simo Sorce @ 2021-04-01 16:22 UTC (permalink / raw) To: Rafael J. Wysocki, Ard Biesheuvel Cc: Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Thu, 2021-04-01 at 18:02 +0200, Rafael J. Wysocki wrote: > On Thu, Apr 1, 2021 at 3:54 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Thu, 1 Apr 2021 at 15:38, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > > > > 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. > > > > > > > > > > > > > > > > > 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. > > > > > > > > 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) > > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 16:22 ` Simo Sorce @ 2021-04-01 16:31 ` Rafael J. Wysocki 2021-04-01 17:53 ` Simo Sorce 0 siblings, 1 reply; 16+ messages in thread From: Rafael J. Wysocki @ 2021-04-01 16:31 UTC (permalink / raw) To: Simo Sorce Cc: Rafael J. Wysocki, Ard Biesheuvel, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Thu, Apr 1, 2021 at 6:22 PM Simo Sorce <simo@redhat.com> wrote: > > On Thu, 2021-04-01 at 18:02 +0200, Rafael J. Wysocki wrote: > > On Thu, Apr 1, 2021 at 3:54 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > On Thu, 1 Apr 2021 at 15:38, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > > > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > 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) > > > > 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. But the amount of data in question is not huge in this case. > > 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. This is not about accidental change which basically is my point. The BIOSes in question change the memory map over hibernation/resume, because they think that the memory layout is now different, so this is about detecting a sort of intentional change. Definitely not random, though. But as stated elsewhere, it is just about failing more gracefully at least in some cases, so let's just go ahead with using CRC32 here (worst case, it will not fail more gracefully in super-corner cases). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Fix hibernation in FIPS mode? 2021-04-01 16:31 ` Rafael J. Wysocki @ 2021-04-01 17:53 ` Simo Sorce 0 siblings, 0 replies; 16+ messages in thread From: Simo Sorce @ 2021-04-01 17:53 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Ard Biesheuvel, Dexuan Cui, linux-pm, crecklin, linux-crypto, linux-kernel On Thu, 2021-04-01 at 18:31 +0200, Rafael J. Wysocki wrote: > On Thu, Apr 1, 2021 at 6:22 PM Simo Sorce <simo@redhat.com> wrote: > > On Thu, 2021-04-01 at 18:02 +0200, Rafael J. Wysocki wrote: > > > On Thu, Apr 1, 2021 at 3:54 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Thu, 1 Apr 2021 at 15:38, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > > On Thu, Apr 1, 2021 at 10:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Tue, 30 Mar 2021 at 21:56, Simo Sorce <simo@redhat.com> wrote: > > > > > > > On Tue, 2021-03-30 at 21:45 +0200, Ard Biesheuvel wrote: > > > > > > > > On Tue, 30 Mar 2021 at 20:05, Simo Sorce <simo@redhat.com> wrote: > > > > > > > > > On Tue, 2021-03-30 at 16:46 +0200, Rafael J. Wysocki wrote: > > > > > > > > > > On Tue, Mar 30, 2021 at 12:14 AM Dexuan Cui <decui@microsoft.com> wrote: > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > 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) > > > > > > 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. > > But the amount of data in question is not huge in this case. > > > > 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. > > This is not about accidental change which basically is my point. > > The BIOSes in question change the memory map over hibernation/resume, > because they think that the memory layout is now different, so this is > about detecting a sort of intentional change. Definitely not random, > though. Ok, not random, but also not intentional, it is just "accidental". > But as stated elsewhere, it is just about failing more gracefully at > least in some cases, so let's just go ahead with using CRC32 here > (worst case, it will not fail more gracefully in super-corner cases). Sounds good. Simo. -- Simo Sorce RHEL Crypto Team Red Hat, Inc ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-04-01 18:47 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-29 22:13 Fix hibernation in FIPS mode? Dexuan Cui 2021-03-30 14:46 ` Rafael J. Wysocki 2021-03-30 14:51 ` Chris von Recklinghausen 2021-03-30 18:04 ` Simo Sorce 2021-03-30 19:45 ` Ard Biesheuvel 2021-03-30 19:55 ` Simo Sorce 2021-04-01 8:46 ` Ard Biesheuvel 2021-04-01 13:38 ` Rafael J. Wysocki 2021-04-01 13:54 ` Chris von Recklinghausen 2021-04-01 13:56 ` Ard Biesheuvel 2021-04-01 18:26 ` Eric Biggers 2021-04-01 13:54 ` Ard Biesheuvel 2021-04-01 16:02 ` Rafael J. Wysocki 2021-04-01 16:22 ` Simo Sorce 2021-04-01 16:31 ` Rafael J. Wysocki 2021-04-01 17:53 ` Simo Sorce
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.