Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ jobs:
cargo update -p syn --precise "2.0.106" --verbose
cargo update -p quote --precise "1.0.41" --verbose
cargo update -p proc-macro2 --precise "1.0.103" --verbose
cargo update -p libc --precise "0.2.183" --verbose
cargo update -p unicode-ident --precise "1.0.22" --verbose
cargo update -p honggfuzz --precise "0.5.58" --verbose
cargo update -p semver --precise "1.0.27" --verbose
RUSTFLAGS="--cfg=fuzzing --cfg=secp256k1_fuzz --cfg=hashes_fuzz" cargo test --verbose --color always
cargo clean
- name: Run fuzzers
Expand Down
6 changes: 6 additions & 0 deletions ci/check-lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }')
# Starting with version 1.0.104, the `proc-macro2` crate has an MSRV of rustc 1.68
[ "$RUSTC_MINOR_VERSION" -lt 68 ] && cargo update -p proc-macro2 --precise "1.0.103" --verbose

# Starting with version 0.2.184, the `libc` crate has an MSRV of rustc 1.65
[ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p libc --precise "0.2.183" --verbose

# Starting with version 1.0.23, the `unicode-ident` crate has an MSRV of rustc 1.71
[ "$RUSTC_MINOR_VERSION" -lt 71 ] && cargo update -p unicode-ident --precise "1.0.22" --verbose

RUSTFLAGS='-D warnings' cargo clippy -- \
`# We use this for sat groupings` \
-A clippy::inconsistent-digit-grouping \
Expand Down
6 changes: 6 additions & 0 deletions ci/ci-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }')
# Starting with version 1.0.104, the `proc-macro2` crate has an MSRV of rustc 1.68
[ "$RUSTC_MINOR_VERSION" -lt 68 ] && cargo update -p proc-macro2 --precise "1.0.103" --verbose

# Starting with version 0.2.184, the `libc` crate has an MSRV of rustc 1.65
[ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p libc --precise "0.2.183" --verbose

# Starting with version 1.0.23, the `unicode-ident` crate has an MSRV of rustc 1.71
[ "$RUSTC_MINOR_VERSION" -lt 71 ] && cargo update -p unicode-ident --precise "1.0.22" --verbose

export RUST_BACKTRACE=1

cargo check --verbose --color always
Expand Down
11 changes: 10 additions & 1 deletion fuzz/ci-fuzz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,22 @@
set -e
set -x

RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }')

# Starting with version 0.5.60, the `honggfuzz` crate has an MSRV of rustc 1.70.
# 0.5.59 nominally has an MSRV of 1.63, but its bundled Cargo.lock pulls in
# transitive deps (`unicode-ident` 1.0.24) that require rustc 1.71. The 0.5.58
# bundled lockfile resolves cleanly on 1.63, so use that with `--locked`.
HONGGFUZZ_INSTALL_ARGS=""
[ "$RUSTC_MINOR_VERSION" -lt 70 ] && HONGGFUZZ_INSTALL_ARGS='--locked --version=0.5.58'

pushd src/bin
rm *_target.rs
./gen_target.sh
[ "$(git diff)" != "" ] && exit 1
popd

cargo install --color always --force honggfuzz --no-default-features
cargo install --color always --force honggfuzz --no-default-features $HONGGFUZZ_INSTALL_ARGS
sed -i 's/lto = true//' Cargo.toml

export RUSTFLAGS="--cfg=secp256k1_fuzz --cfg=hashes_fuzz"
Expand Down
91 changes: 80 additions & 11 deletions src/onion_message_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ use lightning::util::logger::Logger;
use crate::hrn_resolution::{
HrnResolution, HrnResolutionFuture, HrnResolver, HumanReadableName, LNURLResolutionFuture,
};
#[cfg(feature = "http")]
use crate::http_resolver::HTTPHrnResolver;
use crate::Amount;

struct OsRng;
Expand Down Expand Up @@ -93,6 +95,11 @@ fn channel() -> (ChannelSend, ChannelRecv) {
/// query message goes out in a timely manner. You can call [`Self::register_post_queue_action`] to
/// have this happen automatically.
///
/// An optional [`HTTPHrnResolver`] fallback can be enabled via [`Self::new_with_http_fallback`].
/// It is used when the onion-message DNSSEC path cannot be attempted (for example, when no
/// DNS-resolving nodes are known in the network graph) and is the only path used for LNURL
/// resolution, which is not supported by the onion-message resolver itself.
///
/// [`PeerManager::process_events`]: lightning::ln::peer_handler::PeerManager::process_events
pub struct LDKOnionMessageDNSSECHrnResolver<N: Deref<Target = NetworkGraph<L>>, L: Deref>
where
Expand All @@ -104,6 +111,8 @@ where
pending_resolutions: Mutex<HashMap<HumanReadableName, Vec<(PaymentId, ChannelSend)>>>,
message_queue: Mutex<Vec<(DNSResolverMessage, MessageSendInstructions)>>,
pm_event_poker: RwLock<Option<Box<dyn Fn() + Send + Sync>>>,
#[cfg(feature = "http")]
fallback: Option<HTTPHrnResolver>,
}

impl<N: Deref<Target = NetworkGraph<L>>, L: Deref> LDKOnionMessageDNSSECHrnResolver<N, L>
Expand All @@ -117,11 +126,32 @@ where
Self {
network_graph,
next_id: AtomicUsize::new(0),
// TODO: Swap for `new_without_expiry_validation` when we upgrade to LDK 0.2
resolver: OMNameResolver::new(0, 0),
resolver: OMNameResolver::new_without_no_std_expiry_validation(),
pending_resolutions: Mutex::new(HashMap::new()),
message_queue: Mutex::new(Vec::new()),
pm_event_poker: RwLock::new(None),
#[cfg(feature = "http")]
fallback: None,
Comment thread
benthecarman marked this conversation as resolved.
}
}

/// Constructs a new [`LDKOnionMessageDNSSECHrnResolver`] with a fallback [`HTTPHrnResolver`].
///
/// The fallback is consulted whenever the onion-message DNSSEC path cannot be attempted
/// (e.g. no DNS-resolving nodes are known in the network graph) and is also used to satisfy
/// LNURL resolution, which the onion-message resolver does not itself support.
///
/// See the struct-level documentation for more info.
#[cfg(feature = "http")]
pub fn new_with_http_fallback(network_graph: N) -> Self {
Self {
network_graph,
next_id: AtomicUsize::new(0),
resolver: OMNameResolver::new_without_no_std_expiry_validation(),
pending_resolutions: Mutex::new(HashMap::new()),
message_queue: Mutex::new(Vec::new()),
pm_event_poker: RwLock::new(None),
fallback: Some(HTTPHrnResolver::new()),
}
}

Expand All @@ -133,16 +163,24 @@ where
*self.pm_event_poker.write().unwrap() = Some(callback);
}

/// Begins resolving `hrn` via onion-message DNSSEC.
///
/// On failure, returns the error string along with a `should_fallback` flag indicating
/// whether the caller should attempt the HTTP fallback. It is only set when the
/// onion-message path could not be attempted at all (e.g. no DNS-resolving nodes are
/// known); errors that indicate a real problem (bad system clock, malformed HRN) are
/// reported as-is so that callers see them rather than masking them with HTTP.
fn init_resolve_hrn<'a>(
&'a self, hrn: &HumanReadableName,
) -> Result<ChannelRecv, &'static str> {
) -> Result<ChannelRecv, (&'static str, bool)> {
#[cfg(feature = "std")]
{
use std::time::SystemTime;
let clock_err =
"DNSSEC validation relies on having a correct system clock. It is currently set before 1970.";
let now =
SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).map_err(|_| clock_err)?;
let now = SystemTime::now()
.duration_since(SystemTime::UNIX_EPOCH)
.map_err(|_| (clock_err, false))?;
// Use `now / 60` as the block height to expire pending requests after 1-2 minutes.
self.resolver.new_best_block((now.as_secs() / 60) as u32, now.as_secs() as u32);
}
Expand All @@ -168,9 +206,10 @@ where
}
}
if dns_resolvers.is_empty() {
return Err(
return Err((
"Failed to find any DNS resolving nodes, check your network graph is synced",
);
true,
));
}

let counter = self.next_id.fetch_add(1, Ordering::Relaxed) as u64;
Expand All @@ -182,7 +221,7 @@ where
// TODO: Once LDK 0.2 ships with a new context authentication method, we shouldn't need the
// RNG here and can stop depending on std.
let (query, dns_context) =
self.resolver.resolve_name(payment_id, *hrn, &OsRng).map_err(|_| err)?;
self.resolver.resolve_name(payment_id, *hrn, &OsRng).map_err(|_| (err, false))?;
let context = MessageContext::DNSResolver(dns_context);

let (send, recv) = channel();
Expand Down Expand Up @@ -266,19 +305,49 @@ where
{
fn resolve_hrn<'a>(&'a self, hrn: &'a HumanReadableName) -> HrnResolutionFuture<'a> {
match self.init_resolve_hrn(hrn) {
Err(e) => Box::pin(async move { Err(e) }),
Err((e, _should_fallback)) => {
#[cfg(feature = "http")]
{
if _should_fallback {
if let Some(fallback) = &self.fallback {
return fallback.resolve_hrn(hrn);
}
}
}
Box::pin(async move { Err(e) })
},
Ok(recv) => Box::pin(async move { Ok(recv.await) }),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, I believe in the case that the HRN does not resolve to any DNSSEC name (and only uses LNURL) this will simply silently time out and there won't be any LNURL attempt made. We should figure out what to do here, I guess we have to have a second or two timeout here before we attempt LNURL and then we poll both LNURL and DNSSEC and return whichever returns a valid response first (if either does)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

for that then we need a runtime then, no?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmmmmm, I think instead we should do lightning/blips#71 and fallback if we get a failure response.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah that makes sense to me

}
}

fn resolve_lnurl<'a>(&'a self, _url: &'a str) -> HrnResolutionFuture<'a> {
#[cfg_attr(not(feature = "http"), allow(unused))]
fn resolve_lnurl<'a>(&'a self, url: &'a str) -> HrnResolutionFuture<'a> {
#[cfg(feature = "http")]
{
if let Some(fallback) = &self.fallback {
return fallback.resolve_lnurl(url);
}
}

let err = "DNS resolver does not support LNURL resolution";
Box::pin(async move { Err(err) })
}

#[cfg_attr(not(feature = "http"), allow(unused))]
fn resolve_lnurl_to_invoice<'a>(
&'a self, _: String, _: Amount, _: [u8; 32],
&'a self, callback: String, amount: Amount, expected_description_hash: [u8; 32],
) -> LNURLResolutionFuture<'a> {
#[cfg(feature = "http")]
{
if let Some(fallback) = &self.fallback {
return fallback.resolve_lnurl_to_invoice(
callback,
amount,
expected_description_hash,
);
}
}

let err = "resolve_lnurl_to_invoice shouldn't be called when we don't resolve LNURL";
debug_assert!(false, "{err}");
Box::pin(async move { Err(err) })
Expand Down
Loading