node icon indicating copy to clipboard operation
node copied to clipboard

feat: added support for reading certificates from windows system store

Open twitharshil opened this issue 1 year ago • 6 comments

Description of Change

MITM-based proxy environments like ZScaler intercepts the requests sent to a certain endpoint from the user machine and serve a redirect to their own servers ( for eg: https://gateway.zscloud.net/ ) before finally redirecting back to the original endpoint. We have observed root certificates of these proxy vendors present on the user system store. These certificates are needed for the request's SSL certificate verification.

Chrome reads certificates from the system store hence the certificate verification step passes when a request is sent out of the user machine through chrome as a client. Node uses a statically compiled, hardcoded list of certificate authorities, rather than relying on the system's trust store, hence request going out from node as a client fails at the SSL verification step behind such environments.

This PR targets to read the certificates from the user system store and embed them with the existing list of root certificates with the node.

Note:

  1. These changes will make a huge impact on the applications that use open source projects like electron which uses node in their networking layers.

  2. In my current implementation, I've added logic to read the certificates from the system store and embed it with the existing list together in a single file. I am open to suggestions if we want the certificate read logic to be written somewhere else.

  3. This PR only targets the windows platform for now. All the users from which we collected feedback were windows users only.

twitharshil avatar Sep 06 '22 07:09 twitharshil

Review requested:

  • [ ] @nodejs/crypto

nodejs-github-bot avatar Sep 06 '22 07:09 nodejs-github-bot

Related: https://github.com/nodejs/node/issues/39657#issuecomment-1144080731

Having such functionality in node isn't completely out of the question but the default behavior should be consistent across platforms, meaning it should almost certainly be opt-in.

bnoordhuis avatar Sep 06 '22 21:09 bnoordhuis

@bnoordhuis Thanks for taking a look at the draft PR. I've taken a quick look at the thread mentioned in your comment. It seems like for windows we can go ahead with the current approach of reading system store and implement the solution behind an opt-in flag. I believe it will be a build-time flag.

twitharshil avatar Sep 12 '22 11:09 twitharshil

I believe it will be a build-time flag.

Why not a regular cli option?

RaisinTen avatar Sep 12 '22 12:09 RaisinTen

@RaisinTen I choose a build time flag rather than a regular cli option more from my understanding so far. I see a similar use case with node which reads the default OpenSSL cert store for certificates when built with openssl-use-def-ca-store.

twitharshil avatar Sep 15 '22 11:09 twitharshil

@twitharshil that one is actually a runtime flag accompanied by a build time flag. --use-bundled-ca and --use-openssl-ca are the runtime flags. --openssl-use-def-ca-store is the build time flag that is used to select which one of those 2 runtime flags would be enabled by default.

RaisinTen avatar Sep 19 '22 08:09 RaisinTen

Hey @bnoordhuis @RaisinTen I've opened the PR for review, PTAL. Thanks :)

twitharshil avatar Oct 10 '22 08:10 twitharshil

Left some comments. I've commented on some of the technical aspects, but I'm still unsure whether it's a desirable feature.

Speaking for myself, the fact that so far it's Windows-only counts against it.

@bnoordhuis I think this feature has been one of the most demanding features for quite some time now. In recent times I worked closely to support these MITM-based proxies environment only for the windows platform so I thought of upstreaming this feature to node.

Over the years no one picked up this work, so I believe supporting the windows platform at least is better than nothing? We can iteratively get this done for other platforms as well in separate PRs. I am open if someone else wants to pick this work for macOS & Linux otherwise I will pick it up once I find more time.

twitharshil avatar Oct 19 '22 10:10 twitharshil