Merge pull request #109 from mark-adams/algo-validation-fixes

Added some fixes related to algorithm and key choice
jpadilla · Mar 18, 2015 · 88a9fc5 · 88a9fc5
2 parents 1e6b6c5 + f6d0ff2
commit 88a9fc5
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 2 deletions.
diff --git a/jwt/algorithms.py b/jwt/algorithms.py
@@ -2,6 +2,7 @@
 import hmac
 
 from .compat import constant_time_compare, string_types, text_type
+from .exceptions import InvalidKeyError
 
 try:
     from cryptography.hazmat.primitives import interfaces, hashes
@@ -68,7 +69,13 @@ class NoneAlgorithm(Algorithm):
     operations are required.
     """
     def prepare_key(self, key):
-        return None
+        if key == '':
+            key = None
+
+        if key is not None:
+            raise InvalidKeyError('When alg = "none", key value must be None.')
+
+        return key
 
     def sign(self, msg, key):
         return b''
@@ -96,6 +103,17 @@ def prepare_key(self, key):
         if isinstance(key, text_type):
             key = key.encode('utf-8')
 
+        invalid_strings = [
+            b'-----BEGIN PUBLIC KEY-----',
+            b'-----BEGIN CERTIFICATE-----',
+            b'ssh-rsa'
+        ]
+
+        if any([string_value in key for string_value in invalid_strings]):
+            raise InvalidKeyError(
+                'The specified key is an assymetric key or x509 certificate and'
+                ' should not be used as an HMAC secret.')
+
         return key
 
     def sign(self, msg, key):

diff --git a/jwt/exceptions.py b/jwt/exceptions.py
@@ -18,6 +18,10 @@ class InvalidIssuerError(InvalidTokenError):
     pass
 
 
+class InvalidKeyError(Exception):
+    pass
+
+
 # Compatibility aliases (deprecated)
 ExpiredSignature = ExpiredSignatureError
 InvalidAudience = InvalidAudienceError

diff --git a/tests/keys/testkey_rsa.cer b/tests/keys/testkey_rsa.cer
@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDhTCCAm2gAwIBAgIJANE4sir3EkX8MA0GCSqGSIb3DQEBCwUAMFkxCzAJBgNV
+BAYTAlVTMQ4wDAYDVQQIDAVUZXhhczEPMA0GA1UEBwwGQXVzdGluMQ4wDAYDVQQK
+DAVQeUpXVDEZMBcGA1UECwwQVGVzdCBDZXJ0aWZpY2F0ZTAeFw0xNTAzMTgwMTE2
+MTRaFw0xODAzMTcwMTE2MTRaMFkxCzAJBgNVBAYTAlVTMQ4wDAYDVQQIDAVUZXhh
+czEPMA0GA1UEBwwGQXVzdGluMQ4wDAYDVQQKDAVQeUpXVDEZMBcGA1UECwwQVGVz
+dCBDZXJ0aWZpY2F0ZTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANR4
+MwXyb9nDo0K8gsHvDRHpa4jkzRVimVIr3r1K0YZanJmSXQr7giUa/sQjfjpjvKsI
+CSUffH3jbo8VYPifS7N/1DgOB3BfZ2B+mqlVxCwBPB5PwC78YveprNQw7gL0BmmG
+fpQDcZb8XkBTmUm45M//ZofGi3hisKiS6d6fjoVAUKcLwFAD4PNvjlLYE1t50pY4
+3ha9eAfKgJ3hknP8JdJ4vvtUkWVFxUqL83KkDpJWt1tu66y36w+i14I/07A7OLw9
+T5yJtc3FXpyk+032CNe27Bvzv1nnMM9jZdfaS+4A6LDa7hd6ICVjatS8p/4oz0J5
+Dy6WR8ob7osnGHCNw4kCAwEAAaNQME4wHQYDVR0OBBYEFDR6fVdFxZED6YMmD62W
+LlBW+qEBMB8GA1UdIwQYMBaAFDR6fVdFxZED6YMmD62WLlBW+qEBMAwGA1UdEwQF
+MAMBAf8wDQYJKoZIhvcNAQELBQADggEBAFwDNwm+lU/kGfWwiWM0Lv2aosXotoiG
+TsBSWIn2iYphq0vzlgChcNocN9zkaOz3zc9pcREP6lyqHpE0OEbNucHHDdU1L2he
+lLFOLOmkpP5fyPDXs9nKYhO8ygMByEonHm3K/VvCgrsSgJ3JuxMLUxnE55jQXGWV
+OqYQNo2J5h93Zd2HTTe19jCz+bbWnRBP5VvLAAAo5YSmk3iroWSPWAKkWOOecJ2Q
+/xnRyuWERsfvZiF/m9q7yDJ55LXVVm3Rufmy76SoTnJ2acap+XQNXBH/AxayeLUS
+OYmHWH61dUcsQtwXYHYRB8TTtMIwUCXGmthXkDJydEfrGcD0y6APIh8=
+-----END CERTIFICATE-----
diff --git a/tests/test_algorithms.py b/tests/test_algorithms.py
@@ -1,6 +1,7 @@
 import base64
 
-from jwt.algorithms import Algorithm, HMACAlgorithm
+from jwt.algorithms import Algorithm, HMACAlgorithm, NoneAlgorithm
+from jwt.exceptions import InvalidKeyError
 
 from .compat import unittest
 from .utils import ensure_bytes, ensure_unicode, key_path
@@ -35,6 +36,12 @@ def test_algorithm_should_throw_exception_if_verify_not_impl(self):
         with self.assertRaises(NotImplementedError):
             algo.verify('message', 'key', 'signature')
 
+    def test_none_algorithm_should_throw_exception_if_key_is_not_none(self):
+        algo = NoneAlgorithm()
+
+        with self.assertRaises(InvalidKeyError):
+            algo.prepare_key('123')
+
     def test_hmac_should_reject_nonstring_key(self):
         algo = HMACAlgorithm(HMACAlgorithm.SHA256)
 
@@ -49,6 +56,38 @@ def test_hmac_should_accept_unicode_key(self):
 
         algo.prepare_key(ensure_unicode('awesome'))
 
+    @unittest.skipIf(not has_crypto, 'Not supported without cryptography library')
+    def test_hmac_should_throw_exception_if_key_is_pem_public_key(self):
+        algo = HMACAlgorithm(HMACAlgorithm.SHA256)
+
+        with self.assertRaises(InvalidKeyError):
+            with open(key_path('testkey2_rsa.pub.pem'), 'r') as keyfile:
+                algo.prepare_key(keyfile.read())
+
+    @unittest.skipIf(not has_crypto, 'Not supported without cryptography library')
+    def test_hmac_should_throw_exception_if_key_is_x509_certificate(self):
+        algo = HMACAlgorithm(HMACAlgorithm.SHA256)
+
+        with self.assertRaises(InvalidKeyError):
+            with open(key_path('testkey_rsa.cer'), 'r') as keyfile:
+                algo.prepare_key(keyfile.read())
+
+    @unittest.skipIf(not has_crypto, 'Not supported without cryptography library')
+    def test_hmac_should_throw_exception_if_key_is_ssh_public_key(self):
+        algo = HMACAlgorithm(HMACAlgorithm.SHA256)
+
+        with self.assertRaises(InvalidKeyError):
+            with open(key_path('testkey_rsa.pub'), 'r') as keyfile:
+                algo.prepare_key(keyfile.read())
+
+    @unittest.skipIf(not has_crypto, 'Not supported without cryptography library')
+    def test_hmac_should_throw_exception_if_key_is_x509_cert(self):
+        algo = HMACAlgorithm(HMACAlgorithm.SHA256)
+
+        with self.assertRaises(InvalidKeyError):
+            with open(key_path('testkey2_rsa.pub.pem'), 'r') as keyfile:
+                algo.prepare_key(keyfile.read())
+
     @unittest.skipIf(not has_crypto, 'Not supported without cryptography library')
     def test_rsa_should_parse_pem_public_key(self):
         algo = RSAAlgorithm(RSAAlgorithm.SHA256)