Remove custom PrivateKey exponents/coefficient (#71)

Thanks for the improvements!

4 files changed, 73 insertions, 33 deletions

diff --git a/rsa/key.py b/rsa/key.py
index a3a3b65..8170916 100644
--- a/rsa/key.py
+++ b/rsa/key.py
@@ -34,6 +34,7 @@ of pyasn1.
 """
 
 import logging
+import warnings
 
 from rsa._compat import range
 import rsa.prime
@@ -42,6 +43,7 @@ import rsa.common
 import rsa.randnum
 import rsa.core
 
+
 log = logging.getLogger(__name__)
 DEFAULT_EXPONENT = 65537
 
@@ -354,51 +356,30 @@ class PrivateKey(AbstractKey):
     >>> PrivateKey(3247, 65537, 833, 191, 17)
     PrivateKey(3247, 65537, 833, 191, 17)
 
-    exp1, exp2 and coef can be given, but if None or omitted they will be calculated:
+    exp1, exp2 and coef will be calculated:
 
-    >>> pk = PrivateKey(3727264081, 65537, 3349121513, 65063, 57287, exp2=4)
+    >>> pk = PrivateKey(3727264081, 65537, 3349121513, 65063, 57287)
     >>> pk.exp1
     55063
-    >>> pk.exp2  # this is of course not a correct value, but it is the one we passed.
-    4
-    >>> pk.coef
-    50797
-
-    If you give exp1, exp2 or coef, they will be used as-is:
-
-    >>> pk = PrivateKey(1, 2, 3, 4, 5, 6, 7, 8)
-    >>> pk.exp1
-    6
     >>> pk.exp2
-    7
+    10095
     >>> pk.coef
-    8
+    50797
 
     """
 
     __slots__ = ('n', 'e', 'd', 'p', 'q', 'exp1', 'exp2', 'coef')
 
-    def __init__(self, n, e, d, p, q, exp1=None, exp2=None, coef=None):
+    def __init__(self, n, e, d, p, q):
         AbstractKey.__init__(self, n, e)
         self.d = d
         self.p = p
         self.q = q
 
-        # Calculate the other values if they aren't supplied
-        if exp1 is None:
-            self.exp1 = int(d % (p - 1))
-        else:
-            self.exp1 = exp1
-
-        if exp2 is None:
-            self.exp2 = int(d % (q - 1))
-        else:
-            self.exp2 = exp2
-
-        if coef is None:
-            self.coef = rsa.common.inverse(q, p)
-        else:
-            self.coef = coef
+        # Calculate exponents and coefficient.
+        self.exp1 = int(d % (p - 1))
+        self.exp2 = int(d % (q - 1))
+        self.coef = rsa.common.inverse(q, p)
 
     def __getitem__(self, key):
         return getattr(self, key)
@@ -510,8 +491,20 @@ class PrivateKey(AbstractKey):
         if priv[0] != 0:
             raise ValueError('Unable to read this file, version %s != 0' % priv[0])
 
-        as_ints = tuple(int(x) for x in priv[1:9])
-        return cls(*as_ints)
+        as_ints = tuple(map(int, priv[1:6]))
+        key = cls(*as_ints)
+
+        exp1, exp2, coef = map(int, priv[6:9])
+
+        if (key.exp1, key.exp2, key.coef) != (exp1, exp2, coef):
+            warnings.warn(
+                'You have provided a malformed keyfile. Either the exponents '
+                'or the coefficient are incorrect. Using the correct values '
+                'instead.',
+                UserWarning,
+            )
+
+        return key
 
     def _save_pkcs1_der(self):
         """Saves the private key in PKCS#1 DER format.
diff --git a/tests/test_key.py b/tests/test_key.py
index afefa3a..9db30ce 100644
--- a/tests/test_key.py
+++ b/tests/test_key.py
@@ -41,6 +41,13 @@ class KeyGenTest(unittest.TestCase):
         self.assertEqual(0x10001, priv.e)
         self.assertEqual(0x10001, pub.e)
 
+    def test_exponents_coefficient_calculation(self):
+        pk = rsa.key.PrivateKey(3727264081, 65537, 3349121513, 65063, 57287)
+
+        self.assertEqual(pk.exp1, 55063)
+        self.assertEqual(pk.exp2, 10095)
+        self.assertEqual(pk.coef, 50797)
+
     def test_custom_getprime_func(self):
         # List of primes to test with, in order [p, q, p, q, ....]
         # By starting with two of the same primes, we test that this is
diff --git a/tests/test_load_save_keys.py b/tests/test_load_save_keys.py
index ef6584b..55bd5a4 100644
--- a/tests/test_load_save_keys.py
+++ b/tests/test_load_save_keys.py
@@ -17,10 +17,13 @@
 """Unittest for saving and loading keys."""
 
 import base64
-import unittest
+import mock
 import os.path
 import pickle
+import unittest
+import warnings
 
+from rsa._compat import range
 import rsa.key
 
 B64PRIV_DER = b'MC4CAQACBQDeKYlRAgMBAAECBQDHn4npAgMA/icCAwDfxwIDANcXAgInbwIDAMZt'
@@ -80,6 +83,39 @@ class DerTest(unittest.TestCase):
         expected = rsa.key.PrivateKey(3727264081, 65537, 3349121513, 65063, 57287)
 
         self.assertEqual(expected, key)
+        self.assertEqual(key.exp1, 55063)
+        self.assertEqual(key.exp2, 10095)
+        self.assertEqual(key.coef, 50797)
+
+    @mock.patch('pyasn1.codec.der.decoder.decode')
+    def test_load_malformed_private_key(self, der_decode):
+        """Test loading malformed private DER keys."""
+
+        # Decode returns an invalid exp2 value.
+        der_decode.return_value = (
+            [0, 3727264081, 65537, 3349121513, 65063, 57287, 55063, 0, 50797],
+            0,
+        )
+
+        with warnings.catch_warnings(record=True) as w:
+            # Always print warnings
+            warnings.simplefilter('always')
+
+            # Load 3 keys
+            for _ in range(3):
+                key = rsa.key.PrivateKey.load_pkcs1(PRIVATE_DER, 'DER')
+
+            # Check that 3 warnings were generated.
+            self.assertEqual(3, len(w))
+
+            for warning in w:
+                self.assertTrue(issubclass(warning.category, UserWarning))
+                self.assertIn('malformed', str(warning.message))
+
+        # Check that we are creating the key with correct values
+        self.assertEqual(key.exp1, 55063)
+        self.assertEqual(key.exp2, 10095)
+        self.assertEqual(key.coef, 50797)
 
     def test_save_private_key(self):
         """Test saving private DER keys."""
@@ -118,6 +154,9 @@ class PemTest(unittest.TestCase):
         expected = rsa.key.PrivateKey(3727264081, 65537, 3349121513, 65063, 57287)
 
         self.assertEqual(expected, key)
+        self.assertEqual(key.exp1, 55063)
+        self.assertEqual(key.exp2, 10095)
+        self.assertEqual(key.coef, 50797)
 
     def test_save_private_key(self):
         """Test saving private PEM files."""
diff --git a/tox.ini b/tox.ini
index be38566..ae49b27 100644
--- a/tox.ini
+++ b/tox.ini
@@ -12,6 +12,7 @@ deps=pyasn1 >=0.1.3
      PyTest
      pytest-xdist
      pytest-cov
+     mock
 
 [testenv:py35]
 commands=py.test --doctest-modules rsa tests