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

OSX compatible #2511

Merged
merged 15 commits into from
Jul 16, 2021
Merged

OSX compatible #2511

merged 15 commits into from
Jul 16, 2021

Conversation

shargon
Copy link
Member

@shargon shargon commented Jun 24, 2021

Using Bouncy Castle
Close #2499

@Jim8y
Copy link
Contributor

Jim8y commented Jul 9, 2021

Still could not make the UT samples work.
But I created another few test cases and they passed (cases are generated HERE):

            byte[] message = System.Text.Encoding.Default.GetBytes("This is some message");
            byte[] signature = "0c0422df7d6f26a8d6250236060b8acd514fa4e8d260ff3c32c3aad4b6b470376e0f5a27e14e47ad328d01c3d8a4b969febab06ea26c84caa1fbe1779d62a785".HexToBytes();
            byte[] pubKey = new byte[] { 2, 29, 21, 35, 7, 198, 183, 43, 14, 208, 65, 139, 14, 112, 205, 128, 231, 245, 41, 91, 141, 134, 245, 114, 45, 63, 82, 19, 251, 210, 57, 79, 54 };//"03ea01cb94bdaf0cd1c01b159d474f9604f4af35a3e2196f6bdfdb33b2aa4961fa".HexToBytes();

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


             signature = "377beac335a19dee584d422b8c5f4c36004c26d2c663f5dfeba64528b4bfaf78101b4522870c597ff59f35ccfb56024345ca2b3c2ab0812f8dc79a1e91da6379".HexToBytes();
             pubKey = "042eacb81ac1182418e911e6472f53ddd2c6b4726e29e4d6bad17b545aac470675998d76dc345d117e65d0381221cc8d8446b896ef7b8a08481e935859211b4068".HexToBytes();
             message = System.Text.Encoding.Default.GetBytes("world");


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

            message = System.Text.Encoding.Default.GetBytes("world");
            signature = "4de88b4605e830de57ee61289d972e5f1a5d5e704e67f02fa6df3f626c7f55e93e2f7b1eb087ba8788d5b7282de09a82d344514bbba3e7b0361032c6ac75e08b".HexToBytes();
            pubKey = "04917342ce305a2192920a02e06aebd792732c4332285507119a4ff031495d4ffa441b97c2df7103fef47ff55341f0a10b6c32dc7be9734b2330a9c203f2ee096e".HexToBytes();

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

Update the code block in the try catch:

                    var curve = Org.BouncyCastle.Asn1.Sec.SecNamedCurves.GetByName("secp256k1");
                    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);


                    var r = new Org.BouncyCastle.Math.BigInteger(signature.Slice(0, 32).ToArray());
                    var s = new Org.BouncyCastle.Math.BigInteger(signature.Slice(32, 32).ToArray());

                    byte[] sig_der = new Org.BouncyCastle.Asn1.DerSequence(new Org.BouncyCastle.Asn1.DerInteger(r), new Org.BouncyCastle.Asn1.DerInteger(s)).GetDerEncoded();


                    return signer.VerifySignature(sig_der);

@nicolegys
Copy link
Contributor

I tried to run the UT samples on OSX, found that if using the full signature, VerifySignature can pass. But on windows, only using the short signature can pass.

byte[] message = System.Text.Encoding.Default.GetBytes("hello");
            byte[] signature = "5331be791532d157df5b5620620d938bcb622ad02c81cfc184c460efdad18e695480d77440c511e9ad02ea30d773cb54e88f8cbb069644aefa283957085f38b5".HexToBytes();
            byte[] pubKey = "03ea01cb94bdaf0cd1c01b159d474f9604f4af35a3e2196f6bdfdb33b2aa4961fa".HexToBytes();

            var r = new Org.BouncyCastle.Math.BigInteger(signature.AsSpan().Slice(0, 32).ToArray());
            var s = new Org.BouncyCastle.Math.BigInteger(signature.AsSpan().Slice(32, 32).ToArray());
            signature = new Org.BouncyCastle.Asn1.DerSequence(new Org.BouncyCastle.Asn1.DerInteger(r), new Org.BouncyCastle.Asn1.DerInteger(s)).GetDerEncoded();

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

@shargon shargon marked this pull request as ready for review July 9, 2021 12:16
@shargon
Copy link
Member Author

shargon commented Jul 9, 2021

Thanks @nicolegys ! :)

@shargon shargon requested a review from erikzhang July 9, 2021 12:23
shargon and others added 3 commits July 12, 2021 03:45
Co-authored-by: Jinghui Liao <jinghui@wayne.edu>
Co-authored-by: Jinghui Liao <jinghui@wayne.edu>
@ZhangTao1596
Copy link

ZhangTao1596 commented Jul 14, 2021

Passed!

Neo.UnitTests.Cryptography.UT_Crypto.TestSecp256k1:
    Outcome: Passed
    
Total tests: 1. Passed: 1. Failed: 0. Skipped: 0

superboyiii
superboyiii previously approved these changes Jul 14, 2021
@superboyiii
Copy link
Member

Test report here: #2539 (comment)

@superboyiii
Copy link
Member

@erikzhang Need your approve.

@superboyiii superboyiii mentioned this pull request Jul 14, 2021
24 tasks
Jim8y
Jim8y previously approved these changes Jul 14, 2021
@erikzhang erikzhang dismissed stale reviews from Jim8y and superboyiii via bbe1751 July 15, 2021 06:43
}
});
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.

@ZhangTao1596
Copy link

Passed!

----- Test Execution Summary -----

Neo.UnitTests.Cryptography.UT_Crypto.TestSecp256k1:
    Outcome: Passed
    
Total tests: 1. Passed: 1. Failed: 0. Skipped: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

secP256k1 not supported on OSX
7 participants