Merge pull request #161 from mark-adams/158-fix-ecdsa

RE: Fix the ECDSA signature serialization format

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

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)
 

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

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)

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())

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())

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))
 

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