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

digest v0.10 #212

Open
newpavlov opened this issue Nov 23, 2021 · 4 comments
Open

digest v0.10 #212

newpavlov opened this issue Nov 23, 2021 · 4 comments

Comments

@newpavlov
Copy link

newpavlov commented Nov 23, 2021

In RustCrypto we plan to soon migrate our hash crates to digest v0.10, see RustCrypto/traits#819 and RustCrypto/hashes#217.

Probably the most notable change is that crypto-mac got merged into digest, so it should simplify a bit implementation of traits in your case. Most mid-level traits (Update, FixedOutput, etc.) haven't changed much, so migration should be relatively simple.

But I wonder if you will be interested in using the new block-level traits (see the core_api module). By implementing them, buffering can be handled automatically by the CoreWrapper type. There are also wrappers which handle switch between runtime and compile-time selection of output size. Notably, the block-buffer which is used by the wrapper types is designed to completely eliminate unreachable panics in buffering code without unsafe code in hash implementation crates.

If you have any comments, questions or suggestions about the new API, I will be happy to hear them out.

@oconnor663
Copy link
Member

Updating to digest v0.10 should be pretty natural as soon as that stabilizes. We've make a clear "no compatibility guarantees" note in our docs around this.

For the lower-level block-buffer stuff, I'm not totally sure I understand the details, but my guess is that it's not going to be a good fit for BLAKE3. To get the most out of SIMD, BLAKE3 likes to process multiple 1 KiB chunks in parallel, for example 16 chunks at a time on modern x86 CPUs with AVX-512 support. Each chunk is coincidentally also 16 blocks of 64 bytes each, so the block index read order in that case would be parallel groups like (0, 16, 32, ..., 240), (1, 17, 33, ..., 241), ..., (15, 31, 47, ..., 255). This represents 16 KiB of hash input. If .update() is called with even more than that, BLAKE3 will split the input buffer recursively and work with wide arrays of chaining values going "back up the tree", so that these same optimizations can be applied to compressing parent nodes. When multithreading is involved, that's another layer on top of all this. So my guess is that we won't be able to fit all the optimizations we want to do within the block-buffer abstraction. (I'd also expect KangarooTwelve to have a subset of the same problems.) But I'd be really curious to hear your thoughts about this.

@newpavlov
Copy link
Author

newpavlov commented Nov 23, 2021

Note that UpdateCore::update_blocks accepts slice of blocks, so you are not limited to processing data block-by-block. For more details see the BlockBuffer::digest_blocks method (it's used internally by the wrappers). As you can notice, the compress callback can be called with slice of length one if there is unprocessed data in the buffer, but most of the work is done on the line 145. In other words, if you pass to the wrapper ~64 KB of data, then the compress callback will be called with a slice containing ~1000 blocks. So I don't think the block-level API design prevents you from applying the described optimizations.

@oconnor663
Copy link
Member

I've spent 45 minutes trying to add trait impls to let me instantiate an hmac::Hmac<blake3::Hasher>, and so far I haven't succeeded.

I've hacked together a few new impls like this:

impl digest::core_api::BlockSizeUser for Hasher {
    type BlockSize = typenum::U64;
}

impl digest::core_api::BufferKindUser for Hasher {
    type BufferKind = block_buffer::Lazy;
}

impl digest::core_api::UpdateCore for Hasher {
    fn update_blocks(&mut self, blocks: &[digest::core_api::Block<Self>]) {
        for block in blocks {
            // TODO: we need to use larger slices here to avoid a performance hit
            self.update(block);
        }
    }
}

These aren't sufficient, though, and the errors I'm seeing look like this:

error[E0599]: the function or associated item `new_from_slice` exists for struct `CoreWrapper<HmacCore<Hasher>>`, but its trait bounds were not satisfied
   --> src/traits.rs:189:43
    |
189 |           let mut x = hmac::Hmac::<Hasher>::new_from_slice(b"test").unwrap();
    |                                             ^^^^^^^^^^^^^^ function or associated item cannot be called on `CoreWrapper<HmacCore<Hasher>>` due to unsatisfied trait bounds
    |
   ::: /home/jacko/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/digest-0.10.1/src/core_api/wrapper.rs:20:1
    |
20  | / pub struct CoreWrapper<T>
21  | | where
22  | |     T: BufferKindUser,
23  | |     T::BlockSize: IsLess<U256>,
...   |
27  | |     buffer: BlockBuffer<T::BlockSize, T::BufferKind>,
28  | | }
    | | -
    | | |
    | | doesn't satisfy `CoreWrapper<HmacCore<Hasher>>: FixedOutput`
    | | doesn't satisfy `CoreWrapper<HmacCore<Hasher>>: KeyInit`
    | | doesn't satisfy `CoreWrapper<HmacCore<Hasher>>: MacMarker`
    | |_doesn't satisfy `CoreWrapper<HmacCore<Hasher>>: Mac`
    |   doesn't satisfy `CoreWrapper<HmacCore<Hasher>>: Update`
    |
    = note: the following trait bounds were not satisfied:
            `CoreWrapper<HmacCore<Hasher>>: KeyInit`
            which is required by `CoreWrapper<HmacCore<Hasher>>: Mac`
            `CoreWrapper<HmacCore<Hasher>>: Update`
            which is required by `CoreWrapper<HmacCore<Hasher>>: Mac`
            `CoreWrapper<HmacCore<Hasher>>: FixedOutput`
            which is required by `CoreWrapper<HmacCore<Hasher>>: Mac`
            `CoreWrapper<HmacCore<Hasher>>: MacMarker`
            which is required by `CoreWrapper<HmacCore<Hasher>>: Mac`
            `&CoreWrapper<HmacCore<Hasher>>: KeyInit`
            which is required by `&CoreWrapper<HmacCore<Hasher>>: Mac`
            `&CoreWrapper<HmacCore<Hasher>>: Update`
            which is required by `&CoreWrapper<HmacCore<Hasher>>: Mac`
            `&CoreWrapper<HmacCore<Hasher>>: FixedOutput`
            which is required by `&CoreWrapper<HmacCore<Hasher>>: Mac`
            `&CoreWrapper<HmacCore<Hasher>>: MacMarker`
            which is required by `&CoreWrapper<HmacCore<Hasher>>: Mac`
            `&mut CoreWrapper<HmacCore<Hasher>>: KeyInit`
            which is required by `&mut CoreWrapper<HmacCore<Hasher>>: Mac`
            `&mut CoreWrapper<HmacCore<Hasher>>: Update`
            which is required by `&mut CoreWrapper<HmacCore<Hasher>>: Mac`
            `&mut CoreWrapper<HmacCore<Hasher>>: FixedOutput`
            which is required by `&mut CoreWrapper<HmacCore<Hasher>>: Mac`
            `&mut CoreWrapper<HmacCore<Hasher>>: MacMarker`
            which is required by `&mut CoreWrapper<HmacCore<Hasher>>: Mac`

error[E0277]: the trait bound `Hasher: CoreProxy` is not satisfied
   --> src/traits.rs:189:21
    |
189 |         let mut x = hmac::Hmac::<Hasher>::new_from_slice(b"test").unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^ the trait `CoreProxy` is not implemented for `Hasher`
    |
    = note: required because of the requirements on the impl of `BlockSizeUser` for `HmacCore<Hasher>`

Despite this error message, I don't imagine I'm actually supposed to implement CoreProxy myself, and I'm probably making some simple mistake somewhere else that leads to this message. Maybe I'm missing some other trait impl that will take care of all this?

Subjectively, this has been surprisingly complicated. Part of the problem is that digest::core_api doesn't include any examples or suggestions for implementers in its documentation, and I expect that'll naturally get better after things stabilize. But apart from that, there really are a lot of traits flying around here. Is it possible that some of this complexity is accidental?

@jbis9051
Copy link
Contributor

@oconnor663 See #223 (comment). I don't think it's possible for blake3 to work with Hmac. See https://docs.rs/hmac/latest/hmac/struct.SimpleHmac.html

Simplified HMAC instance able to operate over hash functions which do not expose block-level API and hash functions which process blocks lazily (e.g. BLAKE2).

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