Skip to content

Commit

Permalink
channel_control: Forget if unconfirmed for a long time and we are fun…
Browse files Browse the repository at this point in the history
…dee.

We should forget this as it is a potential DoS if we remember every
funding txid that an attacker gave in a `funding_created` but never
broadcasted.
  • Loading branch information
ZmnSCPxj authored and cdecker committed May 23, 2018
1 parent 30daa53 commit 097a8e7
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 2 deletions.
1 change: 1 addition & 0 deletions lightningd/chaintopology.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <common/timeout.h>
#include <common/utils.h>
#include <inttypes.h>
#include <lightningd/channel_control.h>
#include <lightningd/gossip_control.h>

/* Mutual recursion via timer. */
Expand Down
1 change: 0 additions & 1 deletion lightningd/chaintopology.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ void begin_topology(struct chain_topology *topo);

struct txlocator *locate_tx(const void *ctx, const struct chain_topology *topo, const struct bitcoin_txid *txid);

void notify_new_block(struct lightningd *ld, unsigned int height);
void notify_feerate_change(struct lightningd *ld);

#if DEVELOPER
Expand Down
94 changes: 94 additions & 0 deletions lightningd/channel_control.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#include <bitcoin/pubkey.h>
#include <bitcoin/script.h>
#include <ccan/fdpass/fdpass.h>
#include <channeld/gen_channel_wire.h>
#include <common/memleak.h>
#include <common/timeout.h>
#include <common/utils.h>
#include <errno.h>
#include <gossipd/gossip_constants.h>
#include <hsmd/capabilities.h>
Expand Down Expand Up @@ -327,3 +331,93 @@ bool channel_tell_funding_locked(struct lightningd *ld,

return true;
}

/* Check if we are the fundee of this channel, the channel
* funding transaction is still not yet seen onchain, and
* it has been too long since the channel was first opened.
* If so, we should forget the channel. */
static bool
is_fundee_should_forget(struct lightningd *ld,
struct channel *channel,
u32 block_height)
{
u32 max_no_funding_tx = 2016;

/* FIXME: we should be getting max_no_funding_tx
* from an lightningd option, which is why we get
* it as an argument. */
(void) ld;

/* BOLT #2:
*
* A non-funding node (fundee):
* - SHOULD forget the channel if it does not see the
* funding transaction after a reasonable timeout.
*/

/* Only applies if we are fundee. */
if (channel->funder == LOCAL)
return false;

/* Does not apply if we already saw the funding tx. */
if (channel->scid)
return false;

/* Not even reached previous starting blocknum.
* (e.g. if --rescan option is used) */
if (block_height < channel->first_blocknum)
return false;

/* Timeout in blocks not yet reached. */
if (block_height - channel->first_blocknum < max_no_funding_tx)
return false;

/* Ah forget it! */
return true;
}

/* Notify all channels of new blocks. */
void channel_notify_new_block(struct lightningd *ld,
u32 block_height)
{
struct peer *peer;
struct channel *channel;
struct channel **to_forget = tal_arr(NULL, struct channel *, 0);
size_t i;

list_for_each (&ld->peers, peer, list) {
list_for_each (&peer->channels, channel, list)
if (is_fundee_should_forget(ld, channel, block_height)) {
i = tal_count(to_forget);
tal_resize(&to_forget, i + 1);
to_forget[i] = channel;
}
}

/* Need to forget in a separate loop, else the above
* nested loops may crash due to the last channel of
* a peer also deleting the peer, making the inner
* loop crash.
* list_for_each_safe does not work because it is not
* just the freeing of the channel that occurs, but the
* potential destruction of the peer that invalidates
* memory the inner loop is accessing. */
for (i = 0; i < tal_count(to_forget); ++i) {
channel = to_forget[i];
/* Report it first. */
log_unusual(channel->log,
"Forgetting channel: "
"It has been %"PRIu32" blocks without the "
"funding transaction %s getting deeply "
"confirmed. "
"We are fundee and can forget channel without "
"loss of funds.",
block_height - channel->first_blocknum,
type_to_string(tmpctx, struct bitcoin_txid,
&channel->funding_txid));
/* And forget it. */
delete_channel(channel);
}

tal_free(to_forget);
}
3 changes: 3 additions & 0 deletions lightningd/channel_control.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@ bool channel_tell_funding_locked(struct lightningd *ld,
struct channel *channel,
const struct bitcoin_txid *txid,
u32 depth);
/* Notify channels of new blocks. */
void channel_notify_new_block(struct lightningd *ld,
u32 block_height);

#endif /* LIGHTNING_LIGHTNINGD_CHANNEL_CONTROL_H */
9 changes: 9 additions & 0 deletions lightningd/lightningd.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <fcntl.h>
#include <lightningd/bitcoind.h>
#include <lightningd/chaintopology.h>
#include <lightningd/channel_control.h>
#include <lightningd/invoice.h>
#include <lightningd/jsonrpc.h>
#include <lightningd/log.h>
Expand Down Expand Up @@ -292,6 +293,14 @@ static int io_poll_lightningd(struct pollfd *fds, nfds_t nfds, int timeout)
return io_poll_debug(fds, nfds, timeout);
}

void notify_new_block(struct lightningd *ld,
u32 block_height)
{
/* Inform our subcomponents individually. */
htlcs_notify_new_block(ld, block_height);
channel_notify_new_block(ld, block_height);
}

int main(int argc, char *argv[])
{
struct lightningd *ld;
Expand Down
3 changes: 3 additions & 0 deletions lightningd/lightningd.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,4 +192,7 @@ struct backtrace_state *backtrace_state;
/* Check we can run subdaemons, and check their versions */
void test_daemons(const struct lightningd *ld);

/* Notify lightningd about new blocks. */
void notify_new_block(struct lightningd *ld, u32 block_height);

#endif /* LIGHTNING_LIGHTNINGD_LIGHTNINGD_H */
2 changes: 1 addition & 1 deletion lightningd/peer_htlcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1514,7 +1514,7 @@ static u32 htlc_in_deadline(const struct lightningd *ld,
return hin->cltv_expiry - (ld->config.cltv_expiry_delta + 1)/2;
}

void notify_new_block(struct lightningd *ld, u32 height)
void htlcs_notify_new_block(struct lightningd *ld, u32 height)
{
bool removed;

Expand Down
2 changes: 2 additions & 0 deletions lightningd/peer_htlcs.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@ void onchain_failed_our_htlc(const struct channel *channel,
const char *why);
void onchain_fulfilled_htlc(struct channel *channel,
const struct preimage *preimage);

void htlcs_notify_new_block(struct lightningd *ld, u32 height);
#endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */
7 changes: 7 additions & 0 deletions lightningd/test/run-find_my_path.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ void activate_peers(struct lightningd *ld UNNEEDED)
/* Generated stub for begin_topology */
void begin_topology(struct chain_topology *topo UNNEEDED)
{ fprintf(stderr, "begin_topology called!\n"); abort(); }
/* Generated stub for channel_notify_new_block */
void channel_notify_new_block(struct lightningd *ld UNNEEDED,
u32 block_height UNNEEDED)
{ fprintf(stderr, "channel_notify_new_block called!\n"); abort(); }
/* Generated stub for daemon_setup */
void daemon_setup(const char *argv0 UNNEEDED,
void (*backtrace_print)(const char *fmt UNNEEDED, ...) UNNEEDED,
Expand Down Expand Up @@ -58,6 +62,9 @@ size_t hash_htlc_key(const struct htlc_key *htlc_key UNNEEDED)
/* Generated stub for hsm_init */
void hsm_init(struct lightningd *ld UNNEEDED)
{ fprintf(stderr, "hsm_init called!\n"); abort(); }
/* Generated stub for htlcs_notify_new_block */
void htlcs_notify_new_block(struct lightningd *ld UNNEEDED, u32 height UNNEEDED)
{ fprintf(stderr, "htlcs_notify_new_block called!\n"); abort(); }
/* Generated stub for json_escape */
struct json_escaped *json_escape(const tal_t *ctx UNNEEDED, const char *str TAKES UNNEEDED)
{ fprintf(stderr, "json_escape called!\n"); abort(); }
Expand Down

0 comments on commit 097a8e7

Please sign in to comment.