From b0ea52bb3aa9a16c9a4a91fdc0041edbfed10b31 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Thu, 26 Sep 2019 18:07:09 +0200 Subject: [PATCH 01/10] strict error checking in DER decoding of integers and sequences backport of 50e6d9f37 --- ecdsa/der.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/ecdsa/der.py b/ecdsa/der.py index b045952f..8afcf23d 100644 --- a/ecdsa/der.py +++ b/ecdsa/der.py @@ -60,10 +60,15 @@ def remove_constructed(string): return tag, body, rest def remove_sequence(string): + if not string: + raise UnexpectedDER("Empty string does not encode a sequence") if not string.startswith(b("\x30")): - n = string[0] if isinstance(string[0], integer_types) else ord(string[0]) - raise UnexpectedDER("wanted sequence (0x30), got 0x%02x" % n) + n = string[0] if isinstance(string[0], integer_types) else \ + ord(string[0]) + raise UnexpectedDER("wanted type 'sequence' (0x30), got 0x%02x" % n) length, lengthlength = read_length(string[1:]) + if length > len(string) - 1 - lengthlength: + raise UnexpectedDER("Length longer than the provided buffer") endseq = 1+lengthlength+length return string[1+lengthlength:endseq], string[endseq:] @@ -96,14 +101,24 @@ def remove_object(string): return tuple(numbers), rest def remove_integer(string): + if not string: + raise UnexpectedDER("Empty string is an invalid encoding of an " + "integer") if not string.startswith(b("\x02")): - n = string[0] if isinstance(string[0], integer_types) else ord(string[0]) - raise UnexpectedDER("wanted integer (0x02), got 0x%02x" % n) + n = string[0] if isinstance(string[0], integer_types) \ + else ord(string[0]) + raise UnexpectedDER("wanted type 'integer' (0x02), got 0x%02x" % n) length, llen = read_length(string[1:]) + if length > len(string) - 1 - llen: + raise UnexpectedDER("Length longer than provided buffer") + if length == 0: + raise UnexpectedDER("0-byte long encoding of integer") numberbytes = string[1+llen:1+llen+length] rest = string[1+llen+length:] - nbytes = numberbytes[0] if isinstance(numberbytes[0], integer_types) else ord(numberbytes[0]) - assert nbytes < 0x80 # can't support negative numbers yet + nbytes = numberbytes[0] if isinstance(numberbytes[0], integer_types) \ + else ord(numberbytes[0]) + if not nbytes < 0x80: + raise UnexpectedDER("Negative integers are not supported") return int(binascii.hexlify(numberbytes), 16), rest def read_number(string): From 20b377491e2d759a3f47eb7aedba41292cc82238 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Thu, 26 Sep 2019 18:22:09 +0200 Subject: [PATCH 02/10] translate the UnexpectedDER exception to BadSignatureError the users of VerifyingKey.verify() and VerifyingKey.verify_digest() should not need to use multiple exceptions to correctly catch invalid signatures backport of 487f6d3fd6e --- ecdsa/keys.py | 7 ++-- ecdsa/test_malformed_sigs.py | 68 ++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 ecdsa/test_malformed_sigs.py diff --git a/ecdsa/keys.py b/ecdsa/keys.py index 5b521254..929249e9 100644 --- a/ecdsa/keys.py +++ b/ecdsa/keys.py @@ -106,11 +106,14 @@ def verify_digest(self, signature, digest, sigdecode=sigdecode_string): "for your digest (%d)" % (self.curve.name, 8*len(digest))) number = string_to_number(digest) - r, s = sigdecode(signature, self.pubkey.order) + try: + r, s = sigdecode(signature, self.pubkey.order) + except der.UnexpectedDER as e: + raise BadSignatureError("Malformed formatting of signature", e) sig = ecdsa.Signature(r, s) if self.pubkey.verifies(number, sig): return True - raise BadSignatureError + raise BadSignatureError("Signature verification failed") class SigningKey: def __init__(self, _error__please_use_generate=None): diff --git a/ecdsa/test_malformed_sigs.py b/ecdsa/test_malformed_sigs.py new file mode 100644 index 00000000..eaf022e4 --- /dev/null +++ b/ecdsa/test_malformed_sigs.py @@ -0,0 +1,68 @@ +from __future__ import with_statement, division + +import unittest +import os +import time +import shutil +import subprocess +import pytest +from binascii import hexlify, unhexlify +from hashlib import sha1, sha256, sha512 +import hashlib + +from six import b, binary_type +from .keys import SigningKey, VerifyingKey +from .keys import BadSignatureError +from . import util +from .util import sigencode_der, sigencode_strings +from .util import sigdecode_der, sigdecode_strings +from .curves import curves, NIST256p, NIST521p +from .ellipticcurve import Point +from . import der +from . import rfc6979 +import copy + +sigs = [] +example_data = b("some data to sign") + +# Just NIST256p with SHA256 is 560 test cases, all curves with all hashes is +# few thousand slow test cases; execute the most interesting only + +#for curve in curves: +for curve in [NIST256p]: + #for hash_alg in ["md5", "sha1", "sha224", "sha256", "sha384", "sha512"]: + for hash_alg in ["sha256"]: + key = SigningKey.generate(curve) + signature = key.sign(example_data, hashfunc=getattr(hashlib, hash_alg), + sigencode=sigencode_der) + for pos in range(len(signature)): + for xor in (1< Date: Thu, 26 Sep 2019 18:44:51 +0200 Subject: [PATCH 03/10] give the same handling to string encoded signatures as to DER Verify that strings of unexpected lengths are rejected with the same BadSignatureError exception backport of 8533e51384 --- ecdsa/keys.py | 4 +-- ecdsa/test_malformed_sigs.py | 61 +++++++++++++++++++++++------------- ecdsa/util.py | 26 +++++++++++++-- 3 files changed, 65 insertions(+), 26 deletions(-) diff --git a/ecdsa/keys.py b/ecdsa/keys.py index 929249e9..72c0c18f 100644 --- a/ecdsa/keys.py +++ b/ecdsa/keys.py @@ -6,7 +6,7 @@ from .curves import NIST192p, find_curve from .util import string_to_number, number_to_string, randrange from .util import sigencode_string, sigdecode_string -from .util import oid_ecPublicKey, encoded_oid_ecPublicKey +from .util import oid_ecPublicKey, encoded_oid_ecPublicKey, MalformedSignature from .six import PY3, b from hashlib import sha1 @@ -108,7 +108,7 @@ def verify_digest(self, signature, digest, sigdecode=sigdecode_string): number = string_to_number(digest) try: r, s = sigdecode(signature, self.pubkey.order) - except der.UnexpectedDER as e: + except (der.UnexpectedDER, MalformedSignature) as e: raise BadSignatureError("Malformed formatting of signature", e) sig = ecdsa.Signature(r, s) if self.pubkey.verifies(number, sig): diff --git a/ecdsa/test_malformed_sigs.py b/ecdsa/test_malformed_sigs.py index eaf022e4..f6bea9c8 100644 --- a/ecdsa/test_malformed_sigs.py +++ b/ecdsa/test_malformed_sigs.py @@ -1,28 +1,16 @@ from __future__ import with_statement, division -import unittest -import os -import time -import shutil -import subprocess import pytest -from binascii import hexlify, unhexlify -from hashlib import sha1, sha256, sha512 import hashlib from six import b, binary_type from .keys import SigningKey, VerifyingKey from .keys import BadSignatureError -from . import util -from .util import sigencode_der, sigencode_strings -from .util import sigdecode_der, sigdecode_strings +from .util import sigencode_der, sigencode_string +from .util import sigdecode_der, sigdecode_string from .curves import curves, NIST256p, NIST521p -from .ellipticcurve import Point -from . import der -from . import rfc6979 -import copy -sigs = [] +der_sigs = [] example_data = b("some data to sign") # Just NIST256p with SHA256 is 560 test cases, all curves with all hashes is @@ -37,14 +25,14 @@ sigencode=sigencode_der) for pos in range(len(signature)): for xor in (1< Date: Thu, 26 Sep 2019 19:12:45 +0200 Subject: [PATCH 04/10] fix length decoding the same issues as with decoding integers happen with the NIST521p curve as it's large enough that the length encoding of some fields needs to use multi-byte encoding backport of a655d6f382b2 --- ecdsa/der.py | 4 ++++ ecdsa/test_malformed_sigs.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ecdsa/der.py b/ecdsa/der.py index 8afcf23d..7e9c5867 100644 --- a/ecdsa/der.py +++ b/ecdsa/der.py @@ -148,6 +148,8 @@ def encode_length(l): return int2byte(0x80|llen) + s def read_length(string): + if not string: + raise UnexpectedDER("Empty string can't encode valid length value") num = string[0] if isinstance(string[0], integer_types) else ord(string[0]) if not (num & 0x80): # short form @@ -155,6 +157,8 @@ def read_length(string): # else long-form: b0&0x7f is number of additional base256 length bytes, # big-endian llen = num & 0x7f + if not llen: + raise UnexpectedDER("Invalid length encoding, length byte is 0") if llen > len(string)-1: raise UnexpectedDER("ran out of length bytes") return int(binascii.hexlify(string[1:1+llen]), 16), 1+llen diff --git a/ecdsa/test_malformed_sigs.py b/ecdsa/test_malformed_sigs.py index f6bea9c8..fffb1b3b 100644 --- a/ecdsa/test_malformed_sigs.py +++ b/ecdsa/test_malformed_sigs.py @@ -17,7 +17,7 @@ # few thousand slow test cases; execute the most interesting only #for curve in curves: -for curve in [NIST256p]: +for curve in [NIST521p]: #for hash_alg in ["md5", "sha1", "sha224", "sha256", "sha384", "sha512"]: for hash_alg in ["sha256"]: key = SigningKey.generate(curve) From 14abfe020d4907fd9849f269b98f5f8f1060366b Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Thu, 26 Sep 2019 19:15:54 +0200 Subject: [PATCH 05/10] explicitly specify the distro to get py26 and py33 backport of 091ca06422fc0 --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 115e2ea2..5e7a67aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,6 @@ +# workaround for 3.7 not available in default configuration +# travis-ci/travis-ci#9815 +dist: trusty language: python python: - "2.6" From 563d2ee2c07e10ae4f77ccde4161d6a14c681b1b Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Fri, 27 Sep 2019 13:58:32 +0200 Subject: [PATCH 06/10] make variable names in remove_integer more aproppriate backport of 710e7c88243 --- ecdsa/der.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ecdsa/der.py b/ecdsa/der.py index 7e9c5867..2cd48051 100644 --- a/ecdsa/der.py +++ b/ecdsa/der.py @@ -115,9 +115,9 @@ def remove_integer(string): raise UnexpectedDER("0-byte long encoding of integer") numberbytes = string[1+llen:1+llen+length] rest = string[1+llen+length:] - nbytes = numberbytes[0] if isinstance(numberbytes[0], integer_types) \ + msb = numberbytes[0] if isinstance(numberbytes[0], integer_types) \ else ord(numberbytes[0]) - if not nbytes < 0x80: + if not msb < 0x80: raise UnexpectedDER("Negative integers are not supported") return int(binascii.hexlify(numberbytes), 16), rest From 3427fa29f319b27898a28601955807abb44c0830 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Fri, 27 Sep 2019 17:50:10 +0200 Subject: [PATCH 07/10] ensure that the encoding is actually the minimal one for length and integer backport of 2c3db7c57 --- build-requirements-2.6.txt | 1 + ecdsa/der.py | 17 +++++++- ecdsa/test_der.py | 88 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 ecdsa/test_der.py diff --git a/build-requirements-2.6.txt b/build-requirements-2.6.txt index de0022ee..980e00df 100644 --- a/build-requirements-2.6.txt +++ b/build-requirements-2.6.txt @@ -1,3 +1,4 @@ tox coveralls<1.3.0 idna<2.8 +unittest2 diff --git a/ecdsa/der.py b/ecdsa/der.py index 2cd48051..200f00c0 100644 --- a/ecdsa/der.py +++ b/ecdsa/der.py @@ -119,6 +119,15 @@ def remove_integer(string): else ord(numberbytes[0]) if not msb < 0x80: raise UnexpectedDER("Negative integers are not supported") + # check if the encoding is the minimal one (DER requirement) + if length > 1 and not msb: + # leading zero byte is allowed if the integer would have been + # considered a negative number otherwise + smsb = numberbytes[1] if isinstance(numberbytes[1], integer_types) \ + else ord(numberbytes[1]) + if smsb < 0x80: + raise UnexpectedDER("Invalid encoding of integer, unnecessary " + "zero padding bytes") return int(binascii.hexlify(numberbytes), 16), rest def read_number(string): @@ -158,9 +167,13 @@ def read_length(string): # big-endian llen = num & 0x7f if not llen: - raise UnexpectedDER("Invalid length encoding, length byte is 0") + raise UnexpectedDER("Invalid length encoding, length of length is 0") if llen > len(string)-1: - raise UnexpectedDER("ran out of length bytes") + raise UnexpectedDER("Length of length longer than provided buffer") + # verify that the encoding is minimal possible (DER requirement) + msb = string[1] if isinstance(string[1], integer_types) else ord(string[1]) + if not msb or llen == 1 and msb < 0x80: + raise UnexpectedDER("Not minimal encoding of length") return int(binascii.hexlify(string[1:1+llen]), 16), 1+llen def remove_bitstring(string): diff --git a/ecdsa/test_der.py b/ecdsa/test_der.py new file mode 100644 index 00000000..a20a617c --- /dev/null +++ b/ecdsa/test_der.py @@ -0,0 +1,88 @@ + +# compatibility with Python 2.6, for that we need unittest2 package, +# which is not available on 3.3 or 3.4 +try: + import unittest2 as unittest +except ImportError: + import unittest +from .der import remove_integer, UnexpectedDER, read_length +from six import b + +class TestRemoveInteger(unittest.TestCase): + # DER requires the integers to be 0-padded only if they would be + # interpreted as negative, check if those errors are detected + def test_non_minimal_encoding(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x02\x00\x01')) + + def test_negative_with_high_bit_set(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x01\x80')) + + def test_two_zero_bytes_with_high_bit_set(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x03\x00\x00\xff')) + + def test_zero_length_integer(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('\x02\x00')) + + def test_empty_string(self): + with self.assertRaises(UnexpectedDER): + remove_integer(b('')) + + def test_encoding_of_zero(self): + val, rem = remove_integer(b('\x02\x01\x00')) + + self.assertEqual(val, 0) + self.assertFalse(rem) + + def test_encoding_of_127(self): + val, rem = remove_integer(b('\x02\x01\x7f')) + + self.assertEqual(val, 127) + self.assertFalse(rem) + + def test_encoding_of_128(self): + val, rem = remove_integer(b('\x02\x02\x00\x80')) + + self.assertEqual(val, 128) + self.assertFalse(rem) + + +class TestReadLength(unittest.TestCase): + # DER requires the lengths between 0 and 127 to be encoded using the short + # form and lengths above that encoded with minimal number of bytes + # necessary + def test_zero_length(self): + self.assertEqual((0, 1), read_length(b('\x00'))) + + def test_two_byte_zero_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x81\x00')) + + def test_two_byte_small_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x81\x7f')) + + def test_long_form_with_zero_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x80')) + + def test_smallest_two_byte_length(self): + self.assertEqual((128, 2), read_length(b('\x81\x80'))) + + def test_zero_padded_length(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x82\x00\x80')) + + def test_two_three_byte_length(self): + self.assertEqual((256, 3), read_length(b'\x82\x01\x00')) + + def test_empty_string(self): + with self.assertRaises(UnexpectedDER): + read_length(b('')) + + def test_length_overflow(self): + with self.assertRaises(UnexpectedDER): + read_length(b('\x83\x01\x00')) From 99c907d7acc94da6685470328174ea7299863dfd Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Fri, 4 Oct 2019 21:08:05 +0200 Subject: [PATCH 08/10] harden also key decoding as assert statements will be removed in optimised build, do use a custom exception that inherits from AssertionError so that the failures are caught this is reworking of d47a238126a5 to implement the same checks but without implementing support for SEC1/X9.62 formatted keys --- ecdsa/__init__.py | 5 +- ecdsa/keys.py | 43 ++++++++--- ecdsa/test_pyecdsa.py | 173 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 207 insertions(+), 14 deletions(-) diff --git a/ecdsa/__init__.py b/ecdsa/__init__.py index e834b3a8..5c2c3e7c 100644 --- a/ecdsa/__init__.py +++ b/ecdsa/__init__.py @@ -1,9 +1,12 @@ __all__ = ["curves", "der", "ecdsa", "ellipticcurve", "keys", "numbertheory", "test_pyecdsa", "util", "six"] -from .keys import SigningKey, VerifyingKey, BadSignatureError, BadDigestError +from .keys import SigningKey, VerifyingKey, BadSignatureError, BadDigestError,\ + MalformedPointError from .curves import NIST192p, NIST224p, NIST256p, NIST384p, NIST521p, SECP256k1 +from .der import UnexpectedDER _hush_pyflakes = [SigningKey, VerifyingKey, BadSignatureError, BadDigestError, + MalformedPointError, UnexpectedDER, NIST192p, NIST224p, NIST256p, NIST384p, NIST521p, SECP256k1] del _hush_pyflakes diff --git a/ecdsa/keys.py b/ecdsa/keys.py index 72c0c18f..45198170 100644 --- a/ecdsa/keys.py +++ b/ecdsa/keys.py @@ -3,6 +3,7 @@ from . import ecdsa from . import der from . import rfc6979 +from . import ellipticcurve from .curves import NIST192p, find_curve from .util import string_to_number, number_to_string, randrange from .util import sigencode_string, sigdecode_string @@ -15,6 +16,11 @@ class BadSignatureError(Exception): class BadDigestError(Exception): pass + +class MalformedPointError(AssertionError): + pass + + class VerifyingKey: def __init__(self, _error__please_use_generate=None): if not _error__please_use_generate: @@ -33,17 +39,21 @@ def from_public_point(klass, point, curve=NIST192p, hashfunc=sha1): def from_string(klass, string, curve=NIST192p, hashfunc=sha1, validate_point=True): order = curve.order - assert len(string) == curve.verifying_key_length, \ - (len(string), curve.verifying_key_length) + if len(string) != curve.verifying_key_length: + raise MalformedPointError( + "Malformed encoding of public point. Expected string {0} bytes" + " long, received {1} bytes long string".format( + curve.verifying_key_length, len(string))) xs = string[:curve.baselen] ys = string[curve.baselen:] - assert len(xs) == curve.baselen, (len(xs), curve.baselen) - assert len(ys) == curve.baselen, (len(ys), 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") x = string_to_number(xs) y = string_to_number(ys) - if validate_point: - assert ecdsa.point_is_valid(curve.generator, x, y) - from . import ellipticcurve + if validate_point and not ecdsa.point_is_valid(curve.generator, x, y): + raise MalformedPointError("Point does not lie on the curve") point = ellipticcurve.Point(curve.curve, x, y, order) return klass.from_public_point(point, curve, hashfunc) @@ -65,13 +75,18 @@ def from_der(klass, string): if empty != b(""): raise der.UnexpectedDER("trailing junk after DER pubkey objects: %s" % binascii.hexlify(empty)) - assert oid_pk == oid_ecPublicKey, (oid_pk, oid_ecPublicKey) + if oid_pk != oid_ecPublicKey: + raise der.UnexpectedDER( + "Unexpected OID in encoding, received {0}, expected {1}" + .format(oid_pk, oid_ecPublicKey)) curve = find_curve(oid_curve) point_str, empty = der.remove_bitstring(point_str_bitstring) if empty != b(""): raise der.UnexpectedDER("trailing junk after pubkey pointstring: %s" % binascii.hexlify(empty)) - assert point_str.startswith(b("\x00\x04")) + if not point_str.startswith(b("\x00\x04")): + raise der.UnexpectedDER( + "Unsupported or invalid encoding of pubcli key") return klass.from_string(point_str[2:], curve) def to_string(self): @@ -137,7 +152,10 @@ def from_secret_exponent(klass, secexp, curve=NIST192p, hashfunc=sha1): self.default_hashfunc = hashfunc self.baselen = curve.baselen n = curve.order - assert 1 <= secexp < n + if not 1 <= secexp < n: + raise MalformedPointError( + "Invalid value for secexp, expected integer between 1 and {0}" + .format(n)) pubkey_point = curve.generator*secexp pubkey = ecdsa.Public_key(curve.generator, pubkey_point) pubkey.order = n @@ -149,7 +167,10 @@ def from_secret_exponent(klass, secexp, curve=NIST192p, hashfunc=sha1): @classmethod def from_string(klass, string, curve=NIST192p, hashfunc=sha1): - assert len(string) == curve.baselen, (len(string), curve.baselen) + if len(string) != curve.baselen: + raise MalformedPointError( + "Invalid length of private key, received {0}, expected {1}" + .format(len(string), curve.baselen)) secexp = string_to_number(string) return klass.from_secret_exponent(secexp, curve, hashfunc) diff --git a/ecdsa/test_pyecdsa.py b/ecdsa/test_pyecdsa.py index 326cac4a..c750f5a3 100644 --- a/ecdsa/test_pyecdsa.py +++ b/ecdsa/test_pyecdsa.py @@ -1,6 +1,9 @@ from __future__ import with_statement, division -import unittest +try: + import unittest2 as unittest +except ImportError: + import unittest import os import time import shutil @@ -10,15 +13,17 @@ from .six import b, print_, binary_type from .keys import SigningKey, VerifyingKey -from .keys import BadSignatureError +from .keys import BadSignatureError, MalformedPointError, BadDigestError from . import util from .util import sigencode_der, sigencode_strings from .util import sigdecode_der, sigdecode_strings +from .util import encoded_oid_ecPublicKey, MalformedSignature from .curves import Curve, UnknownCurveError from .curves import NIST192p, NIST224p, NIST256p, NIST384p, NIST521p, SECP256k1 from .ellipticcurve import Point from . import der from . import rfc6979 +from . import ecdsa class SubprocessError(Exception): pass @@ -258,6 +263,47 @@ def order(self): return 123456789 pub2 = VerifyingKey.from_pem(pem) self.assertTruePubkeysEqual(pub1, pub2) + def test_vk_from_der_garbage_after_curve_oid(self): + type_oid_der = encoded_oid_ecPublicKey + curve_oid_der = der.encode_oid(*(1, 2, 840, 10045, 3, 1, 1)) + \ + b('garbage') + enc_type_der = der.encode_sequence(type_oid_der, curve_oid_der) + point_der = der.encode_bitstring(b'\x00\xff') + to_decode = der.encode_sequence(enc_type_der, point_der) + + with self.assertRaises(der.UnexpectedDER): + VerifyingKey.from_der(to_decode) + + def test_vk_from_der_invalid_key_type(self): + type_oid_der = der.encode_oid(*(1, 2, 3)) + curve_oid_der = der.encode_oid(*(1, 2, 840, 10045, 3, 1, 1)) + enc_type_der = der.encode_sequence(type_oid_der, curve_oid_der) + point_der = der.encode_bitstring(b'\x00\xff') + to_decode = der.encode_sequence(enc_type_der, point_der) + + with self.assertRaises(der.UnexpectedDER): + VerifyingKey.from_der(to_decode) + + def test_vk_from_der_garbage_after_point_string(self): + type_oid_der = encoded_oid_ecPublicKey + curve_oid_der = der.encode_oid(*(1, 2, 840, 10045, 3, 1, 1)) + enc_type_der = der.encode_sequence(type_oid_der, curve_oid_der) + point_der = der.encode_bitstring(b'\x00\xff') + b('garbage') + to_decode = der.encode_sequence(enc_type_der, point_der) + + with self.assertRaises(der.UnexpectedDER): + VerifyingKey.from_der(to_decode) + + def test_vk_from_der_invalid_bitstring(self): + type_oid_der = encoded_oid_ecPublicKey + curve_oid_der = der.encode_oid(*(1, 2, 840, 10045, 3, 1, 1)) + enc_type_der = der.encode_sequence(type_oid_der, curve_oid_der) + point_der = der.encode_bitstring(b'\x08\xff') + to_decode = der.encode_sequence(enc_type_der, point_der) + + with self.assertRaises(der.UnexpectedDER): + VerifyingKey.from_der(to_decode) + def test_signature_strings(self): priv1 = SigningKey.generate() pub1 = priv1.get_verifying_key() @@ -281,6 +327,86 @@ def test_signature_strings(self): self.assertEqual(type(sig_der), binary_type) self.assertTrue(pub1.verify(sig_der, data, sigdecode=sigdecode_der)) + def test_sig_decode_strings_with_invalid_count(self): + with self.assertRaises(MalformedSignature): + sigdecode_strings([b('one'), b('two'), b('three')], 0xff) + + def test_sig_decode_strings_with_wrong_r_len(self): + with self.assertRaises(MalformedSignature): + sigdecode_strings([b('one'), b('two')], 0xff) + + def test_sig_decode_strings_with_wrong_s_len(self): + with self.assertRaises(MalformedSignature): + sigdecode_strings([b('\xa0'), b('\xb0\xff')], 0xff) + + def test_verify_with_too_long_input(self): + sk = SigningKey.generate() + vk = sk.verifying_key + + with self.assertRaises(BadDigestError): + vk.verify_digest(None, b('\x00') * 128) + + def test_sk_from_secret_exponent_with_wrong_sec_exponent(self): + with self.assertRaises(MalformedPointError): + SigningKey.from_secret_exponent(0) + + def test_sk_from_string_with_wrong_len_string(self): + with self.assertRaises(MalformedPointError): + SigningKey.from_string(b('\x01')) + + def test_sk_from_der_with_junk_after_sequence(self): + ver_der = der.encode_integer(1) + to_decode = der.encode_sequence(ver_der) + b('garbage') + + with self.assertRaises(der.UnexpectedDER): + SigningKey.from_der(to_decode) + + def test_sk_from_der_with_wrong_version(self): + ver_der = der.encode_integer(0) + to_decode = der.encode_sequence(ver_der) + + with self.assertRaises(der.UnexpectedDER): + SigningKey.from_der(to_decode) + + def test_sk_from_der_invalid_const_tag(self): + ver_der = der.encode_integer(1) + privkey_der = der.encode_octet_string(b('\x00\xff')) + curve_oid_der = der.encode_oid(*(1, 2, 3)) + const_der = der.encode_constructed(1, curve_oid_der) + to_decode = der.encode_sequence(ver_der, privkey_der, const_der, + curve_oid_der) + + with self.assertRaises(der.UnexpectedDER): + SigningKey.from_der(to_decode) + + def test_sk_from_der_garbage_after_privkey_oid(self): + ver_der = der.encode_integer(1) + privkey_der = der.encode_octet_string(b('\x00\xff')) + curve_oid_der = der.encode_oid(*(1, 2, 3)) + b('garbage') + const_der = der.encode_constructed(0, curve_oid_der) + to_decode = der.encode_sequence(ver_der, privkey_der, const_der, + curve_oid_der) + + with self.assertRaises(der.UnexpectedDER): + SigningKey.from_der(to_decode) + + def test_sk_from_der_with_short_privkey(self): + ver_der = der.encode_integer(1) + privkey_der = der.encode_octet_string(b('\x00\xff')) + curve_oid_der = der.encode_oid(*(1, 2, 840, 10045, 3, 1, 1)) + const_der = der.encode_constructed(0, curve_oid_der) + to_decode = der.encode_sequence(ver_der, privkey_der, const_der, + curve_oid_der) + + sk = SigningKey.from_der(to_decode) + self.assertEqual(sk.privkey.secret_multiplier, 255) + + def test_sign_with_too_long_hash(self): + sk = SigningKey.from_secret_exponent(12) + + with self.assertRaises(BadDigestError): + sk.sign_digest(b('\xff') * 64) + def test_hashfunc(self): sk = SigningKey.generate(curve=NIST256p, hashfunc=sha256) data = b("security level is 128 bits") @@ -299,6 +425,49 @@ def test_hashfunc(self): curve=NIST256p) self.assertTrue(vk3.verify(sig, data, hashfunc=sha256)) + def test_decoding_with_malformed_uncompressed(self): + enc = b('\x0c\xe0\x1d\xe0d\x1c\x8eS\x8a\xc0\x9eK\xa8x !\xd5\xc2\xc3' + '\xfd\xc8\xa0c\xff\xfb\x02\xb9\xc4\x84)\x1a\x0f\x8b\x87\xa4' + 'z\x8a#\xb5\x97\xecO\xb6\xa0HQ\x89*') + + with self.assertRaises(MalformedPointError): + VerifyingKey.from_string(b('\x02') + enc) + + def test_decoding_with_point_not_on_curve(self): + enc = b('\x0c\xe0\x1d\xe0d\x1c\x8eS\x8a\xc0\x9eK\xa8x !\xd5\xc2\xc3' + '\xfd\xc8\xa0c\xff\xfb\x02\xb9\xc4\x84)\x1a\x0f\x8b\x87\xa4' + 'z\x8a#\xb5\x97\xecO\xb6\xa0HQ\x89*') + + with self.assertRaises(MalformedPointError): + VerifyingKey.from_string(enc[:47] + b('\x00')) + + def test_decoding_with_point_at_infinity(self): + # decoding it is unsupported, as it's not necessary to encode it + with self.assertRaises(MalformedPointError): + VerifyingKey.from_string(b('\x00')) + + def test_from_string_with_invalid_curve_too_short_ver_key_len(self): + # both verifying_key_length and baselen are calculated internally + # by the Curve constructor, but since we depend on them verify + # that inconsistent values are detected + curve = Curve("test", ecdsa.curve_192, ecdsa.generator_192, (1, 2)) + curve.verifying_key_length = 16 + curve.baselen = 32 + + with self.assertRaises(MalformedPointError): + VerifyingKey.from_string(b('\x00')*16, curve) + + def test_from_string_with_invalid_curve_too_long_ver_key_len(self): + # both verifying_key_length and baselen are calculated internally + # by the Curve constructor, but since we depend on them verify + # that inconsistent values are detected + curve = Curve("test", ecdsa.curve_192, ecdsa.generator_192, (1, 2)) + curve.verifying_key_length = 16 + curve.baselen = 16 + + with self.assertRaises(MalformedPointError): + VerifyingKey.from_string(b('\x00')*16, curve) + class OpenSSL(unittest.TestCase): # test interoperability with OpenSSL tools. Note that openssl's ECDSA From b95be03d8540b3a088263cbb3a0a376a8a0efbd0 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Fri, 4 Oct 2019 21:31:19 +0200 Subject: [PATCH 09/10] execute also new tests in Travis not a backport, necessary to make the tests runnable on 0.13 branch --- .coveragerc | 2 ++ .travis.yml | 6 ++++++ build-requirements-3.2.txt | 1 + build-requirements-3.3.txt | 1 + build-requirements.txt | 3 +++ ecdsa/test_der.py | 2 +- ecdsa/test_malformed_sigs.py | 2 +- tox.ini | 18 ++++++++++++++++++ 8 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tox.ini diff --git a/.coveragerc b/.coveragerc index 71240e0a..23c667f9 100644 --- a/.coveragerc +++ b/.coveragerc @@ -7,3 +7,5 @@ omit = ecdsa/six.py ecdsa/_version.py ecdsa/test_ecdsa.py + ecdsa/test_der.py + ecdsa/test_malformed_sigs.py diff --git a/.travis.yml b/.travis.yml index 5e7a67aa..6319f719 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,11 @@ # workaround for 3.7 not available in default configuration # travis-ci/travis-ci#9815 dist: trusty +sudo: false language: python +cache: pip +before_cache: + - rm -f $HOME/.cache/pip/log/debug.log python: - "2.6" - "2.7" @@ -14,6 +18,8 @@ install: - if [[ -e build-requirements-${TRAVIS_PYTHON_VERSION}.txt ]]; then travis_retry pip install -r build-requirements-${TRAVIS_PYTHON_VERSION}.txt; else travis_retry pip install -r build-requirements.txt; fi script: - coverage run setup.py test + - if [[ $TRAVIS_PYTHON_VERSION != 3.2 && ! $TRAVIS_PYTHON_VERSION =~ py ]]; then tox -e py${TRAVIS_PYTHON_VERSION/./}; fi + - if [[ $TRAVIS_PYTHON_VERSION =~ py ]]; then tox -e $TRAVIS_PYTHON_VERSION; fi - python setup.py speed after_success: - coveralls diff --git a/build-requirements-3.2.txt b/build-requirements-3.2.txt index 3b599b55..4995efa0 100644 --- a/build-requirements-3.2.txt +++ b/build-requirements-3.2.txt @@ -7,3 +7,4 @@ PyYAML<3.13 tox<3.0 wheel<0.30 virtualenv==15.2.0 +pytest>2.7.3 diff --git a/build-requirements-3.3.txt b/build-requirements-3.3.txt index de3e0b74..e4b5e775 100644 --- a/build-requirements-3.3.txt +++ b/build-requirements-3.3.txt @@ -2,3 +2,4 @@ python-coveralls tox<3.0 wheel<0.30 virtualenv==15.2.0 +pluggy<0.6 diff --git a/build-requirements.txt b/build-requirements.txt index ae10411c..6c9f5447 100644 --- a/build-requirements.txt +++ b/build-requirements.txt @@ -1 +1,4 @@ python-coveralls +pytest>3.0.7 +pytest-cov<2.7.0 +tox diff --git a/ecdsa/test_der.py b/ecdsa/test_der.py index a20a617c..c0c555e6 100644 --- a/ecdsa/test_der.py +++ b/ecdsa/test_der.py @@ -6,7 +6,7 @@ except ImportError: import unittest from .der import remove_integer, UnexpectedDER, read_length -from six import b +from .six import b class TestRemoveInteger(unittest.TestCase): # DER requires the integers to be 0-padded only if they would be diff --git a/ecdsa/test_malformed_sigs.py b/ecdsa/test_malformed_sigs.py index fffb1b3b..30eee12b 100644 --- a/ecdsa/test_malformed_sigs.py +++ b/ecdsa/test_malformed_sigs.py @@ -3,7 +3,7 @@ import pytest import hashlib -from six import b, binary_type +from .six import b, binary_type from .keys import SigningKey, VerifyingKey from .keys import BadSignatureError from .util import sigencode_der, sigencode_string diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..db7754bf --- /dev/null +++ b/tox.ini @@ -0,0 +1,18 @@ +[tox] +envlist = py26, py27, py33, py34, py35, py36, py37, pypy, pypy3 + + +[testenv] +deps = + py{33}: py<1.5 + py{33}: pytest<3.3 + py{26}: unittest2 + py{26,27,34,35,36,37,py,py3}: pytest + py{33}: wheel<0.30 + coverage + +commands = coverage run --branch -m pytest + +[testenv:coverage] +sitepackages=True +commands = coverage run --branch -m pytest From 1eb2c0410b97ac5101b5db20e2924d79db3e8ec5 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Sat, 5 Oct 2019 14:51:08 +0200 Subject: [PATCH 10/10] update README with error handling of from_string() and from_der() proposed for master in #132 --- README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d7cdcc92..d807793c 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ There are four test suites, three for the original Pearson module, and one more for the wrapper. To run them all, do this: python setup.py test + tox -e coverage On my 2014 Mac Mini, the combined tests take about 20 seconds to run. On a 2.4GHz P4 Linux box, they take 81 seconds. @@ -118,7 +119,8 @@ is to call `s=sk.to_string()`, and then re-create it with `SigningKey.from_string(s, curve)` . This short form does not record the curve, so you must be sure to tell from_string() the same curve you used for the original key. The short form of a NIST192p-based signing key is just 24 -bytes long. +bytes long. If the point encoding is invalid or it does not lie on the +specified curve, `from_string()` will raise MalformedPointError. from ecdsa import SigningKey, NIST384p sk = SigningKey.generate(curve=NIST384p) @@ -132,7 +134,8 @@ formats that OpenSSL uses. The PEM file looks like the familiar ASCII-armored is a shorter binary form of the same data. `SigningKey.from_pem()/.from_der()` will undo this serialization. These formats include the curve name, so you do not need to pass in a curve -identifier to the deserializer. +identifier to the deserializer. In case the file is malformed `from_der()` +and `from_pem()` will raise UnexpectedDER or MalformedPointError. from ecdsa import SigningKey, NIST384p sk = SigningKey.generate(curve=NIST384p)