move where we gc dsa data, improve dsa key checking

2 files changed, 29 insertions, 23 deletions

diff --git a/cryptography/hazmat/backends/openssl/backend.py b/cryptography/hazmat/backends/openssl/backend.py
index e895da83..6245e8e4 100644
--- a/cryptography/hazmat/backends/openssl/backend.py
+++ b/cryptography/hazmat/backends/openssl/backend.py
@@ -628,20 +628,19 @@ class Backend(object):
 
     def create_dsa_signature_ctx(self, private_key, algorithm):
         dsa_cdata = self._dsa_cdata_from_private_key(private_key)
-        dsa_cdata = self._ffi.gc(dsa_cdata, self._lib.DSA_free)
         key = _DSAPrivateKey(self, dsa_cdata)
         return _DSASignatureContext(self, key, algorithm)
 
     def create_dsa_verification_ctx(self, public_key, signature,
                                     algorithm):
         dsa_cdata = self._dsa_cdata_from_public_key(public_key)
-        dsa_cdata = self._ffi.gc(dsa_cdata, self._lib.DSA_free)
         key = _DSAPublicKey(self, dsa_cdata)
         return _DSAVerificationContext(self, key, signature, algorithm)
 
     def load_dsa_private_numbers(self, numbers):
+        dsa._check_dsa_private_numbers(numbers)
         parameter_numbers = numbers.public_numbers.parameter_numbers
-        dsa._check_dsa_parameters(parameter_numbers)
+
         dsa_cdata = self._lib.DSA_new()
         assert dsa_cdata != self._ffi.NULL
         dsa_cdata = self._ffi.gc(dsa_cdata, self._lib.DSA_free)
@@ -656,7 +655,6 @@ class Backend(object):
 
     def load_dsa_public_numbers(self, numbers):
         dsa._check_dsa_parameters(numbers.parameter_numbers)
-        # TODO check more
         dsa_cdata = self._lib.DSA_new()
         assert dsa_cdata != self._ffi.NULL
         dsa_cdata = self._ffi.gc(dsa_cdata, self._lib.DSA_free)
@@ -670,7 +668,6 @@ class Backend(object):
 
     def load_dsa_parameter_numbers(self, numbers):
         dsa._check_dsa_parameters(numbers)
-        # TODO check more
         dsa_cdata = self._lib.DSA_new()
         assert dsa_cdata != self._ffi.NULL
         dsa_cdata = self._ffi.gc(dsa_cdata, self._lib.DSA_free)
@@ -682,10 +679,9 @@ class Backend(object):
         return _DSAParameters(self, dsa_cdata)
 
     def _dsa_cdata_from_public_key(self, public_key):
-        # Does not GC the DSA cdata. You *must* make sure it's freed
-        # correctly yourself!
         ctx = self._lib.DSA_new()
         assert ctx != self._ffi.NULL
+        ctx = self._ffi.gc(ctx, self._lib.DSA_free)
         parameters = public_key.parameters()
         ctx.p = self._int_to_bn(parameters.p)
         ctx.q = self._int_to_bn(parameters.q)
@@ -694,10 +690,9 @@ class Backend(object):
         return ctx
 
     def _dsa_cdata_from_private_key(self, private_key):
-        # Does not GC the DSA cdata. You *must* make sure it's freed
-        # correctly yourself!
         ctx = self._lib.DSA_new()
         assert ctx != self._ffi.NULL
+        ctx = self._ffi.gc(ctx, self._lib.DSA_free)
         parameters = private_key.parameters()
         ctx.p = self._int_to_bn(parameters.p)
         ctx.q = self._int_to_bn(parameters.q)
diff --git a/cryptography/hazmat/primitives/asymmetric/dsa.py b/cryptography/hazmat/primitives/asymmetric/dsa.py
index 4675bd0a..527b6bbc 100644
--- a/cryptography/hazmat/primitives/asymmetric/dsa.py
+++ b/cryptography/hazmat/primitives/asymmetric/dsa.py
@@ -39,8 +39,18 @@ def _check_dsa_parameters(parameters):
                          "one of these pairs (1024, 160) or (2048, 256) "
                          "or (3072, 256).")
 
-    if parameters.g <= 1 or parameters.g >= parameters.p:
-        raise ValueError("g must be > 1 and < p.")
+    if not (1 < parameters.g < parameters.p):
+        raise ValueError("g, p don't satisfy 1 < g < p.")
+
+
+def _check_dsa_private_numbers(numbers):
+    parameters = numbers.public_numbers.parameter_numbers
+    _check_dsa_parameters(parameters)
+    if numbers.x <= 0 or numbers.x >= parameters.q:
+        raise ValueError("x must be > 0 and < q.")
+
+    if numbers.public_numbers.y != pow(parameters.g, numbers.x, parameters.p):
+        raise ValueError("y must be equal to (g ** x % p).")
 
 
 @utils.register_interface(interfaces.DSAParameters)
@@ -102,24 +112,25 @@ class DSAParameters(object):
 @utils.register_interface(interfaces.DSAPrivateKey)
 class DSAPrivateKey(object):
     def __init__(self, modulus, subgroup_order, generator, x, y):
-        _check_dsa_parameters(
-            DSAParameterNumbers(
-                p=modulus,
-                q=subgroup_order,
-                g=generator
-            )
-        )
         if (
             not isinstance(x, six.integer_types) or
             not isinstance(y, six.integer_types)
         ):
             raise TypeError("DSAPrivateKey arguments must be integers.")
 
-        if x <= 0 or x >= subgroup_order:
-            raise ValueError("x must be > 0 and < subgroup_order.")
-
-        if y != pow(generator, x, modulus):
-            raise ValueError("y must be equal to (generator ** x % modulus).")
+        _check_dsa_private_numbers(
+            DSAPrivateNumbers(
+                public_numbers=DSAPublicNumbers(
+                    parameter_numbers=DSAParameterNumbers(
+                        p=modulus,
+                        q=subgroup_order,
+                        g=generator
+                    ),
+                    y=y
+                ),
+                x=x
+            )
+        )
 
         self._modulus = modulus
         self._subgroup_order = subgroup_order