Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added zeroize to blake2_simd#449

Closed
laudiacay wants to merge 5 commits into
RustCrypto:blake2_simd from
laudiacay:blake2_simd
Closed

Added zeroize to blake2_simd #449
laudiacay wants to merge 5 commits into
RustCrypto:blake2_simd from
laudiacay:blake2_simd

Conversation

@laudiacay

@laudiacay laudiacay commented Jan 28, 2023

Copy link
Copy Markdown

This code is much cleaner (although a bit repetitive...). Zeroize added! 🖖

Copy link
Copy Markdown
Author

@tarcieri zeroize's unstable features seem to be angering the inliner ... not sure what you want me to do with this, just let me know what's preferable for the crate's users

Copy link
Copy Markdown
Member

@laudiacay are you talking about these?

error[E0658]: const generics are unstable
 --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/zeroize-1.5.7/src/lib.rs:353:15
 |
353 | impl<Z, const N: usize> Zeroize for [Z; N]
error[E0599]: no associated item named `MAX` found for type `isize` in the current scope
 --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/zeroize-1.5.7/src/lib.rs:431:32
 |
431 | assert!(size <= isize::MAX as usize);
 | ^^^ associated item not found in `isize`

The MSRV of zeroize is 1.51, but you are testing on 1.41, so it won't compile (and isn't expected to).

Copy link
Copy Markdown
Author

So should I just remove the 1.41 tests or configure the crate to only zeroize with a feature, and then only run the 1.41 tests without that feature?

Copy link
Copy Markdown
Member

It may be worth to wait for the next breaking release cycle in which we bump MSRV to 1.57. If you absolutely need zeroization for blake2 v0.10, then you would need to reconfigure the CI job similarly to how we do it for oid features (e.g. see the sha2 job).

Copy link
Copy Markdown
Member

Also I don't think that zeroize support should be enabled by default, similarly to other crates it should be behind disabled-by-default crate feature.

Copy link
Copy Markdown
Author

I don't need zeroize myself- I'm just a little bored and contributing to this after work for fun. If you have another "good first issue" let me know- tarcieri suggested this one.

I'll make it a crate feature, give me a second... :)

Copy link
Copy Markdown
Member

Depending on your knowledge, it could be a good project to migrate the asm-hashes to inline assembly, similarly to #447. Also you could try to implement algorithms as per #1 (you could see other repos for similar issues).

Copy link
Copy Markdown
Author

featurified. Going to go look at the inline assembly one, that should be a fun learning experience, I've only done that in C and solidity before...

Copy link
Copy Markdown
Member

blake2 implementation probably will be replaced before v0.11 is published (see #228). Also see #545.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /