Merge pull request #2704 from murgatroid99/grpc-js_check_server_identity

grpc-js: Call custom `checkServerIdentity` when target name override is set
grpc · Apr 1, 2024 · c3c7cc4 · c3c7cc4
2 parents cc44d78 + e1f831a
commit c3c7cc4
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 5 deletions.
diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.10.4",
+  "version": "1.10.5",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

diff --git a/packages/grpc-js/src/transport.ts b/packages/grpc-js/src/transport.ts
@@ -694,11 +694,13 @@ export class Http2SubchannelConnector implements SubchannelConnector {
         if (options['grpc.ssl_target_name_override']) {
           const sslTargetNameOverride =
             options['grpc.ssl_target_name_override']!;
+          const originalCheckServerIdentity =
+            connectionOptions.checkServerIdentity ?? checkServerIdentity;
           connectionOptions.checkServerIdentity = (
             host: string,
             cert: PeerCertificate
           ): Error | undefined => {
-            return checkServerIdentity(sslTargetNameOverride, cert);
+            return originalCheckServerIdentity(sslTargetNameOverride, cert);
           };
           connectionOptions.servername = sslTargetNameOverride;
         } else {
@@ -804,11 +806,13 @@ export class Http2SubchannelConnector implements SubchannelConnector {
       // This option is used for testing only.
       if (options['grpc.ssl_target_name_override']) {
         const sslTargetNameOverride = options['grpc.ssl_target_name_override']!;
+        const originalCheckServerIdentity =
+          connectionOptions.checkServerIdentity ?? checkServerIdentity;
         connectionOptions.checkServerIdentity = (
           host: string,
           cert: PeerCertificate
         ): Error | undefined => {
-          return checkServerIdentity(sslTargetNameOverride, cert);
+          return originalCheckServerIdentity(sslTargetNameOverride, cert);
         };
         connectionOptions.servername = sslTargetNameOverride;
       } else {

diff --git a/packages/grpc-js/test/test-channel-credentials.ts b/packages/grpc-js/test/test-channel-credentials.ts
@@ -150,8 +150,12 @@ describe('ChannelCredentials Implementation', () => {
 describe('ChannelCredentials usage', () => {
   let client: ServiceClient;
   let server: grpc.Server;
+  let portNum: number;
+  let caCert: Buffer;
+  const hostnameOverride = 'foo.test.google.fr';
   before(async () => {
     const { ca, key, cert } = await pFixtures;
+    caCert = ca;
     const serverCreds = grpc.ServerCredentials.createSsl(null, [
       { private_key: key, cert_chain: cert },
     ]);
@@ -178,9 +182,10 @@ describe('ChannelCredentials usage', () => {
           reject(err);
           return;
         }
+        portNum = port;
         client = new echoService(`localhost:${port}`, combinedCreds, {
-          'grpc.ssl_target_name_override': 'foo.test.google.fr',
-          'grpc.default_authority': 'foo.test.google.fr',
+          'grpc.ssl_target_name_override': hostnameOverride,
+          'grpc.default_authority': hostnameOverride,
         });
         server.start();
         resolve();
@@ -207,4 +212,25 @@ describe('ChannelCredentials usage', () => {
     );
     assert2.afterMustCallsSatisfied(done);
   });
+
+  it('Should call the checkServerIdentity callback', done => {
+    const channelCreds = ChannelCredentials.createSsl(caCert, null, null, {
+      checkServerIdentity: assert2.mustCall((hostname, cert) => {
+        assert.strictEqual(hostname, hostnameOverride);
+        return undefined;
+      }),
+    });
+    const client = new echoService(`localhost:${portNum}`, channelCreds, {
+      'grpc.ssl_target_name_override': hostnameOverride,
+      'grpc.default_authority': hostnameOverride,
+    });
+    client.echo(
+      { value: 'test value', value2: 3 },
+      assert2.mustCall((error: ServiceError, response: any) => {
+        assert.ifError(error);
+        assert.deepStrictEqual(response, { value: 'test value', value2: 3 });
+      })
+    );
+    assert2.afterMustCallsSatisfied(done);
+  });
 });