Re: [PATCH] Use secrets and fall back to random.SystemRandom for keys
On Wed, 2019-10-09 at 18:55 -0400, Jeremy Cline wrote: > On Thu, Oct 10, 2019 at 09:32:13AM +1100, Daniel Axtens wrote: > > > diff --git > > > a/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > > > > > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > > new file mode 100644 > > > index 000..7b101cb > > > --- /dev/null > > > +++ > > > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > > @@ -0,0 +1,5 @@ > > > +--- > > > +security: > > > + - | > > > +Change the recommended method for generating the Django secret key > > > to use a > > > +cryptographically secure random number generator. > > > > Oh, while I remember, I think I've had trouble with the security: > > section before. Have you been able to verify that this shows up in the > > docs? (I build mine with `docker-compose run web tox -e docs`) > > > > Ah, you caught me. I noticed the request for a release note late in the > game and didn't actually check that it worked in the docs. You're right, > it doesn't show up. Some quick debugging indicates that the reno config > is excluding the section, something like: > > diff --git a/releasenotes/config.yaml b/releasenotes/config.yaml > index cd31940..6c7d622 100644 > --- a/releasenotes/config.yaml > +++ b/releasenotes/config.yaml > @@ -11,3 +11,4 @@ sections: >- [fixes, Bug Fixes] >- [api, API Changes] >- [other, Other Notes] > + - [security, Security Notes] > > fixes it. I can re-roll the patch if you like. No need. I've included this and applied it. As you note, the Python 2 fallbacks are easy to clean up later if we need to. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] Use secrets and fall back to random.SystemRandom for keys
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 > > --- > > 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
Re: [PATCH] Use secrets and fall back to random.SystemRandom for keys
On Thu, Oct 10, 2019 at 09:32:13AM +1100, Daniel Axtens wrote: > > diff --git > > a/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > > > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > new file mode 100644 > > index 000..7b101cb > > --- /dev/null > > +++ > > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > @@ -0,0 +1,5 @@ > > +--- > > +security: > > + - | > > +Change the recommended method for generating the Django secret key to > > use a > > +cryptographically secure random number generator. > > Oh, while I remember, I think I've had trouble with the security: > section before. Have you been able to verify that this shows up in the > docs? (I build mine with `docker-compose run web tox -e docs`) > Ah, you caught me. I noticed the request for a release note late in the game and didn't actually check that it worked in the docs. You're right, it doesn't show up. Some quick debugging indicates that the reno config is excluding the section, something like: diff --git a/releasenotes/config.yaml b/releasenotes/config.yaml index cd31940..6c7d622 100644 --- a/releasenotes/config.yaml +++ b/releasenotes/config.yaml @@ -11,3 +11,4 @@ sections: - [fixes, Bug Fixes] - [api, API Changes] - [other, Other Notes] + - [security, Security Notes] fixes it. I can re-roll the patch if you like. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] Use secrets and fall back to random.SystemRandom for keys
> diff --git > a/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > new file mode 100644 > index 000..7b101cb > --- /dev/null > +++ > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > @@ -0,0 +1,5 @@ > +--- > +security: > + - | > +Change the recommended method for generating the Django secret key to > use a > +cryptographically secure random number generator. Oh, while I remember, I think I've had trouble with the security: section before. Have you been able to verify that this shows up in the docs? (I build mine with `docker-compose run web tox -e docs`) Regards, Daniel > -- > 2.21.0 > > ___ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] Use secrets and fall back to random.SystemRandom for keys
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. > Signed-off-by: Jeremy Cline > --- > 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. > + > chars = string.ascii_letters + string.digits + string.punctuation > - print(repr("".join([random.choice(chars) for i in range(0,50)]))) > + print("".join([secrets.choice(chars) for i in range(50)])) > > Once again, store this in ``production.py``. Regards, Daniel > > diff --git a/patchwork/settings/production.example.py > b/patchwork/settings/production.example.py > index c6aa2f2..8058537 100644 > --- a/patchwork/settings/production.example.py > +++ b/patchwork/settings/production.example.py > @@ -21,9 +21,15 @@ from .base import * # noqa > # You'll need to replace this to a random string. The following python code > can > # be used to generate a secret key: > # > -# import string, random > -# chars = string.letters + string.digits + string.punctuation > -# print repr("".join([random.choice(chars) for i in range(0,50)])) > +# import string > +# try: > +# import secrets > +# except ImportError: # Python < 3.6 > +# import random > +# secrets = random.SystemRandom() > +# > +# chars = string.ascii_letters + string.digits + string.punctuation > +# print("".join([secrets.choice(chars) for i in range(50)])) > > SECRET_KEY = os.environ['DJANGO_SECRET_KEY'] > > diff --git > a/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > new file mode 100644 > index 000..7b101cb > --- /dev/null > +++ > b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml > @@ -0,0 +1,5 @@ > +--- > +security: > + - | > +Change the recommended method for generating the Django secret key to > use a > +cryptographically secure random number generator. > -- > 2.21.0 > > ___ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH] Use secrets and fall back to random.SystemRandom for keys
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 Signed-off-by: Jeremy Cline --- 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() + chars = string.ascii_letters + string.digits + string.punctuation - print(repr("".join([random.choice(chars) for i in range(0,50)]))) + print("".join([secrets.choice(chars) for i in range(50)])) Once again, store this in ``production.py``. diff --git a/patchwork/settings/production.example.py b/patchwork/settings/production.example.py index c6aa2f2..8058537 100644 --- a/patchwork/settings/production.example.py +++ b/patchwork/settings/production.example.py @@ -21,9 +21,15 @@ from .base import * # noqa # You'll need to replace this to a random string. The following python code can # be used to generate a secret key: # -# import string, random -# chars = string.letters + string.digits + string.punctuation -# print repr("".join([random.choice(chars) for i in range(0,50)])) +# import string +# try: +# import secrets +# except ImportError: # Python < 3.6 +# import random +# secrets = random.SystemRandom() +# +# chars = string.ascii_letters + string.digits + string.punctuation +# print("".join([secrets.choice(chars) for i in range(50)])) SECRET_KEY = os.environ['DJANGO_SECRET_KEY'] diff --git a/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml new file mode 100644 index 000..7b101cb --- /dev/null +++ b/releasenotes/notes/use-secrets-and-fall-back-to-random.SystemRandom-for-keys-9ceb496919a1bb6f.yaml @@ -0,0 +1,5 @@ +--- +security: + - | +Change the recommended method for generating the Django secret key to use a +cryptographically secure random number generator. -- 2.21.0 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork