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

grpc-js: initiate tls connection through http proxy #1369

Merged
merged 9 commits into from
Apr 20, 2020
27 changes: 22 additions & 5 deletions packages/grpc-js/src/http_proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +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';
mrfelton marked this conversation as resolved.
Show resolved Hide resolved
import * as tls from 'tls';
import * as logging from './logging';
import {
SubchannelAddress,
Expand Down Expand Up @@ -157,7 +160,8 @@ export interface ProxyConnectionResult {

export function getProxiedConnection(
address: SubchannelAddress,
channelOptions: ChannelOptions
channelOptions: ChannelOptions,
connectionOptions: http2.SecureClientSessionOptions
mrfelton marked this conversation as resolved.
Show resolved Hide resolved
): Promise<ProxyConnectionResult> {
if (!('grpc.http_connect_target' in channelOptions)) {
return Promise.resolve<ProxyConnectionResult>({});
Expand Down Expand Up @@ -202,10 +206,23 @@ export function getProxiedConnection(
' through proxy ' +
proxyAddressString
);
resolve({
socket,
realTarget,
});
// The proxy is connecting to a TLS server, so upgrade
// this socket connection to a TLS connection.
mrfelton marked this conversation as resolved.
Show resolved Hide resolved
if ('secureContext' in connectionOptions) {
const cts = tls.connect({
...connectionOptions,
host: getDefaultAuthority(realTarget),
socket: socket,
}, () => {
resolve({ socket: cts, realTarget });
}
);
} else {
resolve({
socket,
realTarget,
});
}
} else {
log(
LogVerbosity.ERROR,
Expand Down
58 changes: 41 additions & 17 deletions packages/grpc-js/src/subchannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -299,24 +300,22 @@ 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) {
connectionOptions.socket = proxyConnectionResult.socket;
return proxyConnectionResult.socket;
} else if ('secureContext' in connectionOptions) {
return tls.connect(this.subchannelAddress);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't better because you're not passing in the options. When connecting with TLS and without a proxy, it should simply not be passing a createConnection option at all.

Copy link
Contributor Author

@mrfelton mrfelton Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My typescript is not up to par :( What I really wanted to do in the ssl block was like this:

      if (proxyConnectionResult.socket) {
        connectionOptions.createConnection = (authority, option) => {
          return proxyConnectionResult.socket;
        };
      }

But I get typescript errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the issue is that TypeScript thinks that proxyConnectionResult.socket might become null between when you check it and when the callback is called. This should work:

if (proxyConnectionResult.socket) {
  connectionOptions.createConnection = (authority, option) => {
    return proxyConnectionResult.socket!; //Exclamation mark here
  };
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! updated.

}
} 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);
}
};
}
/* 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
Expand Down Expand Up @@ -412,7 +411,32 @@ export class Subchannel {
}

private startConnectingInternal() {
getProxiedConnection(this.subchannelAddress, this.options).then(
const connectionOptions: http2.SecureClientSessionOptions =
mrfelton marked this conversation as resolved.
Show resolved Hide resolved
this.credentials._getConnectionOptions() || {};

if ('secureContext' in connectionOptions) {
mrfelton marked this conversation as resolved.
Show resolved Hide resolved
// 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);
},
Expand Down