Memory leak in X509StoreContext class.

[timboddy]

BPO	33795
Nosy	@berkerpeksag, @timboddy

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-06-07.14:22:11.898>
created_at = <Date 2018-06-07.12:27:37.523>
labels = ['performance']
title = 'Memory leak in X509StoreContext class.'
updated_at = <Date 2018-06-07.22:05:35.769>
user = 'https://github.com/timboddy'

bugs.python.org fields:

activity = <Date 2018-06-07.22:05:35.769>
actor = 'timboddy'
assignee = 'none'
closed = True
closed_date = <Date 2018-06-07.14:22:11.898>
closer = 'benjamin.peterson'
components = []
creation = <Date 2018-06-07.12:27:37.523>
creator = 'timboddy'
dependencies = []
files = []
hgrepos = []
issue_num = 33795
keywords = []
message_count = 7.0
messages = ['318927', '318928', '318933', '318944', '318945', '318972', '318975']
nosy_count = 2.0
nosy_names = ['berker.peksag', 'timboddy']
pr_nums = []
priority = 'normal'
resolution = 'third party'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue33795'
versions = ['Python 3.5']

[timboddy]

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()

[berkerpeksag]

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

[timboddy]

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?

[timboddy]

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.

[berkerpeksag]

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.

[timboddy]

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.