noobaa-core
noobaa-core copied to clipboard
fix native MD5 calculation: Bump isa-l_crypto to 4f72976
Signed-off-by: Utkarsh Srivastava [email protected]
Explain the changes
This PR bumps isa-l_crypto to 4f72976
(master). We rely on this package for MD5 calculation (and other use cases) in our crypto package monkey patching. v2.24.0
has the issue where the calculation of MD5, SHA1, etc does not produce the correct result when the given data is not aligned with their respective block sizes. This was fixed in https://github.com/intel/isa-l_crypto/pull/97
For example, node -p 'new (require("./src/util/nb_native")().MD5_MB)().update(Buffer.from(new Array(65).fill("0").join(""))).digest().toString()'
would produce the EXACT same MD5 as node -p 'new (require("./src/util/nb_native")().MD5_MB)().update(Buffer.from(new Array(64).fill("0").join(""))).digest().toString()'
would. This is because the library had an issue where it misses the data which are not multiple of MD_BLOCK_SIZE
.
Incorrect calculation of MD5 in FIPS environment lead to failed deployments of NooBaa in FIPS environment.
Issues: Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2129126
Testing Instructions:
-
node -p 'new (require("./src/util/nb_native")().MD5_MB)().update(Buffer.from(new Array(65).fill("0").join(""))).digest().toString()'
should producef8c702aaa8c658413a4efb3a614d7707
instead of10eab6008d5642cf42abd2aa41f847cb
. - Deploy NooBaa in FIPS environment, the deployment should go through.
- [ ] Doc added/updated
- [ ] Tests added
@liranmauda We need to see we don't break, p&z and FIPS
@utkarsh-pro can you update the version info in https://github.com/noobaa/noobaa-core/blob/master/src/native/third_party/versions.md too?
@liranmauda We need to see we don't break, p&z and FIPS
True, We need to ask for a build on P&Z. As for FIPS, I don't see a reason why it will break, But we definitely need QE to test it on FIPS.
@b-ranto @deepshikhaaa Can we run a build on this commit hash?
@liranmauda @nimrod-becker this PR comes to fix FIPS if I'm not mistaken
@dannyzaken, yes that's right this PR was intended to fix our crypto monkey patching in FIPS environment.
I mentioned the lesser relevant story in the PR description and forgot to mention the clear intention 🤦 .
Edit: Fixed the PR description.