Skip to content

Commit

Permalink
splice: unwrap the old_secret during tx candidates
Browse files Browse the repository at this point in the history 
While running the integration testing in VLS we noted that
there is a problem with the old_secret during that revoke and ack.

The buildin signer of core lightning return always the secret,
but with signer with more strict policy.

In fact, the VLS return the old_secret only if it is the
last tx candidate. So we should keep track of the
old_secret during the recursion.

This is unsafe because we can only revoke transaction M-1 once
we have transaction M signed by the counterparty. If there is a
splice candidate that is unsigned yet, and we revoke commitment
transaction M-1 for it, and it gets into the blockchain, and the peer
goes away, we can't force close anymore.

lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: (null):0 ((null)) 0xffffffffffffffff
lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: FATAL SIGNAL (version v23.08-64-gbffe599)
lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: common/daemon.c:38 (send_backtrace) 0x55c4a2ebd97b
lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: common/status.c:221 (status_failed) 0x55c4a2ecf0ae
lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: common/subdaemon.c:18 (status_backtrace_exit) 0x55c4a2ecf42b
lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: common/daemon.c:78 (crashdump) 0x55c4a2ebdb16
lightningd-1 2023-10-25T15:24:25.151Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0 ((null)) 0x7f084e44251f
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:317 ((null)) 0x7f084e5a09cd
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: wire/towire.c:17 (towire) 0x55c4a2ed610e
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: bitcoin/privkey.c:33 (towire_secret) 0x55c4a2eea03b
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: wire/peer_wiregen.c:2737 (towire_revoke_and_ack) 0x55c4a2ee1d3d
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:1850 (make_revocation_msg_from_secret) 0x55c4a2e9d80e
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:1931 (send_revocation) 0x55c4a2e9de7a
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:2249 (handle_peer_commit_sig) 0x55c4a2e9f175
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:2880 (interactive_send_commitments) 0x55c4a2ea0c2e
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:3937 (splice_initiator_user_finalized) 0x55c4a2ea40ce
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:4011 (splice_initiator_user_update) 0x55c4a2ea44d4
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:5756 (req_in) 0x55c4a2ea8d9d
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: channeld/channeld.c:6151 (main) 0x55c4a2eaa3f8
lightningd-1 2023-10-25T15:24:25.152Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: ../sysdeps/nptl/libc_start_call_main.h:58 (__libc_start_call_main) 0x7f084e429d8f
lightningd-1 2023-10-25T15:24:25.153Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: ../csu/libc-start.c:392 (__libc_start_main_impl) 0x7f084e429e3f
lightningd-1 2023-10-25T15:24:25.153Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: (null):0 ((null)) 0x55c4a2e98734
lightningd-1 2023-10-25T15:24:25.153Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: backtrace: (null):0 ((null)) 0xffffffffffffffff
lightningd-1 2023-10-25T15:24:25.153Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#1: STATUS_FAIL_INTERNAL_ERROR: FATAL SIGNAL

Reported-by: @devrandom
Signed-off-by: Vincenzo Palazzo <[email protected]>
  • Loading branch information
@vincenzopalazzo
vincenzopalazzo committed Oct 27, 2023
1 parent 34692ec commit 20d10b3
Showing 1 changed file with 20 additions and 9 deletions.
29 changes: 20 additions & 9 deletions channeld/channeld.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1963,20 +1963,25 @@ static void send_revocation(struct peer *peer,
peer_write(peer->pps, take(msg));
}

struct commitsig_info {
struct commitsig *commitsig;
struct secret *old_secret;
};

/* Calling `handle_peer_commit_sig` with a `commit_index` of 0 and
* `changed_htlcs` of NULL will process the message, then read & process coming
* consecutive commitment messages equal to the number of inflight splices.
*
* Returns the last commitsig received. When splicing this is the
* newest splice commit sig. */
static struct commitsig *handle_peer_commit_sig(struct peer *peer,
static struct commitsig_info *handle_peer_commit_sig(struct peer *peer,
const u8 *msg,
u32 commit_index,
const struct htlc **changed_htlcs,
s64 splice_amnt,
s64 remote_splice_amnt)
{
struct commitsig *result;
struct commitsig_info *result;
struct channel_id channel_id;
struct bitcoin_signature commit_sig;
secp256k1_ecdsa_signature *raw_sigs;
Expand Down Expand Up @@ -2206,11 +2211,15 @@ static struct commitsig *handle_peer_commit_sig(struct peer *peer,
"Reading validate_commitment_tx reply: %s",
tal_hex(tmpctx, msg2));

result = tal(tmpctx, struct commitsig);
result->tx = clone_bitcoin_tx(result, txs[0]);
result->commit_signature = commit_sig;
result->htlc_signatures = htlc_sigs;
struct commitsig *commitsig;
commitsig = tal(tmpctx, struct commitsig);
commitsig->tx = clone_bitcoin_tx(tmpctx, txs[0]);
commitsig->commit_signature = commit_sig;
commitsig->htlc_signatures = htlc_sigs;

result = tal(tmpctx, struct commitsig_info);
result->commitsig = commitsig;
result->old_secret = old_secret;
/* Only the parent call continues from here.
* Return for all child calls. */
if(commit_index)
Expand Down Expand Up @@ -2238,10 +2247,12 @@ static struct commitsig *handle_peer_commit_sig(struct peer *peer,
result = handle_peer_commit_sig(peer, splice_msg, i + 1,
changed_htlcs, sub_splice_amnt,
funding_diff - sub_splice_amnt);
tal_arr_expand(&commitsigs, result);
old_secret = result->old_secret;
tal_arr_expand(&commitsigs, result->commitsig);
tal_steal(commitsigs, result);
}

assert(old_secret);
peer->splice_state->revoked_count = peer->splice_state->count;

send_revocation(peer, &commit_sig, htlc_sigs, changed_htlcs, txs[0],
Expand Down Expand Up @@ -2841,7 +2852,7 @@ static struct commitsig *interactive_send_commitments(struct peer *peer,
struct wally_psbt *psbt,
enum tx_role our_role)
{
struct commitsig *result;
struct commitsig_info *result;
const u8 *msg, *commit_msg;

commit_msg = NULL;
Expand Down Expand Up @@ -2892,7 +2903,7 @@ static struct commitsig *interactive_send_commitments(struct peer *peer,
handle_peer_revoke_and_ack(peer, msg);
}

return result;
return result->commitsig;
}

static struct wally_psbt_output *find_channel_output(struct peer *peer,
Expand Down

0 comments on commit 20d10b3

Please sign in to comment.