[Bug] TLS server name override not respected

[dan8f]

What are you really trying to do?

Setting a TLS server name override in client connection connect

Describe the bug

When setting a server name override in client connection, the TLS server name is mishandled by grpc-js library and ends up overridden with the address provided

Minimal Reproduction

  import { Connection, Client } from '@temporalio/client';

  const connection = await Connection.connect(
    {
      address: '<address>:7233',

      tls: {
        serverNameOverride: '<override>',
      },

      apiKey,

      metadata
    })

Environment/Versions

TS SDK 1.11.7

Additional context

grpc-js servername override

[kazaz-lumida]

Yes please :) This is blocking us from taking our app to production where we only want to use PrivateLink and not go through the internet

[kazaz-lumida]

Can anyone please let us know what is the status here?
This is a MAJOR security issue from our side and a core functionality of the SDK.
To be honest I'm surprised it was not addressed right away given its importance and what seems to be an easy fix.

[mjameswh]

Very sorry for the delay. I was honestly under the impression that our support team had already provided you a suitable workaround through our Zendesk portal, but this might have been a different conversation. My bad.

The current workaround is to add this dependency to your own package.json:

"@grpc/grpc-js": "1.12.6"

This will get Client connections through Private Link working properly.

[kazaz-lumida]

Thank you @mjameswh , any estimation on getting a real fix?
Fixing the version also means I'm not going to get any new security updates on such critical package which is a security concern on its own

[mjameswh]

any estimation on getting a real fix?

There are actually two solutions coming up:

1. grpc-js: Fix ability to set SNI with ssl_target_name_override option grpc/grpc-node#2956;
2. NativeConnection powered Client #1699 (to be released in TS SDK 1.12.0).

Any of these two will resolve the problem you are facing.

I expect both to be available in the next few days.

[kazaz-lumida]

Hi @mjameswh , seems like https://github.com/grpc/grpc-node/releases/tag/%40grpc%2Fgrpc-js%401.13.4 was merged and released 🎉

Can the Temporal team update the package and release a security patch version so this topic will be resolved?

[mjameswh]

seems like https://github.com/grpc/grpc-node/releases/tag/%40grpc%2Fgrpc-js%401.13.4 was merged and released 🎉

Indeed. We just confirmed that running Temporal TS SDK 1.11.8 with @grpc/grpc-js 1.13.4 works fine and allows connection to Temporal Cloud through Private Link.

Affected users may update that dependency locally by simply doing:

npm update @grpc/grpc-js

Can the Temporal team update the package and release a security patch version so this topic will be resolved?

We won't bump that dependency nor make a new release. That's not necessary, as that dependency is controlled from the user's project package-lock.json file anyway. Simply do npm update @grpc/grpc-js, which is actually easier than having to update all of your @temporalio/* dependencies. And other users may have their own reasons to prefer some older releases of grpc-js; there's no sufficient reason we should prevent them from doing so in this case.