From 2760490d5d4fcc20f6b404fc90807ed756d7114d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 9 Nov 2022 12:00:10 +1030 Subject: [PATCH] common: catch up on latest routeblinding spec. This makes us match eed2ab0c30ad7f93e3b2641ca9d7ade32f3d121d ("Use `invalid_onion_blinding` everywhere"). 1. Numerous typographical changes. 2. Make sure we *always* return WIRE_INVALID_ONION_BLINDING if we're in a blinded path. 3. Handle p->total_msat correctly (MPP payments). 4. Reorganize blinding handling just like spec order. Signed-off-by: Rusty Russell --- common/blindedpath.c | 23 +++--- common/blindedpay.c | 9 ++- common/onion.c | 150 ++++++++++++++++++++++++---------------- lightningd/peer_htlcs.c | 28 +++++--- 4 files changed, 124 insertions(+), 86 deletions(-) diff --git a/common/blindedpath.c b/common/blindedpath.c index da51c6dfc..918aef3bf 100644 --- a/common/blindedpath.c +++ b/common/blindedpath.c @@ -89,9 +89,10 @@ static u8 *enctlv_from_encmsg_raw(const tal_t *ctx, type_to_string(tmpctx, struct secret, &rho)); /* BOLT-route-blinding #4: - * - MUST encrypt them with ChaCha20-Poly1305 using the `rho(i)` key - * and an all-zero nonce - */ + * - MUST encrypt each `encrypted_data_tlv(i)` with ChaCha20-Poly1305 + * using the corresponding `rho(i)` key and an all-zero nonce to + * produce `encrypted_recipient_data(i)` + */ /* Encrypt in place */ towire_pad(&ret, crypto_aead_chacha20poly1305_ietf_ABYTES); ok = crypto_aead_chacha20poly1305_ietf_encrypt(ret, NULL, @@ -127,8 +128,8 @@ bool unblind_onion(const struct pubkey *blinding, struct secret hmac; /* BOLT-route-blinding #4: - * An intermediate node in the blinded route: - * + * A reader: + *... * - MUST compute: * - `ss(i) = SHA256(k(i) * E(i))` (standard ECDH) * - `b(i) = HMAC256("blinded_node_id", ss(i)) * k(i)` @@ -160,15 +161,17 @@ static u8 *decrypt_encmsg_raw(const tal_t *ctx, static const unsigned char npub[crypto_aead_chacha20poly1305_ietf_NPUBBYTES]; /* BOLT-route-blinding #4: - * - If an `encrypted_data` field is provided: - * - MUST decrypt it using `rho(r)` + * A reader: + *... + *- MUST decrypt the `encrypted_data` field using `rho(i)` and use + * the decrypted fields to locate the next node */ subkey_from_hmac("rho", ss, &rho); /* BOLT-onion-message #4: - * - if `enctlv` is not present, or does not decrypt with the - * shared secret from the given `blinding` parameter: - * - MUST drop the message. + *- If the `encrypted_data` field is missing or cannot + * be decrypted: + * - MUST return an error */ /* Too short? */ if (tal_bytelen(enctlv) < crypto_aead_chacha20poly1305_ietf_ABYTES) diff --git a/common/blindedpay.c b/common/blindedpay.c index 1d4aa2502..78d3df834 100644 --- a/common/blindedpay.c +++ b/common/blindedpay.c @@ -19,16 +19,15 @@ u8 **blinded_onion_hops(const tal_t *ctx, /* BOLT-route-blinding #4: * - For every node inside a blinded route: - * - MUST include the `encrypted_data` provided by the + * - MUST include the `encrypted_recipient_data` provided by the * recipient * - For the first node in the blinded route: * - MUST include the `blinding_point` provided by the - * recipient + * recipient in `current_blinding_point` * - If it is the final node: * - MUST include `amt_to_forward` and `outgoing_cltv_value`. - * - Otherwise: - * - MUST NOT include `amt_to_forward` and - * `outgoing_cltv_value`. + * - MUST include `total_amount_msat` when using `basic_mpp`. + * - MUST NOT include any other tlv field. */ onions[i] = onion_blinded_hop(onions, final ? &final_amount : NULL, diff --git a/common/onion.c b/common/onion.c index 0875adc18..a29327e7a 100644 --- a/common/onion.c +++ b/common/onion.c @@ -136,9 +136,9 @@ static bool handle_blinded_forward(struct onion_payload *p, u64 amt = amount_in.millisatoshis; /* Raw: allowed to wrap */ /* BOLT-route-blinding #4: - * - If it not the final node: - * - MUST return an error if fields other - * than `encrypted_recipient_data` or `blinding_point` are present. + * - If it is not the final node: + * - MUST return an error if the payload contains other tlv fields + * than `encrypted_recipient_data` and `current_blinding_point`. */ for (size_t i = 0; i < tal_count(tlv->fields); i++) { if (tlv->fields[i].numtype != TLV_TLV_PAYLOAD_BLINDING_POINT @@ -149,10 +149,10 @@ static bool handle_blinded_forward(struct onion_payload *p, } /* BOLT-route-blinding #4: - * - If it not the final node: + * - If it is not the final node: *... * - MUST return an error if `encrypted_recipient_data` does not - * contain `short_channel_id` or `next_node_id`. + * contain either `short_channel_id` or `next_node_id`. */ if (!enc->short_channel_id && !enc->next_node_id) { *failtlvtype = TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA; @@ -170,7 +170,7 @@ static bool handle_blinded_forward(struct onion_payload *p, p->total_msat = NULL; /* BOLT-route-blinding #4: - * - If it not the final node: + * - If it is not the final node: *... * - MUST return an error if `encrypted_recipient_data` does not * contain `payment_relay`. @@ -196,26 +196,27 @@ static bool handle_blinded_terminal(struct onion_payload *p, { /* BOLT-route-blinding #4: * - If it is the final node: - * - MUST return an error if fields other than - * `encrypted_recipient_data`, `blinding_point`, `amt_to_forward` - * or `outgoing_cltv_value` are present. - * - MUST return an error if the `path_id` in - * `encrypted_recipient_data` does not match the one it created. - * - MUST return an error if `amt_to_forward` or - * `outgoing_cltv_value` are not present. - * - MUST return an error if `amt_to_forward` is below what it expects - * for the payment. + * - MUST return an error if the payload contains other tlv fields than + * `encrypted_recipient_data`, `current_blinding_point`, `amt_to_forward`, + * `outgoing_cltv_value` and `total_amount_msat`. */ for (size_t i = 0; i < tal_count(tlv->fields); i++) { if (tlv->fields[i].numtype != TLV_TLV_PAYLOAD_BLINDING_POINT && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_AMT_TO_FORWARD - && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_OUTGOING_CLTV_VALUE) { + && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_OUTGOING_CLTV_VALUE + && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_TOTAL_AMOUNT_MSAT) { *failtlvtype = tlv->fields[i].numtype; return false; } } + /* BOLT-route-blinding #4: + * - MUST return an error if `amt_to_forward` or + * `outgoing_cltv_value` are not present. + * - MUST return an error if `amt_to_forward` is below what it expects + * for the payment. + */ if (!tlv->amt_to_forward) { *failtlvtype = TLV_TLV_PAYLOAD_AMT_TO_FORWARD; return false; @@ -230,12 +231,17 @@ static bool handle_blinded_terminal(struct onion_payload *p, p->outgoing_cltv = *tlv->outgoing_cltv_value; p->forward_channel = NULL; - /* BOLT #4: - * - if it is the final node: - * - MUST treat `total_msat` as if it were equal to - * `amt_to_forward` if it is not present. */ - p->total_msat = tal_dup(p, struct amount_msat, - &p->amt_to_forward); + if (tlv->total_amount_msat) { + p->total_msat = tal(p, struct amount_msat); + *p->total_msat = amount_msat(*tlv->total_amount_msat); + } else { + /* BOLT #4: + * - if it is the final node: + * - MUST treat `total_msat` as if it were equal to + * `amt_to_forward` if it is not present. */ + p->total_msat = tal_dup(p, struct amount_msat, + &p->amt_to_forward); + } return true; } @@ -276,46 +282,52 @@ struct onion_payload *onion_decode(const tal_t *ctx, return tal_free(p); } - if (blinding || p->tlv->blinding_point) { + /* BOLT-route-blinding #4: + * + * The reader: + * + * - If `encrypted_recipient_data` is present: + */ + if (p->tlv->encrypted_recipient_data) { struct tlv_encrypted_data_tlv *enc; /* Only supported with --experimental-onion-messages! */ if (!blinding_support) { - if (!blinding) - return tal_free(p); - *failtlvtype = TLV_TLV_PAYLOAD_BLINDING_POINT; - goto field_bad; - } - - /* BOLT-route-blinding #4: - * The reader: - * - If `blinding_point` is set (either in the payload or the - * outer message): - * - MUST return an error if it is set in both the payload - * and the outer message - */ - if (blinding && p->tlv->blinding_point) { - *failtlvtype = TLV_TLV_PAYLOAD_BLINDING_POINT; - goto field_bad; - } - if (p->tlv->blinding_point) - p->blinding = tal_dup(p, struct pubkey, - p->tlv->blinding_point); - else - p->blinding = tal_dup(p, struct pubkey, - blinding); - - /* BOLT-route-blinding #4: - * The reader: - *... - * - MUST return an error if `encrypted_recipient_data` is not - * present. - */ - if (!p->tlv->encrypted_recipient_data) { *failtlvtype = TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA; goto field_bad; } + /* BOLT-route-blinding #4: + * + * - If `blinding_point` is set in the incoming `update_add_htlc`: + * - MUST return an error if `current_blinding_point` is present. + * - MUST use that `blinding_point` as the blinding point for decryption. + * - Otherwise: + * - MUST return an error if `current_blinding_point` is not present. + * - MUST use that `current_blinding_point` as the blinding point for decryption. + */ + if (blinding) { + if (p->tlv->blinding_point) { + *failtlvtype = TLV_TLV_PAYLOAD_BLINDING_POINT; + goto field_bad; + } + p->blinding = tal_dup(p, struct pubkey, blinding); + } else { + if (!p->tlv->blinding_point) { + *failtlvtype = TLV_TLV_PAYLOAD_BLINDING_POINT; + goto field_bad; + } + p->blinding = tal_dup(p, struct pubkey, + p->tlv->blinding_point); + } + + /* BOLT-route-blinding #4: + * The reader: + *... + * - MUST return an error if `encrypted_recipient_data` does + * not decrypt using the blinding point as described in + * [Route Blinding](#route-blinding). + */ ecdh(p->blinding, &p->blinding_ss); enc = decrypt_encrypted_data(tmpctx, p->blinding, &p->blinding_ss, p->tlv->encrypted_recipient_data); @@ -326,8 +338,9 @@ struct onion_payload *onion_decode(const tal_t *ctx, if (enc->payment_constraints) { /* BOLT-route-blinding #4: - * - MUST return an error if the expiry is greater than - * `encrypted_recipient_data.payment_constraints.max_cltv_expiry`. + * - MUST return an error if: + * - the expiry is greater than + * `encrypted_recipient_data.payment_constraints.max_cltv_expiry`. */ if (cltv_expiry > enc->payment_constraints->max_cltv_expiry) { *failtlvtype = TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA; @@ -335,8 +348,10 @@ struct onion_payload *onion_decode(const tal_t *ctx, } /* BOLT-route-blinding #4: - * - MUST return an error if the amount is below - * `encrypted_recipient_data.payment_constraints.htlc_minimum_msat`. + * - MUST return an error if: + *... + * - the amount is below + * `encrypted_recipient_data.payment_constraints.htlc_minimum_msat`. */ if (amount_msat_less(amount_in, amount_msat(enc->payment_constraints->htlc_minimum_msat))) { @@ -345,9 +360,11 @@ struct onion_payload *onion_decode(const tal_t *ctx, } /* BOLT-route-blinding #4: - * - MUST return an error if the payment uses a feature - * not included in - * `encrypted_recipient_data.payment_constraints.allowed_features`. + * - If `allowed_features` is present: + * - MUST return an error if: + *... + * - the payment uses a feature not included in + * `encrypted_recipient_data.allowed_features.features` */ /* We don't have any features yet... */ } @@ -383,6 +400,17 @@ struct onion_payload *onion_decode(const tal_t *ctx, return p; } + /* BOLT-route-blinding-fix #4: + * - Otherwise (it is not part of a blinded route): + * - MUST return an error if `blinding_point` is set in the + * incoming `update_add_htlc` or `current_blinding_point` + * is present. + */ + if (blinding || p->tlv->blinding_point) { + *failtlvtype = TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA; + goto field_bad; + } + /* BOLT #4: * * The reader: diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 64ee72cce..89863a772 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -98,6 +98,14 @@ static struct failed_htlc *mk_failed_htlc_badonion(const tal_t *ctx, { struct failed_htlc *f = tal(ctx, struct failed_htlc); + /* BOLT-route-blinding #4: + * - If `blinding_point` is set in the incoming `update_add_htlc`: + * - MUST return `invalid_onion_blinding` for any local error or + * other downstream errors. + */ + if (hin->blinding) + badonion = WIRE_INVALID_ONION_BLINDING; + f->id = hin->key.id; f->onion = NULL; f->badonion = badonion; @@ -113,6 +121,15 @@ static struct failed_htlc *mk_failed_htlc(const tal_t *ctx, { struct failed_htlc *f = tal(ctx, struct failed_htlc); + /* BOLT-route-blinding #4: + * - If `blinding_point` is set in the incoming `update_add_htlc`: + * - MUST return `invalid_onion_blinding` for any local error or + * other downstream errors. + */ + if (hin->blinding) { + return mk_failed_htlc_badonion(ctx, hin, + WIRE_INVALID_ONION_BLINDING); + } f->id = hin->key.id; f->sha256_of_onion = NULL; f->badonion = 0; @@ -149,16 +166,7 @@ static void fail_in_htlc(struct htlc_in *hin, htlc_in_update_state(hin->key.channel, hin, SENT_REMOVE_HTLC); htlc_in_check(hin, __func__); - /* BOLT-route-blinding #4: - * - If `blinding_point` is set in the incoming `update_add_htlc`: - * - MUST return `invalid_onion_blinding` on any error, including - * downstream errors received from forwarding HTLCs. - */ - if (hin->blinding) { - failed_htlc = mk_failed_htlc_badonion(tmpctx, hin, - WIRE_INVALID_ONION_BLINDING); - } else - failed_htlc = mk_failed_htlc(tmpctx, hin, hin->failonion); + failed_htlc = mk_failed_htlc(tmpctx, hin, hin->failonion); bool we_filled = false; wallet_htlc_update(hin->key.channel->peer->ld->wallet,