diff --git a/daemon/htlc.h b/daemon/htlc.h index 7a6e959a1..1f6731fd3 100644 --- a/daemon/htlc.h +++ b/daemon/htlc.h @@ -71,6 +71,8 @@ struct htlc { /* Previous HTLC (if any) which made us offer this (LOCAL only) */ struct htlc *src; const u8 *fail; + /* FIXME: actually an enum onion_type */ + u8 malformed; }; const char *htlc_state_name(enum htlc_state s); diff --git a/lightningd/channel.c b/lightningd/channel.c index 8aa420759..c8058b400 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -67,7 +67,8 @@ static void dump_htlc(const struct htlc *htlc, const char *prefix) htlc->id, htlc_state_name(htlc->state), htlc_state_name(remote_state), - htlc->r ? "FULFILLED" : htlc->fail ? "FAILED" : ""); + htlc->r ? "FULFILLED" : htlc->fail ? "FAILED" : + htlc->malformed ? "MALFORMED" : ""); } void dump_htlcs(const struct channel *channel, const char *prefix) @@ -384,6 +385,7 @@ static enum channel_add_err add_htlc(struct channel *channel, htlc->rhash = *payment_hash; htlc->fail = NULL; + htlc->malformed = 0; htlc->r = NULL; htlc->routing = tal_dup_arr(htlc, u8, routing, TOTAL_PACKET_SIZE, 0); @@ -815,7 +817,7 @@ static bool adjust_balance(struct channel *channel, struct htlc *htlc) if (htlc_has(htlc, HTLC_FLAG(side, HTLC_F_COMMITTED))) continue; - if (!htlc->fail && !htlc->r) { + if (!htlc->fail && !htlc->malformed && !htlc->r) { status_trace("%s HTLC %"PRIu64 " %s neither fail nor fulfull?", htlc_state_owner(htlc->state) == LOCAL @@ -943,6 +945,12 @@ bool channel_force_htlcs(struct channel *channel, failed[i].id); return false; } + if (htlc->malformed) { + status_trace("Fail %s HTLC %"PRIu64" already malformed", + failed_sides[i] == LOCAL ? "out" : "in", + failed[i].id); + return false; + } if (!htlc_has(htlc, HTLC_REMOVING)) { status_trace("Fail %s HTLC %"PRIu64" state %s", failed_sides[i] == LOCAL ? "out" : "in", @@ -950,8 +958,12 @@ bool channel_force_htlcs(struct channel *channel, htlc_state_name(htlc->state)); return false; } - htlc->fail = tal_dup_arr(htlc, u8, failed[i].failreason, - tal_len(failed[i].failreason), 0); + if (failed[i].malformed) + htlc->malformed = failed[i].malformed; + else + htlc->fail = tal_dup_arr(htlc, u8, failed[i].failreason, + tal_len(failed[i].failreason), + 0); } for (i = 0; i < tal_count(htlcs); i++) { diff --git a/lightningd/channel/channel.c b/lightningd/channel/channel.c index 3219e8928..5f695d64a 100644 --- a/lightningd/channel/channel.c +++ b/lightningd/channel/channel.c @@ -663,6 +663,7 @@ static u8 *got_commitsig_msg(const tal_t *ctx, assert(htlc->fail); f = tal_arr_append(&failed); f->id = htlc->id; + f->malformed = htlc->malformed; f->failreason = cast_const(u8 *, htlc->fail); } } else { @@ -1472,23 +1473,39 @@ static void handle_fail(struct peer *peer, const u8 *inmsg) u8 *msg; u64 id; u8 *errpkt; + u16 malformed; enum channel_remove_err e; - if (!fromwire_channel_fail_htlc(inmsg, inmsg, NULL, &id, &errpkt)) + if (!fromwire_channel_fail_htlc(inmsg, inmsg, NULL, &id, &malformed, + &errpkt)) status_failed(WIRE_CHANNEL_BAD_COMMAND, "Invalid channel_fail_htlc"); + if (malformed && !(malformed & BADONION)) + status_failed(WIRE_CHANNEL_BAD_COMMAND, + "Invalid channel_fail_htlc: bad malformed 0x%x", + malformed); + e = channel_fail_htlc(peer->channel, REMOTE, id); switch (e) { case CHANNEL_ERR_REMOVE_OK: - msg = towire_update_fail_htlc(peer, &peer->channel_id, - id, errpkt); + if (malformed) { + struct htlc *h; + struct sha256 sha256_of_onion; + h = channel_get_htlc(peer->channel, REMOTE, id); + sha256(&sha256_of_onion, h->routing, + tal_len(h->routing)); + msg = towire_update_fail_malformed_htlc(peer, + &peer->channel_id, + id, &sha256_of_onion, + malformed); + } else { + msg = towire_update_fail_htlc(peer, &peer->channel_id, + id, errpkt); + } msg_enqueue(&peer->peer_out, take(msg)); start_commit_timer(peer); return; - /* These shouldn't happen, because any offered HTLC (which would give - * us the preimage) should have timed out long before. If we - * were to get preimages from other sources, this could happen. */ case CHANNEL_ERR_NO_SUCH_ID: case CHANNEL_ERR_ALREADY_FULFILLED: case CHANNEL_ERR_HTLC_UNCOMMITTED: diff --git a/lightningd/channel/channel_wire.csv b/lightningd/channel/channel_wire.csv index db9eab1e7..81786ea93 100644 --- a/lightningd/channel/channel_wire.csv +++ b/lightningd/channel/channel_wire.csv @@ -90,6 +90,9 @@ channel_fulfill_htlc,,payment_preimage,struct preimage # Main daemon says HTLC failed channel_fail_htlc,6 channel_fail_htlc,,id,8 +# If malformed is non-zero, it's a BADONION code +channel_fail_htlc,,malformed,u16 +# Otherwise, error_pkt contains failreason. channel_fail_htlc,,len,2 channel_fail_htlc,,error_pkt,len*u8 diff --git a/lightningd/htlc_end.c b/lightningd/htlc_end.c index 8d152bc0a..124fb11e3 100644 --- a/lightningd/htlc_end.c +++ b/lightningd/htlc_end.c @@ -81,6 +81,8 @@ struct htlc_in *htlc_in_check(const struct htlc_in *hin, const char *abortstr) htlc_state_name(hin->hstate)); else if (hin->failuremsg && hin->preimage) return corrupt(hin, abortstr, "Both failed and succeeded"); + else if (hin->failuremsg && hin->malformed) + return corrupt(hin, abortstr, "Both failed and malformed"); return cast_const(struct htlc_in *, hin); } @@ -105,6 +107,7 @@ struct htlc_in *new_htlc_in(const tal_t *ctx, hin->hstate = RCVD_ADD_COMMIT; hin->failuremsg = NULL; + hin->malformed = 0; hin->preimage = NULL; return htlc_in_check(hin, "new_htlc_in"); @@ -147,6 +150,7 @@ struct htlc_out *new_htlc_out(const tal_t *ctx, hout->hstate = SENT_ADD_HTLC; hout->failuremsg = NULL; + hout->malformed = 0; hout->preimage = NULL; hout->in = in; diff --git a/lightningd/htlc_end.h b/lightningd/htlc_end.h index 8f2937210..30142c153 100644 --- a/lightningd/htlc_end.h +++ b/lightningd/htlc_end.h @@ -5,6 +5,7 @@ #include #include #include +#include /* We look up HTLCs by peer & id */ struct htlc_key { @@ -27,9 +28,12 @@ struct htlc_in { /* Shared secret for us to send any failure message. */ struct secret shared_secret; - /* If they failed HTLC, here's the message. */ + /* If we failed HTLC, here's the message. */ const u8 *failuremsg; + /* If it was malformed, here's the error. */ + enum onion_type malformed; + /* If they fulfilled, here's the preimage. */ struct preimage *preimage; }; @@ -48,6 +52,9 @@ struct htlc_out { /* If we failed HTLC, here's the message. */ const u8 *failuremsg; + /* If it was malformed, here's the error. */ + enum onion_type malformed; + /* If we fulfilled, here's the preimage. */ struct preimage *preimage; diff --git a/lightningd/htlc_wire.c b/lightningd/htlc_wire.c index 3e6430728..e7f7048c0 100644 --- a/lightningd/htlc_wire.c +++ b/lightningd/htlc_wire.c @@ -22,6 +22,7 @@ void towire_fulfilled_htlc(u8 **pptr, const struct fulfilled_htlc *fulfilled) void towire_failed_htlc(u8 **pptr, const struct failed_htlc *failed) { towire_u64(pptr, failed->id); + towire_u16(pptr, failed->malformed); towire_u16(pptr, tal_count(failed->failreason)); towire_u8_array(pptr, failed->failreason, tal_count(failed->failreason)); } @@ -66,6 +67,7 @@ void fromwire_failed_htlc(const tal_t *ctx, const u8 **cursor, size_t *max, u16 failreason_len; failed->id = fromwire_u64(cursor, max); + failed->malformed = fromwire_u16(cursor, max); failreason_len = fromwire_u16(cursor, max); failed->failreason = tal_arr(ctx, u8, failreason_len); fromwire_u8_array(cursor, max, failed->failreason, failreason_len); diff --git a/lightningd/htlc_wire.h b/lightningd/htlc_wire.h index 5c918620c..734f75a1e 100644 --- a/lightningd/htlc_wire.h +++ b/lightningd/htlc_wire.h @@ -5,6 +5,7 @@ #include #include #include +#include /* These are how we communicate about HTLC state to the master daemon */ struct added_htlc { @@ -22,6 +23,7 @@ struct fulfilled_htlc { struct failed_htlc { u64 id; + enum onion_type malformed; u8 *failreason; }; diff --git a/lightningd/pay.c b/lightningd/pay.c index f4b1c317b..d596cd2b1 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -76,22 +76,26 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout) enum onion_type failcode; struct onionreply *reply; - reply = unwrap_onionreply(pc, pc->path_secrets, - tal_count(pc->path_secrets), - hout->failuremsg); - if (!reply) { - log_info(hout->key.peer->log, - "htlc %"PRIu64" failed with bad reply (%s)", - hout->key.id, - tal_hex(pc, hout->failuremsg)); - failcode = WIRE_PERMANENT_NODE_FAILURE; - } else { - failcode = fromwire_peektype(reply->msg); - log_info(hout->key.peer->log, - "htlc %"PRIu64" failed from %ith node with code 0x%04x (%s)", - hout->key.id, - reply->origin_index, - failcode, onion_type_name(failcode)); + if (hout->malformed) + failcode = hout->malformed; + else { + reply = unwrap_onionreply(pc, pc->path_secrets, + tal_count(pc->path_secrets), + hout->failuremsg); + if (!reply) { + log_info(hout->key.peer->log, + "htlc %"PRIu64" failed with bad reply (%s)", + hout->key.id, + tal_hex(pc, hout->failuremsg)); + failcode = WIRE_PERMANENT_NODE_FAILURE; + } else { + failcode = fromwire_peektype(reply->msg); + log_info(hout->key.peer->log, + "htlc %"PRIu64" failed from %ith node with code 0x%04x (%s)", + hout->key.id, + reply->origin_index, + failcode, onion_type_name(failcode)); + } } /* FIXME: save ids we can turn reply->origin_index into sender. */ diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index f626eb73b..c3ba5cc66 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -31,20 +31,30 @@ static void save_htlc_stub(struct lightningd *ld, /* This obfuscates the message, whether local or forwarded. */ static void relay_htlc_failmsg(struct htlc_in *hin) { - u8 *reply; - if (!hin->key.peer->owner) return; - reply = wrap_onionreply(hin, &hin->shared_secret, hin->failuremsg); - subd_send_msg(hin->key.peer->owner, - take(towire_channel_fail_htlc(hin, hin->key.id, reply))); - tal_free(reply); + if (hin->malformed) { + subd_send_msg(hin->key.peer->owner, + take(towire_channel_fail_htlc(hin, + hin->key.id, + hin->malformed, + NULL))); + } else { + u8 *reply; + + reply = wrap_onionreply(hin, &hin->shared_secret, + hin->failuremsg); + subd_send_msg(hin->key.peer->owner, + take(towire_channel_fail_htlc(hin, hin->key.id, + 0, reply))); + tal_free(reply); + } } static u8 *make_failmsg(const tal_t *ctx, const struct htlc_in *hin, enum onion_type failcode, - const struct sha256 *onion_sha, const u8 *channel_update) + const u8 *channel_update) { switch (failcode) { case WIRE_INVALID_REALM: @@ -55,12 +65,6 @@ static u8 *make_failmsg(const tal_t *ctx, const struct htlc_in *hin, return towire_permanent_node_failure(ctx); case WIRE_REQUIRED_NODE_FEATURE_MISSING: return towire_required_node_feature_missing(ctx); - case WIRE_INVALID_ONION_VERSION: - return towire_invalid_onion_version(ctx, onion_sha); - case WIRE_INVALID_ONION_HMAC: - return towire_invalid_onion_hmac(ctx, onion_sha); - case WIRE_INVALID_ONION_KEY: - return towire_invalid_onion_key(ctx, onion_sha); case WIRE_TEMPORARY_CHANNEL_FAILURE: return towire_temporary_channel_failure(ctx, channel_update); case WIRE_CHANNEL_DISABLED: @@ -91,26 +95,35 @@ static u8 *make_failmsg(const tal_t *ctx, const struct htlc_in *hin, return towire_final_incorrect_cltv_expiry(ctx, 0); case WIRE_FINAL_INCORRECT_HTLC_AMOUNT: return towire_final_incorrect_htlc_amount(ctx, hin->msatoshi); + case WIRE_INVALID_ONION_VERSION: + case WIRE_INVALID_ONION_HMAC: + case WIRE_INVALID_ONION_KEY: + log_broken(hin->key.peer->log, "Bad failmsg for %s", + onion_type_name(failcode)); } abort(); } -static void fail_htlc(struct htlc_in *hin, enum onion_type failcode, - const struct sha256 *onion_sha) +static void fail_htlc(struct htlc_in *hin, enum onion_type failcode) { - u8 *msg; + log_info(hin->key.peer->log, "failed htlc %"PRIu64" code 0x%04x (%s)", + hin->key.id, failcode, onion_type_name(failcode)); - log_broken(hin->key.peer->log, "failed htlc %"PRIu64" code 0x%04x (%s)", - hin->key.id, failcode, onion_type_name(failcode)); + if (failcode & BADONION) + hin->malformed = failcode; + else { + u8 *msg; - if (failcode & UPDATE) { - /* FIXME: Ask gossip daemon for channel_update. */ + if (failcode & UPDATE) { + /* FIXME: Ask gossip daemon for channel_update. */ + } + + msg = make_failmsg(hin, hin, failcode, NULL); + hin->failuremsg = create_onionreply(hin, &hin->shared_secret, + msg); + tal_free(msg); } - - msg = make_failmsg(hin, hin, failcode, onion_sha, NULL); - hin->failuremsg = create_onionreply(hin, &hin->shared_secret, msg); - tal_free(msg); - + htlc_in_check(hin, __func__); relay_htlc_failmsg(hin); } @@ -269,7 +282,7 @@ static void handle_localpay(struct htlc_in *hin, return; fail: - fail_htlc(hin, failcode, NULL); + fail_htlc(hin, failcode); } /* @@ -283,7 +296,7 @@ static void hend_subd_died(struct htlc_out *hout) "Failing HTLC %"PRIu64" due to peer death", hout->in->key.id); - fail_htlc(hout->in, WIRE_TEMPORARY_CHANNEL_FAILURE, NULL); + fail_htlc(hout->in, WIRE_TEMPORARY_CHANNEL_FAILURE); } /* This is where channeld gives us the HTLC id, and also reports if it @@ -309,7 +322,7 @@ static bool rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds, onion_type_name(failure_code), (int)tal_len(failurestr), (char *)failurestr); - fail_htlc(hout->in, failure_code, NULL); + fail_htlc(hout->in, failure_code); return true; } @@ -427,7 +440,7 @@ static void forward_htlc(struct htlc_in *hin, return; fail: - fail_htlc(hin, failcode, NULL); + fail_htlc(hin, failcode); } /* Temporary information, while we resolve the next hop */ @@ -454,7 +467,7 @@ static bool channel_resolve_reply(struct subd *gossip, const u8 *msg, } if (tal_count(nodes) == 0) { - fail_htlc(gr->hin, WIRE_UNKNOWN_NEXT_PEER, NULL); + fail_htlc(gr->hin, WIRE_UNKNOWN_NEXT_PEER); return true; } else if (tal_count(nodes) != 2) { log_broken(gossip->log, @@ -665,9 +678,13 @@ static bool peer_failed_our_htlc(struct peer *peer, if (!htlc_out_update_state(peer, hout, RCVD_REMOVE_COMMIT)) return false; - log_debug(peer->log, "Our HTLC %"PRIu64" failed", failed->id); - hout->failuremsg = tal_dup_arr(hout, u8, failed->failreason, - tal_len(failed->failreason), 0); + log_debug(peer->log, "Our HTLC %"PRIu64" failed (%u)", failed->id, + failed->malformed); + if (failed->malformed) + hout->malformed = failed->malformed; + else + hout->failuremsg = tal_dup_arr(hout, u8, failed->failreason, + tal_len(failed->failreason), 0); htlc_out_check(hout, __func__); return true; } @@ -675,11 +692,13 @@ static bool peer_failed_our_htlc(struct peer *peer, static void remove_htlc_in(struct peer *peer, struct htlc_in *hin) { htlc_in_check(hin, __func__); - assert(hin->failuremsg || hin->preimage); + assert(hin->failuremsg || hin->preimage || hin->malformed); log_debug(peer->log, "Removing in HTLC %"PRIu64" state %s %s", hin->key.id, htlc_state_name(hin->hstate), - hin->failuremsg ? "FAILED" : "FULFILLED"); + hin->failuremsg ? "FAILED" + : hin->malformed ? "MALFORMED" + : "FULFILLED"); /* If we fulfilled their HTLC, credit us. */ if (hin->preimage) { @@ -695,18 +714,25 @@ static void remove_htlc_in(struct peer *peer, struct htlc_in *hin) static void remove_htlc_out(struct peer *peer, struct htlc_out *hout) { htlc_out_check(hout, __func__); - assert(hout->failuremsg || hout->preimage); + assert(hout->failuremsg || hout->preimage || hout->malformed); log_debug(peer->log, "Removing out HTLC %"PRIu64" state %s %s", hout->key.id, htlc_state_name(hout->hstate), - hout->failuremsg ? "FAILED" : "FULFILLED"); + hout->failuremsg ? "FAILED" + : hout->malformed ? "MALFORMED" + : "FULFILLED"); /* If it's failed, now we can forward since it's completely locked-in */ - if (hout->failuremsg) { + if (!hout->preimage) { if (hout->in) { - hout->in->failuremsg - = tal_dup_arr(hout->in, u8, - hout->failuremsg, - tal_len(hout->failuremsg), 0); + if (hout->in->malformed) + hout->malformed = hout->in->malformed; + else { + hout->in->failuremsg + = tal_dup_arr(hout->in, u8, + hout->failuremsg, + tal_len(hout->failuremsg), + 0); + } relay_htlc_failmsg(hout->in); } else { payment_failed(peer->ld, hout); @@ -1072,16 +1098,13 @@ int peer_got_revoke(struct peer *peer, const u8 *msg) /* Now, any HTLCs we need to immediately fail? */ for (i = 0; i < tal_count(changed); i++) { - struct sha256 bad_onion_sha; struct htlc_in *hin; if (!failcodes[i]) continue; hin = find_htlc_in(&peer->ld->htlcs_in, peer, changed[i].id); - sha256(&bad_onion_sha, hin->onion_routing_packet, - sizeof(hin->onion_routing_packet)); - fail_htlc(hin, failcodes[i], &bad_onion_sha); + fail_htlc(hin, failcodes[i]); } return 0; }