Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix endian issues in md5 name/hash implementation #109

Merged
merged 3 commits into from Jun 25, 2019

Conversation

@jeking3
Copy link
Collaborator

commented Jun 24, 2019

This fix contains a breaking change.

  • The default hash algorithm if not specified is SHA1, so unless you are specifically choosing MD5 then you have nothing to worry about as none of these changes affect SHA1 name generation.

  • The code was not properly handling MD5_Final copy-to-buffer with respect to the overall result of the name generator on little-endian systems. While the SHA1 result is copied as integers, the MD5 result is copied into a byte buffer backed by integers, and not handled properly w.r.t. endianness.

  • The unit test for MD5 was using memcmp instead of accounting for the endianness, thus hiding the original issue.

  • Maintaining consistency in a hash-based implementation over time is critical.

  • The md5 based name generation has changed to ensure the generation of md5 digests is correct (and the same) on both little and big endian systems. Previously it was producing different results on little and big endian systems (and unfortunately, if was the little-endian systems that were incorrect).

  • Define BOOST_UUID_COMPAT_PRE_1_71_MD5 to ensure that any MD5 name generated UUIDs come out the same as they did in 1.66 through 1.70. Note that in this mode little and big endian systems produce a different uuid for md5 name based uuids with the same inputs (which is the defect that was fixed here).

Fix endian issue copying md5 result to byte buffer.
Fix the sha1 and md5 unit tests for endian correctness.
Originally identified by BinCaoWR in GitHub issue #86 and PR #87

This fixes #86
This closes #87

@jeking3 jeking3 added the ci label Jun 24, 2019

@jeking3 jeking3 self-assigned this Jun 24, 2019

@jeking3 jeking3 requested review from pdimov and removed request for pdimov Jun 24, 2019

@jeking3 jeking3 force-pushed the jeking3:endianness branch from aa1e7f0 to 286fbe0 Jun 25, 2019

@jeking3 jeking3 added this to the v1.71.0 milestone Jun 25, 2019

@jeking3 jeking3 merged commit ca0185f into boostorg:develop Jun 25, 2019

3 checks passed

CodeFactor No issues found.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jeking3 jeking3 deleted the jeking3:endianness branch Jun 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.