Skip to content

Commit

Permalink
channeld: Verify the signature sent by the counterparty
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
vincenzopalazzo committed Jul 25, 2023
1 parent 11b5f31 commit f29f544
Showing 1 changed file with 53 additions and 0 deletions.
53 changes: 53 additions & 0 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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_err(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_err(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);

Expand Down

0 comments on commit f29f544

Please sign in to comment.