Implement hash and HMAC stream one shots #63757
Conversation
This implements hashing and HMAC statics for streams. Additionally, "LiteHmac" and "LiteHash" were introduced. The existing HMAC and hash provider functionality do some bookkeeping we don't need for resetting. Since we do not need to use these hash handles after the digest has been finalized, resetting is unnecessary work. For HMAC, that also means keeping a copy of the key around for some implementations which we don't need to do. The LiteHash and LiteHmac types are implemented as structs with a common interface. To avoid boxing, generics are used and constrained to the interface where possible. The Browser implementation just defers to the existing HashDispenser rather than do anything novel.
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones, @krwq Issue DetailsThis implements hashing and HMAC statics for streams. Additionally, The LiteHash and LiteHmac types are implemented as structs with a common The Browser implementation just defers to the existing HashDispenser rather Closes #62489.
|
|
Draft to get feedback from CI, and this is going to conflict with #63685. That should be merged first and I will resolve conflicts. |
|
This does not implement any shortcuts for some streams. We could possibly do this for |
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HMACMD5.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/LiteHashProvider.cs
Outdated
Show resolved
Hide resolved
- Throw exceptions synchronously. - Dispose of hash even if CryptoPool rent/return fails.
vcsjones
force-pushed
the
hash-stream-oneshot-attempt-2
branch
from
277d7a3 to
d2720ee
Compare
| } | ||
|
|
||
| /// This takes ownership of the hash parameter and disposes of it when done. | ||
| private static int ProcessStream<T>(T hash, Stream source, Span<byte> destination) where T : ILiteHash |
There was a problem hiding this comment.
I've done something similar (in readonly struct + generic constrained to interface) in a library I contribute to and while it works, any method called on that interface will force a defensive copy, since the compiler is not able to recognize that the struct method itself is readonly. I think it only sees the interface and thus decides on a defensive copy. I'm also not sure anymore whether without the in modifier as you have here, the compiler is able to figure it out properly or not.
Based on the size of the structs used, I assume it's not as relevant here, just wanted to mention it.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HMACSHA256.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/LiteHash.Apple.cs
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/LiteHash.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/MD5.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/MD5.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/MD5.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/tests/HashAlgorithmTestDriver.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/tests/Sha384Tests.cs
Outdated
Show resolved
Hide resolved
* OpenSsl / Android can wrap. * Apple can wrap. * Windows should not wrap. The HashProviderCng is somewhat specialized in its ability to reset. It did up-front check to determine if the platform supported reusable hash providers, and further had a single implementation for HMAC and Digests. The current Lite hash design requires that they remain separate types.
ghost
locked as resolved and limited conversation to collaborators
bartonjs
added
the
needs-further-triage
This implements hashing and HMAC statics for streams. Additionally,
"LiteHmac" and "LiteHash" were introduced. The existing HMAC and hash
provider functionality do some bookkeeping we don't need for resetting.
Since we do not need to use these hash handles after the digest has
been finalized, resetting is unnecessary work. For HMAC, that also means
keeping a copy of the key around for some implementations which we don't
need to do.
The LiteHash and LiteHmac types are implemented as structs with a common
interface. To avoid boxing, generics are used and constrained to the interface
where possible.
The Browser implementation just defers to the existing HashDispenser rather
than do anything novel.
Closes #62489.