diff --git a/common/onion.c b/common/onion.c index 9da6683f8..193920a32 100644 --- a/common/onion.c +++ b/common/onion.c @@ -111,57 +111,6 @@ u8 *onion_final_hop(const tal_t *ctx, return make_tlv_hop(ctx, tlv); } -/* Returns true if valid, and fills in type. */ -static bool pull_payload_length(const u8 **cursor, - size_t *max, - bool has_realm, - enum onion_payload_type *type, - size_t *len) -{ - /* *len will incorporate bytes we read from cursor */ - const u8 *start = *cursor; - - /* BOLT #4: - * - * The `length` field determines both the length and the format of the - * `hop_payload` field; the following formats are defined: - */ - *len = fromwire_bigsize(cursor, max); - if (!cursor) - return false; - - /* BOLT #4: - * - `tlv_payload` format, identified by any length over `1`. In this - * case the `hop_payload_length` is equal to the numeric value of - * `length`. - */ - if (*len <= 1) - return false; - - /* It's invalid if it claims to be too long! */ - if (*len > *max) - return false; - - if (type) - *type = ONION_TLV_PAYLOAD; - *len += (*cursor - start); - return true; -} - -size_t onion_payload_length(const u8 *raw_payload, size_t len, bool has_realm, - bool *valid, - enum onion_payload_type *type) -{ - size_t max = len, payload_len; - *valid = pull_payload_length(&raw_payload, &max, has_realm, type, &payload_len); - - /* If it's not valid, copy the entire thing. */ - if (!*valid) - return len; - - return payload_len; -} - #if EXPERIMENTAL_FEATURES static struct tlv_tlv_payload *decrypt_tlv(const tal_t *ctx, const struct secret *blinding_ss, @@ -210,7 +159,14 @@ struct onion_payload *onion_decode(const tal_t *ctx, size_t max = tal_bytelen(cursor), len; struct tlv_tlv_payload *tlv; - if (!pull_payload_length(&cursor, &max, true, &p->type, &len)) { + /* BOLT-remove-legacy-onion #4: + * 1. type: `hop_payloads` + * 2. data: + * * [`bigsize`:`length`] + * * [`length*byte`:`payload`] + */ + len = fromwire_bigsize(&cursor, &max); + if (!cursor || len > max) { *failtlvtype = 0; *failtlvpos = tal_bytelen(rs->raw_payload); goto fail_no_tlv; diff --git a/common/onion.h b/common/onion.h index 3476acde4..23dd2c92c 100644 --- a/common/onion.h +++ b/common/onion.h @@ -6,13 +6,7 @@ struct route_step; -enum onion_payload_type { - ONION_TLV_PAYLOAD = 1, -}; - struct onion_payload { - enum onion_payload_type type; - struct amount_msat amt_to_forward; u32 outgoing_cltv; struct amount_msat *total_msat; @@ -45,23 +39,6 @@ u8 *onion_final_hop(const tal_t *ctx, const struct secret *payment_secret, const u8 *payment_metadata); -/** - * onion_payload_length: measure payload length in decrypted onion. - * @raw_payload: payload to look at. - * @len: length of @raw_payload in bytes. - * @has_realm: used for HTLCs, where first byte 0 is magical. - * @valid: set to true if it is valid, false otherwise. - * @type: if non-NULL, set to type of payload if *@valid is true. - * - * If @valid is set, there is room for the HMAC immediately following, - * as the return value is <= ROUTING_INFO_SIZE - HMAC_SIZE. Otherwise, - * the return value is @len (i.e. the entire payload). - */ -size_t onion_payload_length(const u8 *raw_payload, size_t len, - bool has_realm, - bool *valid, - enum onion_payload_type *type); - /** * onion_decode: decode payload from a decrypted onion. * @ctx: context to allocate onion_contents off. diff --git a/common/sphinx.c b/common/sphinx.c index 1eb4496ea..bf25c01aa 100644 --- a/common/sphinx.c +++ b/common/sphinx.c @@ -601,7 +601,8 @@ struct route_step *process_onionpacket( u8 *paddedheader; size_t payload_size; bigsize_t shift_size; - bool valid; + const u8 *cursor; + size_t max; step->next = talz(step, struct onionpacket); step->next->version = msg->version; @@ -624,23 +625,29 @@ struct route_step *process_onionpacket( if (!blind_group_element(&step->next->ephemeralkey, &msg->ephemeralkey, blind)) return tal_free(step); - payload_size = onion_payload_length(paddedheader, - tal_bytelen(msg->routinginfo), - has_realm, - &valid, NULL); + /* Now, try to pull data out. */ + cursor = paddedheader; + max = tal_bytelen(msg->routinginfo); - /* Can't decode? Treat it as terminal. */ - if (!valid) { - shift_size = payload_size; - memset(step->next->hmac.bytes, 0, sizeof(step->next->hmac.bytes)); - } else { - assert(payload_size <= tal_bytelen(msg->routinginfo) - HMAC_SIZE); - /* Copy hmac */ - shift_size = payload_size + HMAC_SIZE; - memcpy(step->next->hmac.bytes, - paddedheader + payload_size, HMAC_SIZE); - } - step->raw_payload = tal_dup_arr(step, u8, paddedheader, payload_size, 0); + /* Any of these could fail, falling thru with cursor == NULL */ + payload_size = fromwire_bigsize(&cursor, &max); + /* FIXME: raw_payload *includes* the length, which is redundant and + * means we can't just ust fromwire_tal_arrn. */ + fromwire_pad(&cursor, &max, payload_size); + if (cursor != NULL) + step->raw_payload = tal_dup_arr(step, u8, paddedheader, + cursor - paddedheader, 0); + fromwire_hmac(&cursor, &max, &step->next->hmac); + + /* BOLT-remove-legacy-onion #4: + * Since no `payload` TLV value can ever be shorter than 2 bytes, `length` values of 0 and 1 are + * reserved. (`0` indicated a legacy format no longer supported, and `1` is reserved for future + * use). */ + if (payload_size < 2 || !cursor) + return tal_free(step); + + /* This includes length field and hmac */ + shift_size = cursor - paddedheader; /* Left shift the current payload out and make the remainder the new onion */ step->next->routinginfo = tal_dup_arr(step->next, diff --git a/common/test/run-sphinx-xor_cipher_stream.c b/common/test/run-sphinx-xor_cipher_stream.c index dc1bd585c..4212fab5d 100644 --- a/common/test/run-sphinx-xor_cipher_stream.c +++ b/common/test/run-sphinx-xor_cipher_stream.c @@ -38,6 +38,9 @@ struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED) /* Generated stub for fromwire */ const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED) { fprintf(stderr, "fromwire called!\n"); abort(); } +/* Generated stub for fromwire_bigsize */ +bigsize_t fromwire_bigsize(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) +{ fprintf(stderr, "fromwire_bigsize called!\n"); abort(); } /* Generated stub for fromwire_bool */ bool fromwire_bool(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) { fprintf(stderr, "fromwire_bool called!\n"); abort(); } @@ -47,6 +50,9 @@ void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) /* Generated stub for fromwire_hmac */ void fromwire_hmac(const u8 **ptr UNNEEDED, size_t *max UNNEEDED, struct hmac *hmac UNNEEDED) { fprintf(stderr, "fromwire_hmac called!\n"); abort(); } +/* Generated stub for fromwire_pad */ +void fromwire_pad(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, size_t num UNNEEDED) +{ fprintf(stderr, "fromwire_pad called!\n"); abort(); } /* Generated stub for fromwire_secp256k1_ecdsa_signature */ void fromwire_secp256k1_ecdsa_signature(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED) @@ -88,12 +94,6 @@ void hmac_update(crypto_auth_hmacsha256_state *state UNNEEDED, /* Generated stub for new_onionreply */ struct onionreply *new_onionreply(const tal_t *ctx UNNEEDED, const u8 *contents TAKES UNNEEDED) { fprintf(stderr, "new_onionreply called!\n"); abort(); } -/* Generated stub for onion_payload_length */ -size_t onion_payload_length(const u8 *raw_payload UNNEEDED, size_t len UNNEEDED, - bool has_realm UNNEEDED, - bool *valid UNNEEDED, - enum onion_payload_type *type UNNEEDED) -{ fprintf(stderr, "onion_payload_length called!\n"); abort(); } /* Generated stub for pubkey_from_node_id */ bool pubkey_from_node_id(struct pubkey *key UNNEEDED, const struct node_id *id UNNEEDED) { fprintf(stderr, "pubkey_from_node_id called!\n"); abort(); } diff --git a/devtools/onion.c b/devtools/onion.c index 0acbbf498..31bbcc56e 100644 --- a/devtools/onion.c +++ b/devtools/onion.c @@ -270,18 +270,12 @@ static void runtest(const char *filename) decodetok = json_get_member(buffer, toks, "decode"); json_for_each_arr(i, hop, decodetok) { - bool valid; - hexprivkey = json_strdup(ctx, buffer, hop); printf("Processing at hop %zu\n", i); step = decode_with_privkey(ctx, serialized, hexprivkey, associated_data); serialized = serialize_onionpacket(ctx, step->next); if (!serialized) errx(1, "Error serializing message."); - onion_payload_length(step->raw_payload, - tal_bytelen(step->raw_payload), - true, &valid, NULL); - assert(valid); printf(" Payload: %s\n", tal_hex(ctx, step->raw_payload)); printf(" Next onion: %s\n", tal_hex(ctx, serialized)); printf(" Next HMAC: %s\n", diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index f9f609a10..dfea74ca5 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -485,21 +485,6 @@ static void destroy_hout_subd_died(struct htlc_out *hout) db_commit_transaction(db); } -static enum forward_style get_onion_style(const struct htlc_in *hin) -{ - /* This happens on reload from db; don't try too hard! */ - if (!hin->payload) - return FORWARD_STYLE_UNKNOWN; - - switch ((int)hin->payload->type) { - case 0: - return FORWARD_STYLE_LEGACY; - case ONION_TLV_PAYLOAD: - return FORWARD_STYLE_TLV; - } - abort(); -} - /* This is where channeld gives us the HTLC id, and also reports if it * failed immediately. */ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED, @@ -539,7 +524,7 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU * so set htlc field with NULL (db wants it to exist!) */ wallet_forwarded_payment_add(ld->wallet, hout->in, - get_onion_style(hout->in), + FORWARD_STYLE_TLV, channel_scid_or_local_alias(hout->key.channel), NULL, FORWARD_LOCAL_FAILED, fromwire_peektype(hout->failmsg)); @@ -713,7 +698,7 @@ static void forward_htlc(struct htlc_in *hin, if (!next || !channel_active(next)) { local_fail_in_htlc(hin, take(towire_unknown_next_peer(NULL))); wallet_forwarded_payment_add(hin->key.channel->peer->ld->wallet, - hin, get_onion_style(hin), + hin, FORWARD_STYLE_TLV, forward_scid, NULL, FORWARD_LOCAL_FAILED, WIRE_UNKNOWN_NEXT_PEER); @@ -812,7 +797,7 @@ static void forward_htlc(struct htlc_in *hin, fail: local_fail_in_htlc(hin, failmsg); wallet_forwarded_payment_add(ld->wallet, - hin, get_onion_style(hin), forward_scid, hout, + hin, FORWARD_STYLE_TLV, forward_scid, hout, FORWARD_LOCAL_FAILED, fromwire_peektype(failmsg)); } @@ -1060,11 +1045,7 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p, json_add_hex_talarr(s, "payload", rs->raw_payload); if (p->payload) { - switch (p->payload->type) { - case ONION_TLV_PAYLOAD: - json_add_string(s, "type", "tlv"); - break; - } + json_add_string(s, "type", "tlv"); if (p->payload->forward_channel) json_add_short_channel_id(s, "short_channel_id", @@ -1406,7 +1387,7 @@ static void fulfill_our_htlc_out(struct channel *channel, struct htlc_out *hout, else if (hout->in) { fulfill_htlc(hout->in, preimage); wallet_forwarded_payment_add(ld->wallet, hout->in, - get_onion_style(hout->in), + FORWARD_STYLE_TLV, channel_scid_or_local_alias(hout->key.channel), hout, FORWARD_SETTLED, 0); } @@ -1534,7 +1515,7 @@ static bool peer_failed_our_htlc(struct channel *channel, if (hout->in) wallet_forwarded_payment_add(ld->wallet, hout->in, - get_onion_style(hout->in), + FORWARD_STYLE_TLV, channel_scid_or_local_alias(channel), hout, FORWARD_FAILED, hout->failmsg @@ -1697,7 +1678,7 @@ void onchain_failed_our_htlc(const struct channel *channel, local_fail_in_htlc(hout->in, take(towire_permanent_channel_failure(NULL))); wallet_forwarded_payment_add(hout->key.channel->peer->ld->wallet, - hout->in, get_onion_style(hout->in), + hout->in, FORWARD_STYLE_TLV, channel_scid_or_local_alias(channel), hout, FORWARD_LOCAL_FAILED, hout->failmsg @@ -1864,7 +1845,7 @@ static bool update_out_htlc(struct channel *channel, if (hout->in) { wallet_forwarded_payment_add(ld->wallet, hout->in, - get_onion_style(hout->in), + FORWARD_STYLE_TLV, channel_scid_or_local_alias(channel), hout, FORWARD_OFFERED, 0); } diff --git a/plugins/bkpr/test/run-bkpr_db.c b/plugins/bkpr/test/run-bkpr_db.c index 9cfccc539..a27cb93ec 100644 --- a/plugins/bkpr/test/run-bkpr_db.c +++ b/plugins/bkpr/test/run-bkpr_db.c @@ -163,9 +163,6 @@ bool json_to_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok bool json_to_txid(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct bitcoin_txid *txid UNNEEDED) { fprintf(stderr, "json_to_txid called!\n"); abort(); } -/* Generated stub for json_to_u64 */ -bool json_to_u64(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, u64 *num UNNEEDED) -{ fprintf(stderr, "json_to_u64 called!\n"); abort(); } /* Generated stub for json_tok_bin_from_hex */ u8 *json_tok_bin_from_hex(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED) { fprintf(stderr, "json_tok_bin_from_hex called!\n"); abort(); } diff --git a/plugins/bkpr/test/run-recorder.c b/plugins/bkpr/test/run-recorder.c index b569eeeb7..c3e449426 100644 --- a/plugins/bkpr/test/run-recorder.c +++ b/plugins/bkpr/test/run-recorder.c @@ -169,9 +169,6 @@ bool json_to_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok bool json_to_txid(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct bitcoin_txid *txid UNNEEDED) { fprintf(stderr, "json_to_txid called!\n"); abort(); } -/* Generated stub for json_to_u64 */ -bool json_to_u64(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, u64 *num UNNEEDED) -{ fprintf(stderr, "json_to_u64 called!\n"); abort(); } /* Generated stub for json_tok_bin_from_hex */ u8 *json_tok_bin_from_hex(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED) { fprintf(stderr, "json_tok_bin_from_hex called!\n"); abort(); } diff --git a/wallet/wallet.h b/wallet/wallet.h index 0522274eb..22bbfbf8f 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -165,7 +165,7 @@ bool string_to_forward_status(const char *status_str, size_t len, * already defined elements (adding is ok) /!\ */ enum forward_style { FORWARD_STYLE_LEGACY = 0, - FORWARD_STYLE_TLV = ONION_TLV_PAYLOAD, + FORWARD_STYLE_TLV = 1, FORWARD_STYLE_UNKNOWN = 2, /* Not actually in db, safe to renumber! */ };