digest: could VariableOutput be replaced entirely with FixedOutput?

[tarcieri]

I believe the current digest::VariableOutput trait could be replaced entirely by FixedOutput and making the type which impls the trait generic around a type-level (typenum) parameter that defines the OutputSize.

Some precedent for this can be found in how the AesGcm crate is generic around a NonceSize:

https://github.com/RustCrypto/AEADs/blob/18a6128/aes-gcm/src/lib.rs#L169-L173

Digests which are only valid for specific sizes could define a marker trait for which sizes are considered valid, and then use that marker trait in the where bounds for when they impl digest::FixedOutput(Dirty).

This approach would make construction of a variable output digest instance infallible with output sizes checked by the type system, and also reduce the total number of traits.

[newpavlov]

Yes, it could be done and IIRC it was the initial approach. But it has a significant drawback: you will not be able to select output size at runtime without creating a huge enum, though I am not sure if anyone needs this capability in practice. I guess we could use this approach and see if anyone complains.

[tarcieri]

Aah yeah, that's a good point, although I think any time I would want to support variable sizes picked at runtime, there's a low cardinality of them

[tarcieri]

@newpavlov it'd definitely be good to improve and/or replace VariableOutput before cutting a final digest v0.10 release.

As a semi-related concern, it's not possible to implement Argon2 using the blake2 crate.

Argon2 requires a BLAKE2(b) output size of 1024-bytes, however the blake2 crate currently caps it at 64.

It seems the reason for the cap is to allocate an output buffer in advance, but that got me wondering: if we did keep VariableOutput around, why can't you just provide it a buffer of your desired size to write into at output time, as opposed to having to choose one in advance?

[newpavlov]

It's already removed in favor of the fixed-size traits.

Argon2 requires a BLAKE2(b) output size of 1024-bytes, however the blake2 crate currently caps it at 64.

IIRC BLAKE2(b) specification states that max output of the variable variant is 64 bytes. The 1024 byte variant is an Argon2 specific modification, which is built on top of the fixed 64-byte variant. I think it can be implemented directly in an argon2 crate, without exposing it in blake2.

[tarcieri]

Aah ok, my bad. I think we can close this issue then?

[tarcieri]

Hmm, I'm still a bit confused how to implement blake2b_long as used by Argon2:

https://github.com/P-H-C/phc-winner-argon2/blob/92cd2e1/src/blake2/blake2b.c#L333-L389

It's used as part of the finalization function, so it needs to be able to operate over a user-provided slice as the output.

[tarcieri]

Here's a rough Rust translation of the C code which uses VarBlake2:

use blake2::{
    digest::{self, VariableOutput},
    Blake2b, Digest, VarBlake2b,
};

const BLAKE2B_OUTBYTES: usize = 64;

fn blake2b_long(input: &[u8], mut out: &mut [u8]) {
    let outlen_bytes = (out.len() as u32).to_le_bytes();

    if out.len() <= BLAKE2B_OUTBYTES {
        let mut digest = VarBlake2b::new(out.len()).unwrap();
        digest::Update::update(&mut digest, &outlen_bytes);
        digest::Update::update(&mut digest, input);
        digest.finalize_variable(|hash| out.copy_from_slice(hash));
    } else {
        let mut digest = Blake2b::new();
        digest.update(&outlen_bytes);
        digest.update(input);

        let mut out_buffer = [0u8; BLAKE2B_OUTBYTES];
        out_buffer.copy_from_slice(&digest.finalize());

        out[..(BLAKE2B_OUTBYTES / 2)].copy_from_slice(&out_buffer[..(BLAKE2B_OUTBYTES / 2)]);
        out = &mut out[(BLAKE2B_OUTBYTES / 2)..];

        let mut in_buffer = [0u8; BLAKE2B_OUTBYTES];

        while out.len() > BLAKE2B_OUTBYTES {
            in_buffer.copy_from_slice(&out_buffer);
            out_buffer.copy_from_slice(&Blake2b::digest(&in_buffer));

            out[..(BLAKE2B_OUTBYTES / 2)].copy_from_slice(&out_buffer[..(BLAKE2B_OUTBYTES / 2)]);
            out = &mut out[(BLAKE2B_OUTBYTES / 2)..];
        }

        let mut digest = VarBlake2b::new(out.len()).unwrap();
        digest::Update::update(&mut digest, &out_buffer);
        digest.finalize_variable(|hash| out.copy_from_slice(hash));
    }
}

[newpavlov]

Ah, so it seems the variable variant is needed for processing the tail after all... The question is then: should we bring back the variable output trait or would be adding an inherent method enough?

[tarcieri]

An inherent method would suffice to solve the problem.

In terms of a trait-based API, I think for this sort of problem it'd be nice if there were something like XofReader, but it consumed self so it could only be used once.

[newpavlov]

Since in the end VariableOutput has use-cases in practice I am gonna close this issue.