Skip to content

RustCrypto Traits #1148

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

Closed
AxelNennker opened this issue Aug 3, 2019 · 11 comments
Closed

RustCrypto Traits #1148

AxelNennker opened this issue Aug 3, 2019 · 11 comments

Comments

@AxelNennker
Copy link

Have you considered to implement rust-openssl so that it implements these traits?
https://github.com/RustCrypto/traits

@sfackler
Copy link
Owner

OpenSSL's APIs don't align cleanly with rust-crypto's.

@AxelNennker
Copy link
Author

I was on vacation when you commented and closed this. Thanks for looking into it.
Do you see a chance that somebody from the contributors agrees to adjust the trait to allow openssl's implementation of e.g. sha256 align cleanly?
https://github.com/RustCrypto/traits/graphs/contributors

I think the Rust cryptography ecosystem would benefit if we had a common set of traits e.g. for digests

@newpavlov
Copy link

newpavlov commented Oct 17, 2019

@sfackler
Can you be more concrete? After a cursory look I think implementing digest traits should be fairly trivial.

Either way, we are open to API changes, next version changes are discussed in RustCrypto/traits#43.

P.S.: FYI we also plan to integrate OpenSSL assembly into our hash crates.

@sfackler
Copy link
Owner

The Hasher type uses fallible methods and has a non-fixed output size https://docs.rs/openssl/0.10.25/openssl/hash/struct.Hasher.html

@newpavlov
Copy link

You always can panic if initialization has failed or algorithm has returned a wrong number of bytes (I don't think the latter should be possible in practice though). We may add fallible initialization methods in future, since they are also can be useful if algorithm is executed on an external hardware.

Also, algorithms in the sha module do not have those issues.

@sfackler
Copy link
Owner

What is the "wrong number" of bytes? Hasher::new(MessageDigest::sha1()) will return 20 bytes, while Hasher::new(MessageDigest::sha256()) will return 32 bytes.

@newpavlov
Copy link

newpavlov commented Oct 17, 2019

You said it yourself:

Hasher type <..> has a non-fixed output size

So IIUC with OpenSSL API type system does not enforce the fact that Hasher::new(MessageDigest::sha256()) returns 32 bytes, so you'll have to check it during runtime when converting the result into [u8; 32]/GenericArray<u8, U32>.

@sfackler
Copy link
Owner

impl Digest for Hasher {
    type OutputSize = /* what goes here? */
}

@newpavlov
Copy link

newpavlov commented Oct 17, 2019

You should not implement Digest for Hasher, instead you need to define a newtype wrapper around Hasher for each supported algorithm and implement Digest for those types. To reduce amount of boilerplate it would be reasonable to use a macro. A good starting point would be to implement digest traits for types in the sha module.

Also note that you should not implement Digest directly, it's just a convenience wrapper around Input, FixedOutput, and Reset traits (i.e. you should implement them and Digest will be implemented via the blanket impl).

@newpavlov
Copy link

newpavlov commented Oct 17, 2019

To be clear: I do not argue that RustCrypto traits should be implemented as part of the openssl crate, just trying to clarify the API. I guess many will not like having RustCrypto crates in the openssl dependency tree even in an optional, disabled by default form. So doing it in a separate "bridge" crate probably will be a better approach.

@AxelNennker
Copy link
Author

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

No branches or pull requests

3 participants