diff --git a/CHANGELOG.md b/CHANGELOG.md index bb0d1316..d0000895 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). [Unreleased][unreleased] ------------------------------------------------------------------------- +### Fixed +- ECDSA (ES256, ES384, ES512) signatures are now being properly serialized [#158][158] + ### Added - Added a new `jwt.get_unverified_header()` to parse and return the header portion of a token prior to signature verification. @@ -78,3 +81,4 @@ rarely used. Users affected by this should upgrade to 3.3+. [132]: https://github.com/jpadilla/pyjwt/pull/132 [128]: https://github.com/jpadilla/pyjwt/pull/128 [141]: https://github.com/jpadilla/pyjwt/pull/141 +[158: https://github.com/jpadilla/pyjwt/pull/158 diff --git a/jwt/algorithms.py b/jwt/algorithms.py index 05fd1944..d4033149 100644 --- a/jwt/algorithms.py +++ b/jwt/algorithms.py @@ -3,6 +3,7 @@ from .compat import constant_time_compare, string_types, text_type from .exceptions import InvalidKeyError +from .utils import der_to_raw_signature, raw_to_der_signature try: from cryptography.hazmat.primitives import hashes @@ -233,10 +234,17 @@ def sign(self, msg, key): signer = key.signer(ec.ECDSA(self.hash_alg())) signer.update(msg) - return signer.finalize() + der_sig = signer.finalize() + + return der_to_raw_signature(der_sig, key.curve) def verify(self, msg, key, sig): - verifier = key.verifier(sig, ec.ECDSA(self.hash_alg())) + try: + der_sig = raw_to_der_signature(sig, key.curve) + except ValueError: + return False + + verifier = key.verifier(der_sig, ec.ECDSA(self.hash_alg())) verifier.update(msg) diff --git a/jwt/contrib/algorithms/py_ecdsa.py b/jwt/contrib/algorithms/py_ecdsa.py index dea2710d..bf0dea5a 100644 --- a/jwt/contrib/algorithms/py_ecdsa.py +++ b/jwt/contrib/algorithms/py_ecdsa.py @@ -50,11 +50,11 @@ def prepare_key(self, key): def sign(self, msg, key): return key.sign(msg, hashfunc=self.hash_alg, - sigencode=ecdsa.util.sigencode_der) + sigencode=ecdsa.util.sigencode_string) def verify(self, msg, key, sig): try: return key.verify(sig, msg, hashfunc=self.hash_alg, - sigdecode=ecdsa.util.sigdecode_der) - except ecdsa.der.UnexpectedDER: + sigdecode=ecdsa.util.sigdecode_string) + except AssertionError: return False diff --git a/jwt/utils.py b/jwt/utils.py index bb7b3a3b..637b8929 100644 --- a/jwt/utils.py +++ b/jwt/utils.py @@ -1,4 +1,12 @@ import base64 +import binascii + +try: + from cryptography.hazmat.primitives.asymmetric.utils import ( + decode_rfc6979_signature, encode_rfc6979_signature + ) +except ImportError: + pass def base64url_decode(input): @@ -25,3 +33,35 @@ def merge_dict(original, updates): raise TypeError('original and updates must be a dictionary: %s' % e) return merged_options + + +def number_to_bytes(num, num_bytes): + padded_hex = '%0*x' % (2 * num_bytes, num) + big_endian = binascii.a2b_hex(padded_hex.encode('ascii')) + return big_endian + + +def bytes_to_number(string): + return int(binascii.b2a_hex(string), 16) + + +def der_to_raw_signature(der_sig, curve): + num_bits = curve.key_size + num_bytes = (num_bits + 7) // 8 + + r, s = decode_rfc6979_signature(der_sig) + + return number_to_bytes(r, num_bytes) + number_to_bytes(s, num_bytes) + + +def raw_to_der_signature(raw_sig, curve): + num_bits = curve.key_size + num_bytes = (num_bits + 7) // 8 + + if len(raw_sig) != 2 * num_bytes: + raise ValueError('Invalid signature') + + r = bytes_to_number(raw_sig[:num_bytes]) + s = bytes_to_number(raw_sig[num_bytes:]) + + return encode_rfc6979_signature(r, s) diff --git a/tests/contrib/test_algorithms.py b/tests/contrib/test_algorithms.py index d59ccc36..1fbe10d1 100644 --- a/tests/contrib/test_algorithms.py +++ b/tests/contrib/test_algorithms.py @@ -130,10 +130,9 @@ def test_ec_sign_should_generate_correct_signature_value(self): jwt_message = ensure_bytes('Hello World!') expected_sig = base64.b64decode(ensure_bytes( - 'MIGIAkIB9vYz+inBL8aOTA4auYz/zVuig7TT1bQgKROIQX9YpViHkFa4DT5' - '5FuFKn9XzVlk90p6ldEj42DC9YecXHbC2t+cCQgCicY+8f3f/KCNtWK7cif' - '6vdsVwm6Lrjs0Ag6ZqCf+olN11hVt1qKBC4lXppqB1gNWEmNQaiz1z2QRyc' - 'zJ8hSJmbw==')) + 'AC+m4Jf/xI3guAC6w0w37t5zRpSCF6F4udEz5LiMiTIjCS4vcVe6dDOxK+M' + 'mvkF8PxJuvqxP2CO3TR3okDPCl/NjATTO1jE+qBZ966CRQSSzcCM+tzcHzw' + 'LZS5kbvKu0Acd/K6Ol2/W3B1NeV5F/gjvZn/jOwaLgWEUYsg0o4XVrAg65')) with open(key_path('testkey_ec'), 'r') as keyfile: jwt_key = algo.prepare_key(keyfile.read()) @@ -151,10 +150,9 @@ def test_ec_verify_should_return_false_if_signature_invalid(self): jwt_message = ensure_bytes('Hello World!') jwt_sig = base64.b64decode(ensure_bytes( - 'MIGIAkIB9vYz+inBL8aOTA4auYz/zVuig7TT1bQgKROIQX9YpViHkFa4DT5' - '5FuFKn9XzVlk90p6ldEj42DC9YecXHbC2t+cCQgCicY+8f3f/KCNtWK7cif' - '6vdsVwm6Lrjs0Ag6ZqCf+olN11hVt1qKBC4lXppqB1gNWEmNQaiz1z2QRyc' - 'zJ8hSJmbw==')) + 'AC+m4Jf/xI3guAC6w0w37t5zRpSCF6F4udEz5LiMiTIjCS4vcVe6dDOxK+M' + 'mvkF8PxJuvqxP2CO3TR3okDPCl/NjATTO1jE+qBZ966CRQSSzcCM+tzcHzw' + 'LZS5kbvKu0Acd/K6Ol2/W3B1NeV5F/gjvZn/jOwaLgWEUYsg0o4XVrAg65')) jwt_sig += ensure_bytes('123') # Signature is now invalid @@ -170,10 +168,9 @@ def test_ec_verify_should_return_true_if_signature_valid(self): jwt_message = ensure_bytes('Hello World!') jwt_sig = base64.b64decode(ensure_bytes( - 'MIGIAkIB9vYz+inBL8aOTA4auYz/zVuig7TT1bQgKROIQX9YpViHkFa4DT5' - '5FuFKn9XzVlk90p6ldEj42DC9YecXHbC2t+cCQgCicY+8f3f/KCNtWK7cif' - '6vdsVwm6Lrjs0Ag6ZqCf+olN11hVt1qKBC4lXppqB1gNWEmNQaiz1z2QRyc' - 'zJ8hSJmbw==')) + 'AC+m4Jf/xI3guAC6w0w37t5zRpSCF6F4udEz5LiMiTIjCS4vcVe6dDOxK+M' + 'mvkF8PxJuvqxP2CO3TR3okDPCl/NjATTO1jE+qBZ966CRQSSzcCM+tzcHzw' + 'LZS5kbvKu0Acd/K6Ol2/W3B1NeV5F/gjvZn/jOwaLgWEUYsg0o4XVrAg65')) with open(key_path('testkey_ec.pub'), 'r') as keyfile: jwt_pub_key = algo.prepare_key(keyfile.read()) diff --git a/tests/test_algorithms.py b/tests/test_algorithms.py index 710a82eb..6260fa06 100644 --- a/tests/test_algorithms.py +++ b/tests/test_algorithms.py @@ -173,10 +173,23 @@ def test_ec_verify_should_return_false_if_signature_invalid(self): # Mess up the signature by replacing a known byte sig = base64.b64decode(ensure_bytes( - 'MIGIAkIB9vYz+inBL8aOTA4auYz/zVuig7TT1bQgKROIQX9YpViHkFa4DT5' - '5FuFKn9XzVlk90p6ldEj42DC9YecXHbC2t+cCQgCicY+8f3f/KCNtWK7cif' - '6vdsVwm6Lrjs0Ag6ZqCf+olN11hVt1qKBC4lXppqB1gNWEmNQaiz1z2QRyc' - 'zJ8hSJmbw=='.replace('r', 's'))) + 'AC+m4Jf/xI3guAC6w0w37t5zRpSCF6F4udEz5LiMiTIjCS4vcVe6dDOxK+M' + 'mvkF8PxJuvqxP2CO3TR3okDPCl/NjATTO1jE+qBZ966CRQSSzcCM+tzcHzw' + 'LZS5kbvKu0Acd/K6Ol2/W3B1NeV5F/gjvZn/jOwaLgWEUYsg0o4XVrAg65'.replace('r', 's'))) + + with open(key_path('testkey_ec.pub'), 'r') as keyfile: + pub_key = algo.prepare_key(keyfile.read()) + + result = algo.verify(message, pub_key, sig) + assert not result + + @pytest.mark.skipif(not has_crypto, reason='Not supported without cryptography library') + def test_ec_verify_should_return_false_if_signature_wrong_length(self): + algo = ECAlgorithm(ECAlgorithm.SHA256) + + message = ensure_bytes('Hello World!') + + sig = base64.b64decode(ensure_bytes('AC+m4Jf/xI3guAC6w0w3')) with open(key_path('testkey_ec.pub'), 'r') as keyfile: pub_key = algo.prepare_key(keyfile.read()) @@ -191,10 +204,9 @@ def test_ec_verify_should_return_true_if_signature_valid(self): message = ensure_bytes('Hello World!') sig = base64.b64decode(ensure_bytes( - 'MIGIAkIB9vYz+inBL8aOTA4auYz/zVuig7TT1bQgKROIQX9YpViHkFa4DT5' - '5FuFKn9XzVlk90p6ldEj42DC9YecXHbC2t+cCQgCicY+8f3f/KCNtWK7cif' - '6vdsVwm6Lrjs0Ag6ZqCf+olN11hVt1qKBC4lXppqB1gNWEmNQaiz1z2QRyc' - 'zJ8hSJmbw==')) + 'AC+m4Jf/xI3guAC6w0w37t5zRpSCF6F4udEz5LiMiTIjCS4vcVe6dDOxK+M' + 'mvkF8PxJuvqxP2CO3TR3okDPCl/NjATTO1jE+qBZ966CRQSSzcCM+tzcHzw' + 'LZS5kbvKu0Acd/K6Ol2/W3B1NeV5F/gjvZn/jOwaLgWEUYsg0o4XVrAg65')) with open(key_path('testkey_ec.pub'), 'r') as keyfile: pub_key = algo.prepare_key(keyfile.read()) diff --git a/tests/test_api_jws.py b/tests/test_api_jws.py index 3e89cc97..9395ae86 100644 --- a/tests/test_api_jws.py +++ b/tests/test_api_jws.py @@ -186,11 +186,10 @@ def test_decodes_valid_es384_jws(self, jws): example_jws = ( b'eyJhbGciOiJFUzM4NCIsInR5cCI6IkpXVCJ9' b'.eyJoZWxsbyI6IndvcmxkIn0' - b'.MIGHAkEdh2kR7IRu5w0tGuY6Xz3Vqa7PHHY2DgXWeee' - b'LXotEqpn9udp2NfVL-XFG0TDoCakzXbIGAWg42S69GFl' - b'KZzxhXAJCAPLPuJoKyAixFnXPBkvkti-UzSIj4s6DePe' - b'uTu7102G_QIXiijY5bx6mdmZa3xUuKeu-zobOIOqR8Zw' - b'FqGjBLZum') + b'.AGtlemKghaIaYh1yeeekFH9fRuNY7hCaw5hUgZ5aG1N' + b'2F8FIbiKLaZKr8SiFdTimXFVTEmxpBQ9sRmdsDsnrM-1' + b'HAG0_zxxu0JyINOFT2iqF3URYl9HZ8kZWMeZAtXmn6Cw' + b'PXRJD2f7N-f7bJ5JeL9VT5beI2XD3FlK3GgRvI-eE-2Ik') decoded_payload = jws.decode(example_jws, example_pubkey) json_payload = json.loads(ensure_unicode(decoded_payload)) diff --git a/tests/test_api_jwt.py b/tests/test_api_jwt.py index 4361d6e7..7d33a9e6 100644 --- a/tests/test_api_jwt.py +++ b/tests/test_api_jwt.py @@ -190,11 +190,10 @@ def test_decodes_valid_es384_jwt(self, jwt): example_jwt = ( b'eyJhbGciOiJFUzM4NCIsInR5cCI6IkpXVCJ9' b'.eyJoZWxsbyI6IndvcmxkIn0' - b'.MIGHAkEdh2kR7IRu5w0tGuY6Xz3Vqa7PHHY2DgXWeee' - b'LXotEqpn9udp2NfVL-XFG0TDoCakzXbIGAWg42S69GFl' - b'KZzxhXAJCAPLPuJoKyAixFnXPBkvkti-UzSIj4s6DePe' - b'uTu7102G_QIXiijY5bx6mdmZa3xUuKeu-zobOIOqR8Zw' - b'FqGjBLZum') + b'.AddMgkmRhzqptDYqlmy_f2dzM6O9YZmVo-txs_CeAJD' + b'NoD8LN7YiPeLmtIhkO5_VZeHHKvtQcGc4lsq-Y72c4dK' + b'pANr1f6HEYhjpBc03u_bv06PYMcr5N2-9k97-qf-JCSb' + b'zqW6R250Q7gNCX5R7NrCl7MTM4DTBZkGbUlqsFUleiGlj') decoded_payload = jwt.decode(example_jwt, example_pubkey) assert decoded_payload == example_payload