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

os.urandom() should use Linux 3.17 getrandom() syscall #66377

Closed
vstinner opened this issue Aug 10, 2014 · 23 comments
Closed

os.urandom() should use Linux 3.17 getrandom() syscall #66377

vstinner opened this issue Aug 10, 2014 · 23 comments
Labels
type-security A security issue

Comments

@vstinner
Copy link
Member

BPO 22181
Nosy @pitrou, @vstinner, @tiran, @jwilk, @alex, @MojoVampire
Files
  • random.patch
  • random-2.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 2015-03-30.11:42:45.852>
    created_at = <Date 2014-08-10.23:32:43.376>
    labels = ['type-security']
    title = 'os.urandom() should use Linux 3.17 getrandom() syscall'
    updated_at = <Date 2015-03-30.11:42:45.851>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2015-03-30.11:42:45.851>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-03-30.11:42:45.852>
    closer = 'vstinner'
    components = []
    creation = <Date 2014-08-10.23:32:43.376>
    creator = 'vstinner'
    dependencies = []
    files = ['38310', '38330']
    hgrepos = []
    issue_num = 22181
    keywords = ['patch']
    message_count = 23.0
    messages = ['225171', '225363', '228250', '228639', '228645', '228655', '228785', '228790', '228791', '228792', '228794', '228795', '228847', '228850', '237102', '237190', '238440', '238504', '238559', '238568', '238676', '238688', '239585']
    nosy_count = 11.0
    nosy_names = ['pitrou', 'vstinner', 'christian.heimes', 'jwilk', 'Arfrever', 'alex', 'neologix', 'python-dev', 'anand.jeyahar', 'josh.r', '700eb415']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue22181'
    versions = ['Python 3.5']

    @vstinner
    Copy link
    Member Author

    The future Linux kernel 3.17 will have a new getrandom() syscall which avoids the need of a file descriptor:
    http://lwn.net/Articles/606141/

    The file descriptor of os.urandom() causes perfomance issues and surprising bugs: bpo-18756, bpo-21207.

    I don't know when the function will land in the libc.

    OpenBSD 5.6 (not released yet) will also have a new getentropy() syscall.

    For Python 2.7, see also the PEP-466 and the issue bpo-21305.

    @pitrou pitrou added the type-feature A feature request or enhancement label Aug 10, 2014
    @vstinner
    Copy link
    Member Author

    Manual page of the OpenBSD getentropy() function:
    http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man2/getentropy.2

    LibreSSL didn't wait for the libc, search for getentropy_getrandom():
    http://openbsd.cs.toronto.edu/cgi-bin/cvsweb/src/lib/libcrypto/crypto/getentropy_linux.c?rev=1.32&content-type=text/x-cvsweb-markup

    The code is currently disabled with "#if 0". The syscall is directly used, the function doesn't handle the ENOSYS error.

    See also this issue of the cryptography project, "Use getentropy(2) and getrandom(2) syscalls when available 1299":
    pyca/cryptography#1299

    @700eb415
    Copy link
    Mannequin

    700eb415 mannequin commented Oct 2, 2014

    It's worth noting that LibreSSL has now enabled the blocked code. If anyone is interested, I would be willing to help port it.

    @anandjeyahar
    Copy link
    Mannequin

    anandjeyahar mannequin commented Oct 6, 2014

    Hi,
    This will need latest kernel to develop, fix and test. I (on Debian 7) couldn't find the latest kernel, but picked up ubuntu kernel from here http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.17-rc7-utopic/. I picked up the latest i.e: linux-image-3.17.0-031700rc7-generic_3.17.0-031700rc7.201409281835_amd64.deb and installed manually, but couldn't find the getrandom() function call either in stdlib.h or linux/random.h.

    Can anyone confirm it's availability in a kernel image (from some other distribution?).

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 6, 2014

    Note that I'm not fussed about it: far from simplifying the code, it
    will make it more complex, thus more error-prone.

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 6, 2014

    The Linux kernel 3.17 has been released with the new getrandom() syscall.

    glibc request to implement the function in the C library:
    https://sourceware.org/bugzilla/show_bug.cgi?id=17252
    "Bug 17252 - getrandom and getentropy syscall"

    It looks like nobody asks for it on the libc-alpha mailing list yet.

    @tiran
    Copy link
    Member

    tiran commented Oct 8, 2014

    Let's not be early adopters here. I suggest we wait until glibc has a proper interface.

    @700eb415
    Copy link
    Mannequin

    700eb415 mannequin commented Oct 8, 2014

    OpenBSD already provides high quality pseudorandom numbers from arc4random(). I don't think this would make us "early adopters" since it has been around for some time on this platform.

    It's also worth mentioning that getentropy() is not recommended in use for normal code as per stated in the man page. arc4random() is recommended, but there may be a reason the first poster has recommended getentropy()

    @pitrou
    Copy link
    Member

    pitrou commented Oct 8, 2014

    This issue is about Linux support. Does the glibc have arc4random? I can't find it on my Ubuntu 13.10 system.

    @alex
    Copy link
    Member

    alex commented Oct 8, 2014

    As I said on the other ticket, using arc4random() indiscriminately would be a very poor idea, on some platforms (such as OS X) arc4random() really does use ARC4, which means there are serious security concerns with it.

    @700eb415
    Copy link
    Mannequin

    700eb415 mannequin commented Oct 8, 2014

    While I agree it may not be wise to use arc4random() globally, OpenBSD is unlikely to create a duplicate interface since it's already available.

    Python is currently unusable in chroots on that platform without reducing the security of the host partition by removing the nodev mount flag.

    I feel like there must be a good solution to this.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 8, 2014

    Since this is a Linux-specific issue (see the title), you should create a separate issue for OpenBSD support. Bonus points if you want to submit a patch as well :-)

    @vstinner vstinner added type-security A security issue and removed type-feature A feature request or enhancement labels Oct 9, 2014
    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 9, 2014

    This issue is specific to Linux: it depends on the Linux kernel version and we are waiting until the new syscall is available in the C library (especially the glibc). For these reasons, I prefer to open a new specific issue for OpenBSD, since they release the kernel and C library at the same time (different release process): issue bpo-22585. OpenBSD 5.6 scheduled in one month will get the new getentropy() syscall and a new getentropy() function at the same time.

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 9, 2014

    Since this is a Linux-specific issue (see the title), you should create a separate issue for OpenBSD support.

    700eb415 opened the issue bpo-22542 for arc4random().

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 3, 2015

    Commit in the Linux kernel:
    https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c6e9d6f38894798696f23c8084ca7edbf16ee895

    --

    Here is a patch to use the new getrandom() syscall of Linux 3.17 in the Python function os.urandom().

    The function falls back to reading /dev/urandom if getrandom() is not supported (returns ENOSYS at runtime).

    On my Linux 3.18, the EINTR path is never taken. But I was able to test it manually by setting flags to GRND_RANDOM (2) and injecting many signals using signal.setitimer(): see my http://bugs.python.org/issue23285#msg237100

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 4, 2015

    random-2.patch: updated patch (I don't understand why random.patch doesn't apply cleanly).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 18, 2015

    New changeset 1fc32bf069ff by Victor Stinner in branch 'default':
    Issue bpo-22181: On Linux, os.urandom() now uses the new getrandom() syscall if
    https://hg.python.org/cpython/rev/1fc32bf069ff

    @vstinner
    Copy link
    Member Author

    Oh, test_os now fails on Linux because os.urandom() doesn't use a file descriptor anymore. The test should be skipped when getrandom() is used. The test is already skipped when getentropy() is used.

    @vstinner vstinner reopened this Mar 19, 2015
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 19, 2015

    New changeset 4491bdb6527b by Victor Stinner in branch 'default':
    Issue bpo-22181: The availability of the getrandom() is now checked in configure,
    https://hg.python.org/cpython/rev/4491bdb6527b

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 19, 2015

    New changeset 8c73af0b3cd9 by Victor Stinner in branch 'default':
    Issue bpo-22181: Fix dev_urandom_noraise(), try calling py_getrandom() before
    https://hg.python.org/cpython/rev/8c73af0b3cd9

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Mar 20, 2015

    New changeset 4491bdb6527b by Victor Stinner in branch 'default':
    Issue bpo-22181: The availability of the getrandom() is now checked in configure,
    https://hg.python.org/cpython/rev/4491bdb6527b

    You forgot to run aclocal, which resulted in PKG_PROG_PKG_CONFIG not being expanded in configure.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 20, 2015

    New changeset b8ceb071159f by Victor Stinner in branch 'default':
    Issue bpo-22181: Run "aclocal; autoconf; autoheader" to regenerate configure
    https://hg.python.org/cpython/rev/b8ceb071159f

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 30, 2015

    New changeset 28b465d8c519 by Victor Stinner in branch 'default':
    Issue bpo-22181: os.urandom() now releases the GIL when the getrandom()
    https://hg.python.org/cpython/rev/28b465d8c519

    @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

    4 participants