Re: [PATCH v3 05/10] migration/dirtyrate: Record hash results for each sampled page

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Aug 20, 2020 at 06:30:09PM +0100, Dr. David Alan Gilbert wrote:
> > * Chuan Zheng (zhengchuan@huawei.com) wrote:
> > > Record hash results for each sampled page.
> > > 
> > > Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> > > Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
> > > ---
> > >  migration/dirtyrate.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  migration/dirtyrate.h |   7 +++
> > >  2 files changed, 151 insertions(+)
> > > 
> > > diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> > > index c4304ef..62b6f69 100644
> > > --- a/migration/dirtyrate.c
> > > +++ b/migration/dirtyrate.c
> > > @@ -25,6 +25,7 @@
> > >  #include "dirtyrate.h"
> > >  
> > >  CalculatingDirtyRateState CalculatingState = CAL_DIRTY_RATE_INIT;
> > > +static unsigned long int qcrypto_hash_len = QCRYPTO_HASH_LEN;
> > 
> > Why do we need this static rather than just using the QCRYPTO_HASH_LEN ?
> > It's never going to change is it?
> > (and anyway it's just a MD5 len?)
> 
> I wouldn't want to bet on that given that this is use of MD5. We might
> claim this isn't security critical, but surprises happen, and we will
> certainly be dinged on security audits for introducing new use of MD5
> no matter what.
> 
> If a cryptographic hash is required, then sha256 should be the choice
> for any new code that doesn't have back compat requirements.
> 
> If a cryptographic hash is not required then how about crc32 

It doesn't need to be cryptographic; is crc32 the fastest reasonable hash for use
in large areas?

Dave

> IOW, it doesn't make a whole lot of sense to say we need a cryptographic
> hash, but then pick the most insecure one.
> 
> sha256 is slower than md5, but it is conceivable that in future we might
> gain support for something like Blake2b which is similar security level
> to SHA3, while being faster than MD5.
> 
> Overall I'm pretty unethusiastic about use of MD5 being introduced and
> worse, being hardcoded as the only option.
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK