Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport random.c from Python 3.5 to Python 2.7 #73374

Closed
vstinner opened this issue Jan 6, 2017 · 6 comments
Closed

Backport random.c from Python 3.5 to Python 2.7 #73374

vstinner opened this issue Jan 6, 2017 · 6 comments
Labels
type-security A security issue

Comments

@vstinner
Copy link
Member

vstinner commented Jan 6, 2017

BPO 29188
Nosy @rhettinger, @vstinner, @tiran, @benjaminp
Files
  • getentropy_linux.patch
  • 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 2017-01-09.10:26:37.586>
    created_at = <Date 2017-01-06.23:53:54.433>
    labels = ['type-security']
    title = 'Backport random.c from Python 3.5 to Python 2.7'
    updated_at = <Date 2017-01-09.10:26:37.584>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-01-09.10:26:37.584>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-01-09.10:26:37.586>
    closer = 'vstinner'
    components = []
    creation = <Date 2017-01-06.23:53:54.433>
    creator = 'vstinner'
    dependencies = []
    files = ['46182']
    hgrepos = []
    issue_num = 29188
    keywords = ['patch']
    message_count = 6.0
    messages = ['284876', '284878', '284880', '284929', '285033', '285034']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'vstinner', 'christian.heimes', 'benjamin.peterson', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue29188'
    versions = ['Python 2.7']

    @vstinner
    Copy link
    Member Author

    vstinner commented Jan 6, 2017

    Python 3.6 uses the new getrandom() function/syscall on Linux and Solaris to get random bytes with no file descriptor: it prevents EMFILE and ENFILE errors or surprises when opening a first file (and looking at its file descriptor).

    I propose to copy and adapt Python/random.c from Python 3.5 when Python 3.5 will be updated for the issue bpo-29157. Python 2.7 requires extra changes:

    • configure.ac: need to check linux/random.h in AC_CHECK_HEADERS()
    • random.c: need to keep vms_urandom() function (Python 2.7 still supports VMS!)
    • Python 2.7 doesn't implement the PEP-475 (EINTR) and so don't have functions like _Py_read() which handles EINTR for us.

    See also the issue bpo-29157 for the latest change in random.c: prefer getrandom() over getentropy() to support the glibc 2.24.

    @vstinner vstinner added the type-security A security issue label Jan 6, 2017
    @rhettinger
    Copy link
    Contributor

    I think it is far too late to be making these kind of changes to 2.7.

    @vstinner
    Copy link
    Member Author

    vstinner commented Jan 7, 2017

    I think it is far too late to be making these kind of changes to 2.7.

    I would prefer to use the "same code" (or almost) on all maintained versions of Python: 2.7, 3.5, 3.6 and 3.7. It should ease the maintenance for bugfixes and enhancements.

    It seems like we want to backport security enhancements from Python 3 to Python 2.7: see the PEP-466. Copying random.c from Python 3 would add support for getrandom() which is nice to have since it avoids a private file descriptor (which causes many issues, even if the most important issues are already worked around in Python 2.7 using fstat()).

    The minimum required change on Python 2.7 is to not use getentropy() on Linux to support the glibc 2.24: see attached getentropy_linux.patch if you don't want the backport.

    @rhettinger
    Copy link
    Contributor

    I would prefer to use the "same code" (or almost) on all maintained
    versions of Python: 2.7, 3.5, 3.6 and 3.7. It should ease the
    maintenance for bugfixes and enhancements.

    This is VERY far from our historical policy for backports. Python 2.7 is supposed to be getting more stable over time (that is one of its chief virtues). We don't want to risk the kind of mini-catastrophe that got published in 3.6 (bpo-29085).

    If you want to push for this, there needs to be a thorough discussion on python-dev (there are tons of possible backports that could be made if the rationale was "I would prefer to use the same code on all maintained versions").

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 9, 2017

    New changeset 13a39142c047 by Victor Stinner in branch '2.7':
    Don't use getentropy() on Linux
    https://hg.python.org/cpython/rev/13a39142c047

    @vstinner
    Copy link
    Member Author

    vstinner commented Jan 9, 2017

    This is VERY far from our historical policy for backports. Python 2.7 is supposed to be getting more stable over time (that is one of its chief virtues). We don't want to risk the kind of mini-catastrophe that got published in 3.6 (bpo-29085).

    I don't consider that the issue bpo-29085 is a catastrophe and it's just a bug which was already fixed.

    Moreover, Python 2.7 and 3.5 don't have _PyOS_URandomNonblock() function and so the _random module is not impacted by this issue.

    If you want to push for this, there needs to be a thorough discussion on python-dev (there are tons of possible backports that could be made if the rationale was "I would prefer to use the same code on all maintained versions").

    Sorry, I suffered from the previous discussion about random numbers. I don't want to reopen a new discussion, people would become crazy again.

    I just fixed Python/random.c in support glibc 2.24 that's all.

    If someone wants the cool getrandom() function/syscall on Python 2.7, please open a new issue. It doesn't really enhance the security, it's just a matter of avoid a file descriptor.

    @vstinner vstinner closed this as completed Jan 9, 2017
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-security A security issue
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants