Skip to content

add rustls-native-certs to support MITM enterprise proxy#1365

Closed
Purexo wants to merge 3 commits intovolta-cli:mainfrom
Purexo:feature/add-rustls-native-certs
Closed

add rustls-native-certs to support MITM enterprise proxy#1365
Purexo wants to merge 3 commits intovolta-cli:mainfrom
Purexo:feature/add-rustls-native-certs

Conversation

@Purexo
Copy link
Copy Markdown

@Purexo Purexo commented Oct 28, 2022

Hello,

I wanted to use Volta on my work computer. Like lot of developpers working for large enterprise, I'm behind a MITM Corporate Proxy. Because of that, all our network is decrypted by the proxy and reencrypted with another certificate, signed by a Corporate CA Authority. and the public certificate of the CA Authority is installed on all computers of the enterprise.

So after installing Volta, I ran volta install node and error Io Error: invalid peer certificate contents: invalid peer certificate: UnknownIssuer append.

I am used to this type of error. Native langages and ssl lib do not work well by default with private CA installed on windows. So I search if volta had a configuration or a variable environment to add CA files like Node.js have NODE_EXTRA_CA_CERTS. I don't find it.
I search at least something to disable SSL check (very insecure…). I don't find it either.

I search if ssl deps of volta (rustls, attohttpc) had a env variable for this. No, it doesn't exist.

I found add_root_certificate method in attohttpc, and rustls-native-certs crate.

So I used it in archive crate (only zip, wich is use by windows). and it seems working.


I'm not a rust developer. So don't hesitate to fix my code if you can do better (I'm not familliar with mut / borrow with rust).
Also, if you know how to load once the certificates store for all attohttpc call. It could be a great improvement.


Resources:

@Purexo
Copy link
Copy Markdown
Author

Purexo commented Oct 28, 2022

after test from other command I see other attohttpc usage.

So I added the load certs on all attohttpc call.

If someone known how ensure the rustls-native-certs is call only once, and configure attohttpc to always use theses certs, I take

@Purexo
Copy link
Copy Markdown
Author

Purexo commented Oct 28, 2022

OK, all attohttpc use rustls-native-certs now.
But I don't know how to make a global lazy Cert Vec, to load once rustls-native-certs, and reuse certs at each call.

@charlespierce
Copy link
Copy Markdown
Contributor

Hi @Purexo, thanks for taking this on! I believe there are some potential issues, not with your changes, but with how attohttpc imports root certs. I opened sbstp/attohttpc#133 (and will likely look to create a PR into attohttpc this week) to address those. The core issue, as noted in the reqwest package, is that some native cert stores have very old certificates with invalid formats. Turning this on for those systems would mean that every attempt to make a network call would fail.

Instead, I'm proposing to take the same approach and skip over any native certs that don't parse correctly, which should make this more robust to those issues. Once that lands, the changes on the Volta side would be limited to enabling the feature flag within attohttpc, which would be a much smaller diff.

If that doesn't wind up working, however, this will be a great starting point to get those situations working!

@Purexo
Copy link
Copy Markdown
Author

Purexo commented Nov 2, 2022

I understand your concerns, but rustls-native-certs check the validity of certs before return them so it should be OK.

About the rest of my PR, what do you think about wrap the attohttpc usage in a subcrate to have the same version of attohttpc in all the projec without need to redeclare attohttpc in differents cargo files ?

@chriskrycho
Copy link
Copy Markdown
Contributor

About the rest of my PR, what do you think about wrap the attohttpc usage in a subcrate to have the same version of attohttpc in all the projec without need to redeclare attohttpc in differents cargo files ?

A good thought, but I think the right solution for that is actually #1367, which will manage that automatically without needing the extra indirection.

@Purexo
Copy link
Copy Markdown
Author

Purexo commented Dec 2, 2022

Issue is fixed by #1375.
And because workspace is a better approach than wrap a dependencies to use the same version everywhere this PR has no reason to exist, so I close it.

Just one question, #1375 is merged since one month now, I can use personally volta on work because I build it for testing and it's perfect. But I wait a new release to share this software with my colleague. Do you have a date @charlespierce or @chriskrycho in mind for a new release ?

@Purexo Purexo closed this Dec 2, 2022
@Purexo Purexo deleted the feature/add-rustls-native-certs branch December 2, 2022 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants