From fc93bd43bc95754c771d974cbde54d84723ded15 Mon Sep 17 00:00:00 2001 From: Drew Hintz Date: Mon, 20 Jul 2020 22:31:33 -0700 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=9A=AB=F0=9F=A5=92=20do=20not=20pickl?= =?UTF-8?q?e=20the=20unpickleable=20RWLock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ecdsa/ellipticcurve.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/ecdsa/ellipticcurve.py b/src/ecdsa/ellipticcurve.py index c016b8b8..a33d2a4e 100644 --- a/src/ecdsa/ellipticcurve.py +++ b/src/ecdsa/ellipticcurve.py @@ -185,6 +185,15 @@ def __init__(self, curve, x, y, z, order=None, generator=False): doubler = doubler.double().scale() self.__precompute.append((doubler.x(), doubler.y())) + def __getstate__(self): + state = self.__dict__.copy() + del state['_scale_lock'] + return state + + def __setstate__(self, state): + self.__dict__.update(state) + self._scale_lock = RWLock() + def __eq__(self, other): """Compare two points with each-other.""" try: From 34ae7a0070eeed114cd563ccadb5bc158b8340d8 Mon Sep 17 00:00:00 2001 From: Drew Hintz Date: Wed, 22 Jul 2020 20:31:05 +0000 Subject: [PATCH 2/4] As requested, acquired the lock before copying PointJacobi. When failing to acquire the lock, it now returns None. Think that's fine or prefer something else? Added `__ne__` for CurveFp. I think `test_inaquality_points_diff_types` from test_ellipticcurve.py was not actually testing equality. Because there was no `__ne__`, it was falling back to identity comparison, causing the test to pass. Now that there's a `__ne__`, it was getting NotImplemented which surprisingly bool casts to True. I changed the `__eq__` to return False for different types. Fix typo in `test_inaquality_points_diff_types` name. As requested, ran black on modified files. It reformmated some lines I hadn't modified. --- src/ecdsa/ellipticcurve.py | 34 ++++++++++++++++------------ src/ecdsa/test_ellipticcurve.py | 22 +++++-------------- src/ecdsa/test_jacobi.py | 39 +++++++++++++++------------------ 3 files changed, 44 insertions(+), 51 deletions(-) diff --git a/src/ecdsa/ellipticcurve.py b/src/ecdsa/ellipticcurve.py index a33d2a4e..6fdc4f09 100644 --- a/src/ecdsa/ellipticcurve.py +++ b/src/ecdsa/ellipticcurve.py @@ -92,14 +92,16 @@ def __init__(self, p, a, b, h=None): self.__h = h def __eq__(self, other): - if isinstance(other, CurveFp): - """Return True if the curves are identical, False otherwise.""" - return ( - self.__p == other.__p - and self.__a == other.__a - and self.__b == other.__b - ) - return NotImplemented + """Return True if the curves are identical, False otherwise.""" + return ( + isinstance(other, CurveFp) + and self.__p == other.__p + and self.__a == other.__a + and self.__b == other.__b + ) + + def __ne__(self, other): + return not self.__eq__(other) def __hash__(self): return hash((self.__p, self.__a, self.__b)) @@ -186,8 +188,14 @@ def __init__(self, curve, x, y, z, order=None, generator=False): self.__precompute.append((doubler.x(), doubler.y())) def __getstate__(self): - state = self.__dict__.copy() - del state['_scale_lock'] + state = None + try: + self._scale_lock.reader_acquire() + state = self.__dict__.copy() + finally: + self._scale_lock.reader_release() + if state: + del state["_scale_lock"] return state def __setstate__(self, state): @@ -721,8 +729,7 @@ def __add__(self, other): p = self.__curve.p() l = ( - (other.__y - self.__y) - * numbertheory.inverse_mod(other.__x - self.__x, p) + (other.__y - self.__y) * numbertheory.inverse_mod(other.__x - self.__x, p) ) % p x3 = (l * l - self.__x - other.__x) % p @@ -788,8 +795,7 @@ def double(self): a = self.__curve.a() l = ( - (3 * self.__x * self.__x + a) - * numbertheory.inverse_mod(2 * self.__y, p) + (3 * self.__x * self.__x + a) * numbertheory.inverse_mod(2 * self.__y, p) ) % p x3 = (l * l - 2 * self.__x) % p diff --git a/src/ecdsa/test_ellipticcurve.py b/src/ecdsa/test_ellipticcurve.py index 6b711fb3..80800f57 100644 --- a/src/ecdsa/test_ellipticcurve.py +++ b/src/ecdsa/test_ellipticcurve.py @@ -121,28 +121,18 @@ def test_p192(self): # in X9.62: d = 651056770906015076056810763456358567190100156695615665659 Q = d * self.p192 - self.assertEqual( - Q.x(), 0x62B12D60690CDCF330BABAB6E69763B471F994DD702D16A5 - ) + self.assertEqual(Q.x(), 0x62B12D60690CDCF330BABAB6E69763B471F994DD702D16A5) k = 6140507067065001063065065565667405560006161556565665656654 R = k * self.p192 - self.assertEqual( - R.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD - ) - self.assertEqual( - R.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835 - ) + self.assertEqual(R.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD) + self.assertEqual(R.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835) u1 = 2563697409189434185194736134579731015366492496392189760599 u2 = 6266643813348617967186477710235785849136406323338782220568 temp = u1 * self.p192 + u2 * Q - self.assertEqual( - temp.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD - ) - self.assertEqual( - temp.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835 - ) + self.assertEqual(temp.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD) + self.assertEqual(temp.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835) def test_double_infinity(self): p1 = INFINITY @@ -195,6 +185,6 @@ def test_inequality_points(self): p = Point(c, 100, 100, 100) self.assertNotEqual(self.g_23, p) - def test_inaquality_points_diff_types(self): + def test_inequality_points_diff_types(self): c = CurveFp(100, -3, 100) self.assertNotEqual(self.g_23, c) diff --git a/src/ecdsa/test_jacobi.py b/src/ecdsa/test_jacobi.py index 7c806f6f..24a96be3 100644 --- a/src/ecdsa/test_jacobi.py +++ b/src/ecdsa/test_jacobi.py @@ -1,3 +1,5 @@ +import pickle + try: import unittest2 as unittest except ImportError: @@ -6,7 +8,7 @@ import hypothesis.strategies as st from hypothesis import given, assume, settings, example -from .ellipticcurve import Point, PointJacobi, INFINITY +from .ellipticcurve import CurveFp, Point, PointJacobi, INFINITY from .ecdsa import generator_256, curve_256, generator_224 from .numbertheory import inverse_mod @@ -303,16 +305,10 @@ def test_add_different_scale_points(self, a_mul, b_mul, new_z): new_zz1 = new_z[1] * new_z[1] % p a = PointJacobi( - curve_256, - a.x() * new_zz0 % p, - a.y() * new_zz0 * new_z[0] % p, - new_z[0], + curve_256, a.x() * new_zz0 % p, a.y() * new_zz0 * new_z[0] % p, new_z[0], ) b = PointJacobi( - curve_256, - b.x() * new_zz1 % p, - b.y() * new_zz1 * new_z[1] % p, - new_z[1], + curve_256, b.x() * new_zz1 % p, b.y() * new_zz1 * new_z[1] % p, new_z[1], ) c = a + b @@ -347,12 +343,8 @@ def test_mul_add_precompute_large(self): b = PointJacobi.from_affine(j_g * 255, True) self.assertEqual(j_g * 256, j_g + b) - self.assertEqual( - j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0 - ) - self.assertEqual( - j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0) - ) + self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0) + self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0)) def test_mul_add_to_mul(self): j_g = PointJacobi.from_affine(generator_256) @@ -378,9 +370,14 @@ def test_mul_add_large(self): b = PointJacobi.from_affine(j_g * 255) self.assertEqual(j_g * 256, j_g + b) - self.assertEqual( - j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0 - ) - self.assertEqual( - j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0) - ) + self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0) + self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0)) + + def test_equality(self): + pj1 = PointJacobi(curve=CurveFp(23, 1, 1, 1), x=2, y=3, z=1, order=1) + pj2 = PointJacobi(curve=CurveFp(23, 1, 1, 1), x=2, y=3, z=1, order=1) + self.assertEqual(pj1, pj2) + + def test_pickle(self): + pj = PointJacobi(curve=CurveFp(23, 1, 1, 1), x=2, y=3, z=1, order=1) + self.assertEqual(pickle.loads(pickle.dumps(pj)), pj) From 9d42c52ea756f415039aeb3fcef8df5d24fb61ab Mon Sep 17 00:00:00 2001 From: Drew Hintz Date: Wed, 22 Jul 2020 21:03:02 +0000 Subject: [PATCH 3/4] ran black with `--line-length 79` as configured in travis-ci for the project --- src/ecdsa/ellipticcurve.py | 6 ++++-- src/ecdsa/test_ellipticcurve.py | 20 +++++++++++++++----- src/ecdsa/test_jacobi.py | 26 ++++++++++++++++++++------ 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/ecdsa/ellipticcurve.py b/src/ecdsa/ellipticcurve.py index 6fdc4f09..9ab848a7 100644 --- a/src/ecdsa/ellipticcurve.py +++ b/src/ecdsa/ellipticcurve.py @@ -729,7 +729,8 @@ def __add__(self, other): p = self.__curve.p() l = ( - (other.__y - self.__y) * numbertheory.inverse_mod(other.__x - self.__x, p) + (other.__y - self.__y) + * numbertheory.inverse_mod(other.__x - self.__x, p) ) % p x3 = (l * l - self.__x - other.__x) % p @@ -795,7 +796,8 @@ def double(self): a = self.__curve.a() l = ( - (3 * self.__x * self.__x + a) * numbertheory.inverse_mod(2 * self.__y, p) + (3 * self.__x * self.__x + a) + * numbertheory.inverse_mod(2 * self.__y, p) ) % p x3 = (l * l - 2 * self.__x) % p diff --git a/src/ecdsa/test_ellipticcurve.py b/src/ecdsa/test_ellipticcurve.py index 80800f57..def53b2a 100644 --- a/src/ecdsa/test_ellipticcurve.py +++ b/src/ecdsa/test_ellipticcurve.py @@ -121,18 +121,28 @@ def test_p192(self): # in X9.62: d = 651056770906015076056810763456358567190100156695615665659 Q = d * self.p192 - self.assertEqual(Q.x(), 0x62B12D60690CDCF330BABAB6E69763B471F994DD702D16A5) + self.assertEqual( + Q.x(), 0x62B12D60690CDCF330BABAB6E69763B471F994DD702D16A5 + ) k = 6140507067065001063065065565667405560006161556565665656654 R = k * self.p192 - self.assertEqual(R.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD) - self.assertEqual(R.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835) + self.assertEqual( + R.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD + ) + self.assertEqual( + R.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835 + ) u1 = 2563697409189434185194736134579731015366492496392189760599 u2 = 6266643813348617967186477710235785849136406323338782220568 temp = u1 * self.p192 + u2 * Q - self.assertEqual(temp.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD) - self.assertEqual(temp.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835) + self.assertEqual( + temp.x(), 0x885052380FF147B734C330C43D39B2C4A89F29B0F749FEAD + ) + self.assertEqual( + temp.y(), 0x9CF9FA1CBEFEFB917747A3BB29C072B9289C2547884FD835 + ) def test_double_infinity(self): p1 = INFINITY diff --git a/src/ecdsa/test_jacobi.py b/src/ecdsa/test_jacobi.py index 24a96be3..5494243d 100644 --- a/src/ecdsa/test_jacobi.py +++ b/src/ecdsa/test_jacobi.py @@ -305,10 +305,16 @@ def test_add_different_scale_points(self, a_mul, b_mul, new_z): new_zz1 = new_z[1] * new_z[1] % p a = PointJacobi( - curve_256, a.x() * new_zz0 % p, a.y() * new_zz0 * new_z[0] % p, new_z[0], + curve_256, + a.x() * new_zz0 % p, + a.y() * new_zz0 * new_z[0] % p, + new_z[0], ) b = PointJacobi( - curve_256, b.x() * new_zz1 % p, b.y() * new_zz1 * new_z[1] % p, new_z[1], + curve_256, + b.x() * new_zz1 % p, + b.y() * new_zz1 * new_z[1] % p, + new_z[1], ) c = a + b @@ -343,8 +349,12 @@ def test_mul_add_precompute_large(self): b = PointJacobi.from_affine(j_g * 255, True) self.assertEqual(j_g * 256, j_g + b) - self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0) - self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0)) + self.assertEqual( + j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0 + ) + self.assertEqual( + j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0) + ) def test_mul_add_to_mul(self): j_g = PointJacobi.from_affine(generator_256) @@ -370,8 +380,12 @@ def test_mul_add_large(self): b = PointJacobi.from_affine(j_g * 255) self.assertEqual(j_g * 256, j_g + b) - self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0) - self.assertEqual(j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0)) + self.assertEqual( + j_g * (0xFF00 + 255 * 0xF0F0), j_g * 0xFF00 + b * 0xF0F0 + ) + self.assertEqual( + j_g * (0xFF00 + 255 * 0xF0F0), j_g.mul_add(0xFF00, b, 0xF0F0) + ) def test_equality(self): pj1 = PointJacobi(curve=CurveFp(23, 1, 1, 1), x=2, y=3, z=1, order=1) From e0840fa562f219477fc0e6804d3011b141eb3d2c Mon Sep 17 00:00:00 2001 From: Drew Hintz Date: Fri, 24 Jul 2020 01:19:25 +0000 Subject: [PATCH 4/4] Revert to returning NotImplemented. For __ne__ use == instead of __eq__ so it handles NotImplemented when the types are different. Remove the not needed state declaration. --- src/ecdsa/ellipticcurve.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/ecdsa/ellipticcurve.py b/src/ecdsa/ellipticcurve.py index 9ab848a7..1bbccb5b 100644 --- a/src/ecdsa/ellipticcurve.py +++ b/src/ecdsa/ellipticcurve.py @@ -92,16 +92,17 @@ def __init__(self, p, a, b, h=None): self.__h = h def __eq__(self, other): - """Return True if the curves are identical, False otherwise.""" - return ( - isinstance(other, CurveFp) - and self.__p == other.__p - and self.__a == other.__a - and self.__b == other.__b - ) + if isinstance(other, CurveFp): + """Return True if the curves are identical, False otherwise.""" + return ( + self.__p == other.__p + and self.__a == other.__a + and self.__b == other.__b + ) + return NotImplemented def __ne__(self, other): - return not self.__eq__(other) + return not (self == other) def __hash__(self): return hash((self.__p, self.__a, self.__b)) @@ -188,14 +189,12 @@ def __init__(self, curve, x, y, z, order=None, generator=False): self.__precompute.append((doubler.x(), doubler.y())) def __getstate__(self): - state = None try: self._scale_lock.reader_acquire() state = self.__dict__.copy() finally: self._scale_lock.reader_release() - if state: - del state["_scale_lock"] + del state["_scale_lock"] return state def __setstate__(self, state):