Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configure database connection ssl settings #220

Open
pbolduc opened this issue Oct 12, 2023 · 3 comments
Open

Allow configure database connection ssl settings #220

pbolduc opened this issue Oct 12, 2023 · 3 comments

Comments

@pbolduc
Copy link
Contributor

pbolduc commented Oct 12, 2023

Is your feature request related to a problem? Please describe.

I am trying to use the Crunchy Postgres Operator (PGO) for deploying Postgres. PGO enforces that all connections are over TLS. In the current version, there appears no way to configure SSL. Knexjs allows setting the SSL settings - https://knexjs.org/guide/#configuration-options. For example,

  connection: {
    connectionString: config.DATABASE_URL,
    host: config["DB_HOST"],
    port: config["DB_PORT"],
    user: config["DB_USER"],
    database: config["DB_NAME"],
    password: config["DB_PASSWORD"],
    ssl: config["DB_SSL"] ? { rejectUnauthorized: false } : false,
  }

The following file would need the extra ssl property.

Without being able to enable the SSL, I get the following error when COMS tries to connect to the database.

> common-object-management-service@0.4.2 migrate
> npm run migrate:latest


> common-object-management-service@0.4.2 migrate:latest
> knex migrate:latest

no pg_hba.conf entry for host "10.97.120.207", user "postgres", database "postgres", no encryption
error: no pg_hba.conf entry for host "10.97.120.207", user "postgres", database "postgres", no encryption
    at Parser.parseErrorMessage (/opt/app-root/src/app/node_modules/pg-protocol/dist/parser.js:287:98)
    at Parser.handlePacket (/opt/app-root/src/app/node_modules/pg-protocol/dist/parser.js:126:29)
    at Parser.parse (/opt/app-root/src/app/node_modules/pg-protocol/dist/parser.js:39:38)
    at Socket.<anonymous> (/opt/app-root/src/app/node_modules/pg-protocol/dist/index.js:11:42)
    at Socket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)
    at Readable.push (node:internal/streams/readable:234:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23)
npm notice 
npm notice New major version of npm available! 8.19.2 -> 10.2.0
npm notice Changelog: <https://github.com/npm/cli/releases/tag/v10.2.0>
npm notice Run `npm install -g npm@10.2.0` to update!
npm notice 

The pg_hba.conf is

sh-4.4$ cat /opt/crunchy/conf/postgres/pg_hba.conf 
# PostgreSQL Client Authentication Configuration File
# ===================================================
#
# Refer to the "Client Authentication" section in the PostgreSQL
# documentation for a complete description of this file.  A short
# synopsis follows.
#
# This file controls: which hosts are allowed to connect, how clients
# are authenticated, which PostgreSQL user names they can use, which
# databases they can access.  Records take one of these forms:
#
# local      DATABASE  USER  METHOD  [OPTIONS]
# host       DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
# hostssl    DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
# hostnossl  DATABASE  USER  ADDRESS  METHOD  [OPTIONS]
#
# (The uppercase items must be replaced by actual values.)
#
# The first field is the connection type: "local" is a Unix-domain
# socket, "host" is either a plain or SSL-encrypted TCP/IP socket,
# "hostssl" is an SSL-encrypted TCP/IP socket, and "hostnossl" is a
# plain TCP/IP socket.
#
# DATABASE can be "all", "sameuser", "samerole", "replication", a
# database name, or a comma-separated list thereof. The "all"
# keyword does not match "replication". Access to replication
# must be enabled in a separate record (see example below).
#
# USER can be "all", a user name, a group name prefixed with "+", or a
# comma-separated list thereof.  In both the DATABASE and USER fields
# you can also write a file name prefixed with "@" to include names
# from a separate file.
#
# ADDRESS specifies the set of hosts the record matches.  It can be a
# host name, or it is made up of an IP address and a CIDR mask that is
# an integer (between 0 and 32 (IPv4) or 128 (IPv6) inclusive) that
# specifies the number of significant bits in the mask.  A host name
# that starts with a dot (.) matches a suffix of the actual host name.
# Alternatively, you can write an IP address and netmask in separate
# columns to specify the set of hosts.  Instead of a CIDR-address, you
# can write "samehost" to match any of the server's own IP addresses,
# or "samenet" to match any address in any subnet that the server is
# directly connected to.
#
# METHOD can be "trust", "reject", "md5", "password", "gss", "sspi",
# "krb5", "ident", "peer", "pam", "ldap", "radius" or "cert".  Note that
# "password" sends passwords in clear text; "md5" is preferred since
# it sends encrypted passwords.
#
# OPTIONS are a set of options for the authentication in the format
# NAME=VALUE.  The available options depend on the different
# authentication methods -- refer to the "Client Authentication"
# section in the documentation for a list of which options are
# available for which authentication methods.
#
# Database and user names containing spaces, commas, quotes and other
# special characters must be quoted.  Quoting one of the keywords
# "all", "sameuser", "samerole" or "replication" makes the name lose
# its special character, and just match a database or username with
# that name.
#
# This file is read on server startup and when the postmaster receives
# a SIGHUP signal.  If you edit the file on a running system, you have
# to SIGHUP the postmaster for the changes to take effect.  You can
# use "pg_ctl reload" to do that.

# Put your actual configuration here
# ----------------------------------
#
# If you want to allow non-local connections, you need to add more
# "host" records.  In that case you will also need to make PostgreSQL
# listen on a non-local interface via the listen_addresses
# configuration parameter, or via the -i or -h command line switches.

# CAUTION: Configuring the system for local "trust" authentication
# allows any local user to connect as any PostgreSQL user, including
# the database superuser.  If you do not trust all your local users,
# use another authentication method.


# TYPE  DATABASE        USER            ADDRESS                 METHOD

# "local" is for Unix domain socket connections only
local   all             all                                     trust
# IPv4 local connections:
host    all             all              127.0.0.1/32           md5
host    all             all              0.0.0.0/0              md5
host    replication     PG_PRIMARY_USER  0.0.0.0/0              md5
# IPv6 local connections:
#host    all             all             ::1/128                trust
# Allow replication connections from localhost, by a user with the
# replication privilege.
#local   replication     postgres                               trust
#host    replication     postgres        127.0.0.1/32           trust
#host    replication     postgres        ::1/128                trust
sh-4.4$ 

Version Number

I am still on 0.4.2, however, looking at the latest version, 0.6.0 it does not appear to support SSL.

Describe the solution you'd like

I would like to enable ssl when connecting to Postgres.

Describe alternatives you've considered

none

Additional context

n/a

@jujaga
Copy link
Member

jujaga commented Oct 12, 2023

I did a quick check of the Knex typescript interfaces, and it seems like their example of rejectUnauthorized is a MariaDB specific parameter. The Knex config uses the Config interface, which can take in an optional StaticConnectionConfig interface, which unions a bunch of configuration types - notably MariaSqlConnectionConfig and PgConnectionConfig. Both of those interfaces do accept an optional ssl boolean or object attribute, but tracking down rejectUnauthorized it appears to be under just the MariaSslConfiguration interface; it does not appear under PgConnectionConfig interface, which leads to the generic TLS ConnectionOptions interface provided by node. I'm going to go ahead and slip in the recommended line based on their documentation recommendation and hope that is sufficient for enabling TLS support.

@pbolduc
Copy link
Contributor Author

pbolduc commented Oct 12, 2023

I was thinking about making it easier by just allowing setting the connecting string directly instead of the fields as Knex allows,

knexfile.js

module.exports = {
  client: 'pg',
  connection: {
    // connectionString is highest priority to use. If left unspecified then connection
    // details will be determined using the individual connection fields (host, port, etc)
    connectionString: config.get('db.connectionString'),
    host: config.get('db.host'),
    user: config.get('db.username'),
    password: config.get('db.password'),
    database: config.get('db.database'),
    port: config.get('db.port')
  },

custom-environment-variables.json

  "db": {
  "connectionString": "DB_CONNECTION_STRING",
  "database": "DB_DATABASE",
  "host": "DB_HOST",
  "password": "DB_PASSWORD",
  "poolMin": "DB_POOL_MIN",
  "poolMax": "DB_POOL_MAX",
  "port": "DB_PORT",
  "username": "DB_USERNAME"
},

@jujaga
Copy link
Member

jujaga commented Oct 12, 2023

We can consider exposing the connectionString for a release after v0.7.0. However, if we were to proceed with this, this would also need a full implementation into the existing helm chart to ensure that the optionality behavior of the connectionString (specifically ensuring that we are optionally taking in a connection string via secret insertion, and only putting in the variable lookup in the deploymentconfig when it exists, as well as excluding out the other db.* values safely). We'll add this to our backlog for consideration thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants