classification
Title: Memory leak in X509StoreContext class.
Type: resource usage Stage: resolved
Components: Versions: Python 3.5
process
Status: closed Resolution: third party
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, timboddy
Priority: normal Keywords:

Created on 2018-06-07 12:27 by timboddy, last changed 2018-06-07 22:05 by timboddy. This issue is now closed.

Messages (7)
msg318927 - (view) Author: Tim Boddy (timboddy) Date: 2018-06-07 12:27
I noticed a memory leak /usr/lib/python3.5/site-packages/OpenSSL/crypto.py in the definition of the class X509StoreContext. The problem is that the __init__ function calls self._init() then later the function verify_certificate calls _init() again. In spite of the disclaimer int __init__ about "no adverse effect", the adverse effect here is that if one does two calls to X509_STORE_CTX_init on the same X509_STORE_CTX without any intervening calls to X509_STORE_CTX_cleanup on that same X509_STORE_CTX it will leak one X509_VERIFY_PARAM and one X509_VERIFY_PARAM_ID.

Here is most of the relevant class:

class X509StoreContext(object):
    """
    An X.509 store context.

    An X.509 store context is used to carry out the actual verification process
    of a certificate in a described context. For describing such a context, see
    :class:`X509Store`.

    :ivar _store_ctx: The underlying X509_STORE_CTX structure used by this
        instance. It is dynamically allocated and automatically garbage
        collected.
    :ivar _store: See the ``store`` ``__init__`` parameter.
    :ivar _cert: See the ``certificate`` ``__init__`` parameter.
    :param X509Store store: The certificates which will be trusted for the
        purposes of any verifications.
    :param X509 certificate: The certificate to be verified.
    """

    def __init__(self, store, certificate):
        store_ctx = _lib.X509_STORE_CTX_new()
        self._store_ctx = _ffi.gc(store_ctx, _lib.X509_STORE_CTX_free)
        self._store = store
        self._cert = certificate
        # Make the store context available for use after instantiating this
        # class by initializing it now. Per testing, subsequent calls to
        # :meth:`_init` have no adverse affect.
        self._init()

    def _init(self):
        """
        Set up the store context for a subsequent verification operation.
        """
        Set up the store context for a subsequent verification operation.
        """
        ret = _lib.X509_STORE_CTX_init(
            self._store_ctx, self._store._store, self._cert._x509, _ffi.NULL
        )
        if ret <= 0:
            _raise_current_error()

    def _cleanup(self):
        """
        Internally cleans up the store context.

        The store context can then be reused with a new call to :meth:`_init`.
        """
        _lib.X509_STORE_CTX_cleanup(self._store_ctx)

    def _exception_from_context(self):
        """
        Convert an OpenSSL native context error failure into a Python
        exception.

        When a call to native OpenSSL X509_verify_cert fails, additional
        information about the failure can be obtained from the store context.
        """
        errors = [
            _lib.X509_STORE_CTX_get_error(self._store_ctx),
            _lib.X509_STORE_CTX_get_error_depth(self._store_ctx),
            _native(_ffi.string(_lib.X509_verify_cert_error_string(
                _lib.X509_STORE_CTX_get_error(self._store_ctx)))),
        ]
        # A context error should always be associated with a certificate, so we
        # expect this call to never return :class:`None`.
        _x509 = _lib.X509_STORE_CTX_get_current_cert(self._store_ctx)
        _cert = _lib.X509_dup(_x509)
        pycert = X509.__new__(X509)
        pycert._x509 = _ffi.gc(_cert, _lib.X509_free)
        return X509StoreContextError(errors, pycert)

    def set_store(self, store):
        """
        Set the context's X.509 store.

        .. versionadded:: 0.15

        :param X509Store store: The store description which will be used for
            the purposes of any *future* verifications.
        """
        self._store = store

    def verify_certificate(self):
        """
        Verify a certificate in a context.

        .. versionadded:: 0.15

        :raises X509StoreContextError: If an error occurred when validating a
          certificate in the context. Sets ``certificate`` attribute to
          indicate which certificate caused the error.
        """
        # Always re-initialize the store context in case
        # :meth:`verify_certificate` is called multiple times.
        self._init()
        ret = _lib.X509_verify_cert(self._store_ctx)
        self._cleanup()
        if ret <= 0:
            raise self._exception_from_context()
msg318928 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-06-07 12:48
Thank you for your report, but OpenSSL/crypto.py is not part of the standard library. I think the correct place to report this is https://github.com/pyca/pyopenssl

There is a X509StoreContext class at  https://github.com/pyca/pyopenssl/blob/179eb1d0917ddc1067d056127e08e952206e0e91/src/OpenSSL/crypto.py#L1696
msg318933 - (view) Author: Tim Boddy (timboddy) Date: 2018-06-07 14:12
Thank you for helping me figure out the correct place to file this.  Is there a quick way for me to evaluate in the future wither a particular file belongs to the standard library?
msg318944 - (view) Author: Tim Boddy (timboddy) Date: 2018-06-07 14:39
It looks as if the issue has been fixed here:

https://github.com/pyca/pyopenssl/blob/179eb1d0917ddc1067d056127e08e952206e0e91/src/OpenSSL/crypto.py#L1790

Thanks again for pointing me to the correct place!

I'm sorry that I accidentally change the status from closed when I added a comment.
msg318945 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-06-07 14:42
> Is there a quick way for me to evaluate in the future wither a
> particular file belongs to the standard library?

If a Python file or package is located in the site-packages/ directory, it's not part of the standard library.
msg318972 - (view) Author: Tim Boddy (timboddy) Date: 2018-06-07 21:15
Would a leak associated with this stack trace fall within the domain of bugs.python.org?  I do see site-packages on ths stack in frames 1 and 2 but frame 3 is in /lib/libpython3.5m.so.1.0:

555555904900
#0  __GI___libc_malloc (bytes=8) at malloc.c:2910
#1  0x00007ffff4b1a6a9 in ?? ()
   from /usr/lib/python3.5/site-packages/_cffi_backend.cpython-35m-x86_64-linux-gnu.so
#2  0x00007ffff4b1ab4d in ?? ()
   from /usr/lib/python3.5/site-packages/_cffi_backend.cpython-35m-x86_64-linux-gnu.so
#3  0x00007ffff7a28019 in ?? () from /lib/libpython3.5m.so.1.0
#4  0x00007ffff79bb229 in PyCFunction_Call () from /lib/libpython3.5m.so.1.0
#5  0x00007ffff7a3474c in PyEval_EvalFrameEx () from /lib/libpython3.5m.so.1.0
#6  0x00007ffff7a33d5c in PyEval_EvalFrameEx () from /lib/libpython3.5m.so.1.0
#7  0x00007ffff7a33d5c in PyEval_EvalFrameEx () from /lib/libpython3.5m.so.1.0

Either way is fine.  I have a bit more debugging to do anyway to understand where the cause of the leak actually is but please let me know your thoughts on whether this bug might be relevant to this site.
msg318975 - (view) Author: Tim Boddy (timboddy) Date: 2018-06-07 22:05
I'm sorry that I changed the resolution by accident.
History
Date User Action Args
2018-06-07 22:05:35timboddysetresolution: not a bug -> third party
messages: + msg318975
2018-06-07 21:15:51timboddysetresolution: third party -> not a bug
messages: + msg318972
2018-06-07 14:42:09berker.peksagsetresolution: not a bug -> third party
messages: + msg318945
2018-06-07 14:39:45timboddysetresolution: third party -> not a bug
messages: + msg318944
2018-06-07 14:22:11benjamin.petersonsetstatus: open -> closed
resolution: third party
2018-06-07 14:12:26timboddysetstatus: closed -> open
resolution: not a bug -> (no value)
messages: + msg318933
2018-06-07 12:48:25berker.peksagsetstatus: open -> closed

nosy: + berker.peksag
messages: + msg318928

resolution: not a bug
stage: resolved
2018-06-07 12:27:37timboddycreate