Skip to content

Commit

Permalink
common: disallow NULL channel_id to peer_failed_err.
Browse files Browse the repository at this point in the history 
No more sending "all-channel" errors; in particular, gossipd now only
sends warnings (which make us hang up), not errors, and peer_connected
rejections are warnings (and disconnect), not errors.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Changed: Plugins: `peer_connected` rejections now send a warning, not an error, to the peer.
  • Loading branch information
@rustyrussell
rustyrussell committed Feb 4, 2021
1 parent f4ee41a commit 6b11cc8
Show file tree
Hide file tree
Showing 25 changed files with 220 additions and 216 deletions.
1 change: 1 addition & 0 deletions common/peer_failed.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ void peer_failed_err(struct per_peer_state *pps,
va_list ap;
const char *desc;

assert(channel_id);
va_start(ap, fmt);
desc = tal_vfmt(tmpctx, fmt, ap);
va_end(ap);
Expand Down
4 changes: 2 additions & 2 deletions common/read_peer_msg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ void handle_gossip_msg(struct per_peer_state *pps, const u8 *msg TAKES)
/* It's a raw gossip msg: this copies or takes() */
gossip = tal_dup_talarr(tmpctx, u8, msg);

/* Gossipd can send us gossip messages, OR errors */
if (fromwire_peektype(gossip) == WIRE_ERROR) {
/* Gossipd can send us gossip messages, OR warnings */
if (fromwire_peektype(gossip) == WIRE_WARNING) {
sync_crypto_write(pps, gossip);
peer_failed_connection_lost();
} else {
Expand Down
8 changes: 1 addition & 7 deletions common/wire_error.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,12 @@ u8 *towire_errorfmtv(const tal_t *ctx,
const char *fmt,
va_list ap)
{
/* BOLT #1:
*
* The channel is referred to by `channel_id`, unless `channel_id` is
* 0 (i.e. all bytes are 0), in which case it refers to all
* channels. */
static const struct channel_id all_channels;
char *estr;
u8 *msg;

estr = tal_vfmt(ctx, fmt, ap);
/* We need tal_len to work, so we use copy. */
msg = towire_error(ctx, channel ? channel : &all_channels,
msg = towire_error(ctx, channel,
(u8 *)tal_dup_arr(estr, char, estr, strlen(estr), 0));
tal_free(estr);

Expand Down
8 changes: 4 additions & 4 deletions common/wire_error.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,25 @@ struct channel_id;
* towire_errorfmt - helper to turn string into WIRE_ERROR.
*
* @ctx: context to allocate from
* @channel: specific channel to complain about, or NULL for all.
* @channel: specific channel to complain about
* @fmt: format for error.
*/
u8 *towire_errorfmt(const tal_t *ctx,
const struct channel_id *channel,
const char *fmt, ...) PRINTF_FMT(3,4);
const char *fmt, ...) PRINTF_FMT(3,4) NON_NULL_ARGS(2);

/**
* towire_errorfmtv - helper to turn string into WIRE_ERROR.
*
* @ctx: context to allocate from
* @channel: specific channel to complain about, or NULL for all.
* @channel: specific channel to complain about
* @fmt: format for error.
* @ap: accumulated varargs.
*/
u8 *towire_errorfmtv(const tal_t *ctx,
const struct channel_id *channel,
const char *fmt,
va_list ap);
va_list ap) NON_NULL_ARGS(2);

/**
* towire_warningfmt - helper to turn string into WIRE_WARNING.
Expand Down
6 changes: 3 additions & 3 deletions connectd/connectd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,14 +463,14 @@ struct io_plan *peer_connected(struct io_conn *conn,
unsup = features_unsupported(daemon->our_features, their_features,
INIT_FEATURE);
if (unsup != -1) {
msg = towire_errorfmt(NULL, NULL, "Unsupported feature %u",
unsup);
msg = towire_warningfmt(NULL, NULL, "Unsupported feature %u",
unsup);
msg = cryptomsg_encrypt_msg(tmpctx, cs, take(msg));
return io_write(conn, msg, tal_count(msg), io_close_cb, NULL);
}

if (!feature_check_depends(their_features, &depender, &missing)) {
msg = towire_errorfmt(NULL, NULL,
msg = towire_warningfmt(NULL, NULL,
"Feature %zu requires feature %zu",
depender, missing);
msg = cryptomsg_encrypt_msg(tmpctx, cs, take(msg));
Expand Down
2 changes: 1 addition & 1 deletion connectd/peer_exchange_initmsg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn,
status_peer_debug(&peer->id,
"No common chain with this peer '%s', closing",
tal_hex(tmpctx, msg));
msg = towire_errorfmt(NULL, NULL, "No common network");
msg = towire_warningfmt(NULL, NULL, "No common network");
msg = cryptomsg_encrypt_msg(NULL, &peer->cs, take(msg));
return io_write(conn, msg, tal_count(msg), io_close_cb, NULL);
}
Expand Down
6 changes: 3 additions & 3 deletions gossipd/gossipd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ static u8 *handle_ping(struct peer *peer, const u8 *ping)
/* This checks the ping packet and makes a pong reply if needed; peer
* can specify it doesn't want a response, to simulate traffic. */
if (!check_ping_make_pong(NULL, ping, &pong))
return towire_errorfmt(peer, NULL, "Bad ping");
return towire_warningfmt(peer, NULL, "Bad ping");

if (pong)
queue_peer_msg(peer, take(pong));
Expand All @@ -333,7 +333,7 @@ static const u8 *handle_pong(struct peer *peer, const u8 *pong)
const char *err = got_pong(pong, &peer->num_pings_outstanding);

if (err)
return towire_errorfmt(peer, NULL, "%s", err);
return towire_warningfmt(peer, NULL, "%s", err);

daemon_conn_send(peer->daemon->master,
take(towire_gossipd_ping_reply(NULL, &peer->id, true,
Expand Down Expand Up @@ -454,7 +454,7 @@ static u8 *handle_onion_message(struct peer *peer, const u8 *msg)

/* FIXME: ratelimit! */
if (!fromwire_onion_message(msg, msg, &onion, tlvs))
return towire_errorfmt(peer, NULL, "Bad onion_message");
return towire_warningfmt(peer, NULL, "Bad onion_message");

/* We unwrap the onion now. */
op = parse_onionpacket(tmpctx, onion, tal_bytelen(onion), &badreason);
Expand Down
114 changes: 57 additions & 57 deletions gossipd/queries.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,9 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg)

if (!fromwire_query_short_channel_ids(tmpctx, msg, &chain, &encoded,
tlvs)) {
return towire_errorfmt(peer, NULL,
"Bad query_short_channel_ids w/tlvs %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"Bad query_short_channel_ids w/tlvs %s",
tal_hex(tmpctx, msg));
}
if (tlvs->query_flags) {
/* BOLT #7:
Expand All @@ -266,9 +266,9 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg)
*/
flags = decode_scid_query_flags(tmpctx, tlvs->query_flags);
if (!flags) {
return towire_errorfmt(peer, NULL,
"Bad query_short_channel_ids query_flags %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"Bad query_short_channel_ids query_flags %s",
tal_hex(tmpctx, msg));
}
} else
flags = NULL;
Expand All @@ -295,15 +295,15 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg)
* - MAY fail the connection.
*/
if (peer->scid_queries || peer->scid_query_nodes) {
return towire_errorfmt(peer, NULL,
"Bad concurrent query_short_channel_ids");
return towire_warningfmt(peer, NULL,
"Bad concurrent query_short_channel_ids");
}

scids = decode_short_ids(tmpctx, encoded);
if (!scids) {
return towire_errorfmt(peer, NULL,
"Bad query_short_channel_ids encoding %s",
tal_hex(tmpctx, encoded));
return towire_warningfmt(peer, NULL,
"Bad query_short_channel_ids encoding %s",
tal_hex(tmpctx, encoded));
}

/* BOLT #7:
Expand All @@ -320,9 +320,9 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg)
memset(flags, 0xFF, tal_bytelen(flags));
} else {
if (tal_count(flags) != tal_count(scids)) {
return towire_errorfmt(peer, NULL,
"Bad query_short_channel_ids flags count %zu scids %zu",
tal_count(flags), tal_count(scids));
return towire_warningfmt(peer, NULL,
"Bad query_short_channel_ids flags count %zu scids %zu",
tal_count(flags), tal_count(scids));
}
}

Expand Down Expand Up @@ -652,9 +652,9 @@ const u8 *handle_query_channel_range(struct peer *peer, const u8 *msg)
if (!fromwire_query_channel_range(msg, &chain_hash,
&first_blocknum, &number_of_blocks,
tlvs)) {
return towire_errorfmt(peer, NULL,
"Bad query_channel_range w/tlvs %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"Bad query_channel_range w/tlvs %s",
tal_hex(tmpctx, msg));
}
if (tlvs->query_option)
query_option_flags = *tlvs->query_option;
Expand Down Expand Up @@ -703,13 +703,13 @@ static u8 *append_range_reply(struct peer *peer,
ts = decode_channel_update_timestamps(tmpctx,
timestamps_tlv);
if (!ts)
return towire_errorfmt(peer, NULL,
"reply_channel_range can't decode timestamps.");
return towire_warningfmt(peer, NULL,
"reply_channel_range can't decode timestamps.");
if (tal_count(ts) != tal_count(scids)) {
return towire_errorfmt(peer, NULL,
"reply_channel_range %zu timestamps when %zu scids?",
tal_count(ts),
tal_count(scids));
return towire_warningfmt(peer, NULL,
"reply_channel_range %zu timestamps when %zu scids?",
tal_count(ts),
tal_count(scids));
}
} else
ts = NULL;
Expand Down Expand Up @@ -749,35 +749,35 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg)
if (!fromwire_reply_channel_range(tmpctx, msg, &chain, &first_blocknum,
&number_of_blocks, &complete,
&encoded, tlvs)) {
return towire_errorfmt(peer, NULL,
"Bad reply_channel_range w/tlvs %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"Bad reply_channel_range w/tlvs %s",
tal_hex(tmpctx, msg));
}

if (!bitcoin_blkid_eq(&chainparams->genesis_blockhash, &chain)) {
return towire_errorfmt(peer, NULL,
"reply_channel_range for bad chain: %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"reply_channel_range for bad chain: %s",
tal_hex(tmpctx, msg));
}

if (!peer->range_replies) {
return towire_errorfmt(peer, NULL,
"reply_channel_range without query: %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"reply_channel_range without query: %s",
tal_hex(tmpctx, msg));
}

/* Beware overflow! */
if (first_blocknum + number_of_blocks < first_blocknum) {
return towire_errorfmt(peer, NULL,
"reply_channel_range invalid %u+%u",
first_blocknum, number_of_blocks);
return towire_warningfmt(peer, NULL,
"reply_channel_range invalid %u+%u",
first_blocknum, number_of_blocks);
}

scids = decode_short_ids(tmpctx, encoded);
if (!scids) {
return towire_errorfmt(peer, NULL,
"Bad reply_channel_range encoding %s",
tal_hex(tmpctx, encoded));
return towire_warningfmt(peer, NULL,
"Bad reply_channel_range encoding %s",
tal_hex(tmpctx, encoded));
}

status_peer_debug(&peer->id,
Expand Down Expand Up @@ -807,12 +807,12 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg)
/* ie. They can be outside range we asked, but they must overlap! */
if (first_blocknum + number_of_blocks <= peer->range_first_blocknum
|| first_blocknum >= peer->range_end_blocknum) {
return towire_errorfmt(peer, NULL,
"reply_channel_range invalid %u+%u for query %u+%u",
first_blocknum, number_of_blocks,
peer->range_first_blocknum,
peer->range_end_blocknum
- peer->range_first_blocknum);
return towire_warningfmt(peer, NULL,
"reply_channel_range invalid %u+%u for query %u+%u",
first_blocknum, number_of_blocks,
peer->range_first_blocknum,
peer->range_end_blocknum
- peer->range_first_blocknum);
}

start = first_blocknum;
Expand All @@ -838,10 +838,10 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg)
* can overlap. */
if (first_blocknum != peer->range_prev_end_blocknum + 1
&& first_blocknum != peer->range_prev_end_blocknum) {
return towire_errorfmt(peer, NULL,
"reply_channel_range %u+%u previous end was block %u",
first_blocknum, number_of_blocks,
peer->range_prev_end_blocknum);
return towire_warningfmt(peer, NULL,
"reply_channel_range %u+%u previous end was block %u",
first_blocknum, number_of_blocks,
peer->range_prev_end_blocknum);
}
peer->range_prev_end_blocknum = end;

Expand Down Expand Up @@ -878,21 +878,21 @@ const u8 *handle_reply_short_channel_ids_end(struct peer *peer, const u8 *msg)
u8 complete;

if (!fromwire_reply_short_channel_ids_end(msg, &chain, &complete)) {
return towire_errorfmt(peer, NULL,
"Bad reply_short_channel_ids_end %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"Bad reply_short_channel_ids_end %s",
tal_hex(tmpctx, msg));
}

if (!bitcoin_blkid_eq(&chainparams->genesis_blockhash, &chain)) {
return towire_errorfmt(peer, NULL,
"reply_short_channel_ids_end for bad chain: %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"reply_short_channel_ids_end for bad chain: %s",
tal_hex(tmpctx, msg));
}

if (!peer->scid_query_outstanding) {
return towire_errorfmt(peer, NULL,
"unexpected reply_short_channel_ids_end: %s",
tal_hex(tmpctx, msg));
return towire_warningfmt(peer, NULL,
"unexpected reply_short_channel_ids_end: %s",
tal_hex(tmpctx, msg));
}

peer->scid_query_outstanding = false;
Expand Down
Loading

0 comments on commit 6b11cc8

Please sign in to comment.