Enable easy configuration of encrypted PostgreSQL connections with new optional DB_SSLMODE (defaults to current value of "disable") following values allowed by PostgreSQL

[3leapsdave]

…PostgreSQL connections, adding a new environment variable DB_SSLMODE with notes about use of existing PGSSLROOTCERT and PGSSLCRL environment variables that may be needed. Default for sslmode in connection string remains current value of disabled

Summary

This pull request enables quick configuration of encrypted PostgreSQL connections when setting up Docker containers. The need for this arises when using a hybrid configuration - application and web servers running in local containers while the database is hosted remotely in a separate server or on some cloud-based Database-as-a-Service. Using a new environment variable DB_SSLMODE, the code to build the default connection string (MM_SQLSETTINGS_DATASOURCE) now supports parameterization of the sslmode parameter (currently hardcoded to a value of disabled)

Ticket Link

This is an unsolicited contribution (I was unable to find a corresponding JIRA ticket for this) arising from my particular deployment challenge. In a particular circumstance, I have an environment where the application and web code runs in Docker containers while the database is hosted in a DaaS configuration remotely. That provider requires an encrypted connection (which is of course good practice anyway) when connecting remotely. PostgreSQL has several parameters to support this behavior, most notably including sslmode, sslrootcert and sslcrl. Best practice for remote databases today is to use verify-ca or verify-full, though require is often used as well. The current code in entrypoint.sh hardcodes the sslmode parameter to disable. This is quite reasonable behavior in the main case where all components run in the same container, or perhaps even in the same Docker network.

My goals were twofold: support configuration of the sslmode parameter and support the associated configuration data such as the root CA file. I initially considered simply overwriting the derived variable MM_SQLSETTINGS_DATASOURCE, on the premise that the case might not be used all that frequently. I decided to propose a new environment variable instead (DB_SSLMODE) for two reasons: 1) The use case of a remote database seems plausible for other users, and a simple replacement of the hardcoded value disable for sslmode makes the least change possible to implement the functionality 2) The convention, at least in the Docker configuration, seems to be that the other variables support the automatic building of MM_SQLSETTINGS_DATASOURCE for PostgreSQL applications, with the documentation recommending override of this variable for MySQL use cases. Therefore, it seemed consistent to extend this and simply make the existing "build" of the variable support this configurable mode for sslmode

Having decided to do this, I was faced with a problem (that incidentally would have arisen even if overriding the MM_SQLSETTINGS_DATASOURCE variable while keeping a URL format). Once one elects to use a mode such as verify-ca or verify-full in PostgreSQL, one must provide the root CA file. Depending on the application, other data such as a certificate revocation list (sslcrl) may be needed also. The URL format for connection strings with PostgreSQL does not always work with these variables, even if one URL-encodes the filenames. I considered changing the default format of the connection string in MM_SQLSETTINGS_DATASOURCE to a parameterized value, but then concluded this would create unnecessary change for users that did not need this new configuration behavior.

Faced with the practical decision to keep the connection string format in current URL format without always having good support for the supplemental configuration information like sslrootcert, I considered creating several new environment variables to handle cases when a user wanted (for example) to specify a CA file in a non-standard location. However, PostgreSQL has built-in support for environment variables PGSSLROOTCERT and PGSSLCRL. So, anyone wishing to use an encrypted connection can supply these environment variables into a Docker container just as one would add any other.

In summary, I feel the approach of adding parameterization for the sslmode value in the MM_SQLSETTINGS_DATASOURCE build-up in entrypoint.sh represents the least overall change while providing the behavior required: 1) because the entrypoint.sh initializes the newly proposed variable DB_SSLMODE to the previously hard-coded value of default, existing applications should run unchanged 2) the syntax for building up MM_SQLSETTINGS_DATASOURCE remains the same, minimizing the possibility that subtle issues might arise for some users if the connection string build-up was made using the variable method 3) the convention of having the entrypoint.sh build-up of MM_SQLSETTINGS_DATASOURCE remains the same for PostgreSQL connections, while users still have ability to override completely 4) there is no unnecessary duplication with additional environment variables, instead leveraging the PostgreSQL-supported variables for specifying CA files and certificate revocation lists when required.

The PostgreSQL reference for encrypted connections can be found here - see sections 33.18.1 and 33.18.2.

[mattermod]

Hello @3leapsdet,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[3leapsdave]

Thanks for catching this - I had not intended that echo to remain! Will update shortly.

On Thu, Dec 10, 2020 at 8:31 AM Carlos Tadeu Panato Junior < notifications@github.com> wrote:

@cpanato requested changes on this pull request.

thanks for your PR, just a small request change

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattermost/mattermost-docker/pull/506#pullrequestreview-549196138, or unsubscribe https://github.com/notifications/unsubscribe-auth/APKMI5OXCBX7RGQW7UHJBU3SUDER7ANCNFSM4UOS4IUQ .