feat(pg): support SSL certs

[mattkrick]

Signed-off-by: Matt Krick [email protected]

Description

Fixes #7174

Demo

N/A

Testing scenarios

• test without changing anything, connecting to PG still works
• add self signed certs root.crt, postgresql.key, postgresql.crt to packages/server/postgres. You are now connecting via SSL
• note: if any of the files are not found, SSL mode will default to process.env.PGSSLMODE per https://github.com/brianc/node-postgres/blob/4b229275cfe41ca17b7d69bd39f91ada0068a5d0/packages/pg-connection-string/README.md#tcp-connections

@rafaelromcar-parabol could I ask you to test this? I think you can probably self sign a cert faster than me 🙂

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[rafaelromcar-parabol]

Just in case, I'm taking this but not yet.. Review is coming soon.

[rafaelromcar-parabol]

Why is the folder __PROJECT_ROOT__/packages/server/postgres not taken from an environment variable? That folder could be the default value if a parameter named PGSSLPATH is empty.

That would allow much more flexibility and remove the data from the application folder.

[rafaelromcar-parabol]

IMHO this PR should have documentation on how to enable SSL for PostgreSQL.

A md file or in the general README.md with the instructions and parameters describing how to configure it. Like what should be in each one of the files root.crt, postgresql.key, postgresql.crt, in which folder those files should be and anything else that could help.

[mattkrick]

documentation added! ready to merge as soon as tests pass

[rafaelromcar-parabol]

I changed rejectUnauthorized: true to false because the certificate provided by GCP CloudSQL only has a as name of the server the one used by the CloudSQL Proxy (we are going to use it in the future). You can see other similar cases on internet like this one.

IMHO, this could be improved using a parameter PG_SSL_REJECT_UNAUTHORIZED. We could set it to true or false depending on the use case. If you think we should do it that way, please add that to the PR. If not, it is working!

[rafaelromcar-parabol]

Forgot to share the error I saw

node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

Error [ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames: Host: postgresql.test-instance-6.parabol.local. is not cert's CN: parabol-proving-ground:test-instance-6-postgresql-1b252b94
    at new NodeError (node:internal/errors:372:5)
    at Object.checkServerIdentity (node:tls:346:12)
    at TLSSocket.onConnectSecure (node:_tls_wrap:1542:27)
    at TLSSocket.emit (node:events:527:28)
    at TLSSocket.emit (node:domain:475:12)
    at TLSSocket._finishInit (node:_tls_wrap:946:8)
    at TLSWrap.ssl.onhandshakedone (node:_tls_wrap:727:12) {
  reason: "Host: postgresql.test-instance-6.parabol.local. is not cert's CN: parabol-proving-ground:test-instance-6-postgresql-1b252b94",
  host: 'postgresql.test-instance-6.parabol.local',

IT GOES AND GOES with more stuff that are less relevant

[Dschoordsch]

-1 I don't know. To me it seems to be a setup issue on our side and I would rather fix that than disabling this check. Basing this solely on this comment: GoogleCloudPlatform/nodejs-docs-samples#1735 (comment)

[Dschoordsch]

-1 I don't know. To me it seems to be a setup issue on our side and I would rather fix that than disabling this check. Basing this solely on this comment: GoogleCloudPlatform/nodejs-docs-samples#1735 (comment)

[rafaelromcar-parabol]

Problem found when running migrations. It shows an error

yarn run v1.22.19
$ node scripts/migrate.js
[migrate-rethinkdb] No new migrations
Done in 0.40s.
yarn run v1.22.19
$ node-pg-migrate -f ./packages/server/postgres/pgmConfig.js up
could not connect to postgres: error: pg_hba.conf rejects connection for host "10.0.85.23", user "parabol-admin", database "parabol", SSL off
    at Parser.parseErrorMessage (/home/node/parabol/node_modules/pg-protocol/src/parser.ts:369:69)
    at Parser.handlePacket (/home/node/parabol/node_modules/pg-protocol/src/parser.ts:188:21)
    at Parser.parse (/home/node/parabol/node_modules/pg-protocol/src/parser.ts:103:30)
    at Socket.<anonymous> (/home/node/parabol/node_modules/pg-protocol/src/index.ts:7:48)
    at Socket.emit (node:events:527:28)
    at Socket.emit (node:domain:475:12)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9)
    at Socket.Readable.push (node:internal/streams/readable:228:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23) {
  length: 166,
  severity: 'FATAL',
  code: '28000',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'auth.c',
  line: '445',
  routine: 'ClientAuthentication'
}
[...] 
error Command failed with exit code 1.

The PG migrations are using the file ./packages/server/postgres/pgmConfig.js as config, which does not have any SSL configuration there. Can you add it as in packages/server/postgres/getPgConfig.ts?

[rafaelromcar-parabol]

Suggestion: change all variables here from PGXXX to POSTGRES_SSL_XXX to follow the same format.

Current variables

POSTGRES_PASSWORD=''
POSTGRES_USER=''
POSTGRES_PASSWORD=''
POSTGRES_USER=''
POSTGRES_DB=''
POSTGRES_HOST=''
POSTGRES_PORT=''
PGADMIN_DEFAULT_EMAIL=''
PGADMIN_DEFAULT_PASSWORD=''

Added in the PR so far:

PGSSLMODE=""
PG_SSL_REJECT_UNAUTHORIZED=""
PG_CERT_DIR=""

Proposed:

POSTGRES_SSL_MODE=""
POSTGRES_SSL_REJECT_UNAUTHORIZED=""
POSTGRES_SSL_CERT_DIR=""
""