On Thu, Oct 10, 2019 at 09:30:50AM +1100, Daniel Axtens wrote: > Hi Jeremy, > > > The random module uses the Mersenne Twister pseudorandom number > > generator and is not a cryptographically secure random number > > generator[0]. The secrets[1] module is intended for generating > > cryptographically strong random numbers, so recommend using that to > > generate the secret key. It's new in Python 3, so if it's unavailable > > fall back to using the ``os.urandom()`` backed implementation of random. > > > > [0] https://docs.python.org/3/library/random.html > > [1] https://docs.python.org/3/library/secrets.html > > > > Thanks for your patch. > > I agree that correctly generated randomness is the right way to go. > > Do you think we need to advise existing implementations to roll their > secret? My feeling is that given the way the twister has been seeded > since Python 2.4 (os.urandom if available), existing installations are > probably OK, but I'd be interested in your take. >
If I were running the service I would, but I do agree that given it's likely it was seeded with os.urandom existing installations are probably fine. > > Signed-off-by: Jeremy Cline <jcl...@redhat.com> > > --- > > docs/deployment/installation.rst | 10 ++++++++-- > > patchwork/settings/production.example.py | 12 +++++++++--- > > ...andom.SystemRandom-for-keys-9ceb496919a1bb6f.yaml | 5 +++++ > > 3 files changed, 22 insertions(+), 5 deletions(-) > > create mode 100644 > > releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > > > diff --git a/docs/deployment/installation.rst > > b/docs/deployment/installation.rst > > index d422573..f477a11 100644 > > --- a/docs/deployment/installation.rst > > +++ b/docs/deployment/installation.rst > > @@ -254,9 +254,15 @@ This should be a random value and kept secret. You can > > generate and a value for > > > > .. code-block:: python > > > > - import string, random > > + import string > > + try: > > + import secrets > > + except ImportError: # Python < 3.6 > > + import random > > + secrets = random.SystemRandom() > > We're dropping Python 2 soon, not in the next version coming out Real > Soon Now, but in the version after that. Would it be worth holding this > patch until then so as we can avoid this messy try:import? I have a > topic branch for this and I'd be happy to include this patch in it. > An alternate approach here would be to just not use the secrets API. CPython is just proxying the calls to secrets.choice to random.SystemRandom().choice() so it comes to the same thing. I just prefer the secrets API since its promise is to be fit for cryptographic purposes and if they find a bug they'll fix it. In my own projects I prefer to use the newer APIs when possible and fall back as I don't find the clean-up to be terribly difficult (I just grep for ImportErrors), but I do get that it's ugly. I don't have a strong opinion about doing just random.SystemRandom().choice(), just secrets.choice() in a deferred branch, or this, so I'll leave it up to you. Thanks, Jeremy _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork