diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js index 6a535a820..877a1e4b7 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -1,6 +1,7 @@ 'use strict' var dns = require('dns') +const _ = require('lodash') var defaults = require('./defaults') @@ -18,7 +19,7 @@ var val = function (key, config, envVar) { return config[key] || envVar || defaults[key] } -var readSSLConfigFromEnvironment = function () { +var readSSLModeFromEnvironment = function () { switch (process.env.PGSSLMODE) { case 'disable': return false @@ -33,6 +34,27 @@ var readSSLConfigFromEnvironment = function () { return defaults.ssl } +var fillInSSLVariablesFromEnvironment = function(existingSsl) { + var caFileName = val('sslrootcert', existingSsl) + var crtFileName = val('sslcert', existingSsl) + var keyFileName = val('sslkey', existingSsl) + + // Only try to load fs if we expect to read from the disk + const fs = caFileName || crtFileName || keyFileName ? require('fs') : null + var result = {} + if (caFileName) { + result.ca = fs.readFileSync(caFileName).toString() + } + if (crtFileName) { + result.cert = fs.readFileSync(crtFileName).toString() + } + if (keyFileName) { + result.key = fs.readFileSync(keyFileName).toString() + } + + return result +} + // Convert arg to a string, surround in single quotes, and escape single quotes and backslashes var quoteParamValue = function (value) { return "'" + ('' + value).replace(/\\/g, '\\\\').replace(/'/g, "\\'") + "'" @@ -78,7 +100,7 @@ class ConnectionParameters { this.binary = val('binary', config) this.options = val('options', config) - this.ssl = typeof config.ssl === 'undefined' ? readSSLConfigFromEnvironment() : config.ssl + this.ssl = typeof config.ssl === 'undefined' ? readSSLModeFromEnvironment() : config.ssl if (typeof this.ssl === 'string') { if (this.ssl === 'true') { @@ -89,6 +111,23 @@ class ConnectionParameters { if (this.ssl === 'no-verify') { this.ssl = { rejectUnauthorized: false } } + + // Fill in possibly missing SSL config parameters from environment variables. + // This lets you mix SSL definitions between connection string, config object and environment variables. + if (this.ssl === true) { + var sslCandidate = fillInSSLVariablesFromEnvironment({}) + + // Change the type of this.ssl from boolean to object only if relevant environment variables were defined + if (!_.isEmpty(sslCandidate)) { + this.ssl = sslCandidate + } + } else if (typeof this.ssl === 'object') { + var sslCandidate = fillInSSLVariablesFromEnvironment({}) + this.ssl = Object.assign(this.ssl, sslCandidate) + } + + // "hiding" the private key so it doesn't show up in stack traces + // or if the client is console.logged if (this.ssl && this.ssl.key) { Object.defineProperty(this.ssl, 'key', { enumerable: false, diff --git a/packages/pg/test/integration/connection-pool/tls-tests.js b/packages/pg/test/integration/connection-pool/tls-tests.js index f85941d45..e1b98af25 100644 --- a/packages/pg/test/integration/connection-pool/tls-tests.js +++ b/packages/pg/test/integration/connection-pool/tls-tests.js @@ -8,6 +8,8 @@ const pg = helper.pg const suite = new helper.Suite() if (process.env.PG_CLIENT_CERT_TEST) { + + // Test SSL using "options object" constructor suite.testAsync('client certificate', async () => { const pool = new pg.Pool({ ssl: { @@ -20,4 +22,13 @@ if (process.env.PG_CLIENT_CERT_TEST) { await pool.query('SELECT 1') await pool.end() }) + + // Test SSL by only defining environment variables without any explicit reference to the cert files in the code + // to be compliant with lib-pg: https://www.postgresql.org/docs/current/libpq-envars.html + suite.testAsync('client certificate', async () => { + const pool = new pg.Pool({}) + + await pool.query('SELECT 1') + await pool.end() + }) } diff --git a/packages/pg/test/unit/connection-parameters/environment-variable-tests.js b/packages/pg/test/unit/connection-parameters/environment-variable-tests.js index b20a7934b..a7a8e4f4d 100644 --- a/packages/pg/test/unit/connection-parameters/environment-variable-tests.js +++ b/packages/pg/test/unit/connection-parameters/environment-variable-tests.js @@ -3,6 +3,11 @@ var helper = require('../test-helper') const Suite = require('../../suite') var assert = require('assert') +const fs = require('fs') + +const tmp = require('tmp') +tmp.setGracefulCleanup() + var ConnectionParameters = require('../../../lib/connection-parameters') var defaults = require('../../../lib').defaults @@ -36,6 +41,38 @@ suite.test('ConnectionParameters initialized from environment variables', functi assert.equal(subject.port, 7890, 'env port') assert.equal(subject.database, 'allyerbase', 'env database') assert.equal(subject.password, 'open', 'env password') + assert.equal(subject.ssl, false, 'ssl') +}) + +suite.test('ConnectionParameters initialized from environment variables - ssl', function () { + createTempTlsFilesAndExecute(function( + certFilePath, keyFilePath, caFilePath, + certFileContents, keyFileContents, caFileContents + ) { + clearEnv() + process.env['PGHOST'] = 'local' + process.env['PGUSER'] = 'bmc2' + process.env['PGPORT'] = 7890 + process.env['PGDATABASE'] = 'allyerbase' + process.env['PGPASSWORD'] = 'open' + + process.env['PGSSLMODE'] = 'verify-full' + process.env['PGSSLCERT'] = certFilePath + process.env['PGSSLKEY'] = keyFilePath + process.env['PGSSLROOTCERT'] = caFilePath + + var subject = new ConnectionParameters() + assert.equal(subject.host, 'local', 'env host') + assert.equal(subject.user, 'bmc2', 'env user') + assert.equal(subject.port, 7890, 'env port') + assert.equal(subject.database, 'allyerbase', 'env database') + assert.equal(subject.password, 'open', 'env password') + + assert.equal(typeof subject.ssl, 'object', 'env ssl') + assert.equal(subject.ssl.cert, certFileContents, 'env ssl cert') + assert.equal(subject.ssl.key, keyFileContents, 'env ssl key') + assert.equal(subject.ssl.ca, caFileContents, 'env ssl ca') + }) }) suite.test('ConnectionParameters initialized from mix', function () { @@ -56,6 +93,77 @@ suite.test('ConnectionParameters initialized from mix', function () { assert.equal(subject.port, 7890, 'env port') assert.equal(subject.database, 'zugzug', 'config database') assert.equal(subject.password, defaults.password, 'defaults password') + assert.equal(subject.ssl, false, 'ssl') +}) + +suite.test('ConnectionParameters initialized from mix - ssl', function () { + createTempTlsFilesAndExecute(function( + certFilePath, keyFilePath, caFilePath, + certFileContents, keyFileContents, caFileContents + ) { + clearEnv() + process.env['PGHOST'] = 'local' + process.env['PGUSER'] = 'bmc2' + process.env['PGPORT'] = 7890 + process.env['PGDATABASE'] = 'allyerbase' + process.env['PGPASSWORD'] = 'open' + process.env['PGSSLMODE'] = 'verify-full' + process.env['PGSSLCERT'] = certFilePath + process.env['PGSSLKEY'] = keyFilePath + delete process.env['PGPASSWORD'] + delete process.env['PGDATABASE'] + + var subject = new ConnectionParameters({ + // The connection string will mostly override this config. See ConnectionParameters constructor. + user: 'testing', + database: 'zugzug', + ssl: { + ca: caFileContents + }, + connectionString: "postgres://user2:pass2@host2:9999/db2" + }) + assert.equal(subject.host, 'host2', 'string host') + assert.equal(subject.user, 'user2', 'string user') + assert.equal(subject.port, 9999, 'string port') + assert.equal(subject.database, 'db2', 'string database') + assert.equal(subject.password, 'pass2', 'string password') + + assert.equal(typeof subject.ssl, 'object', 'env ssl') + assert.equal(subject.ssl.cert, certFileContents, 'env ssl cert') + assert.equal(subject.ssl.key, keyFileContents, 'env ssl key') + assert.equal(subject.ssl.ca, caFileContents, 'config ssl ca') + }) +}) + +suite.test('ConnectionParameters initialized from config - ssl', function () { + createTempTlsFilesAndExecute(function( + certFilePath, keyFilePath, caFilePath, + certFileContents, keyFileContents, caFileContents + ) { + clearEnv() + var subject = new ConnectionParameters({ + host: 'local', + user: 'testing', + password: 'open', + port: 7890, + database: 'zugzug', + ssl: { + cert: certFileContents, + key: keyFileContents, + ca: caFileContents + } + }) + assert.equal(subject.host, 'local', 'env host') + assert.equal(subject.user, 'testing', 'config user') + assert.equal(subject.port, 7890, 'env port') + assert.equal(subject.database, 'zugzug', 'config database') + assert.equal(subject.password, 'open', 'defaults password') + + assert.equal(typeof subject.ssl, 'object', 'config ssl') + assert.equal(subject.ssl.cert, certFileContents, 'config ssl cert') + assert.equal(subject.ssl.key, keyFileContents, 'config ssl key') + assert.equal(subject.ssl.ca, caFileContents, 'config ssl ca') + }) }) suite.test('connection string parsing', function () { @@ -67,6 +175,7 @@ suite.test('connection string parsing', function () { assert.equal(subject.password, 'pw', 'string password') assert.equal(subject.port, 381, 'string port') assert.equal(subject.database, 'lala', 'string database') + assert.equal(subject.ssl, false, 'ssl') }) suite.test('connection string parsing - ssl', function () { @@ -104,6 +213,33 @@ suite.test('ssl is false by default', function () { assert.equal(subject.ssl, false) }) +// Create temp TLS certificate-mock files and run test logic inside this context +function createTempTlsFilesAndExecute(callback) { + tmp.dir(function _tempDirCreated(err, tmpdir) { + if (err) throw err; + + const certFilePath = tmpdir + '/client.crt' + const keyFilePath = tmpdir + '/client.key' + const caFilePath = tmpdir + '/ca.crt' + + const certFileContents = 'client cert file' + const keyFileContents = 'client key file' + const caFileContents = 'CA cert file' + + fs.appendFileSync(certFilePath, certFileContents, function (err) { + if (err) throw err; + }) + fs.appendFileSync(keyFilePath, keyFileContents, function (err) { + if (err) throw err; + }) + fs.appendFileSync(caFilePath, caFileContents, function (err) { + if (err) throw err; + }) + + callback(certFilePath, keyFilePath, caFilePath, certFileContents, keyFileContents, caFileContents) + }) +} + var testVal = function (mode, expected) { suite.test('ssl is ' + expected + ' when $PGSSLMODE=' + mode, function () { clearEnv()