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.

Sign up for GitHub

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

OSX compatible #2511

Merged
merged 15 commits into from
Jul 16, 2021
Merged

OSX compatible #2511

Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
72d4bed
OSX compatible
shargon Jun 24, 2021
d04985a
Remove debug
shargon Jun 24, 2021
3ca1f0a
Simplify
shargon Jun 24, 2021
217f43f
Merge branch 'master' into verify-osx
shargon Jun 30, 2021
fc655ab
Use constant
shargon Jun 30, 2021
ec7c699
Merge branch 'master' into verify-osx
shargon Jul 8, 2021
92c384b
Update src/neo/Cryptography/Crypto.cs
shargon Jul 8, 2021
a0828e8
Use only short signatures
shargon Jul 9, 2021
d48f6b2
Update src/neo/Cryptography/Crypto.cs
shargon Jul 12, 2021
8cc8c03
Update src/neo/Cryptography/Crypto.cs
shargon Jul 12, 2021
cdefaa8
Merge branch 'master' into verify-osx
shargon Jul 12, 2021
bbe1751
Update Crypto.cs
erikzhang Jul 15, 2021
a8e17cc
Merge branch 'master' into verify-osx
shargon Jul 15, 2021
38f9f05
Remove try block
shargon Jul 15, 2021
6e94f2c
Merge branch 'master' into verify-osx
erikzhang Jul 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions src/neo/Cryptography/Crypto.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Runtime.InteropServices;
using System.Security.Cryptography;

namespace Neo.Cryptography
Expand All @@ -8,6 +9,8 @@ namespace Neo.Cryptography
/// </summary>
public static class Crypto
{
private static readonly bool IsOSX = RuntimeInformation.IsOSPlatform(OSPlatform.OSX);

/// <summary>
/// Calculates the 160-bit hash value of the specified message.
/// </summary>
Expand Down Expand Up @@ -59,21 +62,42 @@ public static byte[] Sign(byte[] message, byte[] prikey, byte[] pubkey)
/// <returns><see langword="true"/> if the signature is valid; otherwise, <see langword="false"/>.</returns>
public static bool VerifySignature(ReadOnlySpan<byte> message, ReadOnlySpan<byte> signature, ECC.ECPoint pubkey)
{
ECCurve curve =
pubkey.Curve == ECC.ECCurve.Secp256r1 ? ECCurve.NamedCurves.nistP256 :
pubkey.Curve == ECC.ECCurve.Secp256k1 ? ECCurve.CreateFromFriendlyName("secP256k1") :
throw new NotSupportedException();
byte[] buffer = pubkey.EncodePoint(false);
using var ecdsa = ECDsa.Create(new ECParameters
if (IsOSX)
shargon marked this conversation as resolved.
Show resolved Hide resolved
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
Curve = curve,
Q = new ECPoint
try
{
X = buffer[1..33],
Y = buffer[33..]
var curve = Org.BouncyCastle.Asn1.Sec.SecNamedCurves.GetByName(pubkey.Curve == ECC.ECCurve.Secp256r1 ? "secp256r1" : "secp256k1");
shargon marked this conversation as resolved.
Show resolved Hide resolved
var domain = new Org.BouncyCastle.Crypto.Parameters.ECDomainParameters(curve.Curve, curve.G, curve.N, curve.H);
var point = curve.Curve.CreatePoint(
new Org.BouncyCastle.Math.BigInteger(pubkey.X.Value.ToString()),
new Org.BouncyCastle.Math.BigInteger(pubkey.Y.Value.ToString()));
var pubKey = new Org.BouncyCastle.Crypto.Parameters.ECPublicKeyParameters("ECDSA", point, domain);
var signer = Org.BouncyCastle.Security.SignerUtilities.GetSigner("SHA-256withECDSA");

signer.Init(false, pubKey);
signer.BlockUpdate(message.ToArray(), 0, message.Length);
return signer.VerifySignature(signature.ToArray());
shargon marked this conversation as resolved.
Show resolved Hide resolved
}
});
return ecdsa.VerifyData(message, signature, HashAlgorithmName.SHA256);
catch { return false; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this try-catch?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZhangTao1596 could you test it without the try block in osx?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZhangTao1596 could you test it without the try block in osx?

I think it's OK since we already went into 'try', but we can't go to 'catch'. After remove, no effect on the result.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I still suggest keeping the try-catch for a THIRD-PARTY lib, unless you know it very well.

}
else
{
byte[] buffer = pubkey.EncodePoint(false);
ECCurve curve =
pubkey.Curve == ECC.ECCurve.Secp256r1 ? ECCurve.NamedCurves.nistP256 :
pubkey.Curve == ECC.ECCurve.Secp256k1 ? ECCurve.CreateFromFriendlyName("secP256k1") :
throw new NotSupportedException();
using var ecdsa = ECDsa.Create(new ECParameters
{
Curve = curve,
Q = new ECPoint
{
X = buffer[1..33],
Y = buffer[33..]
}
});
return ecdsa.VerifyData(message, signature, HashAlgorithmName.SHA256);
}
}

/// <summary>
Expand Down