[Django] #33526: Accept truthy/falsy values in settings when performing deployment security checks for SECURE_HSTS_INCLUDE_SUBDOMAINS, SECURE_HSTS_PRELOAD and SECURE_SSL_REDIRECT

22 views
Skip to first unread message

Django

unread,
Feb 18, 2022, 10:05:58 AM2/18/22
to django-...@googlegroups.com
#33526: Accept truthy/falsy values in settings when performing deployment security
checks for SECURE_HSTS_INCLUDE_SUBDOMAINS, SECURE_HSTS_PRELOAD and
SECURE_SSL_REDIRECT
------------------------------------------------+------------------------
Reporter: Will Holmes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (System checks) | Version: 4.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
When running {{{python manage.py check --deploy}}}, deployment security
checks are performed on the following settings values:

SECURE_SSL_REDIRECT
SECURE_HSTS_SECONDS
SECURE_HSTS_INCLUDE_SUBDOMAINS
SECURE_HSTS_PRELOAD
SESSION_COOKIE_SECURE
CSRF_COOKIE_SECURE

For those who use separate modules for dev/prod they can simply hardcore
these settings as booleans. If you use environment variables, truthy and
falsy values are more convenient. Otherwise, you have to write a helper
function to convert the environment variable strings to booleans.

For example, I use the {{{dotenv}}} Python package to read environment
variables in as strings 1 or 0 from a {{{.env}}} file, then use
{{{int()}}} to convert them inline to truthy or falsy integer values:

{{{
from dotenv import load_dotenv
load_dotenv()

SECURE_SSL_REDIRECT = int(os.environ.get("DJANGO_SECURE_SSL_REDIRECT",
default=1))
}}}

It's been brought to my attention that many use {{{env.bool()}}} from the
{{{django-environ}}} package to achieve this. However, I believe
truthy/falsy values should be supported as well to circumvent this
requirement.

The issue: 3 of the above settings don't use truthy or falsy values for
their checks in
[https://github.com/django/django/blob/main/django/core/checks/security/base.py
django.core.security.checks.base], while the rest do.

On lines 178, 188 and 203 the checks for SECURE_HSTS_INCLUDE_SUBDOMAINS,
SECURE_HSTS_PRELOAD and SECURE_SSL_REDIRECT respectively use {{{is
True}}}.

On the other hand,
[https://github.com/django/django/blob/main/django/core/checks/security/csrf.py
django.core.security.checks.csrf] on line 40 and
[https://github.com/django/django/blob/main/django/core/checks/security/sessions.py
django.core.security.checks.sessions] on line 69 both use truthy/falsy
checks.

This causes the following scenario for a setting using {{{is True}}}:

{{{SECURE_SSL_REDIRECT = True}}} <-- Passes checks
{{{SECURE_SSL_REDIRECT = 1}}} <-- Does not pass checks

For a setting that uses truthy/falsy checks:

{{{SESSION_COOKIE_SECURE = True}}} <-- Passes checks
{{{SESSION_COOKIE_SECURE = 1}}} <-- Passes checks

To reproduce this, simply hardcode the settings above using truthy/falsy
values, run the checks, and the 3 settings mentioned above will fail
deployment checks.

I currently get around this limitation by reading in TRUE as a string from
a .env file and converting it to a boolean with a helper function:

{{{
def env_to_bool(value):
if isinstance(value, bool):
return value
else:
return value.upper() == "TRUE"

SECURE_SSL_REDIRECT =
env_to_bool(os.environ.get("DJANGO_SECURE_SSL_REDIRECT", default=True))
}}}

I couldn't find a valid reason for not using truthy/falsy checks for the
three settings that don't pass checks without a boolean True. I would be
happy to take this on as my first contribution but wanted to open a bug
ticket first to see if I'm missing a reason these use {{{is True}}} while
the other checks don't.

If anything, the checks should all use a consistent convention so as to
not inconvenience those who are attempting to run checks with truthy/falsy
settings values.

If more information is needed please let me know.

--
Ticket URL: <https://code.djangoproject.com/ticket/33526>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Feb 18, 2022, 2:51:46 PM2/18/22
to django-...@googlegroups.com
#33526: Accept truthy/falsy values in security checks.
-------------------------------------+-------------------------------------

Reporter: Will Holmes | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Core (System | Version: 4.0
checks) |
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => wontfix


Comment:

Thanks for the ticket. Accepting a truthy/falsey values is risky and can
be confusing e.g. what would you expect by setting `export
SECURE_SSL_REDIRECT=0`?
{{{
>>> os.environ.get('SECURE_SSL_REDIRECT')
'0'
>>> bool(os.environ.get('SECURE_SSL_REDIRECT'))
True
}}}

I prepared [https://github.com/django/django/pull/15443 PR] to make
`CSRF_COOKIE_SECURE`/`SESSION_COOKIE_SECURE` no longer pass on truthy
values.

--
Ticket URL: <https://code.djangoproject.com/ticket/33526#comment:1>

Django

unread,
Feb 21, 2022, 1:54:58 AM2/21/22
to django-...@googlegroups.com
#33526: Accept truthy/falsy values in security checks.
-------------------------------------+-------------------------------------
Reporter: Will H | Owner: nobody

Type: | Status: closed
Cleanup/optimization |
Component: Core (System | Version: 4.0
checks) |
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"1299bc33e131a3c44b544b58c706bc998c4228ed" 1299bc33]:
{{{
#!CommitTicketReference repository=""
revision="1299bc33e131a3c44b544b58c706bc998c4228ed"
Refs #33526 -- Made
CSRF_COOKIE_SECURE/SESSION_COOKIE_SECURE/SESSION_COOKIE_HTTPONLY don't
pass on truthy values.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33526#comment:2>

Reply all
Reply to author
Forward
0 new messages