From c09fd581772ebe9d30bcf6b24665b4b75175029b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 18 Apr 2014 22:00:10 -0400 Subject: Switch to an explicit curve object. Happily, this eliminates just about all of the error cases. --- OpenSSL/SSL.py | 66 +++------------------------- OpenSSL/crypto.py | 69 ++++++++++++++++++++++++++++++ OpenSSL/test/test_crypto.py | 81 ++++++++++++++++++++++++++++++++++- OpenSSL/test/test_ssl.py | 102 +++----------------------------------------- doc/api/crypto.rst | 22 ++++++++++ doc/api/ssl.rst | 27 +++--------- leakcheck/crypto.py | 18 +++++++- 7 files changed, 203 insertions(+), 182 deletions(-) diff --git a/OpenSSL/SSL.py b/OpenSSL/SSL.py index e5ae085..c5b233b 100644 --- a/OpenSSL/SSL.py +++ b/OpenSSL/SSL.py @@ -124,31 +124,6 @@ SSL_CB_CONNECT_EXIT = _lib.SSL_CB_CONNECT_EXIT SSL_CB_HANDSHAKE_START = _lib.SSL_CB_HANDSHAKE_START SSL_CB_HANDSHAKE_DONE = _lib.SSL_CB_HANDSHAKE_DONE -def _get_elliptic_curves(lib): - """ - Load the names of the supported elliptic curves from OpenSSL. - - :param lib: The OpenSSL library binding object. - :return: A set of :py:obj:`unicode` giving the names of the elliptic curves - the underlying library supports. - """ - if lib.Cryptography_HAS_EC: - num_curves = lib.EC_get_builtin_curves(_ffi.NULL, 0) - builtin_curves = _ffi.new('EC_builtin_curve[]', num_curves) - # The return value on this call should be num_curves again. We could - # check it to make sure but if it *isn't* then.. what could we do? - # Abort the whole process, I suppose...? -exarkun - lib.EC_get_builtin_curves(builtin_curves, num_curves) - return set( - _ffi.string(lib.OBJ_nid2sn(c.nid)).decode("ascii") - for c in builtin_curves) - else: - return set() - -ELLIPTIC_CURVES = _get_elliptic_curves() - - - class Error(Exception): """ An error occurred in an `OpenSSL.SSL` API. @@ -653,48 +628,17 @@ class Context(object): _lib.SSL_CTX_set_tmp_dh(self._context, dh) - def _set_tmp_ecdh_curve_by_nid(self, name, nid): - """ - Select a curve to use by the OpenSSL NID associated with that curve. - - :param name: The name of the curve identified by the NID. - :type name: str - - :param nid: The OpenSSL NID to use. - :type nid: int - - :raise UnsupportedEllipticCurve: If the given NID does not identify a - supported curve. - """ - ecdh = _lib.EC_KEY_new_by_curve_name(nid) - if ecdh == _ffi.NULL: - raise UnsupportedEllipticCurve(name) - _lib.SSL_CTX_set_tmp_ecdh(self._context, ecdh) - _lib.EC_KEY_free(ecdh) - - - def set_tmp_ecdh_curve(self, curve_name): + def set_tmp_ecdh_curve(self, curve): """ Select a curve to use for ECDHE key exchange. - The valid values of *curve_name* are the keys in - :py:data:OpenSSL.SSL.ELLIPTIC_CURVE_DESCRIPTIONS. - - Raises a subclass of ``ValueError`` if the linked OpenSSL was - not compiled with elliptical curve support or the specified - curve is not available. You can check the specific subclass, - but, in general, you should just handle ``ValueError``. + :param curve: A curve object to use as returned by either + :py:meth:`OpenSSL.crypto.get_elliptic_curve` or + :py:meth:`OpenSSL.crypto.get_elliptic_curves`. - :param curve_name: The 'short name' of a curve, e.g. 'prime256v1' - :type curve_name: str :return: None """ - if _lib.Cryptography_HAS_EC: - nid = _lib.OBJ_sn2nid(curve_name.encode('ascii')) - if nid == _lib.NID_undef: - raise UnknownObject(curve_name) - return self._set_tmp_ecdh_curve_by_nid(curve_name, nid) - raise ECNotAvailable() + _lib.SSL_CTX_set_tmp_ecdh(self._context, curve._to_EC_KEY()) def set_cipher_list(self, cipher_list): diff --git a/OpenSSL/crypto.py b/OpenSSL/crypto.py index 65e28d7..948402c 100644 --- a/OpenSSL/crypto.py +++ b/OpenSSL/crypto.py @@ -263,6 +263,75 @@ PKeyType = PKey +class _EllipticCurve(object): + """ + :ivar _lib: The :py:mod:`cryptography` binding instance used to interface + with OpenSSL. + + :ivar _nid: The OpenSSL NID identifying the curve this object represents. + :type _nid: :py:class:`int` + + :ivar name: The OpenSSL short name identifying the curve this object + represents. + :type name: :py:class:`unicode` + """ + @classmethod + def _get_elliptic_curves(cls, lib): + """ + Load the names of the supported elliptic curves from OpenSSL. + + :param lib: The OpenSSL library binding object. + :return: A set of :py:obj:`unicode` giving the names of the elliptic curves + the underlying library supports. + """ + if lib.Cryptography_HAS_EC: + num_curves = lib.EC_get_builtin_curves(_ffi.NULL, 0) + builtin_curves = _ffi.new('EC_builtin_curve[]', num_curves) + # The return value on this call should be num_curves again. We could + # check it to make sure but if it *isn't* then.. what could we do? + # Abort the whole process, I suppose...? -exarkun + lib.EC_get_builtin_curves(builtin_curves, num_curves) + return set( + cls.from_nid(lib, c.nid) + for c in builtin_curves) + else: + return set() + + + @classmethod + def from_nid(cls, lib, nid): + return cls(lib, nid, _ffi.string(lib.OBJ_nid2sn(nid)).decode("ascii")) + + + def __init__(self, lib, nid, name): + self._lib = lib + self._nid = nid + self.name = name + + + def __repr__(self): + return "" % (self.name,) + + + def _to_EC_KEY(self): + key = self._lib.EC_KEY_new_by_curve_name(self._nid) + return _ffi.gc(key, _lib.EC_KEY_free) + + + +def get_elliptic_curves(): + return _EllipticCurve._get_elliptic_curves(_lib) + + + +def get_elliptic_curve(name): + for curve in get_elliptic_curves(): + if curve.name == name: + return curve + raise ValueError("unknown curve name", name) + + + class X509Name(object): def __init__(self, name): """ diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py index a3685a9..b288985 100644 --- a/OpenSSL/test/test_crypto.py +++ b/OpenSSL/test/test_crypto.py @@ -25,9 +25,10 @@ from OpenSSL.crypto import PKCS7Type, load_pkcs7_data from OpenSSL.crypto import PKCS12, PKCS12Type, load_pkcs12 from OpenSSL.crypto import CRL, Revoked, load_crl from OpenSSL.crypto import NetscapeSPKI, NetscapeSPKIType -from OpenSSL.crypto import sign, verify +from OpenSSL.crypto import ( + sign, verify, get_elliptic_curve, get_elliptic_curves) from OpenSSL.test.util import TestCase, b -from OpenSSL._util import native +from OpenSSL._util import native, lib def normalize_certificate_pem(pem): return dump_certificate(FILETYPE_PEM, load_certificate(FILETYPE_PEM, pem)) @@ -3058,5 +3059,81 @@ class SignVerifyTests(TestCase): verify(good_cert, sig, content, "sha1") + +class EllipticCurveTests(TestCase): + """ + Tests for :py:class:`_EllipticCurve`, :py:obj:`get_elliptic_curve`, and + :py:obj:`get_elliptic_curves`. + """ + def test_set(self): + """ + :py:obj:`get_elliptic_curves` returns a :py:obj:`set`. + """ + self.assertIsInstance(get_elliptic_curves(), set) + + + def test_some_curves(self): + """ + If :py:mod:`cryptography` has elliptic curve support then the set + returned by :py:obj:`get_elliptic_curves` has some elliptic curves in + it. + + There could be an OpenSSL that violates this assumption. If so, this + test will fail and we'll find out. + """ + curves = get_elliptic_curves() + if lib.Cryptography_HAS_EC: + self.assertTrue(curves) + else: + self.assertFalse(curves) + + + def test_a_curve(self): + """ + :py:obj:`get_elliptic_curve` can be used to retrieve a particular + supported curve. + """ + curves = get_elliptic_curves() + if curves: + curve = next(iter(curves)) + self.assertEqual(curve.name, get_elliptic_curve(curve.name).name) + else: + self.assertRaises(ValueError, get_elliptic_curve, u"prime256v1") + + + def test_not_a_curve(self): + """ + :py:obj:`get_elliptic_curve` raises :py:class:`ValueError` if called + with a name which does not identify a supported curve. + """ + self.assertRaises( + ValueError, get_elliptic_curve, u"this curve was just invented") + + + def test_repr(self): + """ + The string representation of a curve object includes simply states the + object is a curve and what its name is. + """ + curves = get_elliptic_curves() + if curves: + curve = next(iter(curves)) + self.assertEqual("" % (curve.name,), repr(curve)) + + + def test_to_EC_KEY(self): + """ + The curve object can export a version of itself as an EC_KEY* via the + private :py:meth:`_EllipticCurve._to_EC_KEY`. + """ + curves = get_elliptic_curves() + if curves: + curve = next(iter(curves)) + # It's not easy to assert anything about this object. However, see + # leakcheck/crypto.py for a test that demonstrates it at least does + # not leak memory. + curve._to_EC_KEY() + + if __name__ == '__main__': main() diff --git a/OpenSSL/test/test_ssl.py b/OpenSSL/test/test_ssl.py index fdeff9d..6231143 100644 --- a/OpenSSL/test/test_ssl.py +++ b/OpenSSL/test/test_ssl.py @@ -20,6 +20,7 @@ from OpenSSL.crypto import TYPE_RSA, FILETYPE_PEM from OpenSSL.crypto import PKey, X509, X509Extension, X509Store from OpenSSL.crypto import dump_privatekey, load_privatekey from OpenSSL.crypto import dump_certificate, load_certificate +from OpenSSL.crypto import get_elliptic_curves from OpenSSL.SSL import _lib from OpenSSL.SSL import OPENSSL_VERSION_NUMBER, SSLEAY_VERSION, SSLEAY_CFLAGS @@ -36,9 +37,6 @@ from OpenSSL.SSL import ( SESS_CACHE_OFF, SESS_CACHE_CLIENT, SESS_CACHE_SERVER, SESS_CACHE_BOTH, SESS_CACHE_NO_AUTO_CLEAR, SESS_CACHE_NO_INTERNAL_LOOKUP, SESS_CACHE_NO_INTERNAL_STORE, SESS_CACHE_NO_INTERNAL) -from OpenSSL.SSL import ( - _Cryptography_HAS_EC, ELLIPTIC_CURVE_DESCRIPTIONS, - ECNotAvailable, UnknownObject, UnsupportedEllipticCurve) from OpenSSL.SSL import ( Error, SysCallError, WantReadError, WantWriteError, ZeroReturnError) @@ -1179,101 +1177,13 @@ class ContextTests(TestCase, _LoopbackMixin): def test_set_tmp_ecdh_curve(self): """ :py:obj:`Context.set_tmp_ecdh_curve` sets the elliptic curve for - Diffie-Hellman to the specified named curve. - """ - context = Context(TLSv1_METHOD) - for curve in ELLIPTIC_CURVE_DESCRIPTIONS.keys(): - context.set_tmp_ecdh_curve(curve) # Must not throw. - - - def test_set_tmp_ecdh_curve_not_available(self): - """ - :py:obj:`Context.set_tmp_ecdh_curve` raises :py:obj:`ECNotAvailable` if - elliptic curve support is not available from the underlying OpenSSL - version at all. - """ - has_ec = _lib.Cryptography_HAS_EC - try: - _lib.Cryptography_HAS_EC = False - - context = Context(TLSv1_METHOD) - self.assertRaises( - ECNotAvailable, - context.set_tmp_ecdh_curve, next(iter(ELLIPTIC_CURVE_DESCRIPTIONS))) - finally: - _lib.Cryptography_HAS_EC = has_ec - - - def test_set_tmp_ecdh_curve_bad_curve_name(self): - """ - :py:obj:`Context.set_tmp_ecdh_curve` raises :py:obj:`UnknownObject` if - passed a curve_name that OpenSSL does not recognize and EC is - available. - """ - context = Context(TLSv1_METHOD) - try: - context.set_tmp_ecdh_curve('not_an_elliptic_curve') - except ECNotAvailable: - self.assertFalse(_Cryptography_HAS_EC) - except UnknownObject: - self.assertTrue(_Cryptography_HAS_EC) - else: - self.fail( - "set_tmp_ecdh_curve did not fail when called with " - "non-existent curve name") - - - def test_set_tmp_ecdh_curve_bad_nid(self): - """ - :py:obj:`Context._set_tmp_ecdh_curve_by_nid`, an implementation detail - of :py:obj:`Context.set_tmp_ecdh_curve`, raises - :py:obj:`UnsupportedEllipticCurve` raises if passed a NID that does not - identify a supported curve. - """ - context = Context(TLSv1_METHOD) - try: - context._set_tmp_ecdh_curve_by_nid( - u("curve"), _lib.OBJ_sn2nid(b"sha256")) - except UnsupportedEllipticCurve: - pass - else: - self.fail( - "_set_tmp_ecdh_curve_by_nid did not raise " - "UnsupportedEllipticCurve for a NID that does not " - "identify a supported curve.") - - - def test_set_tmp_ecdh_curve_not_a_curve(self): - """ - :py:obj:`Context.set_tmp_ecdh_curve` raises - :py:obj:`UnsupportedEllipticCurve` if passed a curve_name that OpenSSL - cannot instantiate as an elliptic curve. It raises - :py:obj:`ECNotAvailable` if EC is not available at all. + Diffie-Hellman to the specified curve. """ context = Context(TLSv1_METHOD) - try: - context.set_tmp_ecdh_curve('sha256') - except ECNotAvailable: - self.assertFalse(_Cryptography_HAS_EC) - except UnknownObject: - self.assertTrue(_Cryptography_HAS_EC) - else: - self.fail( - "set_tmp_ecdh_curve did not fail when called with " - "a name that is not an elliptic curve") - - - def test_has_curve_descriptions(self): - """ - If the underlying cryptography bindings claim to have elliptic - curve support, there should be at least one curve. - - (In theory there could be an OpenSSL that violates this - assumption. If so, this test will fail and we'll find out.) - - """ - if _Cryptography_HAS_EC: - self.assertNotEqual(len(ELLIPTIC_CURVE_DESCRIPTIONS), 0) + for curve in get_elliptic_curves(): + # The only easily "assertable" thing is that it does not raise an + # exception. + context.set_tmp_ecdh_curve(curve) def test_set_cipher_list_bytes(self): diff --git a/doc/api/crypto.rst b/doc/api/crypto.rst index ee93cfb..974e516 100644 --- a/doc/api/crypto.rst +++ b/doc/api/crypto.rst @@ -119,6 +119,28 @@ Generic exception used in the :py:mod:`.crypto` module. +.. py:function:: get_elliptic_curves + + Return a set of objects representing the elliptic curves supported in the + OpenSSL build in use. + + The curve objects have a :py:class:`unicode` ``name`` attribute by which + they identifying themselves. + + The curve objects are useful as values for the argument accepted by + :py:meth:`Context.set_tmp_ecdh_curve` to specify which elliptical curve + should be used for ECDHE key exchange. + + +.. py:function:: get_elliptic_curve + + Return a single curve object selected by name. + + See :py:func:`get_elliptic_curves` for information about curve objects. + + If the named curve is not supported then :py:class:`ValueError` is raised. + + .. py:function:: dump_certificate(type, cert) Dump the certificate *cert* into a buffer string encoded with the type diff --git a/doc/api/ssl.rst b/doc/api/ssl.rst index b7eca70..9016e98 100644 --- a/doc/api/ssl.rst +++ b/doc/api/ssl.rst @@ -110,17 +110,6 @@ Context, Connection. .. versionadded:: 0.14 -.. py:data:: ELLIPTIC_CURVE_DESCRIPTIONS - - A dictionary mapping short names of elliptic curves to textual - descriptions. This dictionary contains exactly the set of curves - supported by the OpenSSL build in use. - - The keys are the curve names that can be passed into - Constants used with :py:meth:`Context.set_tmp_ecdh_curve` to - specify which elliptical curve should be used for ECDHE key exchange. - - .. py:data:: OPENSSL_VERSION_NUMBER An integer giving the version number of the OpenSSL library used to build this @@ -327,21 +316,15 @@ Context objects have the following methods: Load parameters for Ephemeral Diffie-Hellman from *dhfile*. -.. py:method:: Context.set_tmp_ecdh_curve(curve_name) - Select a curve to use for ECDHE key exchange. +.. py:method:: Context.set_tmp_ecdh_curve(curve) - The valid values of *curve_name* are the keys in - :py:data:`ELLIPTIC_CURVE_DESCRIPTIONS`. + Select a curve to use for ECDHE key exchange. - Raises a subclass of ``ValueError`` if the linked OpenSSL was not - compiled with elliptical curve support or the specified curve is - not available. You can check the specific subclass, but, in - general, you should just handle ``ValueError``. + The valid values of *curve* are the objects returned by + :py:func:`OpenSSL.crypto.get_elliptic_curves` or + :py:func:`OpenSSL.crypto.get_elliptic_curve`. - :param curve_name: The 'short name' of a curve, e.g. 'prime256v1' - :type curve_name: str - :return: None .. py:method:: Context.set_app_data(data) diff --git a/leakcheck/crypto.py b/leakcheck/crypto.py index f5fe2f8..ca79b7c 100644 --- a/leakcheck/crypto.py +++ b/leakcheck/crypto.py @@ -5,7 +5,7 @@ import sys from OpenSSL.crypto import ( FILETYPE_PEM, TYPE_DSA, Error, PKey, X509, load_privatekey, CRL, Revoked, - _X509_REVOKED_dup) + get_elliptic_curves, _X509_REVOKED_dup) from OpenSSL._util import lib as _lib @@ -145,6 +145,22 @@ class Checker_X509_REVOKED_dup(BaseChecker): +class Checker_EllipticCurve(BaseChecker): + """ + Leak checks for :py:obj:`_EllipticCurve`. + """ + def check_to_EC_KEY(self): + """ + Repeatedly create an EC_KEY* from an :py:obj:`_EllipticCurve`. The + structure should be automatically garbage collected. + """ + curves = get_elliptic_curves() + if curves: + curve = next(iter(curves)) + for i in xrange(self.iterations * 1000): + curve._to_EC_KEY() + + def vmsize(): return [x for x in file('/proc/self/status').readlines() if 'VmSize' in x] -- cgit v1.2.1