mirror of
https://github.com/ElementsProject/lightning.git
synced 2025-02-21 14:24:09 +01:00
channeld: Verify the signature sent by the counterparty
This commit addresses an issue to enhance the resilience of core lightning when receiving node announcements. According to BOLT 7 (The announcement_signatures Message), if the node_signature OR the bitcoin_signature is NOT correct, it is recommended to either send a warning and close the connection or send an error and fail the channel. In this commit, we take a strict approach. If any error is detected, we send an error and fail the open channel operation. This is because the announcement_signatures operation is optional, and we assume that it must be correct. lnprototest at commit dea47c29b5541dbfe7fe53cc2598330e897fa4f4 report the following error now. ``` 2023-07-06T21:03:20.930Z DEBUG hsmd: Shutting down ERROR root:helpers.py:170 Traceback (most recent call last): File "/home/vincent/Github/lightning/external/lnprototest/tests/helpers.py", line 167, in run_runner runner.run(test) File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/runner.py", line 99, in run all_done = sequence.action(self) ^^^^^^^^^^^^^^^^^^^^^ File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/structure.py", line 55, in action all_done &= e.action(runner) ^^^^^^^^^^^^^^^^ File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/event.py", line 365, in action raise EventError(self, "{}: message was {}".format(err, msg.to_str())) lnprototest.errors.EventError: `Expected msgtype-warning, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "157"},] ============================================================================================================================================================== short test summary info =============================================================================================================================================================== FAILED tests/test_bolt2-01-close_channel.py::test_close_channel_shutdown_msg_normal_case_receiver_side - AssertionError: `Expected msgtype-shutdown, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "75"},] FAILED tests/test_bolt2-01-close_channel.py::test_close_channel_shutdown_msg_wrong_script_pubkey_receiver_side - AssertionError: `Expected msgtype-warning, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "157"},] ``` Changelog-Fixes: channeld: Verify the signature sent in announcement_signatures by the counterparty Reported-by: lnprototest Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This commit is contained in:
parent
620135335d
commit
f4fd12cc15
2 changed files with 54 additions and 3 deletions
|
@ -647,9 +647,24 @@ static void handle_peer_channel_ready(struct peer *peer, const u8 *msg)
|
|||
billboard_update(peer);
|
||||
}
|
||||
|
||||
/* Checks that key is valid, and signed this hash
|
||||
*
|
||||
* FIXME: move this inside common/utils.h */
|
||||
static bool check_signed_hash_nodeid(const struct sha256_double *hash,
|
||||
const secp256k1_ecdsa_signature *signature,
|
||||
const struct node_id *id)
|
||||
{
|
||||
struct pubkey key;
|
||||
|
||||
return pubkey_from_node_id(&key, id)
|
||||
&& check_signed_hash(hash, signature, &key);
|
||||
}
|
||||
|
||||
static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg)
|
||||
{
|
||||
const u8 *cannounce;
|
||||
struct channel_id chanid;
|
||||
struct sha256_double hash;
|
||||
|
||||
if (!fromwire_announcement_signatures(msg,
|
||||
&chanid,
|
||||
|
@ -669,6 +684,44 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg
|
|||
type_to_string(tmpctx, struct channel_id, &chanid));
|
||||
}
|
||||
|
||||
/* BOLT 7:
|
||||
* - if the node_signature OR the bitcoin_signature is NOT correct:
|
||||
* - MAY send a warning and close the connection, or send an error and fail the channel.
|
||||
*
|
||||
* In our case, we send an error and stop the open channel procedure. This approach is
|
||||
* considered overly strict since the peer can recover from it. However, this step is
|
||||
* optional. If the peer sends it, we assume that the signature must be correct.*/
|
||||
cannounce = create_channel_announcement(tmpctx, peer);
|
||||
|
||||
/* 2 byte msg type + 256 byte signatures */
|
||||
int offset = 258;
|
||||
sha256_double(&hash, cannounce + offset,
|
||||
tal_count(cannounce) - offset);
|
||||
|
||||
if (!check_signed_hash_nodeid(&hash, &peer->announcement_node_sigs[REMOTE], &peer->node_ids[REMOTE])) {
|
||||
peer_failed_warn(peer->pps, &chanid,
|
||||
"Bad node_signature %s hash %s"
|
||||
" on announcement_signatures %s",
|
||||
type_to_string(tmpctx,
|
||||
secp256k1_ecdsa_signature,
|
||||
&peer->announcement_node_sigs[REMOTE]),
|
||||
type_to_string(tmpctx,
|
||||
struct sha256_double,
|
||||
&hash),
|
||||
tal_hex(tmpctx, cannounce));
|
||||
}
|
||||
if (!check_signed_hash(&hash, &peer->announcement_bitcoin_sigs[REMOTE], &peer->channel->funding_pubkey[REMOTE])) {
|
||||
peer_failed_warn(peer->pps, &chanid,
|
||||
"Bad bitcoin_signature %s hash %s"
|
||||
" on announcement_signatures %s",
|
||||
type_to_string(tmpctx,
|
||||
secp256k1_ecdsa_signature,
|
||||
&peer->announcement_bitcoin_sigs[REMOTE]),
|
||||
type_to_string(tmpctx,
|
||||
struct sha256_double,
|
||||
&hash),
|
||||
tal_hex(tmpctx, cannounce));
|
||||
}
|
||||
peer->have_sigs[REMOTE] = true;
|
||||
billboard_update(peer);
|
||||
|
||||
|
|
|
@ -1326,9 +1326,7 @@ def test_funding_reorg_remote_lags(node_factory, bitcoind):
|
|||
bitcoind.generate_block(1)
|
||||
l1.daemon.wait_for_log(r'Peer transient failure .* short_channel_id changed to 104x1x0 \(was 103x1x0\)')
|
||||
|
||||
wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['status'] == [
|
||||
'CHANNELD_NORMAL:Reconnected, and reestablished.',
|
||||
'CHANNELD_NORMAL:Channel ready for use. They need our announcement signatures.'])
|
||||
l2.daemon.wait_for_logs([r'Peer transient failure in CHANNELD_NORMAL: channeld WARNING: Bad node_signature*'])
|
||||
|
||||
# Unblinding l2 brings it back in sync, restarts channeld and sends its announce sig
|
||||
l2.daemon.rpcproxy.mock_rpc('getblockhash', None)
|
||||
|
|
Loading…
Add table
Reference in a new issue