Opened 7 years ago

Closed 5 years ago

#27635 closed Cleanup/optimization (fixed)

django.utils.crypto should use secrets on Python 3.6+

Reported by: Adam Johnson Owned by: Nick Pope
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: aymeric.augustin@…, me@…, desecho@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Aymeric discussing django.utils.crypto on mailing list:

While we’re there, we should use https://docs.python.org/3/library/secrets.html#module-secrets on Python >= 3.6.

Change History (8)

comment:1 by Tim Graham, 7 years ago

Triage Stage: UnreviewedAccepted
Type: New featureCleanup/optimization

Specifically, it looks like that means in place of random.SystemRandom.

comment:2 by Anton Samarchyan, 7 years ago

Cc: desecho@… added
Has patch: set
Version: 1.10master

Added PR

comment:3 by Tim Graham, 7 years ago

Has patch: unset
Triage Stage: AcceptedSomeday/Maybe

Python's secrets.py does from random import SystemRandom so this doesn't change any behavior or add security for now. Adam said, "Presumably the intention is that secrets might one day use a different PRNG's on some OS's." Let's make the change if the benefits become more than theoretical or when only Python 3.6+ is supported.

Another possibility Aymeric mentioned, "In the long run I think we should deprecate get_random_string in favor of similar functions provided by the secrets module. I didn't check whether there was a sensible transition plan to make use of secrets on Python 3.6 while still supporting older versions."

in reply to:  3 comment:4 by Emett Speer, 7 years ago

Replying to Tim Graham:

Python's secrets.py does from random import SystemRandom so this doesn't change any behavior or add security for now. Adam said, "Presumably the intention is that secrets might one day use a different PRNG's on some OS's." Let's make the change if the benefits become more than theoretical or when only Python 3.6+ is supported.

Another possibility Aymeric mentioned, "In the long run I think we should deprecate get_random_string in favor of similar functions provided by the secrets module. I didn't check whether there was a sensible transition plan to make use of secrets on Python 3.6 while still supporting older versions."

I'm with you on this. The vast majority of people are not going to use this for a long time and it will add an extra bit of overhead just to support an update in a single version of Python none of the big distros ship. Once more of the Django community has migrated to Python3.6+ it would be worth looking into.

comment:5 by Claude Paroz, 5 years ago

Has patch: set

New PR now we are on 3.6+.

comment:6 by Nick Pope, 5 years ago

Owner: changed from nobody to Nick Pope
Status: newassigned
Triage Stage: Someday/MaybeAccepted

Alternate PR addressing the non-working fallback and optionally stripping it out based on my comment.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 068005a3:

Refs #27635 -- Removed fallback when SystemRandom() isn't available that doesn't work.

Fallback was untested and likely never triggered.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 1d0bab0:

Fixed #27635 -- Used secrets module in django.utils.crypto.

Note: See TracTickets for help on using tickets.
Back to Top