Skip to content

Commit

Permalink
Fix reachable unwrap on non-channel_type manual channel acceptance
Browse files Browse the repository at this point in the history
If we receive an `OpenChannel` message without a `channel_type`
with `manually_accept_inbound_channels` set, we will `unwrap()`
`None`.

This is uncommon these days as most nodes support `channel_type`,
but sadly is rather trivial for a peer to hit for those with manual
channel acceptance enabled.

Reported in and fixes #2804. Luckily, the updated
`full_stack_target` has no issue reaching this issue quickly.
  • Loading branch information
TheBlueMatt committed Jan 6, 2024
1 parent 3b6a361 commit bceca1e
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 28 deletions.
62 changes: 36 additions & 26 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6845,6 +6845,41 @@ pub(super) struct InboundV1Channel<SP: Deref> where SP::Target: SignerProvider {
pub unfunded_context: UnfundedChannelContext,
}

/// Fetches the [`ChannelTypeFeatures`] that will be used for a channel built from a given
/// [`msgs::OpenChannel`].
pub(super) fn channel_type_from_open_channel(
msg: &msgs::OpenChannel, their_features: &InitFeatures,
our_supported_features: &ChannelTypeFeatures
) -> Result<ChannelTypeFeatures, ChannelError> {
if let Some(channel_type) = &msg.channel_type {
if channel_type.supports_any_optional_bits() {
return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned()));
}

// We only support the channel types defined by the `ChannelManager` in
// `provided_channel_type_features`. The channel type must always support
// `static_remote_key`.
if !channel_type.requires_static_remote_key() {
return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned()));
}
// Make sure we support all of the features behind the channel type.
if !channel_type.is_subset(our_supported_features) {
return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned()));
}
let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false };
if channel_type.requires_scid_privacy() && announced_channel {
return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
}
Ok(channel_type.clone())
} else {
let channel_type = ChannelTypeFeatures::from_init(&their_features);
if channel_type != ChannelTypeFeatures::only_static_remote_key() {
return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
}
Ok(channel_type)
}
}

impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
/// Creates a new channel from a remote sides' request for one.
/// Assumes chain_hash has already been checked and corresponds with what we expect!
Expand All @@ -6863,32 +6898,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {

// First check the channel type is known, failing before we do anything else if we don't
// support this channel type.
let channel_type = if let Some(channel_type) = &msg.channel_type {
if channel_type.supports_any_optional_bits() {
return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned()));
}

// We only support the channel types defined by the `ChannelManager` in
// `provided_channel_type_features`. The channel type must always support
// `static_remote_key`.
if !channel_type.requires_static_remote_key() {
return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned()));
}
// Make sure we support all of the features behind the channel type.
if !channel_type.is_subset(our_supported_features) {
return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned()));
}
if channel_type.requires_scid_privacy() && announced_channel {
return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
}
channel_type.clone()
} else {
let channel_type = ChannelTypeFeatures::from_init(&their_features);
if channel_type != ChannelTypeFeatures::only_static_remote_key() {
return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
}
channel_type
};
let channel_type = channel_type_from_open_channel(msg, their_features, our_supported_features)?;

let channel_keys_id = signer_provider.generate_channel_keys_id(true, msg.funding_satoshis, user_id);
let holder_signer = signer_provider.derive_channel_signer(msg.funding_satoshis, channel_keys_id);
Expand Down
12 changes: 10 additions & 2 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, Messa
// Since this struct is returned in `list_channels` methods, expose it here in case users want to
// construct one themselves.
use crate::ln::{inbound_payment, ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
use crate::ln::channel::{Channel, ChannelPhase, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, UnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
use crate::ln::channel::{self, Channel, ChannelPhase, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, UnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
#[cfg(any(feature = "_test_utils", test))]
use crate::ln::features::Bolt11InvoiceFeatures;
Expand Down Expand Up @@ -6170,13 +6170,21 @@ where

// If we're doing manual acceptance checks on the channel, then defer creation until we're sure we want to accept.
if self.default_configuration.manually_accept_inbound_channels {
let channel_type_res = channel::channel_type_from_open_channel(
&msg, &peer_state.latest_features, &self.channel_type_features()
);
let channel_type = match channel_type_res {
Ok(channel_type) => channel_type,
Err(e) =>
return Err(MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id)),
};
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push_back((events::Event::OpenChannelRequest {
temporary_channel_id: msg.temporary_channel_id.clone(),
counterparty_node_id: counterparty_node_id.clone(),
funding_satoshis: msg.funding_satoshis,
push_msat: msg.push_msat,
channel_type: msg.channel_type.clone().unwrap(),
channel_type,
}, None));
peer_state.inbound_channel_request_by_id.insert(channel_id, InboundChannelRequest {
open_channel_msg: msg.clone(),
Expand Down

0 comments on commit bceca1e

Please sign in to comment.