Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33526 closed Cleanup/optimization (wontfix)

Accept truthy/falsy values in security checks.

Reported by: Will H Owned by: nobody
Component: Core (System checks) Version: 4.0
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

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 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, django.core.security.checks.csrf on line 40 and 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.

Change History (2)

comment:1 by Mariusz Felisiak, 3 years ago

Resolution: wontfix
Status: newclosed
Summary: Accept truthy/falsy values in settings when performing deployment security checks for SECURE_HSTS_INCLUDE_SUBDOMAINS, SECURE_HSTS_PRELOAD and SECURE_SSL_REDIRECTAccept truthy/falsy values in security checks.

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 PR to make CSRF_COOKIE_SECURE/SESSION_COOKIE_SECURE checks no longer pass on truthy values.

Last edited 3 years ago by Mariusz Felisiak (previous) (diff)

comment:2 by GitHub <noreply@…>, 3 years ago

In 1299bc33:

Refs #33526 -- Made CSRF_COOKIE_SECURE/SESSION_COOKIE_SECURE/SESSION_COOKIE_HTTPONLY don't pass on truthy values.

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