Adapt Hashes.HMACSHA256() so that it uses native hashes when they are… #965
Conversation
|
Using following compilation symbols to get a build that works in Blazor wasm: The gist being that you need to add NO_NATIVE_HMACSHA512 and NO_NATIVE_RFC2898_HMACSHA512. |
NBitcoin/Crypto/Hashes.cs
Outdated
| @@ -12,6 +12,8 @@ | |||
|
|
|||
| #if !NONATIVEHASH | |||
| using System.Security.Cryptography; | |||
| using NBitcoin.Crypto.digests; // use managed SHA implementations | |||
| using NBitcoin.BouncyCastle.Crypto.Parameters; // we'll need this to create HMACs based o the managed SHAs. | |||
There was a problem hiding this comment.
introduced 1 warning here: The using directive for 'NBitcoin.BouncyCastle.Crypto.Parameters' appeared previously in this namespace
|
I have no idea why the CI tests timed out on .NET 4.6 |
|
Remove changes on |
|
|
||
| public int DoFinal(byte[] output, int outOff) | ||
| { | ||
| var hash = nativeSha256.ComputeHash(input, 0, input.Length); |
There was a problem hiding this comment.
Can you instead rely on NBitcoin.Secp256k1.SHA256 class which properly implement DoFinal and BlockUpdate and is widely tested and memory efficient?
There was a problem hiding this comment.
Right, I'll take care of it tonight.
There was a problem hiding this comment.
@NicolasDorier I found the NBitcoin.Secpk1.SHA class, but it doesn't look like it implements the IDigest interface, nor the BlockUpdate/DoFinal methods.
There was a problem hiding this comment.
I am not saying to use it instead of ManagedSha256Digest, but to use it rather than that nativeSha256
There was a problem hiding this comment.
Hello @NicolasDorier, I understand now. There is no NBitcoin.Secp512k1.SHA512 equivalent in the Secpk1 project though (to go along with the SHA256). Besides, wouldn't it be great if NBitcoin could rely on the framework's builtins?
It's not clear to me if DoFinal() and BlockUpdate() are BouncyCastle-specific interface methods, or if they are a more wider known concept. I'm not sure how to implement them correctly. I was hoping someone here would review it, and fill in the gaps for me.
NBitcoin/Crypto/Hashes.cs
Outdated
|
|
||
| public static byte[] HMACSHA256(byte[] key, byte[] data) | ||
| { | ||
| var mac = new NBitcoin.BouncyCastle.Crypto.Macs.HMac(new ManagedSha256Digest()); |
There was a problem hiding this comment.
Can you dispose all the ManagedSha256Digest you instanciate?
There was a problem hiding this comment.
I made another update to dispose these.
|
There are conflicts, you need to rebase. |
JVanloofsvelt
force-pushed
the
feature/use-native-hash-with-BC-HMac
branch
from
e9f4bce
to
4d84380
Compare
… present, even if native HMAC is not. If there is no native HMACSHA256, but there is native SHA256, then use Bouncy Castle's HMAC implementation in combination with the native Sha256 digest (instead of Bouncy Castle's SHA256 implementation).
Add guards (compilation symbols) to conditionally add using statements according to the target framework.
JVanloofsvelt
force-pushed
the
feature/use-native-hash-with-BC-HMac
branch
from
4d84380
to
8b1e49d
Compare
|
@knocte I rebased |
|
Ping @JVanloofsvelt |
NicolasDorier
force-pushed
the
master
branch
from
878114e
to
24301c6
Compare
… present, even if native HMAC is not.
If there is no native HMACSHA256, but there is native SHA256, then use Bouncy Castle's HMAC implementation in combination with the native Sha256 digest (instead of Bouncy Castle's SHA256 implementation).