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 <rusty@rustcorp.com.au>

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
+	 * - MUST encrypt each `encrypted_data_tlv(i)` with ChaCha20-Poly1305
-	 *   and an all-zero nonce
+         *   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:
+	 * A reader:
-	 *   - MUST decrypt it using `rho(r)`
+	 *...
+	 *- 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
+	 *- If the `encrypted_data` field is missing or cannot
-	 *     shared secret from the given `blinding` parameter:
+	 *  be decrypted:
-	 *   - MUST drop the message.
+	 *   - MUST return an error
 	 */
 	/* Too short? */
 	if (tal_bytelen(enctlv) < crypto_aead_chacha20poly1305_ietf_ABYTES)

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 include `total_amount_msat` when using `basic_mpp`.
-		 *     - MUST NOT include `amt_to_forward` and
+		 *   - MUST NOT include any other tlv field.
-		 *       `outgoing_cltv_value`.
 		 */
 		onions[i] = onion_blinded_hop(onions,
 					      final ? &final_amount : NULL,

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:
+	 * - If it is not the final node:
-	 *   - MUST return an error if fields other
+	 *   - MUST return an error if the payload contains other tlv fields
-	 *     than `encrypted_recipient_data` or `blinding_point` are present.
+	 *     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
+	 *   - MUST return an error if the payload contains other tlv fields than
-	 *     `encrypted_recipient_data`, `blinding_point`, `amt_to_forward`
+	 *     `encrypted_recipient_data`, `current_blinding_point`, `amt_to_forward`,
-	 *     or `outgoing_cltv_value` are present.
+	 *     `outgoing_cltv_value` and `total_amount_msat`.
-	 *   - 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.
 	 */
 	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 (tlv->total_amount_msat) {
-	 * - if it is the final node:
+		p->total_msat = tal(p, struct amount_msat);
-	 *   - MUST treat `total_msat` as if it were equal to
+		*p->total_msat = amount_msat(*tlv->total_amount_msat);
-	 *     `amt_to_forward` if it is not present. */
+	} else {
-	p->total_msat = tal_dup(p, struct amount_msat,
+		/* BOLT #4:
-				&p->amt_to_forward);
+		 * - 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
+			 * - MUST return an error if:
-			 *   `encrypted_recipient_data.payment_constraints.max_cltv_expiry`.
+			 *   - 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
+			 * - MUST return an error if:
-			 *   `encrypted_recipient_data.payment_constraints.htlc_minimum_msat`.
+			 *...
+			 *   - 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
+			 * - If `allowed_features` is present:
-			 *   not included in
+ 			 *   - MUST return an error if:
-			 *   `encrypted_recipient_data.payment_constraints.allowed_features`.
+			 *...
+			 *     - 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:

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:
+	failed_htlc = mk_failed_htlc(tmpctx, hin, hin->failonion);
-	 * - 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);
 
 	bool we_filled = false;
 	wallet_htlc_update(hin->key.channel->peer->ld->wallet,