[Feature Request] WsProvider mTLS support - SecureContextOptions - Node.js

[agilenc]

• I'm submitting a ...

• Bug report
• Feature request
• Support request
• Other

• Please tell us about your environment:

• Node.js
• Browser
• Other (limited support for other environments)

• What is the current behavior?

On Node when creating a new WsProvider there is no way to specify ca/cert/key (for mTLS).

• What is the expected behavior?

Similar to the ws package where we can do:

const options: SecureContextOptions = {
  ca: fs.readFileSync('ca.crt'),
  cert: fs.readFileSync('client.crt'),
  key: fs.readFileSync('client.crt.key'),
};
const wsClient = new WebSocket('wss://hostname', options);

It would be great to be able to specify a SecureContextOptions object to the constructor when creating a new WsProvider.

• What is the motivation for changing the behavior?

Support more use-cases, currently we are unable to use polkadot-js api because the ws client cannot connect to a secure endpoint with mTLS. Using mTLS is a requirement in our case. We only need this on Node (not browser or other env).

As a general rule client APIs that connect to nodes should provide security guarantees when possible.
This initiative would support this.

• Implications

Talked to @TarikGul who shared this issue #5279. One comment is:

As an aside, due to the overwhelming popularity of ws vs websocket, the polyfill will also be swapped to thatws for Node environments. It has no header support. (Not sure when, it has been on the radar for the last 6 months)

To allow a SecureContextOptions object to be passed to the WsProvider constructor then here when using Node we'd first need to swap the polyfill from websocket to ws. Maybe some work has already been done in that regard?

[jacogr]

I really want to swap to ws, however there are real users out there who uses the supply-headers-capability in a Node.js environment. So that will break their code if a solution is not provided. I have honestly not dug into the internals of the ws package for a long time, so there may be solutions that appeared in the last 18-24 months for that functionality.

EDIT: Indeed, it actually does take header in the options, https://github.com/websockets/ws/blob/d412358ccb5320bc00c8993ecd5d9b992df0753e/lib/websocket.js#L717

Not sure how the rest of the options ca/cert/key/... etc fits into that lib.

[agilenc]

For ca/cert/key those would land here in the lib as options.cert, options.key, options.ca. Please find an example of using cert/key options here. Using ca would follow the same pattern, just adding ca next to cert/key properties.

From the types we see the options parameter is of type WebSocket.ClientOptions which extends SecureContextOptions. This is why it can contain ca/cert/key properties.

Good to hear there wouldn't be any blocker if swapping to ws 👍

[jacogr]

The underlying ws library has been swapped in the last linked PR. As for extending options here, this is completely unlikely to be done by me, so I'll mark it as such. (There is just no way I can reliably setup the testing infrastructure alongside the coding to ensure everything is working, and not quite fond of putting up half-baked code and then playing whack-a-mole)

So, with the fact that the PR won't be done by me, this is what is required -

1. The changes would need to go inside the WSProvider class, the details here specifically -

   api/packages/rpc-provider/src/ws/index.ts

   Line 217 in 4c3b6a6

   : new WebSocket(this.endpoint, undefined, {

2. Options are passed in the constructor, which is a nightmare with the number of options atm -

   api/packages/rpc-provider/src/ws/index.ts

   Line 112 in 4c3b6a6

   constructor (endpoint: string | string[] = defaults.WS_URL, autoConnectMs: number | false = RETRY_DELAY, headers: Record<string, string> = {}, timeout?: number) {

   (will get back to this)
3. So basically, add the options to the constructor, pass it through to the instance in the first point, test, test, test

Now on point 2 - just tacking on more options is horrible. I'm guessing past-me didn't want to break compat so just kept tacking them on. Current and future me still doesn't want to break compat, but certainly doesn't want to tack on more.

So with that in mind, we want to keep the old signature, but add a new signature that takes an options object with headers, timeout, the new ca stuff, etc. On use detect which is use and apply correctly.

So something like (untested, spitballing...) -

interface WsOptions {
  autoConnectMs?: number | false;
  headers?: Record<string, string>;
  timeout?: number;
  // .. additional new ca options go here...
}

/** @deprecated Use constructor(endpoint, options) instead */
constructor (endpoint: string | string[] = defaults.WS_URL, autoConnectMs?: number | false, headers?: Record<string, string>, timeout?: number)
constructor (endpoint: string | string[] = defaults.WS_URL, options: WsOptions = {}) {
  if (typeof options === 'object') {
    // extract stuff from the options object
  } else {
    // old-style, deprecated
    // NOTE Ensure that we still use the defaults as we have in the current code... (they have been removed
    // from the signature above)
  }
}