From af7f4f798dbf3b0092386d803ed9bd701cee0578 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sat, 18 Apr 2020 11:04:13 +0200 Subject: [PATCH 1/9] grpc-js: initiate tls connection through http proxy --- packages/grpc-js/src/http_proxy.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/grpc-js/src/http_proxy.ts b/packages/grpc-js/src/http_proxy.ts index 2b3fb5d5a..fe0bfb7ed 100644 --- a/packages/grpc-js/src/http_proxy.ts +++ b/packages/grpc-js/src/http_proxy.ts @@ -21,6 +21,7 @@ import { LogVerbosity } from './constants'; import { parseTarget } from './resolver-dns'; import { Socket } from 'net'; import * as http from 'http'; +import * as tls from 'tls' import * as logging from './logging'; import { SubchannelAddress, @@ -202,9 +203,14 @@ export function getProxiedConnection( ' through proxy ' + proxyAddressString ); - resolve({ - socket, - realTarget, + var cts = tls.connect({ + host: options.host, + socket: socket + }, function () { + resolve({ + socket: cts, + realTarget, + }); }); } else { log( From c650e59563ea1d4942019c5e7024fbc8487b11ac Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sat, 18 Apr 2020 12:56:51 +0200 Subject: [PATCH 2/9] grpc-js: always explicitly establish connection --- packages/grpc-js/src/subchannel.ts | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index e332df36d..9622e2806 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -302,21 +302,20 @@ export class Subchannel { if (proxyConnectionResult.socket) { connectionOptions.socket = proxyConnectionResult.socket; } - } else { - /* In all but the most recent versions of Node, http2.connect does not use - * the options when establishing plaintext connections, so we need to - * establish that connection explicitly. */ - connectionOptions.createConnection = (authority, option) => { - if (proxyConnectionResult.socket) { - return proxyConnectionResult.socket; - } else { - /* net.NetConnectOpts is declared in a way that is more restrictive - * than what net.connect will actually accept, so we use the type - * assertion to work around that. */ - return net.connect(this.subchannelAddress); - } - }; } + /* In all but the most recent versions of Node, http2.connect does not use + * the options when establishing plaintext connections, so we need to + * establish that connection explicitly. */ + connectionOptions.createConnection = (authority, option) => { + if (proxyConnectionResult.socket) { + return proxyConnectionResult.socket; + } else { + /* net.NetConnectOpts is declared in a way that is more restrictive + * than what net.connect will actually accept, so we use the type + * assertion to work around that. */ + return net.connect(this.subchannelAddress); + } + }; connectionOptions = Object.assign( connectionOptions, this.subchannelAddress From 5af582e31c4116984ffc70243b76d1cc5d894d35 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sat, 18 Apr 2020 13:59:49 +0200 Subject: [PATCH 3/9] grpc-js: pass secureContext through to proxied tls connection --- packages/grpc-js/src/http_proxy.ts | 5 ++++- packages/grpc-js/src/subchannel.ts | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/src/http_proxy.ts b/packages/grpc-js/src/http_proxy.ts index fe0bfb7ed..cde1d4213 100644 --- a/packages/grpc-js/src/http_proxy.ts +++ b/packages/grpc-js/src/http_proxy.ts @@ -21,6 +21,7 @@ import { LogVerbosity } from './constants'; import { parseTarget } from './resolver-dns'; import { Socket } from 'net'; import * as http from 'http'; +import * as http2 from 'http2'; import * as tls from 'tls' import * as logging from './logging'; import { @@ -158,7 +159,8 @@ export interface ProxyConnectionResult { export function getProxiedConnection( address: SubchannelAddress, - channelOptions: ChannelOptions + channelOptions: ChannelOptions, + connectionOptions: http2.SecureClientSessionOptions ): Promise { if (!('grpc.http_connect_target' in channelOptions)) { return Promise.resolve({}); @@ -204,6 +206,7 @@ export function getProxiedConnection( proxyAddressString ); var cts = tls.connect({ + ...connectionOptions, host: options.host, socket: socket }, function () { diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 9622e2806..387d69e8b 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -411,7 +411,28 @@ export class Subchannel { } private startConnectingInternal() { - getProxiedConnection(this.subchannelAddress, this.options).then( + let connectionOptions: http2.SecureClientSessionOptions = + this.credentials._getConnectionOptions() || {}; + + if ('secureContext' in connectionOptions) { + // If provided, the value of grpc.ssl_target_name_override should be used + // to override the target hostname when checking server identity. + // This option is used for testing only. + if (this.options['grpc.ssl_target_name_override']) { + const sslTargetNameOverride = this.options[ + 'grpc.ssl_target_name_override' + ]!; + connectionOptions.checkServerIdentity = ( + host: string, + cert: PeerCertificate + ): Error | undefined => { + return checkServerIdentity(sslTargetNameOverride, cert); + }; + connectionOptions.servername = sslTargetNameOverride; + } + } + + getProxiedConnection(this.subchannelAddress, this.options, connectionOptions).then( (result) => { this.createSession(result); }, From 4e61f21c2fe3a7119de16b5e7459b508fd7d8a99 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sat, 18 Apr 2020 17:58:11 +0200 Subject: [PATCH 4/9] grpc-js: do not set host when instantiating tls socket --- packages/grpc-js/src/http_proxy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/grpc-js/src/http_proxy.ts b/packages/grpc-js/src/http_proxy.ts index cde1d4213..dd5e1bbf8 100644 --- a/packages/grpc-js/src/http_proxy.ts +++ b/packages/grpc-js/src/http_proxy.ts @@ -207,7 +207,6 @@ export function getProxiedConnection( ); var cts = tls.connect({ ...connectionOptions, - host: options.host, socket: socket }, function () { resolve({ From 2c5a8b1a3077f36154d36fecb27c78a13aca3d0e Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sat, 18 Apr 2020 18:41:28 +0200 Subject: [PATCH 5/9] grpc-js: ensure tls connection is used when requested --- packages/grpc-js/src/http_proxy.ts | 23 ++++++++++++++++------- packages/grpc-js/src/subchannel.ts | 24 ++++++++++++++---------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/packages/grpc-js/src/http_proxy.ts b/packages/grpc-js/src/http_proxy.ts index dd5e1bbf8..666877f00 100644 --- a/packages/grpc-js/src/http_proxy.ts +++ b/packages/grpc-js/src/http_proxy.ts @@ -18,11 +18,12 @@ import { URL } from 'url'; import { log } from './logging'; import { LogVerbosity } from './constants'; +import { getDefaultAuthority } from './resolver'; import { parseTarget } from './resolver-dns'; import { Socket } from 'net'; import * as http from 'http'; import * as http2 from 'http2'; -import * as tls from 'tls' +import * as tls from 'tls'; import * as logging from './logging'; import { SubchannelAddress, @@ -205,15 +206,23 @@ export function getProxiedConnection( ' through proxy ' + proxyAddressString ); - var cts = tls.connect({ - ...connectionOptions, - socket: socket - }, function () { + // The proxy is connecting to a TLS server, so upgrade + // this socket connection to a TLS connection. + if ('secureContext' in connectionOptions) { + const cts = tls.connect({ + ...connectionOptions, + host: getDefaultAuthority(realTarget), + socket: socket, + }, () => { + resolve({ socket: cts, realTarget }); + } + ); + } else { resolve({ - socket: cts, + socket, realTarget, }); - }); + } } else { log( LogVerbosity.ERROR, diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 387d69e8b..729ab3d04 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -28,6 +28,7 @@ import * as logging from './logging'; import { LogVerbosity } from './constants'; import { getProxiedConnection, ProxyConnectionResult } from './http_proxy'; import * as net from 'net'; +import * as tls from 'tls'; const clientVersion = require('../../package.json').version; @@ -299,9 +300,6 @@ export class Subchannel { }; connectionOptions.servername = sslTargetNameOverride; } - if (proxyConnectionResult.socket) { - connectionOptions.socket = proxyConnectionResult.socket; - } } /* In all but the most recent versions of Node, http2.connect does not use * the options when establishing plaintext connections, so we need to @@ -309,13 +307,15 @@ export class Subchannel { connectionOptions.createConnection = (authority, option) => { if (proxyConnectionResult.socket) { return proxyConnectionResult.socket; - } else { - /* net.NetConnectOpts is declared in a way that is more restrictive - * than what net.connect will actually accept, so we use the type - * assertion to work around that. */ - return net.connect(this.subchannelAddress); + } else if ('secureContext' in connectionOptions) { + return tls.connect(this.subchannelAddress); } + /* net.NetConnectOpts is declared in a way that is more restrictive + * than what net.connect will actually accept, so we use the type + * assertion to work around that. */ + return net.connect(this.subchannelAddress); }; + connectionOptions = Object.assign( connectionOptions, this.subchannelAddress @@ -411,7 +411,7 @@ export class Subchannel { } private startConnectingInternal() { - let connectionOptions: http2.SecureClientSessionOptions = + const connectionOptions: http2.SecureClientSessionOptions = this.credentials._getConnectionOptions() || {}; if ('secureContext' in connectionOptions) { @@ -432,7 +432,11 @@ export class Subchannel { } } - getProxiedConnection(this.subchannelAddress, this.options, connectionOptions).then( + getProxiedConnection( + this.subchannelAddress, + this.options, + connectionOptions + ).then( (result) => { this.createSession(result); }, From 11965fb0af6b633bf0be4b914d6571b22a34c803 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sat, 18 Apr 2020 22:34:18 +0200 Subject: [PATCH 6/9] grpc-js: dont set createConnection when connecting with TLS and without a proxy --- packages/grpc-js/src/subchannel.ts | 31 +++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 729ab3d04..d1226e967 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -300,21 +300,26 @@ export class Subchannel { }; connectionOptions.servername = sslTargetNameOverride; } - } - /* In all but the most recent versions of Node, http2.connect does not use - * the options when establishing plaintext connections, so we need to - * establish that connection explicitly. */ - connectionOptions.createConnection = (authority, option) => { if (proxyConnectionResult.socket) { - return proxyConnectionResult.socket; - } else if ('secureContext' in connectionOptions) { - return tls.connect(this.subchannelAddress); + connectionOptions.createConnection = (authority, option) => { + return proxyConnectionResult.socket!; + }; } - /* net.NetConnectOpts is declared in a way that is more restrictive - * than what net.connect will actually accept, so we use the type - * assertion to work around that. */ - return net.connect(this.subchannelAddress); - }; + } else { + /* In all but the most recent versions of Node, http2.connect does not use + * the options when establishing plaintext connections, so we need to + * establish that connection explicitly. */ + connectionOptions.createConnection = (authority, option) => { + if (proxyConnectionResult.socket) { + return proxyConnectionResult.socket; + } + /* net.NetConnectOpts is declared in a way that is more restrictive + * than what net.connect will actually accept, so we use the type + * assertion to work around that. */ + return net.connect(this.subchannelAddress); + }; + } + connectionOptions = Object.assign( connectionOptions, From b9e84f499fa687674478885e50b49785ea214712 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sun, 19 Apr 2020 19:58:56 +0200 Subject: [PATCH 7/9] grpc-js: commenting working for node issue 32922 --- packages/grpc-js/src/http_proxy.ts | 10 ++++++---- packages/grpc-js/src/subchannel.ts | 14 +++++++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/grpc-js/src/http_proxy.ts b/packages/grpc-js/src/http_proxy.ts index 666877f00..956eef4ae 100644 --- a/packages/grpc-js/src/http_proxy.ts +++ b/packages/grpc-js/src/http_proxy.ts @@ -22,9 +22,9 @@ import { getDefaultAuthority } from './resolver'; import { parseTarget } from './resolver-dns'; import { Socket } from 'net'; import * as http from 'http'; -import * as http2 from 'http2'; import * as tls from 'tls'; import * as logging from './logging'; +import { SecureClientSessionOptions } from 'http2' import { SubchannelAddress, isTcpSubchannelAddress, @@ -161,7 +161,7 @@ export interface ProxyConnectionResult { export function getProxiedConnection( address: SubchannelAddress, channelOptions: ChannelOptions, - connectionOptions: http2.SecureClientSessionOptions + connectionOptions: SecureClientSessionOptions ): Promise { if (!('grpc.http_connect_target' in channelOptions)) { return Promise.resolve({}); @@ -206,9 +206,11 @@ export function getProxiedConnection( ' through proxy ' + proxyAddressString ); - // The proxy is connecting to a TLS server, so upgrade - // this socket connection to a TLS connection. if ('secureContext' in connectionOptions) { + /* The proxy is connecting to a TLS server, so upgrade this socket + * connection to a TLS connection. + * This is a workaround for https://github.com/nodejs/node/issues/32922 + * See https://github.com/grpc/grpc-node/pull/1369 for more info. */ const cts = tls.connect({ ...connectionOptions, host: getDefaultAuthority(realTarget), diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index d1226e967..a48453470 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -312,15 +312,15 @@ export class Subchannel { connectionOptions.createConnection = (authority, option) => { if (proxyConnectionResult.socket) { return proxyConnectionResult.socket; + } else { + /* net.NetConnectOpts is declared in a way that is more restrictive + * than what net.connect will actually accept, so we use the type + * assertion to work around that. */ + return net.connect(this.subchannelAddress); } - /* net.NetConnectOpts is declared in a way that is more restrictive - * than what net.connect will actually accept, so we use the type - * assertion to work around that. */ - return net.connect(this.subchannelAddress); }; } - connectionOptions = Object.assign( connectionOptions, this.subchannelAddress @@ -416,6 +416,10 @@ export class Subchannel { } private startConnectingInternal() { + /* Pass connection options through to the proxy so that it's able to + * upgrade it's connection to support tls if needed. + * This is a workaround for https://github.com/nodejs/node/issues/32922 + * See https://github.com/grpc/grpc-node/pull/1369 for more info. */ const connectionOptions: http2.SecureClientSessionOptions = this.credentials._getConnectionOptions() || {}; From 48072d5f4f01cc82c1cf06e45b95cc423093fc09 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sun, 19 Apr 2020 20:00:05 +0200 Subject: [PATCH 8/9] grpc-js: setting ALPNProtocols option for tls proxy --- packages/grpc-js/src/subchannel.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index a48453470..f44d1b857 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -424,6 +424,7 @@ export class Subchannel { this.credentials._getConnectionOptions() || {}; if ('secureContext' in connectionOptions) { + connectionOptions.ALPNProtocols = ['h2']; // If provided, the value of grpc.ssl_target_name_override should be used // to override the target hostname when checking server identity. // This option is used for testing only. From eef75a5c1b9de00b55cc8b44c44a7b930ac1bc31 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Sun, 19 Apr 2020 20:02:38 +0200 Subject: [PATCH 9/9] grpc-js: use tls.ConnectionOptions type for proxy connection options --- packages/grpc-js/src/http_proxy.ts | 3 +-- packages/grpc-js/src/subchannel.ts | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js/src/http_proxy.ts b/packages/grpc-js/src/http_proxy.ts index 956eef4ae..708b30c6e 100644 --- a/packages/grpc-js/src/http_proxy.ts +++ b/packages/grpc-js/src/http_proxy.ts @@ -24,7 +24,6 @@ import { Socket } from 'net'; import * as http from 'http'; import * as tls from 'tls'; import * as logging from './logging'; -import { SecureClientSessionOptions } from 'http2' import { SubchannelAddress, isTcpSubchannelAddress, @@ -161,7 +160,7 @@ export interface ProxyConnectionResult { export function getProxiedConnection( address: SubchannelAddress, channelOptions: ChannelOptions, - connectionOptions: SecureClientSessionOptions + connectionOptions: tls.ConnectionOptions ): Promise { if (!('grpc.http_connect_target' in channelOptions)) { return Promise.resolve({}); diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index f44d1b857..414e797d3 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -28,7 +28,7 @@ import * as logging from './logging'; import { LogVerbosity } from './constants'; import { getProxiedConnection, ProxyConnectionResult } from './http_proxy'; import * as net from 'net'; -import * as tls from 'tls'; +import { ConnectionOptions } from 'tls'; const clientVersion = require('../../package.json').version; @@ -420,7 +420,7 @@ export class Subchannel { * upgrade it's connection to support tls if needed. * This is a workaround for https://github.com/nodejs/node/issues/32922 * See https://github.com/grpc/grpc-node/pull/1369 for more info. */ - const connectionOptions: http2.SecureClientSessionOptions = + const connectionOptions: ConnectionOptions = this.credentials._getConnectionOptions() || {}; if ('secureContext' in connectionOptions) {