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

sha3 v0.10 - chain_update(): unsatisfied trait bounds #335

Closed
aewag opened this issue Dec 13, 2021 · 9 comments · Fixed by RustCrypto/traits#846
Closed

sha3 v0.10 - chain_update(): unsatisfied trait bounds #335

aewag opened this issue Dec 13, 2021 · 9 comments · Fixed by RustCrypto/traits#846

Comments

@aewag
Copy link
Contributor

aewag commented Dec 13, 2021

I just updated sha3 v0.9 to v0.10, but struggle to use the chain_update method:

error[E0599]: the method `chain_update` exists for struct `CoreWrapper<Shake256Core>`, but its trait bounds were not satisfied
  --> src/main.rs:13:10
   |
13 |           .chain_update(b"abc");
   |            ^^^^^^^^^^^^ method cannot be called on `CoreWrapper<Shake256Core>` due to unsatisfied trait bounds
   |
  ::: /home/alwagner/.cargo/registry/src/github.com-1ecc6299db9ec823/digest-0.10.0/src/core_api/wrapper.rs:20:1

I skimmed through the Changelog and the docs, but failed to see a hint or note to fix the issue. Do you have any suggestions to get this working? :)

I added a minimal "working" example below:

Example

Cargo.toml

[dependencies]
sha2 = "0.10.0"
sha3 = "0.10.0"

main.rs

fn main() {
    shake256();
    sha256();
}

fn shake256() {
    use sha3::{
        Digest, Shake256,
        digest::{ExtendableOutput, Update, XofReader},
    };

    let mut hasher = Shake256::default()
        .chain_update(b"abc");

    // read hash digest
    let mut digest = [0u8; 32];
    hasher.finalize_xof().read(&mut digest);

    assert_eq!(digest, [0u8; 32]);
}

fn sha256() {
    use sha2::{Digest, Sha256};

    // write input message
    let digest = Sha256::default()
        .chain_update(b"abc")
        .finalize();

    assert_eq!(&digest[..], [0u8; 32]);
}
@newpavlov
Copy link
Member

newpavlov commented Dec 13, 2021

Shake256 is an extendable output function (XOF) and it does not implement the Digest trait (and never was), which gets implemented only for fixed output functions. With Shake256 you have to use the "mid-level" traits directly (i.e. Update, ExtendableOutput, and others). Maybe you want to use sha3::Sha3_256?

@aewag
Copy link
Contributor Author

aewag commented Dec 13, 2021

Maybe you want to use sha3::Sha3_256?

Unfortunately, I can't because of the different padding, which would result to different digests. (Shake256 is expected by the standard).

With sha3 v0.9 I used the chain() method, which works with the code above. I fail to see which other mid-level traits need to be used with the new version v0.10 to use the chain_update() method. Could you maybe provide me a link to the docs or similar? Or is it no longer possible to use the chain method together with Shake256?

@tarcieri
Copy link
Member

@newpavlov it seems like chained input should still work for XOFs? I would expect them to only differ from other hash functions in terms of finalization (with a XOF returning a stateful reader object in that case)

@newpavlov
Copy link
Member

Ah, yes. I forgot that in digest v0.9 we had the Update::chain method. It got removed for some reason in one of several pre versions without reflecting it in the changelog. It's probably worth to bring it back. Maybe it also could be worth to add a method like this?

trait Update {
    fn update(&mut self, data: &[u8]);
    fn new_with_prefix(data: impl AsRef<[u8]>) -> Self
    where Self: Sized + Default
    { let mut h = Self::default(); h.update(data.as_ref()); h }
}

@aewag
Copy link
Contributor Author

aewag commented Dec 13, 2021

It's probably worth to bring it back.

Thanks, that would be nice. :)

Maybe it also could be worth to add a method like this?

For me it would probably simplify some internals, but I am not sure if it is helpful in general.

@aewag
Copy link
Contributor Author

aewag commented Dec 14, 2021

Thanks, just tested it with Shake256 :)

BTW was the renaming of Digest::chain to Digest::chain_update with a bigger purpose in mind?

RustCrypto/traits@5acb9f7#diff-2d770173354c22c7a64855fd000b85e4fd0db68e370a0d571b11df3295578aceL72

I ask, because this will probably break code at other places, at least in the READMEs it requires some updates, but at a first glance it seems to effect other RustCrypto traits as well, for example:

https://github.com/RustCrypto/traits/blob/e922122260e8ed2f73094483b6e9e3238f73df36/signature/derive/src/lib.rs#L27

@newpavlov
Copy link
Member

The issue with having both Digest::chain and Update::chain is that having both traits in scope causes conflicts (this is why we have Digest::finalize and FixedOutput::finalize_fixed, even though the former simply wraps the latter), but I guess we already have the update method on both traits...

Ideally Digest would be a simple trait alias or an extension trait, but, unfortunately, the former is not supported by Rust, while the latter is not ergonomic (IIRC in user code you would have to import both Digest and Update to use the update method, even though Digest is an extension trait.). Another potential solution is hypothetical inherent traits.

@aewag
Copy link
Contributor Author

aewag commented Dec 14, 2021

The issue with having both Digest::chain and Update::chain is that having both traits in scope causes conflicts (this is why we have Digest::finalize and FixedOutput::finalize_fixed, even though the former simply wraps the latter), but I guess we already have the update method on both traits...

Yep, having both in scope won't work. But from my perspective, I understood the Digest trait as a wrapper around commonly used traits. So to say, you only use the Digest trait if it fits your use-case, if not you would have to select the wanted mid-level traits (e.g. Update, FixedOutput).

(There are more methods that are overlapping, for instance finalize_into & finalize_into_reset.)

IMO using the same names for the method in the high-level trait and in the mid-level trait would make it easier to understand and use.

@newpavlov
Copy link
Member

Yeah, in digest v0.11 we probably should reconsider method names. Also it could be worth to remove the *_into methods and leave only methods which return array. Both are mostly equivalent to each other ABI-wise, but the latter results in a bit of unneccecary stack register twiddling, which hopefully can be removed by compiler improvements.

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 a pull request may close this issue.

3 participants