From 3fc5f0e61be2f9f6169d88b143e8c196d5e63b5e Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 29 Jul 2019 22:15:23 +0700 Subject: Fix field reduction for custom secp128r1 curve - see https://github.com/bcgit/bc-java/issues/566 --- crypto/crypto.csproj | 5 +++ crypto/src/math/ec/custom/sec/SecP128R1Field.cs | 5 +++ crypto/test/UnitTests.csproj | 1 + .../math/ec/custom/sec/test/SecP128R1FieldTest.cs | 46 ++++++++++++++++++++++ 4 files changed, 57 insertions(+) create mode 100644 crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs (limited to 'crypto') diff --git a/crypto/crypto.csproj b/crypto/crypto.csproj index 55fcf1704..1b4cccd52 100644 --- a/crypto/crypto.csproj +++ b/crypto/crypto.csproj @@ -12932,6 +12932,11 @@ SubType = "Code" BuildAction = "Compile" /> + = P3 && Nat128.Gte(z, P)) + { + AddPInvTo(z); + } } public static void Square(uint[] x, uint[] z) diff --git a/crypto/test/UnitTests.csproj b/crypto/test/UnitTests.csproj index 1378034a6..0ab02f02e 100644 --- a/crypto/test/UnitTests.csproj +++ b/crypto/test/UnitTests.csproj @@ -337,6 +337,7 @@ + diff --git a/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs b/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs new file mode 100644 index 000000000..211d95c9c --- /dev/null +++ b/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs @@ -0,0 +1,46 @@ +using System; + +using NUnit.Framework; + +using Org.BouncyCastle.Asn1.Sec; +using Org.BouncyCastle.Asn1.X9; +using Org.BouncyCastle.Crypto.EC; +using Org.BouncyCastle.Math.Raw; +using Org.BouncyCastle.Security; +using Org.BouncyCastle.Utilities; + +namespace Org.BouncyCastle.Math.EC.Custom.Sec.Tests +{ + [TestFixture] + public class SecP128R1FieldTest + { + [Test] + public void Test_GitHub566() + { + uint[] x = new uint[]{ 0x4B1E2F5E, 0x09E29D21, 0xA58407ED, 0x6FC3C7CF }; + uint[] y = new uint[]{ 0x2FFE8892, 0x55CA61CA, 0x0AF780B5, 0x4BD7B797 }; + uint[] z = Nat128.Create(); + + SecP128R1Field.Multiply(x, y, z); + + uint[] expected = new uint[]{ 0x01FFFF01, 0, 0, 0 }; + Assert.IsTrue(Arrays.AreEqual(expected, z)); + } + + [Test] + public void TestReduce32() + { + uint[] z = Nat128.Create(); + //Arrays.Fill(z, 0xFFFFFFFF); + for (int i = 0; i < z.Length; ++i) + { + z[i] = 0xFFFFFFFF; + } + + SecP128R1Field.Reduce32(0xFFFFFFFF, z); + + uint[] expected = new uint[]{ 1, 1, 0, 4 }; + Assert.IsTrue(Arrays.AreEqual(expected, z)); + } + } +} -- cgit 1.4.1 From 144b0664aa8f22dfd1e4a9cde526cda81a1ed252 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 29 Jul 2019 22:18:16 +0700 Subject: Remove unused imports --- crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs | 4 ---- 1 file changed, 4 deletions(-) (limited to 'crypto') diff --git a/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs b/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs index 211d95c9c..40e1a61c1 100644 --- a/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs +++ b/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs @@ -2,11 +2,7 @@ using NUnit.Framework; -using Org.BouncyCastle.Asn1.Sec; -using Org.BouncyCastle.Asn1.X9; -using Org.BouncyCastle.Crypto.EC; using Org.BouncyCastle.Math.Raw; -using Org.BouncyCastle.Security; using Org.BouncyCastle.Utilities; namespace Org.BouncyCastle.Math.EC.Custom.Sec.Tests -- cgit 1.4.1 From 3d27c78a8822120ce09328304b98895845ee4f88 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 29 Jul 2019 22:29:43 +0700 Subject: Make main SMix array 1-dimensional --- crypto/src/crypto/generators/SCrypt.cs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'crypto') diff --git a/crypto/src/crypto/generators/SCrypt.cs b/crypto/src/crypto/generators/SCrypt.cs index 4d15bb3d7..51dc50b81 100644 --- a/crypto/src/crypto/generators/SCrypt.cs +++ b/crypto/src/crypto/generators/SCrypt.cs @@ -98,31 +98,37 @@ namespace Org.BouncyCastle.Crypto.Generators uint[] blockY = new uint[BCount]; uint[] X = new uint[BCount]; - uint[][] V = new uint[N][]; + uint[] V = new uint[N * BCount]; try { Array.Copy(B, BOff, X, 0, BCount); - for (int i = 0; i < N; ++i) - { - V[i] = (uint[])X.Clone(); - BlockMix(X, blockX1, blockX2, blockY, r); - } + int off = 0; + for (int i = 0; i < N; i += 2) + { + Array.Copy(X, 0, V, off, BCount); + off += BCount; + BlockMix(X, blockX1, blockX2, blockY, r); + Array.Copy(blockY, 0, V, off, BCount); + off += BCount; + BlockMix(blockY, blockX1, blockX2, X, r); + } uint mask = (uint)N - 1; for (int i = 0; i < N; ++i) { uint j = X[BCount - 16] & mask; - Xor(X, V[j], 0, X); - BlockMix(X, blockX1, blockX2, blockY, r); - } + Array.Copy(V, j * BCount, blockY, 0, BCount); + Xor(blockY, X, 0, blockY); + BlockMix(blockY, blockX1, blockX2, X, r); + } Array.Copy(X, 0, B, BOff, BCount); } finally { - ClearAll(V); + Clear(V); ClearAll(X, blockX1, blockX2, blockY); } } @@ -143,8 +149,6 @@ namespace Org.BouncyCastle.Crypto.Generators YOff = halfLen + BOff - YOff; BOff += 16; } - - Array.Copy(Y, 0, B, 0, Y.Length); } private static void Xor(uint[] a, uint[] b, int bOff, uint[] output) -- cgit 1.4.1 From 405768d3761478acaec455903ae3af4187e34525 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 29 Jul 2019 22:33:47 +0700 Subject: Add several copy64 methods --- crypto/src/math/raw/Nat.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'crypto') diff --git a/crypto/src/math/raw/Nat.cs b/crypto/src/math/raw/Nat.cs index 040ade74f..f9e4e6714 100644 --- a/crypto/src/math/raw/Nat.cs +++ b/crypto/src/math/raw/Nat.cs @@ -270,6 +270,23 @@ namespace Org.BouncyCastle.Math.Raw Array.Copy(x, xOff, z, zOff, len); } + public static ulong[] Copy64(int len, ulong[] x) + { + ulong[] z = new ulong[len]; + Array.Copy(x, 0, z, 0, len); + return z; + } + + public static void Copy64(int len, ulong[] x, ulong[] z) + { + Array.Copy(x, 0, z, 0, len); + } + + public static void Copy64(int len, ulong[] x, int xOff, ulong[] z, int zOff) + { + Array.Copy(x, xOff, z, zOff, len); + } + public static uint[] Create(int len) { return new uint[len]; -- cgit 1.4.1 From e14888232256fa1d880938395c995bc969e5e295 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 29 Jul 2019 22:49:02 +0700 Subject: Fix a corner-case for DER set-value sorting --- crypto/src/asn1/Asn1Set.cs | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) (limited to 'crypto') diff --git a/crypto/src/asn1/Asn1Set.cs b/crypto/src/asn1/Asn1Set.cs index bf83dbdc1..7fa072c0d 100644 --- a/crypto/src/asn1/Asn1Set.cs +++ b/crypto/src/asn1/Asn1Set.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Diagnostics; using System.IO; #if PORTABLE @@ -344,29 +345,35 @@ namespace Org.BouncyCastle.Asn1 { byte[] a = (byte[])x, b = (byte[])y; #endif + Debug.Assert(a.Length >= 2 && b.Length >= 2); + + /* + * NOTE: Set elements in DER encodings are ordered first according to their tags (class and + * number); the CONSTRUCTED bit is not part of the tag. + * + * For SET-OF, this is unimportant. All elements have the same tag and DER requires them to + * either all be in constructed form or all in primitive form, according to that tag. The + * elements are effectively ordered according to their content octets. + * + * For SET, the elements will have distinct tags, and each will be in constructed or + * primitive form accordingly. Failing to ignore the CONSTRUCTED bit could therefore lead to + * ordering inversions. + */ + int a0 = a[0] & ~Asn1Tags.Constructed; + int b0 = b[0] & ~Asn1Tags.Constructed; + if (a0 != b0) + return a0 < b0 ? -1 : 1; + int len = System.Math.Min(a.Length, b.Length); - for (int i = 0; i != len; ++i) + for (int i = 1; i < len; ++i) { byte ai = a[i], bi = b[i]; if (ai != bi) return ai < bi ? -1 : 1; } - if (a.Length > b.Length) - return AllZeroesFrom(a, len) ? 0 : 1; - if (a.Length < b.Length) - return AllZeroesFrom(b, len) ? 0 : -1; + Debug.Assert(a.Length == b.Length); return 0; } - - private bool AllZeroesFrom(byte[] bs, int pos) - { - while (pos < bs.Length) - { - if (bs[pos++] != 0) - return false; - } - return true; - } } } } -- cgit 1.4.1 From 3d4e40e51c2eefe48cc927c6efa8c10317abaa8b Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 29 Jul 2019 22:59:50 +0700 Subject: Adapt test to access restrictions --- .../math/ec/custom/sec/test/SecP128R1FieldTest.cs | 51 +++++++++++++++------- 1 file changed, 35 insertions(+), 16 deletions(-) (limited to 'crypto') diff --git a/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs b/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs index 40e1a61c1..26b4060b0 100644 --- a/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs +++ b/crypto/test/src/math/ec/custom/sec/test/SecP128R1FieldTest.cs @@ -2,41 +2,60 @@ using NUnit.Framework; -using Org.BouncyCastle.Math.Raw; -using Org.BouncyCastle.Utilities; +using Org.BouncyCastle.Asn1.Sec; +using Org.BouncyCastle.Asn1.X9; +using Org.BouncyCastle.Crypto.EC; namespace Org.BouncyCastle.Math.EC.Custom.Sec.Tests { [TestFixture] public class SecP128R1FieldTest { + private static readonly X9ECParameters DP = CustomNamedCurves + .GetByOid(SecObjectIdentifiers.SecP128r1); + [Test] public void Test_GitHub566() { uint[] x = new uint[]{ 0x4B1E2F5E, 0x09E29D21, 0xA58407ED, 0x6FC3C7CF }; uint[] y = new uint[]{ 0x2FFE8892, 0x55CA61CA, 0x0AF780B5, 0x4BD7B797 }; - uint[] z = Nat128.Create(); - SecP128R1Field.Multiply(x, y, z); + ECFieldElement Z = FE(x).Multiply(FE(y)); - uint[] expected = new uint[]{ 0x01FFFF01, 0, 0, 0 }; - Assert.IsTrue(Arrays.AreEqual(expected, z)); + uint[] expected = new uint[] { 0x01FFFF01, 0, 0, 0 }; + Assert.AreEqual(FE(expected), Z); } - [Test] - public void TestReduce32() + private ECFieldElement FE(BigInteger x) + { + return DP.Curve.FromBigInteger(x); + } + + private ECFieldElement FE(uint[] x) { - uint[] z = Nat128.Create(); - //Arrays.Fill(z, 0xFFFFFFFF); - for (int i = 0; i < z.Length; ++i) + return FE(Nat128_ToBigInteger(x)); + } + + private static BigInteger Nat128_ToBigInteger(uint[] x) + { + byte[] bs = new byte[16]; + for (int i = 0; i < 4; ++i) { - z[i] = 0xFFFFFFFF; + uint x_i = x[i]; + if (x_i != 0) + { + Pack_UInt32_To_BE(x_i, bs, (3 - i) << 2); + } } + return new BigInteger(1, bs); + } - SecP128R1Field.Reduce32(0xFFFFFFFF, z); - - uint[] expected = new uint[]{ 1, 1, 0, 4 }; - Assert.IsTrue(Arrays.AreEqual(expected, z)); + private static void Pack_UInt32_To_BE(uint n, byte[] bs, int off) + { + bs[off] = (byte)(n >> 24); + bs[off + 1] = (byte)(n >> 16); + bs[off + 2] = (byte)(n >> 8); + bs[off + 3] = (byte)(n); } } } -- cgit 1.4.1 From 551bd07fd737f2460b015f7b8300056fa012baff Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Tue, 30 Jul 2019 00:39:23 +0700 Subject: Work on EC parameters classes - make fields private/readonly where possible - add public validation methods to ECDomainParameters - add validation to ECPrivateKeyParameters - ECDomainParameters equality/GetHashCode ignore (optional) cofactor --- crypto/src/crypto/generators/ECKeyPairGenerator.cs | 2 +- crypto/src/crypto/parameters/ECDomainParameters.cs | 51 ++++++++++++++-------- .../crypto/parameters/ECNamedDomainParameters.cs | 2 +- .../crypto/parameters/ECPrivateKeyParameters.cs | 15 ++----- .../src/crypto/parameters/ECPublicKeyParameters.cs | 15 ++----- 5 files changed, 42 insertions(+), 43 deletions(-) (limited to 'crypto') diff --git a/crypto/src/crypto/generators/ECKeyPairGenerator.cs b/crypto/src/crypto/generators/ECKeyPairGenerator.cs index 26bc06e14..6a710c62e 100644 --- a/crypto/src/crypto/generators/ECKeyPairGenerator.cs +++ b/crypto/src/crypto/generators/ECKeyPairGenerator.cs @@ -105,7 +105,7 @@ namespace Org.BouncyCastle.Crypto.Generators { d = new BigInteger(n.BitLength, random); - if (d.CompareTo(BigInteger.Two) < 0 || d.CompareTo(n) >= 0) + if (d.CompareTo(BigInteger.One) < 0 || d.CompareTo(n) >= 0) continue; if (WNafUtilities.GetNafWeight(d) < minWeight) diff --git a/crypto/src/crypto/parameters/ECDomainParameters.cs b/crypto/src/crypto/parameters/ECDomainParameters.cs index e377f7760..3ff7d809f 100644 --- a/crypto/src/crypto/parameters/ECDomainParameters.cs +++ b/crypto/src/crypto/parameters/ECDomainParameters.cs @@ -8,12 +8,13 @@ namespace Org.BouncyCastle.Crypto.Parameters { public class ECDomainParameters { - internal ECCurve curve; - internal byte[] seed; - internal ECPoint g; - internal BigInteger n; - internal BigInteger h; - internal BigInteger hInv; + private readonly ECCurve curve; + private readonly byte[] seed; + private readonly ECPoint g; + private readonly BigInteger n; + private readonly BigInteger h; + + private BigInteger hInv; public ECDomainParameters( ECCurve curve, @@ -48,7 +49,7 @@ namespace Org.BouncyCastle.Crypto.Parameters // we can't check for h == null here as h is optional in X9.62 as it is not required for ECDSA this.curve = curve; - this.g = Validate(curve, g); + this.g = ValidatePublicPoint(curve, g); this.n = n; this.h = h; this.seed = Arrays.Clone(seed); @@ -113,26 +114,42 @@ namespace Org.BouncyCastle.Crypto.Parameters { return curve.Equals(other.curve) && g.Equals(other.g) - && n.Equals(other.n) - && h.Equals(other.h); + && n.Equals(other.n); } public override int GetHashCode() { - int hc = curve.GetHashCode(); - hc *= 37; + //return Arrays.GetHashCode(new object[]{ curve, g, n }); + int hc = 4; + hc *= 257; + hc ^= curve.GetHashCode(); + hc *= 257; hc ^= g.GetHashCode(); - hc *= 37; + hc *= 257; hc ^= n.GetHashCode(); - hc *= 37; - hc ^= h.GetHashCode(); return hc; } - internal static ECPoint Validate(ECCurve c, ECPoint q) + public BigInteger ValidatePrivateScalar(BigInteger d) + { + if (null == d) + throw new ArgumentNullException("d", "Scalar cannot be null"); + + if (d.CompareTo(BigInteger.One) < 0 || (d.CompareTo(N) >= 0)) + throw new ArgumentException("Scalar is not in the interval [1, n - 1]", "d"); + + return d; + } + + public ECPoint ValidatePublicPoint(ECPoint q) + { + return ValidatePublicPoint(Curve, q); + } + + internal static ECPoint ValidatePublicPoint(ECCurve c, ECPoint q) { - if (q == null) - throw new ArgumentException("Point has null value", "q"); + if (null == q) + throw new ArgumentNullException("q", "Point cannot be null"); q = ECAlgorithms.ImportPoint(c, q).Normalize(); diff --git a/crypto/src/crypto/parameters/ECNamedDomainParameters.cs b/crypto/src/crypto/parameters/ECNamedDomainParameters.cs index 4b8e2558f..2279c7dcc 100644 --- a/crypto/src/crypto/parameters/ECNamedDomainParameters.cs +++ b/crypto/src/crypto/parameters/ECNamedDomainParameters.cs @@ -17,7 +17,7 @@ namespace Org.BouncyCastle.Crypto.Parameters } public ECNamedDomainParameters(DerObjectIdentifier name, ECDomainParameters dp) - : this(name, dp.curve, dp.g, dp.n, dp.h, dp.seed) + : this(name, dp.Curve, dp.G, dp.N, dp.H, dp.GetSeed()) { } diff --git a/crypto/src/crypto/parameters/ECPrivateKeyParameters.cs b/crypto/src/crypto/parameters/ECPrivateKeyParameters.cs index 4d0fa1fc6..47e53ef2d 100644 --- a/crypto/src/crypto/parameters/ECPrivateKeyParameters.cs +++ b/crypto/src/crypto/parameters/ECPrivateKeyParameters.cs @@ -24,10 +24,7 @@ namespace Org.BouncyCastle.Crypto.Parameters DerObjectIdentifier publicKeyParamSet) : base("ECGOST3410", true, publicKeyParamSet) { - if (d == null) - throw new ArgumentNullException("d"); - - this.d = d; + this.d = Parameters.ValidatePrivateScalar(d); } public ECPrivateKeyParameters( @@ -36,10 +33,7 @@ namespace Org.BouncyCastle.Crypto.Parameters ECDomainParameters parameters) : base(algorithm, true, parameters) { - if (d == null) - throw new ArgumentNullException("d"); - - this.d = d; + this.d = Parameters.ValidatePrivateScalar(d); } public ECPrivateKeyParameters( @@ -48,10 +42,7 @@ namespace Org.BouncyCastle.Crypto.Parameters DerObjectIdentifier publicKeyParamSet) : base(algorithm, true, publicKeyParamSet) { - if (d == null) - throw new ArgumentNullException("d"); - - this.d = d; + this.d = Parameters.ValidatePrivateScalar(d); } public BigInteger D diff --git a/crypto/src/crypto/parameters/ECPublicKeyParameters.cs b/crypto/src/crypto/parameters/ECPublicKeyParameters.cs index 69916e525..d43ac7e0e 100644 --- a/crypto/src/crypto/parameters/ECPublicKeyParameters.cs +++ b/crypto/src/crypto/parameters/ECPublicKeyParameters.cs @@ -24,10 +24,7 @@ namespace Org.BouncyCastle.Crypto.Parameters DerObjectIdentifier publicKeyParamSet) : base("ECGOST3410", false, publicKeyParamSet) { - if (q == null) - throw new ArgumentNullException("q"); - - this.q = ECDomainParameters.Validate(Parameters.Curve, q); + this.q = ECDomainParameters.ValidatePublicPoint(Parameters.Curve, q); } public ECPublicKeyParameters( @@ -36,10 +33,7 @@ namespace Org.BouncyCastle.Crypto.Parameters ECDomainParameters parameters) : base(algorithm, false, parameters) { - if (q == null) - throw new ArgumentNullException("q"); - - this.q = ECDomainParameters.Validate(Parameters.Curve, q); + this.q = ECDomainParameters.ValidatePublicPoint(Parameters.Curve, q); } public ECPublicKeyParameters( @@ -48,10 +42,7 @@ namespace Org.BouncyCastle.Crypto.Parameters DerObjectIdentifier publicKeyParamSet) : base(algorithm, false, publicKeyParamSet) { - if (q == null) - throw new ArgumentNullException("q"); - - this.q = ECDomainParameters.Validate(Parameters.Curve, q); + this.q = ECDomainParameters.ValidatePublicPoint(Parameters.Curve, q); } public ECPoint Q -- cgit 1.4.1