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.

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

EcRecover #3633

Open
wants to merge 21 commits into
base: HF_Echidna
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
141 changes: 115 additions & 26 deletions src/Neo/Cryptography/Crypto.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
// modifications are permitted.

using Neo.IO.Caching;
using Org.BouncyCastle.Asn1;
using Org.BouncyCastle.Asn1.X9;
using Org.BouncyCastle.Crypto.Parameters;
using Org.BouncyCastle.Math;
using Org.BouncyCastle.Utilities.Encoders;
using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Security.Cryptography;

Expand All @@ -24,21 +27,18 @@ namespace Neo.Cryptography
/// </summary>
public static class Crypto
{
private static readonly ECDsaCache CacheECDsa = new();
private static readonly bool IsOSX = RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
private static readonly ECCurve secP256k1 = ECCurve.CreateFromFriendlyName("secP256k1");
private static readonly X9ECParameters bouncySecp256k1 = Org.BouncyCastle.Asn1.Sec.SecNamedCurves.GetByName("secp256k1");
private static readonly X9ECParameters bouncySecp256r1 = Org.BouncyCastle.Asn1.Sec.SecNamedCurves.GetByName("secp256r1");

/// <summary>
/// Holds domain parameters for Secp256r1 elliptic curve.
/// 64 bytes ECDSA signature + 1 byte recovery id
/// </summary>
private static readonly ECDomainParameters secp256r1DomainParams = new ECDomainParameters(bouncySecp256r1.Curve, bouncySecp256r1.G, bouncySecp256r1.N, bouncySecp256r1.H);

private const int RecuperableSignatureLength = 64 + 1;
/// <summary>
/// Holds domain parameters for Secp256k1 elliptic curve.
/// 64 bytes ECDSA signature
/// </summary>
private static readonly ECDomainParameters secp256k1DomainParams = new ECDomainParameters(bouncySecp256k1.Curve, bouncySecp256k1.G, bouncySecp256k1.N, bouncySecp256k1.H);
private const int SignatureLength = 64;
private static readonly BigInteger s_prime = new(1, Hex.Decode("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F"));
private static readonly ECDsaCache CacheECDsa = new();
private static readonly bool IsOSX = RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
private static readonly ECCurve secP256k1 = ECCurve.CreateFromFriendlyName("secP256k1");

/// <summary>
/// Calculates the 160-bit hash value of the specified message.
Expand Down Expand Up @@ -72,13 +72,9 @@ public static byte[] Sign(byte[] message, byte[] priKey, ECC.ECCurve ecCurve = n
{
if (hasher == Hasher.Keccak256 || (IsOSX && ecCurve == ECC.ECCurve.Secp256k1))
{
var domain =
ecCurve == null || ecCurve == ECC.ECCurve.Secp256r1 ? secp256r1DomainParams :
ecCurve == ECC.ECCurve.Secp256k1 ? secp256k1DomainParams :
throw new NotSupportedException(nameof(ecCurve));
var signer = new Org.BouncyCastle.Crypto.Signers.ECDsaSigner();
var privateKey = new BigInteger(1, priKey);
var priKeyParameters = new ECPrivateKeyParameters(privateKey, domain);
var priKeyParameters = new ECPrivateKeyParameters(privateKey, ecCurve.BouncyCastleDomainParams);
signer.Init(true, priKeyParameters);
var messageHash =
hasher == Hasher.SHA256 ? message.Sha256() :
Expand Down Expand Up @@ -112,6 +108,107 @@ public static byte[] Sign(byte[] message, byte[] priKey, ECC.ECCurve ecCurve = n
return ecdsa.SignData(message, hashAlg);
}

/// <summary>
/// ECRecover
/// </summary>
/// <param name="curve">Curve</param>
/// <param name="signature">Signature</param>
/// <param name="hash">Message hash</param>
/// <param name="format">Signature format</param>
/// <returns>Allowed Public keys</returns>
public static ECC.ECPoint[] ECRecover(ECC.ECCurve curve, byte[] signature, byte[] hash, SignatureFormat format = SignatureFormat.Der)
{
BigInteger r, s;
int recId = 0, recIdTo = 4;

// Decode signature

switch (format)
{
case SignatureFormat.Der:
{
var derSequence = (DerSequence)Asn1Object.FromByteArray(signature);
r = ((DerInteger)derSequence[0]).Value;
s = ((DerInteger)derSequence[1]).Value;

if (derSequence.Count == 3)
{
recId = ((DerInteger)derSequence[2]).IntValueExact;
recIdTo = recId + 1;
}
break;
}
case SignatureFormat.Fixed32:
{
r = new(1, signature, 0, 32);
s = new(1, signature, 32, 32);

if (signature.Length == RecuperableSignatureLength)
{
recId = signature[SignatureLength];
recIdTo = recId + 1;
}
break;
}
default: throw new InvalidOperationException("Invalid signature format");
}

// Validate values

Choose a reason for hiding this comment

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

on the topic of validation, maybe having the s value in the lower half of the domain would be a good guard against signature maleability. At least that's how that was addressed in ethereum where the wallets only generate (r,s) pairs with s lower than n/2 (where n is the order of the curve). Because a pair (r, n-s) is also a valid solution leading to possible issues.


if (recId < 0 || recId >= 4) throw new ArgumentException("v should be positive less than 4");
if (r.SignValue < 0) throw new ArgumentException("r should be positive");
if (s.SignValue < 0) throw new ArgumentException("s should be positive");
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/Hecate2/ECrecover/blob/c3ec9b4ca4f74e66c426b48452f86cf643a9f6fb/Program.cs#L83-L84

        if (r.CompareTo(BigInteger.One) < 0 || r.CompareTo(curve.N) > 0) throw new ArgumentException($"Invalid r value {r}; expected [1, {curve.N}]");
        if (s.CompareTo(BigInteger.One) < 0 || s.CompareTo(curve.N) > 0) throw new ArgumentException($"Invalid s value {s}; expected [1, {curve.N}]");

Copy link
Contributor

Choose a reason for hiding this comment

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

Optionally we can require s < n / 2 if v is not specified, for Ethereum


// Precompute variables

var n = curve.BouncyCastleCurve.N;
var e = new BigInteger(1, hash);
var eInv = BigInteger.Zero.Subtract(e).Mod(n);
var rInv = r.ModInverse(n);
var srInv = rInv.Multiply(s).Mod(n);
var eInvrInv = rInv.Multiply(eInv).Mod(n);

// Do the work

var recovered = new List<ECC.ECPoint>();

for (; recId < recIdTo; ++recId)
{
var i = BigInteger.ValueOf((long)recId / 2);
var x = r.Add(i.Multiply(n));

if (x.CompareTo(s_prime) >= 0)
{
continue;
}

var decompressedRKey = DecompressKey(curve.BouncyCastleCurve.Curve, x, (recId & 1) == 1);
if (!decompressedRKey.Multiply(n).IsInfinity)
{
continue;
}

var q = Org.BouncyCastle.Math.EC.ECAlgorithms.SumOfTwoMultiplies(curve.BouncyCastleCurve.G, eInvrInv, decompressedRKey, srInv);
recovered.Add(ECC.ECPoint.FromBytes(q.Normalize().GetEncoded(false), curve));
}

return [.. recovered];
}

/// <summary>
/// Decompress key
/// </summary>
/// <param name="curve">ECC curve</param>
/// <param name="xBN">xBN</param>
/// <param name="yBit">yBit</param>
/// <returns>ECPoint</returns>
private static Org.BouncyCastle.Math.EC.ECPoint DecompressKey(Org.BouncyCastle.Math.EC.ECCurve curve, BigInteger xBN, bool yBit)
{
var compEnc = X9IntegerConverter.IntegerToBytes(xBN, 1 + X9IntegerConverter.GetByteLength(curve));
compEnc[0] = (byte)(yBit ? 0x03 : 0x02);
return curve.DecodePoint(compEnc);
}

/// <summary>
/// Verifies that a digital signature is appropriate for the provided key, message and hash algorithm.
/// </summary>
Expand All @@ -126,18 +223,10 @@ public static bool VerifySignature(ReadOnlySpan<byte> message, ReadOnlySpan<byte

if (hasher == Hasher.Keccak256 || (IsOSX && pubkey.Curve == ECC.ECCurve.Secp256k1))
{
var domain =
pubkey.Curve == ECC.ECCurve.Secp256r1 ? secp256r1DomainParams :
pubkey.Curve == ECC.ECCurve.Secp256k1 ? secp256k1DomainParams :
throw new NotSupportedException(nameof(pubkey.Curve));
var curve =
pubkey.Curve == ECC.ECCurve.Secp256r1 ? bouncySecp256r1.Curve :
bouncySecp256k1.Curve;

var point = curve.CreatePoint(
var point = pubkey.Curve.BouncyCastleCurve.Curve.CreatePoint(
new BigInteger(pubkey.X.Value.ToString()),
new BigInteger(pubkey.Y.Value.ToString()));
var pubKey = new ECPublicKeyParameters("ECDSA", point, domain);
var pubKey = new ECPublicKeyParameters("ECDSA", point, pubkey.Curve.BouncyCastleDomainParams);
var signer = new Org.BouncyCastle.Crypto.Signers.ECDsaSigner();
signer.Init(false, pubKey);

Expand Down
16 changes: 13 additions & 3 deletions src/Neo/Cryptography/ECC/ECCurve.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// modifications are permitted.

using Neo.Extensions;
using Org.BouncyCastle.Crypto.Parameters;
using System.Globalization;
using System.Numerics;

Expand All @@ -33,9 +34,14 @@ public class ECCurve
/// </summary>
public readonly ECPoint G;

public readonly Org.BouncyCastle.Asn1.X9.X9ECParameters BouncyCastleCurve;
/// <summary>
/// Holds domain parameters for Secp256r1 elliptic curve.
/// </summary>
public readonly ECDomainParameters BouncyCastleDomainParams;
internal readonly int ExpectedECPointLength;

private ECCurve(BigInteger Q, BigInteger A, BigInteger B, BigInteger N, byte[] G)
private ECCurve(BigInteger Q, BigInteger A, BigInteger B, BigInteger N, byte[] G, string curveName)
{
this.Q = Q;
ExpectedECPointLength = ((int)VM.Utility.GetBitLength(Q) + 7) / 8;
Expand All @@ -44,6 +50,8 @@ private ECCurve(BigInteger Q, BigInteger A, BigInteger B, BigInteger N, byte[] G
this.N = N;
Infinity = new ECPoint(null, null, this);
this.G = ECPoint.DecodePoint(G, this);
BouncyCastleCurve = Org.BouncyCastle.Asn1.Sec.SecNamedCurves.GetByName(curveName);
BouncyCastleDomainParams = new ECDomainParameters(BouncyCastleCurve.Curve, BouncyCastleCurve.G, BouncyCastleCurve.N, BouncyCastleCurve.H);
}

/// <summary>
Expand All @@ -55,7 +63,8 @@ private ECCurve(BigInteger Q, BigInteger A, BigInteger B, BigInteger N, byte[] G
BigInteger.Zero,
7,
BigInteger.Parse("00FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", NumberStyles.AllowHexSpecifier),
("04" + "79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798" + "483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8").HexToBytes()
("04" + "79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798" + "483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8").HexToBytes(),
"secp256k1"
);

/// <summary>
Expand All @@ -67,7 +76,8 @@ private ECCurve(BigInteger Q, BigInteger A, BigInteger B, BigInteger N, byte[] G
BigInteger.Parse("00FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFC", NumberStyles.AllowHexSpecifier),
BigInteger.Parse("005AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B", NumberStyles.AllowHexSpecifier),
BigInteger.Parse("00FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551", NumberStyles.AllowHexSpecifier),
("04" + "6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296" + "4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5").HexToBytes()
("04" + "6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296" + "4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5").HexToBytes(),
"secp256r1"
);
}
}
26 changes: 26 additions & 0 deletions src/Neo/Cryptography/SignatureFormat.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (C) 2015-2024 The Neo Project.
//
// SignatureFormat.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

namespace Neo.Cryptography
{
public enum SignatureFormat : byte
{
/// <summary>
/// Der
/// </summary>
Der = 0,

/// <summary>
/// Fixed 32 bytes per BigInteger
/// </summary>
Fixed32 = 1
}
}
43 changes: 43 additions & 0 deletions src/Neo/SmartContract/Native/CryptoLib.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,49 @@ public static byte[] Keccak256(byte[] data)
return data.Keccak256();
}

/// <summary>
/// Verifies that a digital signature is appropriate for the provided key and message using the ECDSA algorithm.
/// </summary>
/// <param name="message">The signed message.</param>
/// <param name="signature">The signature to be verified.</param>
/// <param name="curveHash">A pair of the curve to be used by the ECDSA algorithm and the hasher function to be used to hash message.</param>
/// <returns><see langword="true"/> if the signature is valid; otherwise, <see langword="false"/>.</returns>
[ContractMethod(Hardfork.HF_Aspidochelone, CpuFee = 1 << 10)]
public static ECPoint[] ECrecover(byte[] message, byte[] signature, NamedCurveHash curveHash)
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
try
{
var ch = s_curves[curveHash];
var messageHash =
ch.Hasher == Hasher.SHA256 ? message.Sha256() :
ch.Hasher == Hasher.Keccak256 ? message.Keccak256() :
throw new NotSupportedException(nameof(ch.Hasher));

switch (curveHash)
{
case NamedCurveHash.secp256k1Keccak256:
case NamedCurveHash.secp256k1SHA256:
{
return Crypto.ECRecover(ECCurve.Secp256k1, signature, messageHash,
// TODO: only accept 65?

Choose a reason for hiding this comment

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

64bytes signatures MAY store a parity bit in the first bit of the s value which is always 0.
From what I deduce, you assume the parity value to be 0 if the size is 64 so checking the first bit of s will not change the current behavior if this is not a compressed signature (i.e. parity will always be 0) But in case it is a compressed ERC-2098 signature, it will be supported out-of-the-box

Note: this is predicated on the assumption that N3 contracts might need to verify signatures that originate in NEOX where EIP standards might apply (eg: someone migrates a platform written for Ethereum to NeoX)
for reference, this is the compressed signature standard I am referring to: https://eips.ethereum.org/EIPS/eip-2098

signature.Length == 64 ? SignatureFormat.Fixed32 : SignatureFormat.Der);
}

// TODO: Not tested, check if it works
case NamedCurveHash.secp256r1Keccak256:
case NamedCurveHash.secp256r1SHA256:
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
return Crypto.ECRecover(ECCurve.Secp256r1, signature, messageHash,
// TODO: only accept 65?
signature.Length == 64 ? SignatureFormat.Fixed32 : SignatureFormat.Der);
}
}
}
catch { }
Copy link
Member

Choose a reason for hiding this comment

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

Several lines below we do throw new NotSupportedException(nameof(ch.Hasher)); if hashing algorythm is not supported. This exception should be properly thrown, so we don't need to catch it here.


throw new NotSupportedException(nameof(curveHash));
}

/// <summary>
/// Verifies that a digital signature is appropriate for the provided key and message using the ECDSA algorithm.
/// </summary>
Expand Down
6 changes: 6 additions & 0 deletions tests/Neo.UnitTests/Cryptography/UT_Crypto.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ public void TestSecp256k1()

Crypto.VerifySignature(message, signature, pubKey, Neo.Cryptography.ECC.ECCurve.Secp256k1)
.Should().BeTrue();

var messageHash = message.Sha256();
var recover = Crypto.ECRecover(Neo.Cryptography.ECC.ECCurve.Secp256k1, signature, messageHash, SignatureFormat.Fixed32);

recover.Length.Should().Be(2);
shargon marked this conversation as resolved.
Show resolved Hide resolved
recover[0].ToArray().Should().BeEquivalentTo(pubKey);
}
}
}
Loading