Incremental Hash, allowing to continue to hash

[Drawaes]

In a number of protocols signing is based on a hash of all messages up to the signing point in the protocol. Incremental Hash is great for that because it means we don't need to hold all the current messages in the conversation in memory. However if we are required to sign at a specific point but then continue hashing for a future signing point we are stuck as we can only do a FinishAndReset

My proposal is to add a Clone or Duplicate method

public partial class IncrementalHash
{
    // Returns a new IncrementalHash in the same state
    public IncrementalHash DuplicateHash() => throw null;

    // Gets the completed hash for the already applied values,
    // but does not block AppendData from accepting more data.
    // These produce equivalent (hash1, hash2) values:
    // A)
    //   hasher.AppendData(data1);
    //   byte[] hash1 = hasher.GetCurrentHash();
    //   hasher.AppendData(data2);
    //   byte[] hash2 = hasher.GetCurrentHash();
    // B)
    //   hasher.AppendData(data1);
    //   byte[] hash1 = hasher.GetHashAndReset();
    //   hasher.AppendData(data1);
    //   hasher.AppendData(data2);
    //   byte[] hash2 = hasher.GetHashAndReset();
    // C)
    //   hasher.AppendData(data1);
    //   using (IncrementalHash tmp = hasher.DuplicateHash())
    //   {
    //       hash1 = tmp.GetHashAndReset();
    //   }
    //   hasher.AppendData(data2);
    //   hash2 = hasher.GetHashAndReset();
    public bool TryGetCurrentHash(Span<byte> destination, out int bytesWritten) => throw null;
    public byte[] GetCurrentHash() => throw null;
}

I know for a fact that both OpenSsl and BCrypt support this operation, I am unsure on osx.

Such implementation on OpenSsl would look like

            var ctx = EVP_MD_CTX_copy_ex(_ctx);

And in BCrypt something like

           var newHash = BCryptDuplicateHash(_hashHandle);

Obviously with differences due to using the .net core PAL etc

1. Updated to put back the (Try) GetCurrentHash methods as suggested by @bartonjs

[GrabYourPitchforks]

I'd honestly go with a Clone or Duplicate approach. Usability is a little more clear with a dedicated method to clone the internal state than the AndContinue proposal IMO.

[Drawaes]

I am fine with that. Have updated to reflect it

[bartonjs]

This came up previously and my initial thought was (Try)GetPartialHash or (Try)GetCurrentHash. DuplicateHash is slightly more generalized, since it allows for not only partial-and-continue but also divergent hashes.

If the "Partial" (or "Current") seems self-identifying enough then we could do both; but if we want to only do one then DuplicateHash is probably the one to do (because of generalization). And, as noted, DuplicateHash might get renamed to Clone in API review, even though that word has a certain amount of smell to it 😄.

[Drawaes]

So my initial API was either of those, but actually I like you idea of both.

I 100% agree that clone is very overloaded in meaning already and I wouldn't like it as much. How about I put back the (Try)GetCurrentHash as well (we can always negotiate down to only the duplicate).

[terrajobst]

Video

• There don't seem to be any scenarios for DuplicateHash, so let's remove it for now. If we were to do it, we should probably name it Clone though.
• Rest looks good as proposed.
• We should add a HashSizeInBytes property so that callers don't have to call TryGetCurrentHash in a loop
• While we're at it, we should also add overloads for the new TryGetCurrentHash and the existing TryGetHashAndReset to return the length instead of returning a bool.

namespace System.Security.Cryptography
{
    public partial class IncrementalHash
    {
        public bool TryGetCurrentHash(Span<byte> destination, out int bytesWritten);
        public int GetCurrentHash(Span<byte> destination);
        // public bool TryGetHashAndReset(Span<byte> destination, out int bytesWritten);
        public int GetHashAndReset(Span<byte> destination);
        public byte[] GetCurrentHash();
        public int HashLengthInBytes { get; }
    }
}

[Drawaes]

@terrajobst It appears that some API changes are needed on the public type HashProvider. They are in the PR

public virtual int GetCurrentHash(Span<byte> destination);
public virtual bool TryGetCurrentHash(Span<byte> destination, out int bytesWritten);

[bartonjs]

HashProvider isn't public...