Skip to content
This repository has been archived by the owner on Mar 23, 2021. It is now read-only.

add danger clause for ignoring invalid certificates #2470

Merged
merged 1 commit into from
Apr 17, 2020
Merged

add danger clause for ignoring invalid certificates #2470

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions cnd/src/swap_protocols/halight/connector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,24 @@ fn client(certificate: &Certificate, macaroon: &Macaroon) -> Result<reqwest::Cli
HeaderValue::from_str(&macaroon.0)?,
);

Ok(reqwest::Client::builder()
// The generated, self-signed lnd certificate is deemed invalid on macOS
// Catalina because of new certificate requirements in macOS Catalina: https://support.apple.com/en-us/HT210176
// By using this conditional compilation step for macOS we accept invalid
// certificates. This is only a minimal security risk because by default the
// certificate that lnd generates is configured to only allow connections
// from localhost. Ticket that will resolve that issue: https://github.com/lightningnetwork/lnd/issues/4201
#[cfg(target_os = "macos")]
let client = reqwest::Client::builder()
.danger_accept_invalid_certs(true)
.add_root_certificate(cert)
.default_headers(default_headers)
.build()?)
.build()?;

#[cfg(not(target_os = "macos"))]
let client = reqwest::Client::builder()
.add_root_certificate(cert)
.default_headers(default_headers)
.build()?;
Comment on lines +453 to +464
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope it is not too much bike-shedding but I found this: https://doc.rust-lang.org/std/macro.cfg.html

So I think we could do:

// The generated, self-signed lnd certificate is deemed invalid on macOS
// Catalina because of new certificate requirements in macOS Catalina: https://support.apple.com/en-us/HT210176
// By using this conditional compilation step for macOS we accept invalid
// certificates. This is only a minimal security risk because by default the
// certificate that lnd generates is configured to only allow connections
// from localhost. Ticket that will resolve that issue: https://github.com/lightningnetwork/lnd/issues/4201
let accept_invalid_certificates = if cfg!(target_os = "macos") {
	true
} else {
	false
};    

let client = reqwest::Client::builder()
	.danger_accept_invalid_certs(accept_invalid_certificates)
    .add_root_certificate(cert)
    .default_headers(default_headers)
    .build()?;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you if you like it better :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the 🚲🏠 comment reaction when we need it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beautiful, but saw it too late. Feel free to push a follow up :)


Ok(client)
}