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

Allow using BLAKE2b checksum instead of bundled MD4 #406

Closed
wants to merge 1 commit into from

Conversation

afbjorklund
Copy link
Contributor

This is mostly for doing performance comparisons, before switching the default checksum.

As discussed, we might want to use more than just 128 bits (out of the available 512 bits).

@afbjorklund afbjorklund requested a review from jrosdahl May 2, 2019 21:12
@afbjorklund
Copy link
Contributor Author

Currently this is a compile-time flag. It could be made into a runtime option, if wanted (probably not)

@afbjorklund afbjorklund changed the title Allow using BLAKE2b crypto instead of bundled MD4 Allow using BLAKE2b checksum instead of bundled MD4 May 2, 2019
@afbjorklund
Copy link
Contributor Author

As per discussion in #412 this should be made into runtime configuration, so that we can compare.

@afbjorklund
Copy link
Contributor Author

Made into runtime configuration, but still needs --enable-blake2 to compile.

@afbjorklund afbjorklund added feature New or improved feature and removed work in progress Do not merge labels May 13, 2019
@afbjorklund
Copy link
Contributor Author

Now it is back to a compiletime configuration again, removed the checksum setting.

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Jun 16, 2019

Removed the total counter, and increased the digest from 16 bytes to 20 bytes (out of the 64)

Note that this (digest size) changes the entire checksum, it's not just truncating the full output:

$ echo -n "" | b2sum -l 512
786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce  -
$ echo -n "" | b2sum -l 160
3345524abf6bbe1809449224b5972c41790b6cf2  -

@afbjorklund
Copy link
Contributor Author

Closing this branch where it is optional, will do another PR where it is simply changed/replaced...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New or improved feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants