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.

Sign up for GitHub

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

Jump to bottom

CPython uses deprecated randomness API #88777

Closed
strombrg mannequin opened this issue Jul 12, 2021 · 9 comments
Closed

CPython uses deprecated randomness API #88777

strombrg mannequin opened this issue Jul 12, 2021 · 9 comments
Labels
3.11 only security fixes OS-windows type-feature A feature request or enhancement

Comments

@strombrg
Copy link
Mannequin

strombrg mannequin commented Jul 12, 2021

BPO 44611
Nosy @tim-one, @pfmoore, @vstinner, @tjguk, @ambv, @zware, @zooba, @graingert, @corona10
PRs
  • bpo-44611: Use BCryptGenRandom instead of CryptGenRandom on Windows #27168
  • bpo-44611: Update docs for os and whatsnew 3.11 #27314
    		•

    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 2021-07-23.14:04:46.166>
created_at = <Date 2021-07-12.17:16:33.330>
labels = ['type-feature', 'OS-windows', '3.11']
title = 'CPython uses deprecated randomness API'
updated_at = <Date 2021-08-04.15:01:09.774>
user = 'https://bugs.python.org/strombrg'

    bugs.python.org fields:

    activity = <Date 2021-08-04.15:01:09.774>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-07-23.14:04:46.166>
closer = 'corona10'
components = ['Windows']
creation = <Date 2021-07-12.17:16:33.330>
creator = 'strombrg'
dependencies = []
files = []
hgrepos = []
issue_num = 44611
keywords = ['patch']
message_count = 9.0
messages = ['397339', '397361', '397362', '397371', '397920', '398056', '398126', '398901', '398902']
nosy_count = 10.0
nosy_names = ['tim.peters', 'paul.moore', 'vstinner', 'tim.golden', 'lukasz.langa', 'strombrg', 'zach.ware', 'steve.dower', 'graingert', 'corona10']
pr_nums = ['27168', '27314']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue44611'
versions = ['Python 3.11']
    @strombrg
    Copy link
    Mannequin Author

    strombrg mannequin commented Jul 12, 2021

    CPython 3.9 uses CryptGenRandom(), which has been deprecated by Microsoft.

    I'm told the randomness produced by CryptGenRandom() is fine, but Microsoft has introduced a newer API for getting randomness.

    For these reasons, Python/bootstrap_hash.c should be updated to use https://docs.microsoft.com/en-us/windows/win32/seccng/cng-por , but it is not urgent, and is not needed in older versions of CPython.

    Also the documentation that references CryptGenRandom() should be updated, EG: https://docs.python.org/3/library/os.html#os.urandom

    @strombrg strombrg mannequin added 3.11 only security fixes OS-windows type-feature A feature request or enhancement labels Jul 12, 2021
    @tim-one
    Copy link
    Member

    tim-one commented Jul 12, 2021

    Dan, the Microsoft URL in your message gives a 404 for me. Did you perhaps mean to end it with "cng-portal" (instead of "cng-por")?

    @graingert
    Copy link
    Mannequin

    graingert mannequin commented Jul 12, 2021

    https://docs.microsoft.com/en-us/windows/win32/seccng/cng-portal ?

    @strombrg
    Copy link
    Mannequin Author

    strombrg mannequin commented Jul 13, 2021

    Yes, cng-portal.

    On Mon, Jul 12, 2021 at 3:24 PM Thomas Grainger <report@bugs.python.org>
    wrote:

    Thomas Grainger <tagrain@gmail.com> added the comment:

    https://docs.microsoft.com/en-us/windows/win32/seccng/cng-portal ?

    ----------
    nosy: +graingert

    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue44611\>

    --

    Dan Stromberg | Senior Software Engineer

    Mobile +1.949.342.6502

    <https://keepersecurity.com/\>

    ** This email is confidential and is intended for the recipient(s)
    addressed herein **

    @corona10
    Copy link
    Member

    @tim.peters

    Can you please take a look at #71355?
    I would like to get your review before merging this PR :)

    @corona10
    Copy link
    Member

    New changeset 906fe47 by Dong-hee Na in branch 'main':
    bpo-44611: Use BCryptGenRandom instead of CryptGenRandom on Windows (GH-27168)
    906fe47

    @ambv
    Copy link
    Contributor

    ambv commented Jul 24, 2021

    New changeset 4463fa2 by Dong-hee Na in branch 'main':
    bpo-44611: Update docs for os and whatsnew 3.11 (bpo-27314)
    4463fa2

    @vstinner
    Copy link
    Member

    vstinner commented Aug 4, 2021

    Would it be possible to document in os.urandom() documentation that the BCryptGenRandom() function is used on Windows with the "system-preferred random number generator algorithm"? (I don't think that we should mention the BCRYPT_USE_SYSTEM_PREFERRED_RNG constant.)

    @vstinner
    Copy link
    Member

    vstinner commented Aug 4, 2021

    Thanks for this change, I like the fact that hCryptProv variable could be removed!

    @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
    Assignees
    No one assigned
    Labels
    3.11 only security fixes OS-windows type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @ambv @vstinner @corona10 @tim-one