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

RIPEMD-320 implementation #68

Merged
merged 1 commit into from
Feb 11, 2019
Merged

Conversation

magnet
Copy link
Contributor

@magnet magnet commented Dec 13, 2018

This partly addresses issue #13.

This RIPEMD-320 implementation is based on the existing RIPEMD-160 implementation. It would be possible and relatively easy to refactor code to avoid so much duplication, but making this a separate crate breaks less code and makes it easy to add.

RIPEMD-160 and RIPEMD-320 also share a similar relationship as RIPEMD-128 and RIPEMD-256 do. A quick diff between RIPEMD-160 and RIPEMD-320 implementations should be useful anyone writing RIPEMD-128/256.

I don't know if that's OK with regard to the conventions in place, but in the tests I drew some samples from the RIPEMD-160 webpage[1] that describes expected hashes for some strings.

Also, I have uploaded a 0.8.0-rc1 version to crates.io (under the name ripemd320). If/when you can merge this into master and publish, please ping me so I can give you access and optionally yank the rc1.

[1] https://homes.esat.kuleuven.be/~bosselae/ripemd160.html

@magnet
Copy link
Contributor Author

magnet commented Dec 13, 2018

The travis-ci fail on Rust 1.21.0 seems unrelated to my changes.

Running `rustc --crate-name blake2 blake2/src/lib.rs --crate-type lib --emit=dep-info,link -C opt-level=3 --cfg 'feature="crypto-mac"' --cfg 'feature="default"' --cfg 'feature="digest"' --cfg 'feature="std"' -C metadata=9262f4b445d7d7a9 -C extra-filename=-9262f4b445d7d7a9 --out-dir /home/travis/build/RustCrypto/hashes/target/release/deps -L dependency=/home/travis/build/RustCrypto/hashes/target/release/deps --extern byte_tools=/home/travis/build/RustCrypto/hashes/target/release/deps/libbyte_tools-1f1e017fe4446f53.rlib --extern crypto_mac=/home/travis/build/RustCrypto/hashes/target/release/deps/libcrypto_mac-a079c1b565c6202c.rlib --extern opaque_debug=/home/travis/build/RustCrypto/hashes/target/release/deps/libopaque_debug-4d4d716ba82a9b9d.rlib --extern digest=/home/travis/build/RustCrypto/hashes/target/release/deps/libdigest-a4c2be69685ced18.rlib` error[E0425]: cannot find function `from_ref` in module `slice` --> /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/block-buffer-0.7.1/src/lib.rs:72:22 | 72 | f(slice::from_ref(&self.buffer)); | ^^^^^^^^ not found in `slice

slice::from_ref was added in Rust 1.28.0[1]. That call is made by the block-buffer crate, itself used by blake2.

[1] https://doc.rust-lang.org/std/slice/fn.from_ref.html

@newpavlov
Copy link
Member

Thank you for your PR! At first glance it looks good to me, I will check it a bit more thoroughly on the next week and will probably merge it immediately after that.

I've yanked block-buffer v0.7.1, it's great that we found it thanks to your PR and not by breaking someone's build. But there is still a small problem in your code which should be fixable with a simple use core::mem;.

@magnet
Copy link
Contributor Author

magnet commented Dec 14, 2018

Hey @newpavlov . Thanks for your comment! Do you mean you want me to change the call to core::mem::swap to simply use core::mem + swap? I can update the PR if that's what you suggest, or leave it as it is for you to change as you see fit.
I have also sent you an invite to manage ripemd320 on crates.io because I will be away next week.

@newpavlov
Copy link
Member

Do you mean you want me to change the call to core::mem::swap to simply use core::mem + swap?

Yes, exactly this. I think it will be better for you to do it.

@magnet
Copy link
Contributor Author

magnet commented Dec 14, 2018

I have updated the PR.
The build is now failing on Rust nightly TARGET=thumbv7em-none-eabi for unrelated reasons, seems like I won't get that nice green light from Travis ;)

@magnet
Copy link
Contributor Author

magnet commented Jan 1, 2019

Hey @newpavlov, happy new year! Any news on this? :-)

@magnet
Copy link
Contributor Author

magnet commented Feb 7, 2019

@newpavlov or anyone, could we get this merged? 🙏 🙂

@tarcieri
Copy link
Member

tarcieri commented Feb 7, 2019

The build is now failing on Rust nightly TARGET=thumbv7em-none-eabi for unrelated reasons, seems like I won't get that nice green light from Travis ;)

Try again?

@magnet magnet closed this Feb 7, 2019
@magnet magnet reopened this Feb 7, 2019
@magnet
Copy link
Contributor Author

magnet commented Feb 7, 2019

@tarcieri the build now fails on Rust 1.21.0 for the same reason as the first time -- that is, blobby or block-buffer crate use slice::from_ref which was added in Rust 1.28.0. This is unrelated to this pull request.

@tarcieri
Copy link
Member

tarcieri commented Feb 7, 2019

@magnet unfortunate... any ideas @newpavlov ?

@newpavlov
Copy link
Member

Arrrgh, I've added input2 method for testing bindings to OpenSSL assembly, but missed the fact that slice::from_ref was not stable for Rust 1.21. (I guess I really should add MSRV CI tests to RustCrypto/utils) I will yank block-buffer v0.7.2 and will re-run the failing test.

And sorry for the long waiting time! I couldn't find time to properly review PRs. I will do my best to do it either on this or next weekend.

@magnet
Copy link
Contributor Author

magnet commented Feb 8, 2019

Thanks @newpavlov ! The checks are green finally 🙂.

@newpavlov newpavlov merged commit b7524a2 into RustCrypto:master Feb 11, 2019
@newpavlov
Copy link
Member

Thank you again! It will be nice if you'll share publishing rights to ripemd320 crate either with me or RustCrypto:hashes group.

@magnet
Copy link
Contributor Author

magnet commented Feb 11, 2019

@newpavlov Thanks for merging, I had already sent you an invite, and just sent a new one to you and RustCrypto:hashes

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

Successfully merging this pull request may close these issues.

3 participants