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

Jump to bottom

PEP 466: Backport hashlib.pbkdf2_hmac to Python 2.7 #65503

Closed
alex opened this issue Apr 19, 2014 · 17 comments
Closed

PEP 466: Backport hashlib.pbkdf2_hmac to Python 2.7 #65503

alex opened this issue Apr 19, 2014 · 17 comments
Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir

Comments

@alex
Copy link
Member

alex commented Apr 19, 2014

BPO 21304
Nosy @malemburg, @gpshead, @ncoghlan, @pitrou, @tiran, @benjaminp, @alex, @dstufft
Files
  • pbkdf2.diff
  • pbkdf2.diff
  • pbkdf2.diff
  • pbkdf2.diff
    		•

    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 2014-05-31.20:26:54.174>
created_at = <Date 2014-04-19.00:49:06.064>
labels = ['extension-modules', 'library']
title = 'PEP 466: Backport hashlib.pbkdf2_hmac to Python 2.7'
updated_at = <Date 2014-05-31.20:26:54.065>
user = 'https://github.com/alex'

    bugs.python.org fields:

    activity = <Date 2014-05-31.20:26:54.065>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2014-05-31.20:26:54.174>
closer = 'python-dev'
components = ['Extension Modules', 'Library (Lib)']
creation = <Date 2014-04-19.00:49:06.064>
creator = 'alex'
dependencies = []
files = ['34968', '35242', '35309', '35375']
hgrepos = []
issue_num = 21304
keywords = ['patch', 'needs review', 'security_issue']
message_count = 17.0
messages = ['216823', '216841', '216842', '218454', '218456', '218457', '218458', '218459', '218460', '218785', '218786', '218787', '218885', '218887', '219188', '219197', '219475']
nosy_count = 9.0
nosy_names = ['lemburg', 'gregory.p.smith', 'ncoghlan', 'pitrou', 'christian.heimes', 'benjamin.peterson', 'alex', 'python-dev', 'dstufft']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue21304'
versions = ['Python 2.7']
    @alex
    Copy link
    Member Author

    alex commented Apr 19, 2014

    Pursuant to PEP-466, this is a backport of Python 3.4's hashlib.pbkdf2_hmac.

    Of note in this patch:

    • None of the utilities for testing both a python and a C implementation simultaneously were present, so this only tests whichever implementation is available.
    • Due to a variety of API changes and missing APIs, the code is not an exact copy-paste, tough luck :-)
    • I haven't done docs yet.
    • It currently accepts unicode values because the y* format from Python3 doesn't have any parallel in Python2. I'm not sure whether consistency with the rest of the 2-verse is more important than consistency with a sane way to treat data / the 3-verse.

    @alex alex added extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir labels Apr 19, 2014
    @bitdancer bitdancer changed the title Backport hashlib.pbkdf2_hmac to Python 2.7 PEP 446: Backport hashlib.pbkdf2_hmac to Python 2.7 Apr 19, 2014
    @ncoghlan ncoghlan changed the title PEP 446: Backport hashlib.pbkdf2_hmac to Python 2.7 PEP 466: Backport hashlib.pbkdf2_hmac to Python 2.7 Apr 19, 2014
    @gpshead
    Copy link
    Member

    gpshead commented Apr 19, 2014

    See also http://bugs.python.org/issue21288 to consider one fix/oversite/addition to the existing API as part of this process. (discuss that there)

    by default: use the exact same API as 3.4 if it is suitable for PEP-466 and 2.7.7's needs. the above issue is about fixing a possible oversight; so if it happens in 3.4 it should happen in 2.7.7.

    @alex
    Copy link
    Member Author

    alex commented Apr 19, 2014

    Yup, I've got my eyes on it, if anything lands there I'll include it in this in the 2.7 code, whether it's before or after this patch lands :-)

    @alex
    Copy link
    Member Author

    alex commented May 13, 2014

    New patch includes the documentation as well.

    @dstufft
    Copy link
    Member

    dstufft commented May 13, 2014

    The attached patch looks pretty good to me.

    @alex
    Copy link
    Member Author

    alex commented May 13, 2014

    I'm still concerned about the unicode issue, but I'm not sure what the right way to fix it is.

    @dstufft
    Copy link
    Member

    dstufft commented May 13, 2014

    I don't think there's any way around it, nor do I think that it actually leaks any meaningful timing.

    @alex
    Copy link
    Member Author

    alex commented May 13, 2014

    Sorry, I wasn't concerned from a timing attack perspective here, I was concerned from an "oh my god implicit coercion is terrible" perspective :-)

    @dstufft
    Copy link
    Member

    dstufft commented May 13, 2014

    Oh, gotcha.

    Yea I agree, but it's Python 2.x that's par for the course.

    @malemburg
    Copy link
    Member

    Some comments:

    • Python 2.7 ships with OpenSSL 0.9.8 on Windows, so the Python version will always get used on that platform, so it needs to be fast.

    • The iterations loop should use xrange instead of range

    • The .encode('ascii') in _long_to_bin() is not necessary in Python 2

    • Given that _long_to_bin() and _bin_to_long() are only used once in the function, it's better to inline the code directly.

    • bytes(buffer()) should not be necessary in Python 2, since objects with a buffer interface will usually also implement the tp_str slot used by bytes().

    @tiran
    Copy link
    Member

    tiran commented May 19, 2014

    Sorry that I join the party rather late.

    How about you take my back port from https://bitbucket.org/tiran/backports.pbkdf2/ and remove all Python 3.x related code? :) I spent a lot of time to make the code as fast as possible.

    @malemburg
    Copy link
    Member

    On 19.05.2014 12:24, Christian Heimes wrote:

    How about you take my back port from https://bitbucket.org/tiran/backports.pbkdf2/ and remove all Python 3.x related code? :) I spent a lot of time to make the code as fast as possible.

    Could you perhaps compare this to the proposed patch ?

    @alex
    Copy link
    Member Author

    alex commented May 21, 2014

    Updated patch applies all of MAL's suggestions. Except the buffer() one, the purpose of the buffer() call is to make it an error to pass a list (or random other types) since you can call bytes() on any object.

    @alex
    Copy link
    Member Author

    alex commented May 21, 2014

    As a note, the current code is basically identical to the code in Christain's backport, without the py3k compat.

    @benjaminp
    Copy link
    Contributor

    Looks like there's a debugging turd in the test.

    @alex
    Copy link
    Member Author

    alex commented May 27, 2014

    New patch removes the pdb nonsense in the test.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 31, 2014

    New changeset e4da3ba9dcac by Benjamin Peterson in branch '2.7':
    backport hashlib.pbkdf2_hmac per PEP-466 (closes bpo-21304)
    http://hg.python.org/cpython/rev/e4da3ba9dcac

    @python-dev python-dev mannequin closed this as completed May 31, 2014
    @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
    extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    6 participants
    @alex @gpshead @dstufft @benjaminp @tiran @malemburg