Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Working around invalid self-signed certificates #772

Closed
casey opened this issue Jun 29, 2021 · 5 comments
Closed

Working around invalid self-signed certificates #772

casey opened this issue Jun 29, 2021 · 5 comments

Comments

@casey
Copy link

casey commented Jun 29, 2021

I'm working on a project that uses rustls with tonic to connect to LND. By default, LND creates a self-signed end-entity certificate with no CA.

From reading other issues, in particular briansmith/webpki#114, we gather that such certificates can't be used with webpki, and thus rustls with the default configuration.

We don't have control of the certificates that LND generates, so we're trying to find a way to work around this.

Our hacky solution is have the user pass in LND's certificate, and pass it to a ServerCertVerifier implementation, SingleCertVerifier, whose verify_server_cert function checks that the certificate presented by the server is byte-for-byte equal to the certificate that the user passed in. We aren't planning on doing hostname or expiration checking, because this cert was given to us by the user.

Here is the current version of SingleCertVerifier: single_cert_verifier.rs

Our assumption is that when using SingleCertVerifier, rustls, during other parts of the TLS handshake, will still make sure that the server has the private key that matches the presented certificate, e.g. by verifying a signature from the server, so man-in-the-middle attacks are not possible.

Is that assumption correct?

@djc
Copy link
Member

djc commented Jun 30, 2021

You can see here what the client does after receiving the server certificate:

https://github.com/ctz/rustls/blob/main/rustls/src/client/tls13.rs#L724

In general, not doing hostname or expiration checking sounds like a really bad idea, where you essentially shift blame for any security issues that come up to your users.

@Kixunil
Copy link

Kixunil commented Jul 2, 2021

@djc I believe not checking hostname and probably also expiration is fine in this case. The short version is that if the certificate located on client is invalid (compromised) no additional check can help.

If you have a bit of time I'd appreciate reviewing my detailed explanation of why I believe it is the case and letting me know if you see any mistake. If you don't I can understand.

@jbg
Copy link
Contributor

jbg commented Jul 3, 2021

@Kixunil if you are asserting that you received the exact certificate, byte-for-byte, that you know the server is supposed to be sending, then it's irrelevant whether you check hostname or expiry in terms of preventing a MITM from impersonating the server. But as @djc says, you're pushing responsibility onto the user of your app/library to securely transfer that certificate out-of-band and store it, and things like revocation (e.g. if the server cert is compromised) become more complex and error-prone.

IMO the automatic certificate generation of LND is a bit of an anti-feature for a production deployment. I helped with a deployment of LND where security was a concern, and we removed its automatic certificate generation and used cert-manager in CA mode to generate a certificate for it. It injects the cert + key on the LND side, and the cert-manager CA cert on the client side, where it's added as a trusted root. This way renewal and revocation can be handled sensibly too. You might consider something similar.

@Kixunil
Copy link

Kixunil commented Jul 3, 2021

I think I understand. I will add the option to use CAs instead then. I believe the direct verification still has its merits:

  • with LND there's usually exactly one user - I don't want other people to spend my money, there's not a huge difference between securely transferring certificate to CA and to the single user. Especially since it requires transferring macaroon securely anyway, so bundling in certificate is fine.
  • CAs are occasionally compromised even by state actors. LND is by nature intended to be resistant against state actors and other attackers.

So despite allowing other forms of verification I do not intend to remove this feature nor stop using it myself.

@djc
Copy link
Member

djc commented Jul 3, 2021

As Brian already locked the webpki thread, this is also off-topic for this repo. Please take further discussion to some other venue.

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

No branches or pull requests

4 participants