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

[Daniel P . Berrangé]

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 

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 :|