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

Add hash auto-randomization #76356

Closed
bjarvis mannequin opened this issue Nov 30, 2017 · 6 comments
Closed

Add hash auto-randomization #76356

bjarvis mannequin opened this issue Nov 30, 2017 · 6 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@bjarvis
Copy link
Mannequin

bjarvis mannequin commented Nov 30, 2017

BPO 32175
Nosy @rhettinger, @pitrou, @vstinner, @tiran
Files
  • auto_rand_2.7.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 = 'https://github.com/rhettinger'
    closed_at = <Date 2017-12-04.20:03:38.678>
    created_at = <Date 2017-11-30.05:39:15.418>
    labels = ['type-feature']
    title = 'Add hash auto-randomization'
    updated_at = <Date 2017-12-04.22:22:50.769>
    user = 'https://bugs.python.org/bjarvis'

    bugs.python.org fields:

    activity = <Date 2017-12-04.22:22:50.769>
    actor = 'vstinner'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2017-12-04.20:03:38.678>
    closer = 'rhettinger'
    components = []
    creation = <Date 2017-11-30.05:39:15.418>
    creator = 'bjarvis'
    dependencies = []
    files = ['47305']
    hgrepos = []
    issue_num = 32175
    keywords = ['patch']
    message_count = 6.0
    messages = ['307278', '307295', '307314', '307590', '307593', '307601']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'pitrou', 'vstinner', 'christian.heimes', 'bjarvis']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue32175'
    versions = ['Python 2.7']

    @bjarvis
    Copy link
    Mannequin Author

    bjarvis mannequin commented Nov 30, 2017

    Hash auto-randomization is a mechanism to detect when a collision attack is underway and switch to a randomized keying scheme at that point.

    This patch is for the 2.7 branch, where hash randomization is not on by default.

    Using collided strings from https://github.com/Storyyeller/fnv-collider/tree/master/collided_strings, 10 "attacks" of roughly 50,000 collided strings were launched against this. The unmodified Python had a median insert time of roughly 4.32 seconds and a median retrieve time of roughly 4.40 seconds. With the auto-randomized version of Python, the median insert time was roughly 3.99 seconds and median retrieve time was roughly 3.57 seconds. This is a 7.7% and 18.9% savings, respectively.

    @bjarvis bjarvis mannequin added the type-feature A feature request or enhancement label Nov 30, 2017
    @tiran
    Copy link
    Member

    tiran commented Nov 30, 2017

    Raymond, dicts are your area of expertise.

    I'm -0 on the patch. The check is going to slow down dicts and it's really easy to enable randomization with an env var or command line argument.

    @vstinner
    Copy link
    Member

    It was decided to leave the hash randomization disabled by default for backward compatibility. It's a deliberate choice.

    I don't think that we need to go further for Python 2.7. I never considered this denial of service attach as major, there are many other ways to trigger a DoS, and fixing the dict type is not the right way to prevent this class of attacks.

    HTTP clients and frameworks like http.client and Django implemented other countermeasures like limiting the number of HTTP headers.

    The problem was correctly fixed in Python 3: randomization enabled by default since Python 3.3, and Python 3.4 now uses SipHash which better hides the hash secret.

    More info at:

    @pitrou
    Copy link
    Member

    pitrou commented Dec 4, 2017

    Agreed this is not desirable at this point. Let's leave 2.7 like it is.

    @rhettinger
    Copy link
    Contributor

    Marking as closed for the reasons listed by the other respondents.

    @vstinner
    Copy link
    Member

    vstinner commented Dec 4, 2017

    By the way, for a few other reasons, Python 3 is more secure than Python 2.7. If you care of security, please upgrade!

    @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-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants