From 793018d1e339062e7920ec63e6b555b1e943f3e0 Mon Sep 17 00:00:00 2001 From: delvedor Date: Thu, 22 Jul 2021 14:24:13 +0200 Subject: [PATCH 01/22] WIP: support CA fingerprint validation --- index.js | 2 ++ lib/Connection.js | 26 ++++++++++++++++++++++++++ lib/pool/BaseConnectionPool.js | 3 +++ 3 files changed, 31 insertions(+) diff --git a/index.js b/index.js index 60c44014a..4e880dc0f 100644 --- a/index.js +++ b/index.js @@ -102,6 +102,7 @@ class Client extends ESAPI { suggestCompression: false, compression: false, ssl: null, + certFingerprint: null, agent: null, headers: {}, nodeFilter: null, @@ -146,6 +147,7 @@ class Client extends ESAPI { Connection: options.Connection, auth: options.auth, emit: this[kEventEmitter].emit.bind(this[kEventEmitter]), + certFingerprint: options.certFingerprint, sniffEnabled: options.sniffInterval !== false || options.sniffOnStart !== false || options.sniffOnConnectionFault !== false diff --git a/lib/Connection.js b/lib/Connection.js index 6eda7c539..4b5595725 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -42,6 +42,7 @@ class Connection { this.headers = prepareHeaders(opts.headers, opts.auth) this.deadCount = 0 this.resurrectTimeout = 0 + this.certFingerprint = opts.certFingerprint this._openRequests = 0 this._status = opts.status || Connection.statuses.ALIVE @@ -123,10 +124,34 @@ class Connection { callback(new RequestAbortedError(), null) } + const onSocket = socket => { + /* istanbul ignore else */ + if (!socket.isSessionReused()) { + socket.once('secureConnect', () => { + // Check if certificate is valid + if (socket.authorized === false) { + onError(new Error(socket.authorizationError)) + request.once('error', () => {}) // we need to catch the request aborted error + return request.abort() + } + + // Check if fingerprint matches + if (this.certFingerprint !== socket.getPeerCertificate().fingerprint) { + onError(new Error('Fingerprint does not match')) + request.once('error', () => {}) // we need to catch the request aborted error + return request.abort() + } + }) + } + } + request.on('response', onResponse) request.on('timeout', onTimeout) request.on('error', onError) request.on('abort', onAbort) + if (this.certFingerprint != null) { + request.on('socket', onSocket) + } // Disables the Nagle algorithm request.setNoDelay(true) @@ -152,6 +177,7 @@ class Connection { request.removeListener('timeout', onTimeout) request.removeListener('error', onError) request.removeListener('abort', onAbort) + request.removeListener('socket', onSocket) cleanedListeners = true } } diff --git a/lib/pool/BaseConnectionPool.js b/lib/pool/BaseConnectionPool.js index 2b3081153..ba01a60c6 100644 --- a/lib/pool/BaseConnectionPool.js +++ b/lib/pool/BaseConnectionPool.js @@ -36,6 +36,7 @@ class BaseConnectionPool { this._ssl = opts.ssl this._agent = opts.agent this._proxy = opts.proxy || null + this._certFingerprint = opts.certFingerprint || null } getConnection () { @@ -72,6 +73,8 @@ class BaseConnectionPool { if (opts.agent == null) opts.agent = this._agent /* istanbul ignore else */ if (opts.proxy == null) opts.proxy = this._proxy + /* istanbul ignore else */ + if (opts.certFingerprint == null) opts.certFingerprint = this._certFingerprint const connection = new this.Connection(opts) From 8f60fa07128f361bd7d488ca97eb47f91354424f Mon Sep 17 00:00:00 2001 From: delvedor Date: Thu, 22 Jul 2021 14:24:20 +0200 Subject: [PATCH 02/22] Updated test --- test/unit/connection.test.js | 57 +++++++++++++++++++++++++++++++++++- test/utils/buildServer.js | 17 ++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/test/unit/connection.test.js b/test/unit/connection.test.js index a951a9e6b..472bd42fa 100644 --- a/test/unit/connection.test.js +++ b/test/unit/connection.test.js @@ -28,7 +28,7 @@ const hpagent = require('hpagent') const intoStream = require('into-stream') const { buildServer } = require('../utils') const Connection = require('../../lib/Connection') -const { TimeoutError, ConfigurationError, RequestAbortedError } = require('../../lib/errors') +const { TimeoutError, ConfigurationError, RequestAbortedError, ConnectionError } = require('../../lib/errors') test('Basic (http)', t => { t.plan(4) @@ -947,3 +947,58 @@ test('Abort with a slow body', t => { setImmediate(() => request.abort()) }) + +test('Check server fingerprint (success)', t => { + t.plan(2) + + function handler (req, res) { + res.end('ok') + } + + buildServer(handler, { secure: true }, ({ port, certFingerprint }, server) => { + const connection = new Connection({ + url: new URL(`https://localhost:${port}`), + certFingerprint + }) + + connection.request({ + path: '/hello', + method: 'GET' + }, (err, res) => { + t.error(err) + + let payload = '' + res.setEncoding('utf8') + res.on('data', chunk => { payload += chunk }) + res.on('error', err => t.fail(err)) + res.on('end', () => { + t.equal(payload, 'ok') + server.stop() + }) + }) + }) +}) + +test('Check server fingerprint (failure)', t => { + t.plan(2) + + function handler (req, res) { + res.end('ok') + } + + buildServer(handler, { secure: true }, ({ port }, server) => { + const connection = new Connection({ + url: new URL(`https://localhost:${port}`), + certFingerprint: 'FO:OB:AR' + }) + + connection.request({ + path: '/hello', + method: 'GET' + }, (err, res) => { + t.ok(err instanceof ConnectionError) + t.equal(err.message, 'Fingerprint does not match') + server.stop() + }) + }) +}) diff --git a/test/utils/buildServer.js b/test/utils/buildServer.js index b47b2fec2..d0ba750e2 100644 --- a/test/utils/buildServer.js +++ b/test/utils/buildServer.js @@ -19,6 +19,7 @@ 'use strict' +const crypto = require('crypto') const debug = require('debug')('elasticsearch-test') const stoppable = require('stoppable') @@ -35,6 +36,13 @@ const secureOpts = { cert: readFileSync(join(__dirname, '..', 'fixtures', 'https.cert'), 'utf8') } +const certFingerprint = getFingerprint(secureOpts.cert + .split('\n') + .slice(1, -1) + .map(line => line.trim()) + .join('') +) + let id = 0 function buildServer (handler, opts, cb) { const serverId = id++ @@ -58,7 +66,7 @@ function buildServer (handler, opts, cb) { server.listen(0, () => { const port = server.address().port debug(`Server '${serverId}' booted on port ${port}`) - resolve([Object.assign({}, secureOpts, { port }), server]) + resolve([Object.assign({}, secureOpts, { port, certFingerprint }), server]) }) }) } else { @@ -70,4 +78,11 @@ function buildServer (handler, opts, cb) { } } +function getFingerprint (content, inputEncoding = 'base64', outputEncoding = 'hex') { + const shasum = crypto.createHash('sha1') + shasum.update(content, inputEncoding) + const res = shasum.digest(outputEncoding) + return res.toUpperCase().match(/.{1,2}/g).join(':') +} + module.exports = buildServer From ad3b90bf7e9479d3ff5999c075a50f4ab32e8c9a Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 26 Jul 2021 16:19:38 +0200 Subject: [PATCH 03/22] Address feedback --- index.d.ts | 1 + index.js | 4 ++-- lib/Connection.d.ts | 1 + lib/Connection.js | 29 +++++++++++++++++++---------- lib/pool/BaseConnectionPool.js | 4 ++-- lib/pool/index.d.ts | 1 + 6 files changed, 26 insertions(+), 14 deletions(-) diff --git a/index.d.ts b/index.d.ts index e7d00efeb..0dae77481 100644 --- a/index.d.ts +++ b/index.d.ts @@ -118,6 +118,7 @@ interface ClientOptions { password?: string; }; disablePrototypePoisoningProtection?: boolean | 'proto' | 'constructor'; + caFingerprint?: string; } declare class Client { diff --git a/index.js b/index.js index 4e880dc0f..ceae0fc4c 100644 --- a/index.js +++ b/index.js @@ -102,7 +102,7 @@ class Client extends ESAPI { suggestCompression: false, compression: false, ssl: null, - certFingerprint: null, + caFingerprint: null, agent: null, headers: {}, nodeFilter: null, @@ -147,7 +147,7 @@ class Client extends ESAPI { Connection: options.Connection, auth: options.auth, emit: this[kEventEmitter].emit.bind(this[kEventEmitter]), - certFingerprint: options.certFingerprint, + caFingerprint: options.caFingerprint, sniffEnabled: options.sniffInterval !== false || options.sniffOnStart !== false || options.sniffOnConnectionFault !== false diff --git a/lib/Connection.d.ts b/lib/Connection.d.ts index 933a6a8eb..6b5c6cb7d 100644 --- a/lib/Connection.d.ts +++ b/lib/Connection.d.ts @@ -40,6 +40,7 @@ export interface ConnectionOptions { roles?: ConnectionRoles; auth?: BasicAuth | ApiKeyAuth; proxy?: string | URL; + caFingerprint?: string; } interface ConnectionRoles { diff --git a/lib/Connection.js b/lib/Connection.js index 4b5595725..da552f757 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -42,7 +42,7 @@ class Connection { this.headers = prepareHeaders(opts.headers, opts.auth) this.deadCount = 0 this.resurrectTimeout = 0 - this.certFingerprint = opts.certFingerprint + this.caFingerprint = opts.caFingerprint this._openRequests = 0 this._status = opts.status || Connection.statuses.ALIVE @@ -128,15 +128,8 @@ class Connection { /* istanbul ignore else */ if (!socket.isSessionReused()) { socket.once('secureConnect', () => { - // Check if certificate is valid - if (socket.authorized === false) { - onError(new Error(socket.authorizationError)) - request.once('error', () => {}) // we need to catch the request aborted error - return request.abort() - } - // Check if fingerprint matches - if (this.certFingerprint !== socket.getPeerCertificate().fingerprint) { + if (this.caFingerprint !== getIssuerCertificate(socket).fingerprint256) { onError(new Error('Fingerprint does not match')) request.once('error', () => {}) // we need to catch the request aborted error return request.abort() @@ -149,7 +142,7 @@ class Connection { request.on('timeout', onTimeout) request.on('error', onError) request.on('abort', onAbort) - if (this.certFingerprint != null) { + if (this.caFingerprint != null) { request.on('socket', onSocket) } @@ -366,5 +359,21 @@ function prepareHeaders (headers = {}, auth) { return headers } +function getIssuerCertificate (socket) { + let certificate = socket.getPeerCertificate(true) + while (certificate && Object.keys(certificate).length > 0) { + if (certificate.issuerCertificate !== undefined) { + // For self-signed certificates, `issuerCertificate` may be a circular reference. + if (certificate.fingerprint256 === certificate.issuerCertificate.fingerprint256) { + break + } + certificate = certificate.issuerCertificate + } else { + break + } + } + return certificate +} + module.exports = Connection module.exports.internals = { prepareHeaders } diff --git a/lib/pool/BaseConnectionPool.js b/lib/pool/BaseConnectionPool.js index ba01a60c6..a168adaa8 100644 --- a/lib/pool/BaseConnectionPool.js +++ b/lib/pool/BaseConnectionPool.js @@ -36,7 +36,7 @@ class BaseConnectionPool { this._ssl = opts.ssl this._agent = opts.agent this._proxy = opts.proxy || null - this._certFingerprint = opts.certFingerprint || null + this._certFingerprint = opts.caFingerprint || null } getConnection () { @@ -74,7 +74,7 @@ class BaseConnectionPool { /* istanbul ignore else */ if (opts.proxy == null) opts.proxy = this._proxy /* istanbul ignore else */ - if (opts.certFingerprint == null) opts.certFingerprint = this._certFingerprint + if (opts.caFingerprint == null) opts.caFingerprint = this._certFingerprint const connection = new this.Connection(opts) diff --git a/lib/pool/index.d.ts b/lib/pool/index.d.ts index c1ebbdad6..7b3f62f94 100644 --- a/lib/pool/index.d.ts +++ b/lib/pool/index.d.ts @@ -31,6 +31,7 @@ interface BaseConnectionPoolOptions { auth?: BasicAuth | ApiKeyAuth; emit: (event: string | symbol, ...args: any[]) => boolean; Connection: typeof Connection; + caFingerprint?: string; } interface ConnectionPoolOptions extends BaseConnectionPoolOptions { From d4844f6313ecf6104683487871befe0c22c39fc1 Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 26 Jul 2021 16:19:42 +0200 Subject: [PATCH 04/22] Updated test --- test/acceptance/sniff.test.js | 1 + test/unit/connection.test.js | 6 +++--- test/utils/buildServer.js | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/test/acceptance/sniff.test.js b/test/acceptance/sniff.test.js index 5dfaa3f76..d18e8a2a9 100644 --- a/test/acceptance/sniff.test.js +++ b/test/acceptance/sniff.test.js @@ -77,6 +77,7 @@ test('Should update the connection pool', t => { t.same(hosts[i], { url: new URL(nodes[id].url), id: id, + caFingerprint: null, roles: { master: true, data: true, diff --git a/test/unit/connection.test.js b/test/unit/connection.test.js index 472bd42fa..d9f33d47c 100644 --- a/test/unit/connection.test.js +++ b/test/unit/connection.test.js @@ -955,10 +955,10 @@ test('Check server fingerprint (success)', t => { res.end('ok') } - buildServer(handler, { secure: true }, ({ port, certFingerprint }, server) => { + buildServer(handler, { secure: true }, ({ port, caFingerprint }, server) => { const connection = new Connection({ url: new URL(`https://localhost:${port}`), - certFingerprint + caFingerprint }) connection.request({ @@ -989,7 +989,7 @@ test('Check server fingerprint (failure)', t => { buildServer(handler, { secure: true }, ({ port }, server) => { const connection = new Connection({ url: new URL(`https://localhost:${port}`), - certFingerprint: 'FO:OB:AR' + caFingerprint: 'FO:OB:AR' }) connection.request({ diff --git a/test/utils/buildServer.js b/test/utils/buildServer.js index d0ba750e2..ef907c05f 100644 --- a/test/utils/buildServer.js +++ b/test/utils/buildServer.js @@ -36,7 +36,7 @@ const secureOpts = { cert: readFileSync(join(__dirname, '..', 'fixtures', 'https.cert'), 'utf8') } -const certFingerprint = getFingerprint(secureOpts.cert +const caFingerprint = getFingerprint(secureOpts.cert .split('\n') .slice(1, -1) .map(line => line.trim()) @@ -66,7 +66,7 @@ function buildServer (handler, opts, cb) { server.listen(0, () => { const port = server.address().port debug(`Server '${serverId}' booted on port ${port}`) - resolve([Object.assign({}, secureOpts, { port, certFingerprint }), server]) + resolve([Object.assign({}, secureOpts, { port, caFingerprint }), server]) }) }) } else { @@ -79,7 +79,7 @@ function buildServer (handler, opts, cb) { } function getFingerprint (content, inputEncoding = 'base64', outputEncoding = 'hex') { - const shasum = crypto.createHash('sha1') + const shasum = crypto.createHash('sha256') shasum.update(content, inputEncoding) const res = shasum.digest(outputEncoding) return res.toUpperCase().match(/.{1,2}/g).join(':') From b0f786f559e2616ad94ecaf0f34e7a901d794cac Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 26 Jul 2021 16:35:57 +0200 Subject: [PATCH 05/22] Moar test --- test/unit/client.test.js | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/unit/client.test.js b/test/unit/client.test.js index d9a26c110..418d8dc02 100644 --- a/test/unit/client.test.js +++ b/test/unit/client.test.js @@ -1498,3 +1498,44 @@ test('Bearer auth', t => { }) }) }) + +test('Check server fingerprint (success)', t => { + t.plan(1) + + function handler (req, res) { + res.end('ok') + } + + buildServer(handler, { secure: true }, ({ port, caFingerprint }, server) => { + const client = new Client({ + node: `https://localhost:${port}`, + caFingerprint + }) + + client.info((err, res) => { + t.error(err) + server.stop() + }) + }) +}) + +test('Check server fingerprint (failure)', t => { + t.plan(2) + + function handler (req, res) { + res.end('ok') + } + + buildServer(handler, { secure: true }, ({ port }, server) => { + const client = new Client({ + node: `https://localhost:${port}`, + caFingerprint: 'FO:OB:AR' + }) + + client.info((err, res) => { + t.ok(err instanceof errors.ConnectionError) + t.equal(err.message, 'Fingerprint does not match') + server.stop() + }) + }) +}) From ebbbee889463404061241fdb858ab38f61983c5e Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 26 Jul 2021 16:36:01 +0200 Subject: [PATCH 06/22] Updated docs --- docs/basic-config.asciidoc | 4 ++++ docs/connecting.asciidoc | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/docs/basic-config.asciidoc b/docs/basic-config.asciidoc index 04e95ca9e..7678b37fc 100644 --- a/docs/basic-config.asciidoc +++ b/docs/basic-config.asciidoc @@ -255,4 +255,8 @@ const client = new Client({ |`boolean`, `'proto'`, `'constructor'` - By the default the client will protect you against prototype poisoning attacks. Read https://web.archive.org/web/20200319091159/https://hueniverse.com/square-brackets-are-the-enemy-ff5b9fd8a3e8?gi=184a27ee2a08[this article] to learn more. If needed you can disable prototype poisoning protection entirely or one of the two checks. Read the `secure-json-parse` https://github.com/fastify/secure-json-parse[documentation] to learn more. + _Default:_ `false` +|`caFingerprint` +|`string` - If configured, verify the supplied certificate authority fingerprint. + +_Default:_ `null` + |=== diff --git a/docs/connecting.asciidoc b/docs/connecting.asciidoc index 529046e36..0bbdded28 100644 --- a/docs/connecting.asciidoc +++ b/docs/connecting.asciidoc @@ -176,6 +176,28 @@ const client = new Client({ }) ---- +[discrete] +[[auth-ca-fingerprint]] +==== CA fingerprint + +To improve the security of your connection to Elasticsearch, you can provide +a `caFingerprint` option, which will verify the supplied certificate authority fingerprint. + +[source,js] +---- +const { Client } = require('@elastic/elasticsearch') +const client = new Client({ + node: 'https://example.com' + auth: { ... }, + // the fingerprint (SHA256) of the CA certificate that is used to sign the certificate that the Elasticsearch node presents for TLS. + caFingerprint: 'SHA-256-DER', + ssl: { + // might be required if it's a self-signed certificate + rejectUnauthorized: false + } +}) +---- + [discrete] [[client-usage]] From b7b05454bac995b43573cc35dcc18b5f4e8fa0f7 Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 26 Jul 2021 16:52:00 +0200 Subject: [PATCH 07/22] Improve example --- docs/connecting.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/connecting.asciidoc b/docs/connecting.asciidoc index 0bbdded28..9f743d4b9 100644 --- a/docs/connecting.asciidoc +++ b/docs/connecting.asciidoc @@ -190,7 +190,7 @@ const client = new Client({ node: 'https://example.com' auth: { ... }, // the fingerprint (SHA256) of the CA certificate that is used to sign the certificate that the Elasticsearch node presents for TLS. - caFingerprint: 'SHA-256-DER', + caFingerprint: '20:0D:CA:FA:76:...', ssl: { // might be required if it's a self-signed certificate rejectUnauthorized: false From d0c104030a60752a786258a4967f8f9c97926387 Mon Sep 17 00:00:00 2001 From: delvedor Date: Mon, 26 Jul 2021 18:09:08 +0200 Subject: [PATCH 08/22] Typo --- lib/pool/BaseConnectionPool.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pool/BaseConnectionPool.js b/lib/pool/BaseConnectionPool.js index a168adaa8..80e80a318 100644 --- a/lib/pool/BaseConnectionPool.js +++ b/lib/pool/BaseConnectionPool.js @@ -36,7 +36,7 @@ class BaseConnectionPool { this._ssl = opts.ssl this._agent = opts.agent this._proxy = opts.proxy || null - this._certFingerprint = opts.caFingerprint || null + this._caFingerprint = opts.caFingerprint || null } getConnection () { @@ -74,7 +74,7 @@ class BaseConnectionPool { /* istanbul ignore else */ if (opts.proxy == null) opts.proxy = this._proxy /* istanbul ignore else */ - if (opts.caFingerprint == null) opts.caFingerprint = this._certFingerprint + if (opts.caFingerprint == null) opts.caFingerprint = this._caFingerprint const connection = new this.Connection(opts) From a5c59db792075b281dc7d1185120d53769aa24f7 Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Tue, 27 Jul 2021 13:55:54 +0200 Subject: [PATCH 09/22] Update docs/basic-config.asciidoc Co-authored-by: Aleh Zasypkin --- docs/basic-config.asciidoc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/basic-config.asciidoc b/docs/basic-config.asciidoc index 6de2b21d5..c2ffff0dd 100644 --- a/docs/basic-config.asciidoc +++ b/docs/basic-config.asciidoc @@ -256,7 +256,7 @@ const client = new Client({ _Default:_ `false` |`caFingerprint` -|`string` - If configured, verify the supplied certificate authority fingerprint. + +|`string` - If configured, verify the supplied CA certificate fingerprint. + _Default:_ `null` |=== @@ -280,4 +280,3 @@ const client = new Client({ disablePrototypePoisoningProtection: true }) ---- - From ff279ccbc699b8bfb61cb3e64cb31c7bfbae656f Mon Sep 17 00:00:00 2001 From: delvedor Date: Tue, 27 Jul 2021 14:11:46 +0200 Subject: [PATCH 10/22] Throw if caFingerprint is configured with http --- index.js | 17 +++++++++++++++++ lib/Connection.js | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/index.js b/index.js index ceae0fc4c..e240a6208 100644 --- a/index.js +++ b/index.js @@ -117,6 +117,10 @@ class Client extends ESAPI { disablePrototypePoisoningProtection: false }, opts) + if (options.caFingerprint !== null && isHttpConnection(opts.node || opts.nodes)) { + throw new ConfigurationError('You can\'t configure the caFingerprint with a http connection') + } + if (process.env.ELASTIC_CLIENT_APIVERSIONING === 'true') { options.headers = Object.assign({ accept: 'application/vnd.elasticsearch+json; compatible-with=7' }, options.headers) } @@ -317,6 +321,19 @@ function getAuth (node) { } } +function isHttpConnection (node) { + if (Array.isArray(node)) { + for (const n of node) { + if (new URL(n).protocol === 'http:') { + return true + } + } + return false + } else { + return new URL(node).protocol === 'http:' + } +} + const events = { RESPONSE: 'response', REQUEST: 'request', diff --git a/lib/Connection.js b/lib/Connection.js index da552f757..aa56ce56e 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -129,6 +129,7 @@ class Connection { if (!socket.isSessionReused()) { socket.once('secureConnect', () => { // Check if fingerprint matches + /* istanbul ignore else */ if (this.caFingerprint !== getIssuerCertificate(socket).fingerprint256) { onError(new Error('Fingerprint does not match')) request.once('error', () => {}) // we need to catch the request aborted error @@ -362,11 +363,14 @@ function prepareHeaders (headers = {}, auth) { function getIssuerCertificate (socket) { let certificate = socket.getPeerCertificate(true) while (certificate && Object.keys(certificate).length > 0) { + /* istanbul ignore else */ if (certificate.issuerCertificate !== undefined) { // For self-signed certificates, `issuerCertificate` may be a circular reference. + /* istanbul ignore else */ if (certificate.fingerprint256 === certificate.issuerCertificate.fingerprint256) { break } + /* istanbul ignore next */ certificate = certificate.issuerCertificate } else { break From 9e1babda4e19008426d37f498e46a8a25b2ffc2d Mon Sep 17 00:00:00 2001 From: delvedor Date: Tue, 27 Jul 2021 14:11:49 +0200 Subject: [PATCH 11/22] Updated test --- test/unit/client.test.js | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/unit/client.test.js b/test/unit/client.test.js index 418d8dc02..a85a17413 100644 --- a/test/unit/client.test.js +++ b/test/unit/client.test.js @@ -26,6 +26,7 @@ const intoStream = require('into-stream') const { ConnectionPool, Transport, Connection, errors } = require('../../index') const { CloudConnectionPool } = require('../../lib/pool') const { Client, buildServer } = require('../utils') + let clientVersion = require('../../package.json').version if (clientVersion.includes('-')) { clientVersion = clientVersion.slice(0, clientVersion.indexOf('-')) + 'p' @@ -1539,3 +1540,47 @@ test('Check server fingerprint (failure)', t => { }) }) }) + +test('caFingerprint can\'t be configured over http / 1', t => { + t.plan(2) + + try { + new Client({ // eslint-disable-line + node: 'http://localhost:9200', + caFingerprint: 'FO:OB:AR' + }) + t.fail('shuld throw') + } catch (err) { + t.ok(err instanceof errors.ConfigurationError) + t.equal(err.message, 'You can\'t configure the caFingerprint with a http connection') + } +}) + +test('caFingerprint can\'t be configured over http / 2', t => { + t.plan(2) + + try { + new Client({ // eslint-disable-line + nodes: ['http://localhost:9200'], + caFingerprint: 'FO:OB:AR' + }) + t.fail('shuld throw') + } catch (err) { + t.ok(err instanceof errors.ConfigurationError) + t.equal(err.message, 'You can\'t configure the caFingerprint with a http connection') + } +}) + +test('caFingerprint can\'t be configured over http / 3', t => { + t.plan(1) + + try { + new Client({ // eslint-disable-line + nodes: ['https://localhost:9200'], + caFingerprint: 'FO:OB:AR' + }) + t.pass('should not throw') + } catch (err) { + t.fail('shuld not throw') + } +}) From fcf947d9782e490fd96598e95f2571f68ab07556 Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Wed, 28 Jul 2021 16:10:26 +0200 Subject: [PATCH 12/22] Update index.js Co-authored-by: Aleh Zasypkin --- index.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/index.js b/index.js index e240a6208..11b60a56d 100644 --- a/index.js +++ b/index.js @@ -323,12 +323,7 @@ function getAuth (node) { function isHttpConnection (node) { if (Array.isArray(node)) { - for (const n of node) { - if (new URL(n).protocol === 'http:') { - return true - } - } - return false + return node.some((n) => new URL(n).protocol === 'http:') } else { return new URL(node).protocol === 'http:' } From 1cc3281fc84a21262077377c867ed1d1a7caeb85 Mon Sep 17 00:00:00 2001 From: delvedor Date: Wed, 28 Jul 2021 16:12:29 +0200 Subject: [PATCH 13/22] Updated docs --- docs/basic-config.asciidoc | 2 +- docs/connecting.asciidoc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/basic-config.asciidoc b/docs/basic-config.asciidoc index c2ffff0dd..846afe986 100644 --- a/docs/basic-config.asciidoc +++ b/docs/basic-config.asciidoc @@ -256,7 +256,7 @@ const client = new Client({ _Default:_ `false` |`caFingerprint` -|`string` - If configured, verify the supplied CA certificate fingerprint. + +|`string` - If configured, verify the supplied CA certificate fingerprint. You must configure a SHA256 diagest. + _Default:_ `null` |=== diff --git a/docs/connecting.asciidoc b/docs/connecting.asciidoc index 9f743d4b9..c0d425e4b 100644 --- a/docs/connecting.asciidoc +++ b/docs/connecting.asciidoc @@ -182,6 +182,7 @@ const client = new Client({ To improve the security of your connection to Elasticsearch, you can provide a `caFingerprint` option, which will verify the supplied certificate authority fingerprint. +You must configure a SHA256 digest. [source,js] ---- From 33f90b00cd84832b47605b3dc12ba326864d47fd Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Thu, 29 Jul 2021 10:25:05 +0200 Subject: [PATCH 14/22] Update docs/basic-config.asciidoc Co-authored-by: Ioannis Kakavas --- docs/basic-config.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/basic-config.asciidoc b/docs/basic-config.asciidoc index 846afe986..c800b38c0 100644 --- a/docs/basic-config.asciidoc +++ b/docs/basic-config.asciidoc @@ -256,7 +256,7 @@ const client = new Client({ _Default:_ `false` |`caFingerprint` -|`string` - If configured, verify the supplied CA certificate fingerprint. You must configure a SHA256 diagest. + +|`string` - If configured, verify that the fingerprint of the CA certificate that has signed the certificate of the server matches the supplied fingerprint. Only accepts SHA256 digest fingerprints. + _Default:_ `null` |=== From 1b69efed0d904350ef30753506224c56c60f077d Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Thu, 29 Jul 2021 10:25:56 +0200 Subject: [PATCH 15/22] Update docs/connecting.asciidoc Co-authored-by: Ioannis Kakavas --- docs/connecting.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/connecting.asciidoc b/docs/connecting.asciidoc index c0d425e4b..6cf1f0e4c 100644 --- a/docs/connecting.asciidoc +++ b/docs/connecting.asciidoc @@ -180,7 +180,7 @@ const client = new Client({ [[auth-ca-fingerprint]] ==== CA fingerprint -To improve the security of your connection to Elasticsearch, you can provide +You can configure to only trust certificates that are signed by a specific CA certificate ( CA certificate pinning ) by providing a `caFingerprint` option. This will verify that the fingerprint of the CA certificate that has signed the certificate of the server matches the supplied value. a `caFingerprint` option, which will verify the supplied certificate authority fingerprint. You must configure a SHA256 digest. From 5f85ded8e2b49fa8e84c470a36720eb7e46afbcc Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Thu, 29 Jul 2021 10:26:05 +0200 Subject: [PATCH 16/22] Update lib/Connection.js Co-authored-by: Ioannis Kakavas --- lib/Connection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Connection.js b/lib/Connection.js index aa56ce56e..cb266d486 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -131,7 +131,7 @@ class Connection { // Check if fingerprint matches /* istanbul ignore else */ if (this.caFingerprint !== getIssuerCertificate(socket).fingerprint256) { - onError(new Error('Fingerprint does not match')) + onError(new Error('Server certificate CA fingerprint does not match the value configured in caFingerprint')) request.once('error', () => {}) // we need to catch the request aborted error return request.abort() } From a6a6dee5abcc5ae51df2f9658738e0ae2a0e48e8 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 30 Jul 2021 12:48:02 +0200 Subject: [PATCH 17/22] Improve getIssuerCertificate utility --- lib/Connection.js | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/lib/Connection.js b/lib/Connection.js index cb266d486..c6cfa7899 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -128,9 +128,17 @@ class Connection { /* istanbul ignore else */ if (!socket.isSessionReused()) { socket.once('secureConnect', () => { + const issuerCertificate = getIssuerCertificate(socket) + /* istanbul ignore next */ + if (issuerCertificate == null) { + onError(new Error('Invalid or malformed certificate')) + request.once('error', () => {}) // we need to catch the request aborted error + return request.abort() + } + // Check if fingerprint matches /* istanbul ignore else */ - if (this.caFingerprint !== getIssuerCertificate(socket).fingerprint256) { + if (this.caFingerprint !== issuerCertificate.fingerprint256) { onError(new Error('Server certificate CA fingerprint does not match the value configured in caFingerprint')) request.once('error', () => {}) // we need to catch the request aborted error return request.abort() @@ -363,21 +371,31 @@ function prepareHeaders (headers = {}, auth) { function getIssuerCertificate (socket) { let certificate = socket.getPeerCertificate(true) while (certificate && Object.keys(certificate).length > 0) { - /* istanbul ignore else */ - if (certificate.issuerCertificate !== undefined) { - // For self-signed certificates, `issuerCertificate` may be a circular reference. - /* istanbul ignore else */ - if (certificate.fingerprint256 === certificate.issuerCertificate.fingerprint256) { - break - } - /* istanbul ignore next */ - certificate = certificate.issuerCertificate - } else { + // invalid certificate + if (certificate.issuerCertificate == null) { + return null + } + + // we have reached the root certificate + if (certificate.subject.C === certificate.issuer.C && + certificate.subject.ST === certificate.issuer.ST && + certificate.subject.L === certificate.issuer.L && + certificate.subject.O === certificate.issuer.O && + certificate.subject.OU === certificate.issuer.OU && + certificate.subject.CN === certificate.issuer.CN) { break } + + // For self-signed certificates, `issuerCertificate` may be a circular reference. + if (certificate.fingerprint256 === certificate.issuerCertificate.fingerprint256) { + break + } + + // continue the loop + certificate = certificate.issuerCertificate } return certificate } module.exports = Connection -module.exports.internals = { prepareHeaders } +module.exports.internals = { prepareHeaders, getIssuerCertificate } From d9192e9870843f34eaf1ff89e5a4dc7744d1451a Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 30 Jul 2021 12:48:13 +0200 Subject: [PATCH 18/22] Updated test --- test/unit/client.test.js | 2 +- test/unit/connection.test.js | 185 ++++++++++++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 2 deletions(-) diff --git a/test/unit/client.test.js b/test/unit/client.test.js index a85a17413..d857a54c4 100644 --- a/test/unit/client.test.js +++ b/test/unit/client.test.js @@ -1535,7 +1535,7 @@ test('Check server fingerprint (failure)', t => { client.info((err, res) => { t.ok(err instanceof errors.ConnectionError) - t.equal(err.message, 'Fingerprint does not match') + t.equal(err.message, 'Server certificate CA fingerprint does not match the value configured in caFingerprint') server.stop() }) }) diff --git a/test/unit/connection.test.js b/test/unit/connection.test.js index d9f33d47c..dcf5b0b7d 100644 --- a/test/unit/connection.test.js +++ b/test/unit/connection.test.js @@ -29,6 +29,7 @@ const intoStream = require('into-stream') const { buildServer } = require('../utils') const Connection = require('../../lib/Connection') const { TimeoutError, ConfigurationError, RequestAbortedError, ConnectionError } = require('../../lib/errors') +const { getIssuerCertificate } = Connection.internals test('Basic (http)', t => { t.plan(4) @@ -997,8 +998,190 @@ test('Check server fingerprint (failure)', t => { method: 'GET' }, (err, res) => { t.ok(err instanceof ConnectionError) - t.equal(err.message, 'Fingerprint does not match') + t.equal(err.message, 'Server certificate CA fingerprint does not match the value configured in caFingerprint') server.stop() }) }) }) + +test('getIssuerCertificate returns the root CA / 1', t => { + t.plan(2) + const issuerCertificate = { + fingerprint256: 'BA:ZF:AZ', + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + } + } + issuerCertificate.issuerCertificate = issuerCertificate + + const socket = { + getPeerCertificate (bool) { + t.ok(bool) + return { + fingerprint256: 'FO:OB:AR', + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '2', + ST: '2', + L: '2', + O: '2', + OU: '2', + CN: '2' + }, + issuerCertificate + } + } + } + t.same(getIssuerCertificate(socket), issuerCertificate) +}) + +test('getIssuerCertificate returns the root CA / 2', t => { + t.plan(2) + const issuerCertificate = { + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + } + } + issuerCertificate.issuerCertificate = issuerCertificate + + const socket = { + getPeerCertificate (bool) { + t.ok(bool) + return { + fingerprint256: 'FO:OB:AR', + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '2', + ST: '2', + L: '2', + O: '2', + OU: '2', + CN: '2' + }, + issuerCertificate + } + } + } + t.same(getIssuerCertificate(socket), issuerCertificate) +}) + +test('getIssuerCertificate returns the root CA / 3', t => { + t.plan(2) + const issuerCertificate = { + fingerprint256: 'FO:OB:AR', + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + } + } + issuerCertificate.issuerCertificate = issuerCertificate + + const socket = { + getPeerCertificate (bool) { + t.ok(bool) + return { + fingerprint256: 'FO:OB:AR', + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '2', + ST: '2', + L: '2', + O: '2', + OU: '2', + CN: '2' + }, + issuerCertificate + } + } + } + t.equal(getIssuerCertificate(socket).fingerprint256, 'FO:OB:AR') +}) + +test('getIssuerCertificate detects invalid/malformed certificates', t => { + t.plan(2) + const socket = { + getPeerCertificate (bool) { + t.ok(bool) + return { + fingerprint256: 'FO:OB:AR', + subject: { + C: '1', + ST: '1', + L: '1', + O: '1', + OU: '1', + CN: '1' + }, + issuer: { + C: '2', + ST: '2', + L: '2', + O: '2', + OU: '2', + CN: '2' + } + // missing issuerCertificate + } + } + } + t.equal(getIssuerCertificate(socket), null) +}) From 208d316a915036622de7deac13c3dba69283c1f2 Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 30 Jul 2021 12:52:13 +0200 Subject: [PATCH 19/22] Nit --- lib/Connection.js | 2 +- test/unit/client.test.js | 2 +- test/unit/connection.test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Connection.js b/lib/Connection.js index c6cfa7899..966dcff0a 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -139,7 +139,7 @@ class Connection { // Check if fingerprint matches /* istanbul ignore else */ if (this.caFingerprint !== issuerCertificate.fingerprint256) { - onError(new Error('Server certificate CA fingerprint does not match the value configured in caFingerprint')) + onError(new Error('Server certificate CA fingerprint does not match the value configured in caFingerprint')) request.once('error', () => {}) // we need to catch the request aborted error return request.abort() } diff --git a/test/unit/client.test.js b/test/unit/client.test.js index d857a54c4..ef5718f86 100644 --- a/test/unit/client.test.js +++ b/test/unit/client.test.js @@ -1535,7 +1535,7 @@ test('Check server fingerprint (failure)', t => { client.info((err, res) => { t.ok(err instanceof errors.ConnectionError) - t.equal(err.message, 'Server certificate CA fingerprint does not match the value configured in caFingerprint') + t.equal(err.message, 'Server certificate CA fingerprint does not match the value configured in caFingerprint') server.stop() }) }) diff --git a/test/unit/connection.test.js b/test/unit/connection.test.js index dcf5b0b7d..b4dd17c3d 100644 --- a/test/unit/connection.test.js +++ b/test/unit/connection.test.js @@ -998,7 +998,7 @@ test('Check server fingerprint (failure)', t => { method: 'GET' }, (err, res) => { t.ok(err instanceof ConnectionError) - t.equal(err.message, 'Server certificate CA fingerprint does not match the value configured in caFingerprint') + t.equal(err.message, 'Server certificate CA fingerprint does not match the value configured in caFingerprint') server.stop() }) }) From fecd15b3de72875eba75fdd3d025ef52663d537f Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Fri, 30 Jul 2021 15:45:55 +0200 Subject: [PATCH 20/22] Update docs/connecting.asciidoc Co-authored-by: Ioannis Kakavas --- docs/connecting.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/connecting.asciidoc b/docs/connecting.asciidoc index 6cf1f0e4c..819e3a64b 100644 --- a/docs/connecting.asciidoc +++ b/docs/connecting.asciidoc @@ -180,7 +180,7 @@ const client = new Client({ [[auth-ca-fingerprint]] ==== CA fingerprint -You can configure to only trust certificates that are signed by a specific CA certificate ( CA certificate pinning ) by providing a `caFingerprint` option. This will verify that the fingerprint of the CA certificate that has signed the certificate of the server matches the supplied value. +You can configure the client to only trust certificates that are signed by a specific CA certificate ( CA certificate pinning ) by providing a `caFingerprint` option. This will verify that the fingerprint of the CA certificate that has signed the certificate of the server matches the supplied value. a `caFingerprint` option, which will verify the supplied certificate authority fingerprint. You must configure a SHA256 digest. From 58867188a969943fa777380f05ee33d6042bf8b9 Mon Sep 17 00:00:00 2001 From: Tomas Della Vedova Date: Fri, 30 Jul 2021 15:46:18 +0200 Subject: [PATCH 21/22] Update test/unit/client.test.js Co-authored-by: Ioannis Kakavas --- test/unit/client.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/client.test.js b/test/unit/client.test.js index ef5718f86..fbc45dc82 100644 --- a/test/unit/client.test.js +++ b/test/unit/client.test.js @@ -1564,7 +1564,7 @@ test('caFingerprint can\'t be configured over http / 2', t => { nodes: ['http://localhost:9200'], caFingerprint: 'FO:OB:AR' }) - t.fail('shuld throw') + t.fail('should throw') } catch (err) { t.ok(err instanceof errors.ConfigurationError) t.equal(err.message, 'You can\'t configure the caFingerprint with a http connection') From b4ea6aae2cdb1ec3d1432eabd98f08ea3bd9ea3c Mon Sep 17 00:00:00 2001 From: delvedor Date: Fri, 30 Jul 2021 15:48:26 +0200 Subject: [PATCH 22/22] Improve getIssuerCertificate utility --- lib/Connection.js | 13 +---- test/unit/connection.test.js | 103 +---------------------------------- 2 files changed, 3 insertions(+), 113 deletions(-) diff --git a/lib/Connection.js b/lib/Connection.js index 966dcff0a..88a154ae6 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -376,17 +376,8 @@ function getIssuerCertificate (socket) { return null } - // we have reached the root certificate - if (certificate.subject.C === certificate.issuer.C && - certificate.subject.ST === certificate.issuer.ST && - certificate.subject.L === certificate.issuer.L && - certificate.subject.O === certificate.issuer.O && - certificate.subject.OU === certificate.issuer.OU && - certificate.subject.CN === certificate.issuer.CN) { - break - } - - // For self-signed certificates, `issuerCertificate` may be a circular reference. + // We have reached the root certificate. + // In case of self-signed certificates, `issuerCertificate` may be a circular reference. if (certificate.fingerprint256 === certificate.issuerCertificate.fingerprint256) { break } diff --git a/test/unit/connection.test.js b/test/unit/connection.test.js index b4dd17c3d..5ba1c494a 100644 --- a/test/unit/connection.test.js +++ b/test/unit/connection.test.js @@ -1004,7 +1004,7 @@ test('Check server fingerprint (failure)', t => { }) }) -test('getIssuerCertificate returns the root CA / 1', t => { +test('getIssuerCertificate returns the root CA', t => { t.plan(2) const issuerCertificate = { fingerprint256: 'BA:ZF:AZ', @@ -1055,107 +1055,6 @@ test('getIssuerCertificate returns the root CA / 1', t => { t.same(getIssuerCertificate(socket), issuerCertificate) }) -test('getIssuerCertificate returns the root CA / 2', t => { - t.plan(2) - const issuerCertificate = { - subject: { - C: '1', - ST: '1', - L: '1', - O: '1', - OU: '1', - CN: '1' - }, - issuer: { - C: '1', - ST: '1', - L: '1', - O: '1', - OU: '1', - CN: '1' - } - } - issuerCertificate.issuerCertificate = issuerCertificate - - const socket = { - getPeerCertificate (bool) { - t.ok(bool) - return { - fingerprint256: 'FO:OB:AR', - subject: { - C: '1', - ST: '1', - L: '1', - O: '1', - OU: '1', - CN: '1' - }, - issuer: { - C: '2', - ST: '2', - L: '2', - O: '2', - OU: '2', - CN: '2' - }, - issuerCertificate - } - } - } - t.same(getIssuerCertificate(socket), issuerCertificate) -}) - -test('getIssuerCertificate returns the root CA / 3', t => { - t.plan(2) - const issuerCertificate = { - fingerprint256: 'FO:OB:AR', - subject: { - C: '1', - ST: '1', - L: '1', - O: '1', - OU: '1', - CN: '1' - }, - issuer: { - C: '1', - ST: '1', - L: '1', - O: '1', - OU: '1', - CN: '1' - } - } - issuerCertificate.issuerCertificate = issuerCertificate - - const socket = { - getPeerCertificate (bool) { - t.ok(bool) - return { - fingerprint256: 'FO:OB:AR', - subject: { - C: '1', - ST: '1', - L: '1', - O: '1', - OU: '1', - CN: '1' - }, - issuer: { - C: '2', - ST: '2', - L: '2', - O: '2', - OU: '2', - CN: '2' - }, - issuerCertificate - } - } - } - t.equal(getIssuerCertificate(socket).fingerprint256, 'FO:OB:AR') -}) - test('getIssuerCertificate detects invalid/malformed certificates', t => { t.plan(2) const socket = {