rapiz1 / rathole

A lightweight and high-performance reverse proxy for NAT traversal, written in Rust. An alternative to frp and ngrok.
Apache License 2.0
9.43k stars 475 forks source link

feat: optional rustls support #330

Closed sunmy2019 closed 7 months ago

sunmy2019 commented 7 months ago

This is the initial implementation of rustls support.

Use it by disabling native-tls-support flag and enabling rustls-support flag.

Notes:

  1. It passes all tests. Don't know if that's enough.
  2. There is no available package other than openssl that can handle all pkcs12 pbe methods. The only available create is p12 that handles some of the PBE methods. I choose one of them as you can see in examples/tls/create_self_signed_cert.sh
  3. features flags like: "rustls-support" -> "tls-support", "native-tls-support" -> "tls-support".

Feedback is welcome. I am new to this project.

sunmy2019 commented 7 months ago

Seem the check requires it passes on both native-tls and rustls support enabled.

Supporting both enabled is not common in other crates. I deliberately disabled this.

What should we do here?

rapiz1 commented 7 months ago

https://github.com/taiki-e/cargo-hack has some flags to adjust the powerset behaviour

sunmy2019 commented 7 months ago

I will add docs and more tests for rustls when the implementation is done and reviewed.

rapiz1 commented 7 months ago

It passes all tests. Don't know if that's enough.

Did you check how will the integration test run with this new compile flag? I think we're only running with the default tls implementation ( native-tls ). If we run the test with both native-tls and rustls, we can be sure it's correct.

sunmy2019 commented 7 months ago

Did you check how will the integration test run with this new compile flag? I think we're only running with the default tls implementation ( native-tls )

I run it locally by replacing native-tls with rustls.

I am figuring out how to test both in CI.

sunmy2019 commented 7 months ago

I added both tests to CI.

sunmy2019 commented 7 months ago

I will add docs in the sequential PR.

skbeh commented 7 months ago

@sunmy2019 The p12 crate is incompatible with pkcs12 cert generated by OpenSSL3: https://github.com/hjiayz/p12/issues/13.

sunmy2019 commented 7 months ago

@sunmy2019 The p12 crate is incompatible with pkcs12 cert generated by OpenSSL3: hjiayz/p12#13.

See my updated docs here: #337

One difference is that, the crate we use for loading PKCS#12 archives can only handle limited types of PBE algorithms. We only support PKCS#12 archives that they (crate p12) support. So we need to specify the PBE algorithm when creating the PKCS#12 archive.

In short, the command to create the PKCS#12 archive with rustls support is:

openssl pkcs12 -export -out identity.pfx -inkey server.key -in server.crt -certfile ca_chain_certs.crt \
    -keypbe PBE-SHA1-3DES -certpbe PBE-SHA1-3DES
skbeh commented 7 months ago

@sunmy2019 Both SHA1 and 3DES are not secure according to today's security standards. IMHO, a more updated crate should be used to handle modern algorithms.

sunmy2019 commented 7 months ago

@sunmy2019 Both SHA1 and 3DES are not secure according to today's security standards. IMHO, a more updated crate should be used to handle modern algorithms.

Do you have suggestions?

sunmy2019 commented 7 months ago

@sunmy2019 Both SHA1 and 3DES are not secure according to today's security standards. IMHO, a more updated crate should be used to handle modern algorithms.

That makes sense. I think we should support the default PBEs for newer formats, but I am unsure how.

Currently, the best-supported crate is still the openssl.

One strategy is to fork p12 and maintain our use case for it. There is a PR implementing what we need (https://github.com/hjiayz/p12/pull/10), but no one is responding.

One strategy is to deprecate PKCS#12, and provide PCKS#8 options which is maintained by rustls itself.

skbeh commented 6 months ago

According to https://stackoverflow.com/a/62863490/7878845, PKCS#12 file can be generated without encryption, which could be an alternative to not provide false sense of security.

sunmy2019 commented 6 months ago

According to https://stackoverflow.com/a/62863490/7878845, PKCS#12 file can be generated without encryption, which could be an alternative to not provide false sense of security.

I've been wondering what's the point of this password in this program when you write down the password in a separate file.

skbeh commented 6 months ago

@rapiz1 Also please update the document to use 3DES instead of RC2 (i.e. -keypbe PBE-SHA1-3DES -certpbe PBE-SHA1-3DES since 3DES provides not good but still acceptable encryption if someone ever wants it.