Opened 4 years ago

Closed 4 years ago

#32456 closed New feature (fixed)

Add support for PostgreSQL passfile to dbshell.

Reported by: Dominik George Owned by: Hasan Ramezani
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords:
Cc: Florian Apolloner, Daniel Bowring, Simon Charette Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Mariusz Felisiak)

The dbshell management commands all carry the risk of leaking passwords through process information (as noted in a comment in db.backends.mysql.client). As of Django 3.2, there is the settings_to_cmd_args_env class method, which provides an API to generate the environment needed to call the utility.

Using the environment is somewhat more secure, but the environment of processes can potentially still be read. Both MySQL and PostgreSQL advise against using the respective environment variables.

Specifying a password file works for connections but dbshell doesn't support it, see comment.

I want to propose a way to solve this. I already did this in django-dbbackup, which also happened to construct a command line before:

https://github.com/django-dbbackup/django-dbbackup/pull/385/commits/222152afe9032e98249cada6d7e200a3eb751e63

The mechanism is that in addition to the environment and args, a temporary file is generated. For PostgreSQL, this is a file in .pgpass format; for MySQL, it could be an options file. I wrapped that handling in a neat context manager.

For Django itself, I did a quick shot at PostgreSQL as well, as attached in the patch. The patch is not complete, and is only intended as a base for discussion. If we find consensus about the mechanism, I will happily complete it and extend to the other backends.

Attachments (1)

django_postgresql_pgpass.diff (2.5 KB ) - added by Dominik George 4 years ago.

Download all attachments as: .zip

Change History (9)

by Dominik George, 4 years ago

comment:1 by Dominik George, 4 years ago

Description: modified (diff)
Summary: Pass runshell passwords as option filesPass dbshell passwords as option files

comment:2 by Mariusz Felisiak, 4 years ago

Cc: Florian Apolloner Daniel Bowring Simon Charette added
Has patch: unset
Type: UncategorizedNew feature

The proposed solution was used in Django < 3.0 and changed to PGPASSWORD in cf826c9a91015c8da2ad4910b12e2ed83e2fb20f, see #30173. We discussed this recently @django-security and agreed that using environment variables should be enough for all backends, because only old UNIX systems such as AIX allow non-root users to see process environment variables via ps.

We could, however, add dbshell support for a custom passfile:

diff --git a/django/db/backends/postgresql/client.py b/django/db/backends/postgresql/client.py
index 2339880967..fa41d25228 100644
--- a/django/db/backends/postgresql/client.py
+++ b/django/db/backends/postgresql/client.py
@@ -16,6 +16,7 @@ class DatabaseClient(BaseDatabaseClient):
         dbname = settings_dict.get('NAME') or 'postgres'
         user = settings_dict.get('USER')
         passwd = settings_dict.get('PASSWORD')
+        passfile = options.get('passfile')
         service = options.get('service')
         sslmode = options.get('sslmode')
         sslrootcert = options.get('sslrootcert')
@@ -44,6 +45,8 @@ class DatabaseClient(BaseDatabaseClient):
             env['PGSSLCERT'] = str(sslcert)
         if sslkey:
             env['PGSSLKEY'] = str(sslkey)
+        if passfile:
+            env['PGPASSFILE'] = str(passfile)
         return args, env
 
     def runshell(self, parameters):

If you agree, I'd be happy to accept such ticket.

comment:3 by Dominik George, 4 years ago

I understand the intention why it was removed, although I do not agree.

What is the plan forward once MySQL removes support for MYSQL_PWD? The comment there already acknowledges that it was deprecated and will be removed, and using a deprecated interface does not seem to good an idea.

Supporting an explicit password file seems appropriate, but in that case, it should be honoured by the database backend itself as well (psycopg2 uses libpq, so this should work). Having doubled configuration for the regular backend and dbshell does not seem appropriate to me.

comment:4 by Mariusz Felisiak, 4 years ago

What is the plan forward once MySQL removes support for MYSQL_PWD? The comment there already acknowledges that it was deprecated and will be removed, and using a deprecated interface does not seem to good an idea.

Yes it's deprecated but as far as I'm aware without any reasonable alternative, creating a MySQL on the fly is not an option, IMO. Users can always use option files without providing passwords in settings.

Supporting an explicit password file seems appropriate, but in that case, it should be honoured by the database backend itself as well (psycopg2 uses libpq, so this should work). Having doubled configuration for the regular backend and dbshell does not seem appropriate to me.

All OPTIONS are already passed to the new connections, so:

    DATABASES = {
        'default': {
            'ENGINE': 'django.db.backends.postgresql',
            ...
            'OPTIONS': {'passfile': '/home/user/my_custom_pgpass'},
        }
    }

works for connections, but it is not set for dbshell.

comment:5 by Mariusz Felisiak, 4 years ago

Description: modified (diff)
Easy pickings: set
Summary: Pass dbshell passwords as option filesAdd support for PostgreSQL passfile to dbshell.
Triage Stage: UnreviewedAccepted
Version: 3.24.0

comment:6 by Hasan Ramezani, 4 years ago

Has patch: set
Owner: changed from nobody to Hasan Ramezani
Status: newassigned

comment:7 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In 8380fe0:

Fixed #32456 -- Added dbshell support for specifying a password file on PostgreSQL.

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