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

Migrate from rustls to openssl #25

Closed
wants to merge 3 commits into from
Closed

Migrate from rustls to openssl #25

wants to merge 3 commits into from

Conversation

okjodom
Copy link

@okjodom okjodom commented Dec 14, 2022

In the interest of moving forward with the proposals on PR #20 , this PR cherry picks migration from rustls to openssl for the TLS certificate handling, introduced by @yzernik because of #17.

closes #17

yzernik and others added 3 commits December 14, 2022 12:10
* Update library to use openssl instead of rustls

* Include docstring in lib module
@okjodom okjodom changed the title Cherry pick: Openssl support Migrate from rustls to openssl Dec 14, 2022
@okjodom
Copy link
Author

okjodom commented Dec 16, 2022

@Kixunil, what do you thing of this proposal to migrate to openssl

@Kixunil
Copy link
Owner

Kixunil commented Dec 16, 2022

To be completely honest, after recent fiascos with vulnerabilities in OpenSSL the idea of forcing it onto users seems disgusting and scary. It also make building the library more annoying. I'd be fine with optional support that is off by default.

I think that ideally whichever limitations Rustls has should be resolved by improving Rustls or some workaround.

@Kixunil
Copy link
Owner

Kixunil commented Dec 16, 2022

Did a bit of digging around and found this: rustls/rustls#1032

I think this is the way to go. This library has custom verification which simply compares the certificate against the expected one (which is usually better for LND anyway) and thus IP address should work. It may require upgrading rustls though and maybe we'll hit MSRV issues. If you could look into it it'd be nice. If we hit these issues I will look at a possible solution myself.

@okjodom
Copy link
Author

okjodom commented Dec 17, 2022

Ack. on feedback. marking this PR draft while I look into the alternative path suggested

@okjodom okjodom marked this pull request as draft December 17, 2022 21:15
@okjodom
Copy link
Author

okjodom commented Feb 1, 2023

Presently attempting an upgrade to rustls v0.20.8 and I've observed lot's of breaking changes to be worked through.

@okjodom
Copy link
Author

okjodom commented Feb 28, 2023

Not the direction to go!

@okjodom okjodom closed this Feb 28, 2023
@okjodom okjodom deleted the openssl-support branch July 10, 2023 15:24
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.

Unable to connect to LND using IP address
3 participants