From e0d1c054d3c8966772a04b074a795540b7703456 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Tue, 8 Dec 2020 01:14:35 +0100 Subject: [PATCH 01/11] fix from_public_key_recovery() and sign() for large hashes the public key recovery needs to truncate large hashes just like the signature verification and creation, so reuse the code that does that --- src/ecdsa/keys.py | 95 +++++++++++++++++++++++++-------------- src/ecdsa/test_jacobi.py | 7 +-- src/ecdsa/test_pyecdsa.py | 7 +-- 3 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/ecdsa/keys.py b/src/ecdsa/keys.py index c4b3f06c..cc675107 100644 --- a/src/ecdsa/keys.py +++ b/src/ecdsa/keys.py @@ -124,6 +124,38 @@ class MalformedPointError(AssertionError): pass +def _truncate_and_convert_digest(digest, curve, allow_truncate): + """Truncates and converts digest to an integer.""" + if not allow_truncate: + if len(digest) > curve.baselen: + raise BadDigestError( + "this curve ({0}) is too short " + "for the length of your digest ({1})".format( + curve.name, 8 * len(digest) + ) + ) + else: + digest = digest[: curve.baselen] + number = string_to_number(digest) + if allow_truncate: + max_length = bit_length(curve.order) + # we don't use bit_length(number) as that truncates leading zeros + length = len(digest) * 8 + + # See NIST FIPS 186-4: + # + # When the length of the output of the hash function is greater + # than N (i.e., the bit length of q), then the leftmost N bits of + # the hash function output block shall be used in any calculation + # using the hash function output during the generation or + # verification of a digital signature. + # + # as such, we need to shift-out the low-order bits: + number >>= max(0, length - max_length) + + return number + + class VerifyingKey(object): """ Class for handling keys that can verify signatures (public keys). @@ -435,7 +467,13 @@ def from_der(cls, string, hashfunc=sha1): @classmethod def from_public_key_recovery( - cls, signature, data, curve, hashfunc=sha1, sigdecode=sigdecode_string + cls, + signature, + data, + curve, + hashfunc=sha1, + sigdecode=sigdecode_string, + allow_truncate=True, ): """ Return keys that can be used as verifiers of the provided signature. @@ -458,6 +496,9 @@ def from_public_key_recovery( a tuple with two integers, "r" as the first one and "s" as the second one. See :func:`ecdsa.util.sigdecode_string` and :func:`ecdsa.util.sigdecode_der` for examples. + :param bool allow_truncate: if True, the provided hashfunc can generate + values larger than the bit size of the order of the curve, the + extra bits (at the end of the digest) will be truncated. :type sigdecode: callable :return: Initialised VerifyingKey objects @@ -466,7 +507,12 @@ def from_public_key_recovery( data = normalise_bytes(data) digest = hashfunc(data).digest() return cls.from_public_key_recovery_with_digest( - signature, digest, curve, hashfunc=hashfunc, sigdecode=sigdecode + signature, + digest, + curve, + hashfunc=hashfunc, + sigdecode=sigdecode, + allow_truncate=allow_truncate, ) @classmethod @@ -477,6 +523,7 @@ def from_public_key_recovery_with_digest( curve, hashfunc=sha1, sigdecode=sigdecode_string, + allow_truncate=False, ): """ Return keys that can be used as verifiers of the provided signature. @@ -500,7 +547,10 @@ def from_public_key_recovery_with_digest( second one. See :func:`ecdsa.util.sigdecode_string` and :func:`ecdsa.util.sigdecode_der` for examples. :type sigdecode: callable - + :param bool allow_truncate: if True, the provided hashfunc can generate + values larger than the bit size of the order of the curve (and + the length of provided `digest`), the extra bits (at the end of the + digest) will be truncated. :return: Initialised VerifyingKey object :rtype: VerifyingKey @@ -510,7 +560,9 @@ def from_public_key_recovery_with_digest( sig = ecdsa.Signature(r, s) digest = normalise_bytes(digest) - digest_as_number = string_to_number(digest) + digest_as_number = _truncate_and_convert_digest( + digest, curve, allow_truncate + ) pks = sig.recover_public_keys(digest_as_number, generator) # Transforms the ecdsa.Public_key object into a VerifyingKey @@ -717,27 +769,9 @@ def verify_digest( # signature doesn't have to be a bytes-like-object so don't normalise # it, the decoders will do that digest = normalise_bytes(digest) - if not allow_truncate and len(digest) > self.curve.baselen: - raise BadDigestError( - "this curve (%s) is too short " - "for your digest (%d)" % (self.curve.name, 8 * len(digest)) - ) - number = string_to_number(digest) - if allow_truncate: - max_length = bit_length(self.curve.order) - # we don't use bit_length(number) as that truncates leading zeros - length = len(digest) * 8 - - # See NIST FIPS 186-4: - # - # When the length of the output of the hash function is greater - # than N (i.e., the bit length of q), then the leftmost N bits of - # the hash function output block shall be used in any calculation - # using the hash function output during the generation or - # verification of a digital signature. - # - # as such, we need to shift-out the low-order bits: - number >>= max(0, length - max_length) + number = _truncate_and_convert_digest( + digest, self.curve, allow_truncate, + ) try: r, s = sigdecode(signature, self.pubkey.order) @@ -1409,14 +1443,9 @@ def sign_digest( :rtype: bytes or sigencode function dependant type """ digest = normalise_bytes(digest) - if allow_truncate: - digest = digest[: self.curve.baselen] - if len(digest) > self.curve.baselen: - raise BadDigestError( - "this curve (%s) is too short " - "for your digest (%d)" % (self.curve.name, 8 * len(digest)) - ) - number = string_to_number(digest) + number = _truncate_and_convert_digest( + digest, self.curve, allow_truncate, + ) r, s = self.sign_number(number, entropy, k) return sigencode(r, s, self.privkey.order) diff --git a/src/ecdsa/test_jacobi.py b/src/ecdsa/test_jacobi.py index 4a938a32..4fdbe545 100644 --- a/src/ecdsa/test_jacobi.py +++ b/src/ecdsa/test_jacobi.py @@ -210,7 +210,8 @@ def test_multiplications(self, mul): @example(0) @example(int(generator_brainpoolp160r1.order())) def test_precompute(self, mul): - precomp = PointJacobi.from_affine(generator_brainpoolp160r1, True) + precomp = generator_brainpoolp160r1 + self.assertTrue(precomp._PointJacobi__precompute) pj = PointJacobi.from_affine(generator_brainpoolp160r1) a = precomp * mul @@ -383,7 +384,7 @@ def test_mul_add_same(self): self.assertEqual(j_g * 2, j_g.mul_add(1, j_g, 1)) def test_mul_add_precompute(self): - j_g = PointJacobi.from_affine(generator_256, True) + j_g = PointJacobi.from_affine(generator_brainpoolp160r1, True) b = PointJacobi.from_affine(j_g * 255, True) self.assertEqual(j_g * 256, j_g + b) @@ -391,7 +392,7 @@ def test_mul_add_precompute(self): self.assertEqual(j_g * (5 + 255 * 7), j_g.mul_add(5, b, 7)) def test_mul_add_precompute_large(self): - j_g = PointJacobi.from_affine(generator_256, True) + j_g = PointJacobi.from_affine(generator_brainpoolp160r1, True) b = PointJacobi.from_affine(j_g * 255, True) self.assertEqual(j_g * 256, j_g + b) diff --git a/src/ecdsa/test_pyecdsa.py b/src/ecdsa/test_pyecdsa.py index 65b67160..288b2eb0 100644 --- a/src/ecdsa/test_pyecdsa.py +++ b/src/ecdsa/test_pyecdsa.py @@ -616,7 +616,7 @@ def test_hashfunc(self): def test_public_key_recovery(self): # Create keys - curve = NIST256p + curve = BRAINPOOLP160r1 sk = SigningKey.generate(curve=curve) vk = sk.get_verifying_key() @@ -649,7 +649,7 @@ def test_public_key_recovery(self): def test_public_key_recovery_with_custom_hash(self): # Create keys - curve = NIST256p + curve = BRAINPOOLP160r1 sk = SigningKey.generate(curve=curve, hashfunc=sha256) vk = sk.get_verifying_key() @@ -660,7 +660,7 @@ def test_public_key_recovery_with_custom_hash(self): # Recover verifying keys recovered_vks = VerifyingKey.from_public_key_recovery( - signature, data, curve, hashfunc=sha256 + signature, data, curve, hashfunc=sha256, allow_truncate=True ) # Test if each pk is valid @@ -829,6 +829,7 @@ def test_VerifyingKey_encode_decode(curve, encoding): assert vk.pubkey.point == from_enc.pubkey.point + class OpenSSL(unittest.TestCase): # test interoperability with OpenSSL tools. Note that openssl's ECDSA # sign/verify arguments changed between 0.9.8 and 1.0.0: the early From 36e2e922f9fc638338fb4364012227bf33c049cc Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Mon, 7 Dec 2020 23:27:33 +0100 Subject: [PATCH 02/11] add secp112r1 --- src/ecdsa/__init__.py | 2 ++ src/ecdsa/curves.py | 13 +++++++++++++ src/ecdsa/ecdsa.py | 15 +++++++++++++++ src/ecdsa/test_pyecdsa.py | 15 +++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/src/ecdsa/__init__.py b/src/ecdsa/__init__.py index 4ae0a114..d4516bf8 100644 --- a/src/ecdsa/__init__.py +++ b/src/ecdsa/__init__.py @@ -19,6 +19,7 @@ BRAINPOOLP320r1, BRAINPOOLP384r1, BRAINPOOLP512r1, + SECP112r1, ) from .ecdh import ( ECDH, @@ -72,5 +73,6 @@ BRAINPOOLP320r1, BRAINPOOLP384r1, BRAINPOOLP512r1, + SECP112r1, ] del _hush_pyflakes diff --git a/src/ecdsa/curves.py b/src/ecdsa/curves.py index 9a103803..7cb5c9ca 100644 --- a/src/ecdsa/curves.py +++ b/src/ecdsa/curves.py @@ -10,6 +10,7 @@ "UnknownCurveError", "orderlen", "Curve", + "SECP112r1", "NIST192p", "NIST224p", "NIST256p", @@ -49,6 +50,16 @@ def __repr__(self): return self.name +# the SEC curves +SECP112r1 = Curve( + "SECP112r1", + ecdsa.curve_112r1, + ecdsa.generator_112r1, + (1, 3, 132, 0, 6), + "secp112r1", +) + + # the NIST curves NIST192p = Curve( "NIST192p", @@ -167,6 +178,7 @@ def __repr__(self): ) +# no order in particular, but keep previously added curves first curves = [ NIST192p, NIST224p, @@ -181,6 +193,7 @@ def __repr__(self): BRAINPOOLP320r1, BRAINPOOLP384r1, BRAINPOOLP512r1, + SECP112r1, ] diff --git a/src/ecdsa/ecdsa.py b/src/ecdsa/ecdsa.py index d785a457..d110990c 100644 --- a/src/ecdsa/ecdsa.py +++ b/src/ecdsa/ecdsa.py @@ -294,6 +294,21 @@ def point_is_valid(generator, x, y): return True +# secp112r1 curve +_p = int(remove_whitespace("DB7C 2ABF62E3 5E668076 BEAD208B"), 16) +# s = 00F50B02 8E4D696E 67687561 51752904 72783FB1 +_a = int(remove_whitespace("DB7C 2ABF62E3 5E668076 BEAD2088"), 16) +_b = int(remove_whitespace("659E F8BA0439 16EEDE89 11702B22"), 16) +_Gx = int(remove_whitespace("09487239 995A5EE7 6B55F9C2 F098"), 16) +_Gy = int(remove_whitespace("A89C E5AF8724 C0A23E0E 0FF77500"), 16) +_r = int(remove_whitespace("DB7C 2ABF62E3 5E7628DF AC6561C5"), 16) +_h = 1 +curve_112r1 = ellipticcurve.CurveFp(_p, _a, _b, _h) +generator_112r1 = ellipticcurve.PointJacobi( + curve_112r1, _Gx, _Gy, 1, _r, generator=True +) + + # NIST Curve P-192: _p = 6277101735386680763835789423207666416083908700390324961279 _r = 6277101735386680763835789423176059013767194773182842284081 diff --git a/src/ecdsa/test_pyecdsa.py b/src/ecdsa/test_pyecdsa.py index 288b2eb0..2cbeb75c 100644 --- a/src/ecdsa/test_pyecdsa.py +++ b/src/ecdsa/test_pyecdsa.py @@ -26,6 +26,7 @@ from .util import number_to_string, encoded_oid_ecPublicKey, MalformedSignature from .curves import Curve, UnknownCurveError from .curves import ( + SECP112r1, NIST192p, NIST224p, NIST256p, @@ -866,6 +867,13 @@ def get_openssl_messagedigest_arg(self, hash_name): # vk: 3:OpenSSL->python 4:python->OpenSSL # sig: 5:OpenSSL->python 6:python->OpenSSL + @pytest.mark.skipif( + "secp112r1" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp112r1", + ) + def test_from_openssl_secp112r1(self): + return self.do_test_from_openssl(SECP112r1) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", @@ -1030,6 +1038,13 @@ def do_test_from_openssl(self, curve, hash_name="SHA1"): sk_from_p8 = SigningKey.from_pem(privkey_p8_pem) self.assertEqual(sk, sk_from_p8) + @pytest.mark.skipif( + "secp112r1" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp112r1", + ) + def test_to_openssl_secp112r1(self): + self.do_test_to_openssl(SECP112r1) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", From de821521cc3eafe5843557e719803537e478fb26 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Mon, 7 Dec 2020 23:40:39 +0100 Subject: [PATCH 03/11] add secp112r2 --- src/ecdsa/__init__.py | 2 ++ src/ecdsa/curves.py | 11 +++++++++++ src/ecdsa/ecdsa.py | 15 +++++++++++++++ src/ecdsa/test_pyecdsa.py | 15 +++++++++++++++ 4 files changed, 43 insertions(+) diff --git a/src/ecdsa/__init__.py b/src/ecdsa/__init__.py index d4516bf8..0e834acf 100644 --- a/src/ecdsa/__init__.py +++ b/src/ecdsa/__init__.py @@ -20,6 +20,7 @@ BRAINPOOLP384r1, BRAINPOOLP512r1, SECP112r1, + SECP112r2, ) from .ecdh import ( ECDH, @@ -74,5 +75,6 @@ BRAINPOOLP384r1, BRAINPOOLP512r1, SECP112r1, + SECP112r2, ] del _hush_pyflakes diff --git a/src/ecdsa/curves.py b/src/ecdsa/curves.py index 7cb5c9ca..d14e3c10 100644 --- a/src/ecdsa/curves.py +++ b/src/ecdsa/curves.py @@ -11,6 +11,7 @@ "orderlen", "Curve", "SECP112r1", + "SECP112r2", "NIST192p", "NIST224p", "NIST256p", @@ -60,6 +61,15 @@ def __repr__(self): ) +SECP112r2 = Curve( + "SECP112r2", + ecdsa.curve_112r2, + ecdsa.generator_112r2, + (1, 3, 132, 0, 7), + "secp112r2", +) + + # the NIST curves NIST192p = Curve( "NIST192p", @@ -194,6 +204,7 @@ def __repr__(self): BRAINPOOLP384r1, BRAINPOOLP512r1, SECP112r1, + SECP112r2, ] diff --git a/src/ecdsa/ecdsa.py b/src/ecdsa/ecdsa.py index d110990c..b572143d 100644 --- a/src/ecdsa/ecdsa.py +++ b/src/ecdsa/ecdsa.py @@ -309,6 +309,21 @@ def point_is_valid(generator, x, y): ) +# secp112r2 curve +_p = int(remove_whitespace("DB7C 2ABF62E3 5E668076 BEAD208B"), 16) +# s = 022757A1 114D69E 67687561 51755316 C05E0BD4 +_a = int(remove_whitespace("6127 C24C05F3 8A0AAAF6 5C0EF02C"), 16) +_b = int(remove_whitespace("51DE F1815DB5 ED74FCC3 4C85D709"), 16) +_Gx = int(remove_whitespace("4BA30AB5 E892B4E1 649DD092 8643"), 16) +_Gy = int(remove_whitespace("ADCD 46F5882E 3747DEF3 6E956E97"), 16) +_r = int(remove_whitespace("36DF 0AAFD8B8 D7597CA1 0520D04B"), 16) +_h = 4 +curve_112r2 = ellipticcurve.CurveFp(_p, _a, _b, _h) +generator_112r2 = ellipticcurve.PointJacobi( + curve_112r2, _Gx, _Gy, 1, _r, generator=True +) + + # NIST Curve P-192: _p = 6277101735386680763835789423207666416083908700390324961279 _r = 6277101735386680763835789423176059013767194773182842284081 diff --git a/src/ecdsa/test_pyecdsa.py b/src/ecdsa/test_pyecdsa.py index 2cbeb75c..bdd5f9b4 100644 --- a/src/ecdsa/test_pyecdsa.py +++ b/src/ecdsa/test_pyecdsa.py @@ -27,6 +27,7 @@ from .curves import Curve, UnknownCurveError from .curves import ( SECP112r1, + SECP112r2, NIST192p, NIST224p, NIST256p, @@ -874,6 +875,13 @@ def get_openssl_messagedigest_arg(self, hash_name): def test_from_openssl_secp112r1(self): return self.do_test_from_openssl(SECP112r1) + @pytest.mark.skipif( + "secp112r2" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp112r2", + ) + def test_from_openssl_secp112r2(self): + return self.do_test_from_openssl(SECP112r2) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", @@ -1045,6 +1053,13 @@ def do_test_from_openssl(self, curve, hash_name="SHA1"): def test_to_openssl_secp112r1(self): self.do_test_to_openssl(SECP112r1) + @pytest.mark.skipif( + "secp112r2" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp112r2", + ) + def test_to_openssl_secp112r2(self): + self.do_test_to_openssl(SECP112r2) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", From e74f8e6540c9a6f5a0c4bc1e186e0403948c9563 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Mon, 7 Dec 2020 23:53:18 +0100 Subject: [PATCH 04/11] add secp128r1 --- src/ecdsa/__init__.py | 2 ++ src/ecdsa/curves.py | 11 +++++++++++ src/ecdsa/ecdsa.py | 16 ++++++++++++++++ src/ecdsa/test_pyecdsa.py | 15 +++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/src/ecdsa/__init__.py b/src/ecdsa/__init__.py index 0e834acf..b429e253 100644 --- a/src/ecdsa/__init__.py +++ b/src/ecdsa/__init__.py @@ -21,6 +21,7 @@ BRAINPOOLP512r1, SECP112r1, SECP112r2, + SECP128r1, ) from .ecdh import ( ECDH, @@ -76,5 +77,6 @@ BRAINPOOLP512r1, SECP112r1, SECP112r2, + SECP128r1, ] del _hush_pyflakes diff --git a/src/ecdsa/curves.py b/src/ecdsa/curves.py index d14e3c10..1de13e81 100644 --- a/src/ecdsa/curves.py +++ b/src/ecdsa/curves.py @@ -12,6 +12,7 @@ "Curve", "SECP112r1", "SECP112r2", + "SECP128r1", "NIST192p", "NIST224p", "NIST256p", @@ -70,6 +71,15 @@ def __repr__(self): ) +SECP128r1 = Curve( + "SECP128r1", + ecdsa.curve_128r1, + ecdsa.generator_128r1, + (1, 3, 132, 0, 28), + "secp128r1", +) + + # the NIST curves NIST192p = Curve( "NIST192p", @@ -205,6 +215,7 @@ def __repr__(self): BRAINPOOLP512r1, SECP112r1, SECP112r2, + SECP128r1, ] diff --git a/src/ecdsa/ecdsa.py b/src/ecdsa/ecdsa.py index b572143d..b1f4a151 100644 --- a/src/ecdsa/ecdsa.py +++ b/src/ecdsa/ecdsa.py @@ -324,6 +324,22 @@ def point_is_valid(generator, x, y): ) +# secp128r1 curve +_p = int(remove_whitespace("FFFFFFFD FFFFFFFF FFFFFFFF FFFFFFFF"), 16) +# S = 000E0D4D 69E6768 75615175 0CC03A44 73D03679 +# a and b are mod p, so a is equal to p-3, or simply -3 +# _a = -3 +_b = int(remove_whitespace("E87579C1 1079F43D D824993C 2CEE5ED3"), 16) +_Gx = int(remove_whitespace("161FF752 8B899B2D 0C28607C A52C5B86"), 16) +_Gy = int(remove_whitespace("CF5AC839 5BAFEB13 C02DA292 DDED7A83"), 16) +_r = int(remove_whitespace("FFFFFFFE 00000000 75A30D1B 9038A115"), 16) +_h = 1 +curve_128r1 = ellipticcurve.CurveFp(_p, -3, _b, _h) +generator_128r1 = ellipticcurve.PointJacobi( + curve_128r1, _Gx, _Gy, 1, _r, generator=True +) + + # NIST Curve P-192: _p = 6277101735386680763835789423207666416083908700390324961279 _r = 6277101735386680763835789423176059013767194773182842284081 diff --git a/src/ecdsa/test_pyecdsa.py b/src/ecdsa/test_pyecdsa.py index bdd5f9b4..de79d6ba 100644 --- a/src/ecdsa/test_pyecdsa.py +++ b/src/ecdsa/test_pyecdsa.py @@ -28,6 +28,7 @@ from .curves import ( SECP112r1, SECP112r2, + SECP128r1, NIST192p, NIST224p, NIST256p, @@ -882,6 +883,13 @@ def test_from_openssl_secp112r1(self): def test_from_openssl_secp112r2(self): return self.do_test_from_openssl(SECP112r2) + @pytest.mark.skipif( + "secp128r1" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp128r1", + ) + def test_from_openssl_secp128r1(self): + return self.do_test_from_openssl(SECP128r1) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", @@ -1060,6 +1068,13 @@ def test_to_openssl_secp112r1(self): def test_to_openssl_secp112r2(self): self.do_test_to_openssl(SECP112r2) + @pytest.mark.skipif( + "secp128r1" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp128r1", + ) + def test_to_openssl_secp128r1(self): + self.do_test_to_openssl(SECP128r1) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", From f3607fda058986d15cb3d68bd6b9e30a52b2cf3a Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Wed, 9 Dec 2020 19:13:43 +0100 Subject: [PATCH 05/11] add secp160r1; fix handling curves with order > p the secret multiplier is limited by the order of the base point, that also informs the size of elements for the signature (as they are calculated modulo order), but the public point is a point, so its elements are modulo prime from the curve. The same thing applies to the shared secret: it's just one coordinate of the point, so it's modulo p of the curve, not modulo order of generator. for all curves up till now the size of order and size of the prime was the same so it worked fine, but secp160r1 is different, so it showed the bugs so fix this bug and add secp160r1 as the test coverage for it --- src/ecdsa/__init__.py | 2 ++ src/ecdsa/curves.py | 13 ++++++++++++- src/ecdsa/ecdh.py | 2 +- src/ecdsa/ecdsa.py | 22 ++++++++++++++++++++++ src/ecdsa/keys.py | 18 +++++++++--------- src/ecdsa/test_ecdh.py | 23 +++++++++-------------- src/ecdsa/test_pyecdsa.py | 25 +++++++++++++++++++++++-- 7 files changed, 78 insertions(+), 27 deletions(-) diff --git a/src/ecdsa/__init__.py b/src/ecdsa/__init__.py index b429e253..80ae50f7 100644 --- a/src/ecdsa/__init__.py +++ b/src/ecdsa/__init__.py @@ -22,6 +22,7 @@ SECP112r1, SECP112r2, SECP128r1, + SECP160r1, ) from .ecdh import ( ECDH, @@ -78,5 +79,6 @@ SECP112r1, SECP112r2, SECP128r1, + SECP160r1, ] del _hush_pyflakes diff --git a/src/ecdsa/curves.py b/src/ecdsa/curves.py index 1de13e81..90cdd52e 100644 --- a/src/ecdsa/curves.py +++ b/src/ecdsa/curves.py @@ -13,6 +13,7 @@ "SECP112r1", "SECP112r2", "SECP128r1", + "SECP160r1", "NIST192p", "NIST224p", "NIST256p", @@ -43,7 +44,7 @@ def __init__(self, name, curve, generator, oid, openssl_name=None): self.generator = generator self.order = generator.order() self.baselen = orderlen(self.order) - self.verifying_key_length = 2 * self.baselen + self.verifying_key_length = 2 * orderlen(curve.p()) self.signature_length = 2 * self.baselen self.oid = oid self.encoded_oid = der.encode_oid(*oid) @@ -80,6 +81,15 @@ def __repr__(self): ) +SECP160r1 = Curve( + "SECP160r1", + ecdsa.curve_160r1, + ecdsa.generator_160r1, + (1, 3, 132, 0, 8), + "secp160r1", +) + + # the NIST curves NIST192p = Curve( "NIST192p", @@ -216,6 +226,7 @@ def __repr__(self): SECP112r1, SECP112r2, SECP128r1, + SECP160r1, ] diff --git a/src/ecdsa/ecdh.py b/src/ecdsa/ecdh.py index 9173279f..a12e94ee 100644 --- a/src/ecdsa/ecdh.py +++ b/src/ecdsa/ecdh.py @@ -304,7 +304,7 @@ def generate_sharedsecret_bytes(self): :rtype: byte string """ return number_to_string( - self.generate_sharedsecret(), self.private_key.curve.order + self.generate_sharedsecret(), self.private_key.curve.curve.p() ) def generate_sharedsecret(self): diff --git a/src/ecdsa/ecdsa.py b/src/ecdsa/ecdsa.py index b1f4a151..9f11d50c 100644 --- a/src/ecdsa/ecdsa.py +++ b/src/ecdsa/ecdsa.py @@ -340,6 +340,28 @@ def point_is_valid(generator, x, y): ) +# secp160r1 +_p = int(remove_whitespace("FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 7FFFFFFF"), 16) +# S = 1053CDE4 2C14D696 E6768756 1517533B F3F83345 +# a and b are mod p, so a is equal to p-3, or simply -3 +# _a = -3 +_b = int(remove_whitespace("1C97BEFC 54BD7A8B 65ACF89F 81D4D4AD C565FA45"), 16) +_Gx = int( + remove_whitespace("4A96B568 8EF57328 46646989 68C38BB9 13CBFC82"), 16, +) +_Gy = int( + remove_whitespace("23A62855 3168947D 59DCC912 04235137 7AC5FB32"), 16, +) +_r = int( + remove_whitespace("01 00000000 00000000 0001F4C8 F927AED3 CA752257"), 16, +) +_h = 1 +curve_160r1 = ellipticcurve.CurveFp(_p, -3, _b, _h) +generator_160r1 = ellipticcurve.PointJacobi( + curve_160r1, _Gx, _Gy, 1, _r, generator=True +) + + # NIST Curve P-192: _p = 6277101735386680763835789423207666416083908700390324961279 _r = 6277101735386680763835789423176059013767194773182842284081 diff --git a/src/ecdsa/keys.py b/src/ecdsa/keys.py index cc675107..8edd99c6 100644 --- a/src/ecdsa/keys.py +++ b/src/ecdsa/keys.py @@ -272,12 +272,12 @@ def _from_raw_encoding(string, curve): order = curve.order # real assert, from_string() should not call us with different length assert len(string) == curve.verifying_key_length - xs = string[: curve.baselen] - ys = string[curve.baselen :] - if len(xs) != curve.baselen: - raise MalformedPointError("Unexpected length of encoded x") - if len(ys) != curve.baselen: - raise MalformedPointError("Unexpected length of encoded y") + xs = string[: curve.verifying_key_length // 2] + ys = string[curve.verifying_key_length // 2 :] + # real assert, verifying_key_length is calculated by multiplying an + # integer by two so it will always be even + assert len(xs) == curve.verifying_key_length // 2 + assert len(ys) == curve.verifying_key_length // 2 x = string_to_number(xs) y = string_to_number(ys) @@ -371,7 +371,7 @@ def from_string( raise MalformedPointError( "Invalid X9.62 encoding of the public point" ) - elif sig_len == curve.baselen + 1: + elif sig_len == curve.verifying_key_length // 2 + 1: point = cls._from_compressed(string, curve) else: raise MalformedPointError( @@ -573,14 +573,14 @@ def from_public_key_recovery_with_digest( def _raw_encode(self): """Convert the public key to the :term:`raw encoding`.""" - order = self.pubkey.order + order = self.curve.curve.p() x_str = number_to_string(self.pubkey.point.x(), order) y_str = number_to_string(self.pubkey.point.y(), order) return x_str + y_str def _compressed_encode(self): """Encode the public point into the compressed form.""" - order = self.pubkey.order + order = self.curve.curve.p() x_str = number_to_string(self.pubkey.point.x(), order) if self.pubkey.point.y() & 1: return b("\x03") + x_str diff --git a/src/ecdsa/test_ecdh.py b/src/ecdsa/test_ecdh.py index d84429ce..32d88252 100644 --- a/src/ecdsa/test_ecdh.py +++ b/src/ecdsa/test_ecdh.py @@ -375,7 +375,7 @@ def test_ecdh_with_openssl(vcurve): if hlp.find("-derive") == 0: # pragma: no cover pytest.skip("system openssl does not support `pkeyutl -derive`") except RunOpenSslError: # pragma: no cover - pytest.skip("system openssl does not support `pkeyutl -derive`") + pytest.skip("system openssl could not be executed") if os.path.isdir("t"): # pragma: no branch shutil.rmtree("t") @@ -412,25 +412,20 @@ def test_ecdh_with_openssl(vcurve): assert secret1 == secret2 - try: - run_openssl( - "pkeyutl -derive -inkey t/privkey1.pem -peerkey t/pubkey2.pem -out t/secret1" - ) - run_openssl( - "pkeyutl -derive -inkey t/privkey2.pem -peerkey t/pubkey1.pem -out t/secret2" - ) - except RunOpenSslError: # pragma: no cover - pytest.skip("system openssl does not support `pkeyutl -derive`") - return + run_openssl( + "pkeyutl -derive -inkey t/privkey1.pem -peerkey t/pubkey2.pem -out t/secret1" + ) + run_openssl( + "pkeyutl -derive -inkey t/privkey2.pem -peerkey t/pubkey1.pem -out t/secret2" + ) with open("t/secret1", "rb") as e: ssl_secret1 = e.read() with open("t/secret1", "rb") as e: ssl_secret2 = e.read() - if len(ssl_secret1) != vk1.curve.baselen: # pragma: no cover - pytest.skip("system openssl does not support `pkeyutl -derive`") - return + assert len(ssl_secret1) == vk1.curve.verifying_key_length // 2 + assert len(secret1) == vk1.curve.verifying_key_length // 2 assert ssl_secret1 == ssl_secret2 assert secret1 == ssl_secret1 diff --git a/src/ecdsa/test_pyecdsa.py b/src/ecdsa/test_pyecdsa.py index de79d6ba..d904b939 100644 --- a/src/ecdsa/test_pyecdsa.py +++ b/src/ecdsa/test_pyecdsa.py @@ -29,6 +29,7 @@ SECP112r1, SECP112r2, SECP128r1, + SECP160r1, NIST192p, NIST224p, NIST256p, @@ -313,8 +314,15 @@ class FakeGenerator: def order(self): return 123456789 + class FakeCurveFp: + def p(self): + return int( + "6525534529039240705020950546962731340" + "4541085228058844382513856749047873406763" + ) + badcurve = Curve( - "unknown", None, FakeGenerator(), (1, 2, 3, 4, 5, 6), None + "unknown", FakeCurveFp(), FakeGenerator(), (1, 2, 3, 4, 5, 6), None ) badpub.curve = badcurve badder = badpub.to_der() @@ -832,7 +840,6 @@ def test_VerifyingKey_encode_decode(curve, encoding): assert vk.pubkey.point == from_enc.pubkey.point - class OpenSSL(unittest.TestCase): # test interoperability with OpenSSL tools. Note that openssl's ECDSA # sign/verify arguments changed between 0.9.8 and 1.0.0: the early @@ -890,6 +897,13 @@ def test_from_openssl_secp112r2(self): def test_from_openssl_secp128r1(self): return self.do_test_from_openssl(SECP128r1) + @pytest.mark.skipif( + "secp160r1" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp160r1", + ) + def test_from_openssl_secp160r1(self): + return self.do_test_from_openssl(SECP160r1) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", @@ -1075,6 +1089,13 @@ def test_to_openssl_secp112r2(self): def test_to_openssl_secp128r1(self): self.do_test_to_openssl(SECP128r1) + @pytest.mark.skipif( + "secp160r1" not in OPENSSL_SUPPORTED_CURVES, + reason="system openssl does not support secp160r1", + ) + def test_to_openssl_secp160r1(self): + self.do_test_to_openssl(SECP160r1) + @pytest.mark.skipif( "prime192v1" not in OPENSSL_SUPPORTED_CURVES, reason="system openssl does not support prime192v1", From 57b3f7b626594ea6e40a42e3e7177e65e9ca26a3 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Tue, 8 Dec 2020 11:22:21 +0100 Subject: [PATCH 06/11] ignore _compat module both for old and new checkout if we don't ignore _compat for the new checkout, we will get a big decrease in code coverage --- .travis.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3ebeff48..bddea117 100644 --- a/.travis.yml +++ b/.travis.yml @@ -117,17 +117,16 @@ script: - | if [[ $INSTRUMENTAL && $TRAVIS_PULL_REQUEST != "false" ]]; then git checkout $PR_FIRST^ - files="$(ls src/ecdsa/test*.py)" - instrumental -t ecdsa -i 'test.*|.*_version|.*_compat' `which pytest` $files + instrumental -t ecdsa -i 'test.*|.*_version|.*_compat' `which pytest` src/ecdsa/test*.py instrumental -f .instrumental.cov -s instrumental -f .instrumental.cov -s | python diff-instrumental.py --save .diff-instrumental git checkout $BRANCH - instrumental -t ecdsa -i 'test.*|.*_version' `which pytest` src/ecdsa + instrumental -t ecdsa -i 'test.*|.*_version|.*_compat' `which pytest` src/ecdsa/test*.py instrumental -f .instrumental.cov -sr fi - | if [[ $INSTRUMENTAL && $TRAVIS_PULL_REQUEST == "false" ]]; then - instrumental -t ecdsa -i 'test.*|.*_version' `which pytest` src/ecdsa + instrumental -t ecdsa -i 'test.*|.*_version|.*_compat' `which pytest` src/ecdsa instrumental -f .instrumental.cov -s # just log the values when merging instrumental -f .instrumental.cov -s | python diff-instrumental.py From 28795824642531db10736067eceb3e510cd2b5f3 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Tue, 8 Dec 2020 22:37:45 +0100 Subject: [PATCH 07/11] run instrumental with new OpenSSL many tests (in particular ecdh) require new OpenSSL, so run it on a distro with new OpenSSL --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index bddea117..3bbd9acd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,6 +17,8 @@ matrix: include: - python: 2.7 env: INSTRUMENTAL=yes + dist: bionic + sudo: true - python: 2.6 env: TOX_ENV=py26 - python: 2.7 From 3bda7d703ef0e41969405485799a40691b7069eb Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Mon, 7 Dec 2020 03:30:01 +0100 Subject: [PATCH 08/11] add instrumental to tox config make it easier to test instrumental coverage --- tox.ini | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tox.ini b/tox.ini index 4b072c3f..60562ded 100644 --- a/tox.ini +++ b/tox.ini @@ -55,6 +55,19 @@ basepython=python2.7 [testenv:gmpy2py39] basepython=python3.9 +[testenv:instrumental] +basepython = python2.7 +deps = + gmpy2 + instrumental + hypothesis + pytest>=4.6.0 + coverage==4.5.4 + six +commands = + instrumental -t ecdsa -i 'test.*|.*_version|.*_compat' {envbindir}/pytest {posargs:src/ecdsa} + instrumental -f .instrumental.cov -sr + [testenv:coverage] sitepackages=True whitelist_externals=coverage From 95aa83fcf45ffff179c42520630ed8cdbd7254d9 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Tue, 8 Dec 2020 23:15:55 +0100 Subject: [PATCH 09/11] add some test coverage to ecdsa.keys --- src/ecdsa/test_keys.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/ecdsa/test_keys.py b/src/ecdsa/test_keys.py index 406a5bf6..b25403f2 100644 --- a/src/ecdsa/test_keys.py +++ b/src/ecdsa/test_keys.py @@ -150,6 +150,8 @@ def setUpClass(cls): ) cls.vk2 = VerifyingKey.from_pem(key_str) + cls.sk2 = SigningKey.generate(vk.curve) + def test_custom_hashfunc(self): vk = VerifyingKey.from_der(self.key_bytes, hashlib.sha256) @@ -196,10 +198,20 @@ def test_equality_on_verifying_keys(self): self.assertEqual(self.vk, self.sk.get_verifying_key()) def test_inequality_on_verifying_keys(self): - self.assertNotEqual(self.vk, self.vk2) + # use `==` to workaround instrumental <-> unittest compat issue + self.assertFalse(self.vk == self.vk2) def test_inequality_on_verifying_keys_not_implemented(self): - self.assertNotEqual(self.vk, None) + # use `==` to workaround instrumental <-> unittest compat issue + self.assertFalse(self.vk == None) + + def test_VerifyingKey_inequality_on_same_curve(self): + # use `==` to workaround instrumental <-> unittest compat issue + self.assertFalse(self.vk == self.sk2.verifying_key) + + def test_SigningKey_inequality_on_same_curve(self): + # use `==` to workaround instrumental <-> unittest compat issue + self.assertFalse(self.sk == self.sk2) class TestSigningKey(unittest.TestCase): @@ -271,10 +283,12 @@ def test_verify_with_lazy_precompute(self): self.assertTrue(vk.verify(sig, b"other message")) def test_inequality_on_signing_keys(self): - self.assertNotEqual(self.sk1, self.sk2) + # use `==` to workaround instrumental <-> unittest compat issue + self.assertFalse(self.sk1 == self.sk2) def test_inequality_on_signing_keys_not_implemented(self): - self.assertNotEqual(self.sk1, None) + # use `==` to workaround instrumental <-> unittest compat issue + self.assertFalse(self.sk1 == None) # test VerifyingKey.verify() From bae6ddc771a468a6c1520fad72ea6fc21bf09f2e Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Wed, 9 Dec 2020 09:36:27 +0100 Subject: [PATCH 10/11] is_prime() test coverage looks like there is some test coverage variability from hypothesis so do few static examples --- src/ecdsa/test_numbertheory.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/ecdsa/test_numbertheory.py b/src/ecdsa/test_numbertheory.py index 4912c578..b121b873 100644 --- a/src/ecdsa/test_numbertheory.py +++ b/src/ecdsa/test_numbertheory.py @@ -207,6 +207,38 @@ def st_comp_no_com_fac(draw): HYP_SLOW_SETTINGS["max_examples"] = 10 +class TestIsPrime(unittest.TestCase): + def test_very_small_prime(self): + assert is_prime(23) + + def test_very_small_composite(self): + assert not is_prime(22) + + def test_small_prime(self): + assert is_prime(123456791) + + def test_special_composite(self): + assert not is_prime(10261) + + def test_medium_prime_1(self): + # nextPrime[2^256] + assert is_prime(2 ** 256 + 0x129) + + def test_medium_prime_2(self): + # nextPrime(2^256+0x129) + assert is_prime(2 ** 256 + 0x12D) + + def test_medium_trivial_composite(self): + assert not is_prime(2 ** 256 + 0x130) + + def test_medium_non_trivial_composite(self): + assert not is_prime(2 ** 256 + 0x12F) + + def test_large_prime(self): + # nextPrime[2^2048] + assert is_prime(2 ** 2048 + 0x3D5) + + class TestNumbertheory(unittest.TestCase): def test_gcd(self): assert gcd(3 * 5 * 7, 3 * 5 * 11, 3 * 5 * 13) == 3 * 5 From 4bd1d1c9e55085aee1e4eb53f7a3d20ddefa0c4f Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Wed, 9 Dec 2020 09:44:22 +0100 Subject: [PATCH 11/11] ignore environment-caused lack of branch coverage for inverse_mod() --- src/ecdsa/numbertheory.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ecdsa/numbertheory.py b/src/ecdsa/numbertheory.py index 5ff1c272..03577c72 100644 --- a/src/ecdsa/numbertheory.py +++ b/src/ecdsa/numbertheory.py @@ -220,11 +220,14 @@ def square_root_mod_prime(a, p): raise RuntimeError("No b found.") +# because all the inverse_mod code is arch/environment specific, and coveralls +# expects it to execute equal number of times, we need to waive it by +# adding the "no branch" pragma to all branches if GMPY2: # pragma: no branch def inverse_mod(a, m): """Inverse of a mod m.""" - if a == 0: + if a == 0: # pragma: no branch return 0 return powmod(a, -1, m) @@ -237,14 +240,14 @@ def inverse_mod(a, m): # only using the native `pow()` function, and `pow()` in gmpy sanity # checks the parameters before passing them on to underlying # implementation - if a == 0: + if a == 0: # pragma: no branch return 0 a = mpz(a) m = mpz(m) lm, hm = mpz(1), mpz(0) low, high = a % m, m - while low > 1: + while low > 1: # pragma: no branch r = high // low lm, low, hm, high = hm - lm * r, high - low * r, lm, low @@ -255,7 +258,7 @@ def inverse_mod(a, m): def inverse_mod(a, m): """Inverse of a mod m.""" - if a == 0: + if a == 0: # pragma: no branch return 0 return pow(a, -1, m) @@ -265,12 +268,12 @@ def inverse_mod(a, m): def inverse_mod(a, m): """Inverse of a mod m.""" - if a == 0: + if a == 0: # pragma: no branch return 0 lm, hm = 1, 0 low, high = a % m, m - while low > 1: + while low > 1: # pragma: no branch r = high // low lm, low, hm, high = hm - lm * r, high - low * r, lm, low