From 3508331fc44cb64d3ca7f77b9fb319634e0ec434 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 31 Jan 2024 13:46:18 +1030 Subject: [PATCH] lightningd: check channel_announcement signatures we get from peer. We hoise check_signed_hash_nodeid from gossipd's internals, into common/. Signed-off-by: Rusty Russell --- common/node_id.c | 11 +++++++++++ common/node_id.h | 16 +++++++++++++++ gossipd/routing.c | 11 ----------- lightningd/channel_gossip.c | 22 +++++++++++++++++---- lightningd/gossip_generation.c | 36 +++++++++++++++++++++++++++++++++- lightningd/gossip_generation.h | 16 +++++++++++++++ wallet/test/run-wallet.c | 7 +++++++ 7 files changed, 103 insertions(+), 16 deletions(-) diff --git a/common/node_id.c b/common/node_id.c index 34cda6d54..902b651b6 100644 --- a/common/node_id.c +++ b/common/node_id.c @@ -1,5 +1,6 @@ #include "config.h" #include +#include #include #include #include @@ -66,3 +67,13 @@ void towire_node_id(u8 **pptr, const struct node_id *id) #endif towire(pptr, id->k, sizeof(id->k)); } + +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); +} diff --git a/common/node_id.h b/common/node_id.h index 1e2f8d063..110c590dd 100644 --- a/common/node_id.h +++ b/common/node_id.h @@ -7,6 +7,8 @@ #include #include +struct sha256_double; + struct node_id { u8 k[PUBKEY_CMPR_LEN]; }; @@ -63,4 +65,18 @@ static inline size_t node_id_hash(const struct node_id *id) { return siphash24(siphash_seed(), id->k, sizeof(id->k)); } + +/** + * check_signed_hash_nodeid - check a raw secp256k1 signature. + * @h: hash which was signed. + * @signature: signature. + * @id: node_id corresponding to private key used to sign. + * + * Returns true if the id, hash and signature are correct. Changing any + * one of these will make it fail. + */ +bool check_signed_hash_nodeid(const struct sha256_double *hash, + const secp256k1_ecdsa_signature *signature, + const struct node_id *id); + #endif /* LIGHTNING_COMMON_NODE_ID_H */ diff --git a/gossipd/routing.c b/gossipd/routing.c index 548c63bc6..1e543a7d7 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -618,17 +618,6 @@ struct chan *new_chan(struct routing_state *rstate, return chan; } -/* Checks that key is valid, and signed this hash */ -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); -} - /* Verify the signature of a channel_update message */ static u8 *check_channel_update(const tal_t *ctx, const struct node_id *node_id, diff --git a/lightningd/channel_gossip.c b/lightningd/channel_gossip.c index 6ba99ecd3..afc985cc4 100644 --- a/lightningd/channel_gossip.c +++ b/lightningd/channel_gossip.c @@ -351,6 +351,21 @@ static void stash_remote_announce_sigs(struct channel *channel, const secp256k1_ecdsa_signature *bitcoin_sig) { struct channel_gossip *cg = channel->channel_gossip; + const char *err; + + /* 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. + */ + err = check_announce_sigs(channel, scid, node_sig, bitcoin_sig); + if (err) { + channel_fail_transient(channel, true, + "Bad gossip announcement_signatures for scid %s: %s", + short_channel_id_to_str(tmpctx, &scid), + err); + return; + } tal_free(cg->remote_sigs); cg->remote_sigs = tal(cg, struct remote_announce_sigs); @@ -375,7 +390,6 @@ static bool apply_remote_sigs(struct channel *channel) return false; } - /* FIXME: check sigs are valid! */ wallet_announcement_save(channel->peer->ld->wallet, channel->dbid, &cg->remote_sigs->node_sig, @@ -397,7 +411,7 @@ static void send_channel_announce_sigs(struct channel *channel) if (!channel_state_can_add_htlc(channel->state)) return; - ca = create_channel_announcement(tmpctx, channel, + ca = create_channel_announcement(tmpctx, channel, *channel->scid, NULL, NULL, NULL, NULL); msg = hsm_sync_req(tmpctx, ld, @@ -453,7 +467,7 @@ static void send_channel_announcement(struct channel *channel) const u8 *ca, *msg; struct channel_gossip *cg = channel->channel_gossip; - ca = create_channel_announcement(tmpctx, channel, + ca = create_channel_announcement(tmpctx, channel, *channel->scid, NULL, NULL, &cg->remote_sigs->node_sig, &cg->remote_sigs->bitcoin_sig); @@ -471,7 +485,7 @@ static void send_channel_announcement(struct channel *channel) if (!ld->gossip) return; - ca = create_channel_announcement(tmpctx, channel, + ca = create_channel_announcement(tmpctx, channel, *channel->scid, &local_node_sig, &local_bitcoin_sig, &cg->remote_sigs->node_sig, diff --git a/lightningd/gossip_generation.c b/lightningd/gossip_generation.c index ef9aee72a..1ec4700fc 100644 --- a/lightningd/gossip_generation.c +++ b/lightningd/gossip_generation.c @@ -51,6 +51,7 @@ static void copysig_or_zero(secp256k1_ecdsa_signature *dst, u8 *create_channel_announcement(const tal_t *ctx, const struct channel *channel, + struct short_channel_id scid, const secp256k1_ecdsa_signature *local_node_signature, const secp256k1_ecdsa_signature *local_bitcoin_signature, const secp256k1_ecdsa_signature *remote_node_signature, @@ -72,7 +73,7 @@ u8 *create_channel_announcement(const tal_t *ctx, node_id[REMOTE] = channel->peer->id; funding_pubkey[LOCAL] = channel->local_funding_pubkey; funding_pubkey[REMOTE] = channel->channel_info.remote_fundingkey; - return create_channel_announcement_dir(ctx, features, *channel->scid, + return create_channel_announcement_dir(ctx, features, scid, node_signature, bitcoin_signature, node_id, funding_pubkey); } @@ -205,3 +206,36 @@ bool channel_update_details(const u8 *channel_update, *enabled = !(channel_flags & ROUTING_FLAGS_DISABLED); return true; } + +const char *check_announce_sigs(const struct channel *channel, + struct short_channel_id scid, + const secp256k1_ecdsa_signature *remote_node_signature, + const secp256k1_ecdsa_signature *remote_bitcoin_signature) +{ + struct sha256_double hash; + const u8 *cannounce; + + cannounce = create_channel_announcement(tmpctx, channel, scid, + NULL, NULL, NULL, NULL); + + /* BOLT #7: + * + * - MUST compute the double-SHA256 hash `h` of the message, beginning + * at offset 256, up to the end of the message. + * - Note: the hash skips the 4 signatures but hashes the rest of the + * message, including any future fields appended to the end. + */ + /* First two bytes are the msg type */ + int offset = 258; + sha256_double(&hash, cannounce + offset, tal_count(cannounce) - offset); + + if (!check_signed_hash_nodeid(&hash, remote_node_signature, + &channel->peer->id)) + return "invalid node_signature"; + + if (!check_signed_hash(&hash, remote_bitcoin_signature, + &channel->channel_info.remote_fundingkey)) + return "invalid bitcoin_signature"; + + return NULL; +} diff --git a/lightningd/gossip_generation.h b/lightningd/gossip_generation.h index 675143a8d..2208960a0 100644 --- a/lightningd/gossip_generation.h +++ b/lightningd/gossip_generation.h @@ -10,6 +10,7 @@ struct channel; * create_channel_announcement: create a channel_announcement message * @ctx: the tal context to allocate return from * @channel: the channel to announce + * @scid: channel->scid * @local_node_signature: optional local node signature * @local_bitcoin_signature: optional local node signature * @remote_node_signature: optional peer node signature @@ -20,6 +21,7 @@ struct channel; */ u8 *create_channel_announcement(const tal_t *ctx, const struct channel *channel, + struct short_channel_id scid, const secp256k1_ecdsa_signature *local_node_signature, const secp256k1_ecdsa_signature *local_bitcoin_signature, const secp256k1_ecdsa_signature *remote_node_signature, @@ -61,4 +63,18 @@ bool channel_update_details(const u8 *channel_update, u32 *timestamp, bool *enabled); +/** + * check_announce_sigs: check that signatures are correct for this scid + * @channel: the channel (for peer's id / bitcoin key and the channel features) + * @scid: the short_channel_id it's proposing + * @remote_node_signature: node signature + * @remote_bitcoin_signature: bitcoin signature + * + * Returns a string literal if one signature is bad. + */ +const char *check_announce_sigs(const struct channel *channel, + struct short_channel_id scid, + const secp256k1_ecdsa_signature *remote_node_signature, + const secp256k1_ecdsa_signature *remote_bitcoin_signature); + #endif /* LIGHTNING_LIGHTNINGD_GOSSIP_GENERATION_H */ diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index a3faf0285..e7c30daae 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -107,6 +107,12 @@ void channeld_tell_depth(struct channel *channel UNNEEDED, const struct bitcoin_txid *txid UNNEEDED, u32 depth UNNEEDED) { fprintf(stderr, "channeld_tell_depth called!\n"); abort(); } +/* Generated stub for check_announce_sigs */ +const char *check_announce_sigs(const struct channel *channel UNNEEDED, + struct short_channel_id scid UNNEEDED, + const secp256k1_ecdsa_signature *remote_node_signature UNNEEDED, + const secp256k1_ecdsa_signature *remote_bitcoin_signature UNNEEDED) +{ fprintf(stderr, "check_announce_sigs called!\n"); abort(); } /* Generated stub for cmd_id_from_close_command */ const char *cmd_id_from_close_command(const tal_t *ctx UNNEEDED, struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED) @@ -174,6 +180,7 @@ struct anchor_details *create_anchor_details(const tal_t *ctx UNNEEDED, /* Generated stub for create_channel_announcement */ u8 *create_channel_announcement(const tal_t *ctx UNNEEDED, const struct channel *channel UNNEEDED, + struct short_channel_id scid UNNEEDED, const secp256k1_ecdsa_signature *local_node_signature UNNEEDED, const secp256k1_ecdsa_signature *local_bitcoin_signature UNNEEDED, const secp256k1_ecdsa_signature *remote_node_signature UNNEEDED,