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

Self-signed TLS certificates don't work with rustls #5450

Open
casey opened this issue Jun 29, 2021 · 13 comments · May be fixed by #9325
Open

Self-signed TLS certificates don't work with rustls #5450

casey opened this issue Jun 29, 2021 · 13 comments · May be fixed by #9325

Comments

@casey
Copy link

casey commented Jun 29, 2021

We're working on an lnd gRPC client in Rust, using rustls, and rustls doesn't accept the self-signed certificates generated by LND 0.13.0-beta.

The error reported by rustls, WebPKIError(CAUsedAsEndEntity), is caused by lnd's certificate being a CA certificate, when servers should present end-entity certificates.

These certificates work with OpenSSL (which we assume other client implementations are using), and we're trying to figure out the difference in behavior, cf. briansmith/webpki#114 (comment). We're not totally sure, but we think that OpenSSL just doesn't check that servers present end-entity certificates, when according to the spec it should.

We think the solution would be to make lnd generate both a CA cert (e.g. ca.cert) and an end-entity cert (e.g. ee.cert) signed by that CA. Clients would then add ca.cert as a trusted root certificate, and would then accept ee.cert when presented by lnd.

@Roasbeef
Copy link
Member

Roasbeef commented Jun 29, 2021

Note that you can create your own cert, and drop it into place (with the proper name) and lnd will just run with it and use it as normal.

I don't think it's an OpenSSL specific issue, as the certs work with all Go programs (which don't use openssl) and many other languages that may not be directly linking to openssl.

@Roasbeef
Copy link
Member

It looks like this came up in the past: #4201

We eventually resolved things by specifying some additional attributes to specify ExtendedKeyUsage: #4209

@casey
Copy link
Author

casey commented Jun 30, 2021

Note that you can create your own cert, and drop it into place (with the proper name) and lnd will just run with it and use it as normal.

We're writing an app that users will run with their own nodes, so we don't necessarily control the certificates that will be used, and assume that many node operators will have their own self-signed certificates.

I don't think it's an OpenSSL specific issue, as the certs work with all Go programs (which don't use openssl) and many other languages that may not be directly linking to openssl.

Ah, gotcha. I just assumed that Go used openssl.

We eventually resolved things by specifying some additional attributes to specify ExtendedKeyUsage: #4209

Thanks for the pointer! I think it might be the case that rustls / webpki doesn't support the serverAuth extended key usage value. I pinged the developers to ask.

@mbacarella
Copy link

I would like to reiterate @casey here

We're writing an app that users will run with their own nodes, so we don't necessarily control the certificates that will be used, and assume that many node operators will have their own self-signed certificates.

Similar issue here mirleft/ocaml-tls#446, except with ocaml-tls instead of rusttls 😄

@guggero
Copy link
Collaborator

guggero commented Apr 27, 2022

It sounds to me like the problem is that the cert has the CA cert=true flag? Could be that the way the cert is generated is non-standard. But it sounds like there is a workaround on the client side? That you just need to allow that CA cert=true flag?

Changing the way the TLS cert works in lnd is probably not an easy thing. Dealing with the tls.cert and tls.key is already quite cumbersome, I'd rather avoid adding an extra CA cert file into the mix.

@mbacarella
Copy link

But it sounds like there is a workaround on the client side? That you just need to allow that CA cert=true flag?

Sure, it's a workaround. I have to say I'm sad about this on, uhm, spiritual grounds?

None of us here is using openssl, having all made a clean break (rustls, ocaml-tls, go's tls), but we're talking about adopting bad protocol adherence behavior inherited from openssl to proceed.

@guggero
Copy link
Collaborator

guggero commented Apr 29, 2022

I haven't had time to look into this in more detail, it's been a while since I last did a TLS deep dive.
Is the problem that in the cert presented by lnd the server cert and CA cert are basically the same one (issued from the same private/public key pair)? Or is it just a flag that is set in the cert that could be set differently to fix the problem?
If the latter is true, then I'd definitely be in favor of changing it. If the former is true and we need to introduce new key pairs and a CA cert then I'd be rather hesitant, since it will complicate the setup for many users with not that many benefits (IMO).

@mbacarella
Copy link

TLS is kind of a black box to me as well so I can just say what I did to get it to work. The way I'm fixing this client side is to fork the ocaml-x509 library into my project, and hacking it not to blow up just because the certificate from lnd has a flag in it that says it's a CA certificate. That works.

I didn't open this issue though, and @casey may have different thoughts re: rustls.

@guggero
Copy link
Collaborator

guggero commented May 2, 2022

Hmm, it really just looks like we set a flag that could be set to false instead. Did a few quick tests and nothing seems to break with this patch (on top of current master, aca2299):

diff --git a/cert/selfsigned.go b/cert/selfsigned.go
index dd87fb0b7..3a41f8ba0 100644
--- a/cert/selfsigned.go
+++ b/cert/selfsigned.go
@@ -240,7 +240,6 @@ func GenCertPair(org, certFile, keyFile string, tlsExtraIPs,
                KeyUsage: x509.KeyUsageKeyEncipherment |
                        x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
                ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-               IsCA:                  true, // so can sign self.
                BasicConstraintsValid: true,
 
                DNSNames:    dnsNames,
diff --git a/go.mod b/go.mod
index 2ed2d56d3..dbc0a7ba0 100644
--- a/go.mod
+++ b/go.mod
@@ -172,6 +172,8 @@ replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2
 // https://github.com/lightninglabs/neutrino/pull/247 is merged.
 replace github.com/lightninglabs/neutrino => github.com/lightninglabs/neutrino v0.13.2-0.20220209052920-0c79b771272b
 
+replace github.com/lightningnetwork/lnd/cert => ./cert
+
 // If you change this please also update .github/pull_request_template.md and
 // docs/INSTALL.md.
 go 1.17

@mbacarella and @casey, does this patch resolve your issues?

@mbacarella
Copy link

mbacarella commented Jun 27, 2022

I applied that patch on top of 0.14.1-beta and tried it out. My client now reports an "invalid certificate chain" error instead.

lncli and openssl based clients continue to work. Not sure if the issue is another disagreement in protocol adherence or what.

hannesm added a commit to hannesm/opam-repository that referenced this issue Feb 4, 2023
CHANGES:

* Validation: allow self-signed server certificate with BasicConstraints CA=true
  (reported by @mbacarella in mirleft/ocaml-tls#446
   (lightningnetwork/lnd#5450), fix mirleft/ocaml-x509#161 by @hannesm)
@guggero
Copy link
Collaborator

guggero commented May 12, 2023

@mbacarella was this fixed with ocaml/opam-repository#23231? Can this issue be closed?

@mbacarella
Copy link

Well, the problem is fixed in OCaml TLS, but this issue is about the Rust TLS library so I can't comment on whether it should be closed. :)

@darioAnongba
Copy link

Hi guys,

Just wanted to add my 2 cents to this issue. It is true that the rustls is most likely the stricter lib around so it is the only one having this issue but rustls validation is correct as per the specification.

The error CaUsedAsEndEntity indicates that Rustls is rejecting the server’s certificate because it is a CA certificate being used as an end-entity certificate, which is not allowed by default in TLS protocols. Rustls adheres strictly to the TLS specification and security best practices, so it flags this usage as invalid.
In TLS, the server certificate presented during the handshake should be an end-entity certificate (CA:FALSE). Using a CA certificate as a server certificate violates the specification.

I see many people trying to integrate LND in rust using tonic having to implement custom verifiers or having to generate their own certs for LND to use, making devOps tasks harder.

Quoting @guggero:

Changing the way the TLS cert works in lnd is probably not an easy thing. Dealing with the tls.cert and tls.key is already quite cumbersome, I'd rather avoid adding an extra CA cert file into the mix.

I actually recommend having a CA, TLS key and TLS cert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants