Clarify onionmessage decryption

[rustyrussell]

I recently re-implemented this to get our code production ready, and I found the spec unclear and even misleading. This series seeks to rework it to be clearer and more coherent.

Most pointedly, it's now clear that blinding applies to the onion, not the internal encrypted blob: the spec was confused about this at various points. The onion decoding is now specified exactly, and is general. The old references to "realms" (from legacy onions) is removed.

[thomash-acinq]

blinding applies to the onion, not the internal encrypted blob.

That's not what's implemented in eclair and phoenix, and it contradicts the Route Blinding Requirements section.
I guess the error comes from the fact that there are two ways to use the blinding:

• For the onion itself we use $B_i = HMAC256(\text{"blinded\_node\_id"}, ss_i) * N_i$
• For the encrypted blob inside the onion we use $rho_i = HMAC256(\text{"rho"}, ss_i)$

[thomash-acinq]

This is incorrect, the blob is not encrypted to the real node id, but with a key that is derived from both the blinding and the node's private key:
$ss_i = SHA256(e_i * N_i) = SHA256(k_i * E_i)$ (ECDH shared secret known only by $N_r$ and $N_i$)
$rho_i = HMAC256(\text{"rho"}, ss_i)$ (key used to encrypt the payload for $N_i$ by $N_r$)

[valentinewallace]

I believe this is what we have in LDK as well, although I'm not too familiar with the notation being used. E.g., for ss_0, we compute ecdh(N_0, session_priv), which I think is equivalent to SHA256(e_0 * N_0) as in the above comment. Eclair is the most interop tested with us so far so pretty sure we're on the same page.

[rustyrussell]

This is incorrect, the blob is not encrypted to the real node id, but with a key that is derived from both the blinding and the node's private key: ssi=SHA256(ei∗Ni)=SHA256(ki∗Ei) (ECDH shared secret known only by Nr and Ni) rhoi=HMAC256("rho",ssi) (key used to encrypt the payload for Ni by Nr)

Well, yes. I should spell that out, indeed you use that same blinding as the ECDH ephemeral key.

What you don't do, is ssi=SHA256(ei*Bi). That would be a valid construction (which I would call "encrypting to the blinded node pubkey"). i.e. you don't use blinding as blinding at all here...

i.e. the onion is encrypted to the blinded node id, the inner encrypted_recipient_data is not. This is important!

[thomash-acinq]

Why did you remove this? blinding_point needs to be used as $E_i$ for the Route Blinding Requirements section.

[rustyrussell]

Yes, but it's not used as a blinding point. I changed this and the next one:

    - If `blinding_point` is set in the incoming `update_add_htlc`:
      - MUST return an error if `current_blinding_point` is present.
	  - MUST use `blinding_point` as $`E_i`$
    - Otherwise:
      - MUST return an error if `current_blinding_point` is not present.
      - MUST use `current_blinding_point` as $`E_i`$
      - SHOULD add a random delay before returning errors.
    - MUST return an error if `encrypted_recipient_data` does not decrypt to a valid `encrypted_data_tlv` as described in [Route Blinding](#route-blinding).

?

[thomash-acinq]

I don't understand what you mean by "following blinding point" here.

[rustyrussell]

This is wrong, it's used both to derive the ss and to derive the next blinding point. Will fix...

[thomash-acinq]

This is confusing. We need to tweak the private key. And then we need to use the tweaked key instead of the public one.

[rustyrussell]

This is the alternative construction, where we tweak the onion ephemeral key (which is equivalent). (CLN needs to do this because we have an HSM and we didn't want to teach it to tweak, but it's also easier in the text).

We could specify both, but we do note the private key method alternative in the rationale?

[thomash-acinq]

Suggested change

	- $`ss_i = SHA256(e_i * N_i) = SHA256(k_i * E_i)$` (ECDH shared secret known only by $`N_r`$ and $`N_i`$)
	- $ss_i = SHA256(e_i * N_i) = SHA256(k_i * E_i)$ (ECDH shared secret known only by $N_r$ and $N_i$)

Markdown rendering is broken because of a misplaced `, we should probably remove all the ` when we already have $.

[rustyrussell]

Wow, I didn't even know that worked. But if we take out ` the formatting changes from monospaced to normal, which would be confusing unless we do it everywhere.

Fixed for now...

[rustyrussell]

OK, so I rolled in Thomas' feedback, which lead me to think that we should simply avoid calling the point "blinding" (as that's not all it's used for). Example commit which changes it to "path_key" at the end?

I think this is clearer. WDYT?

[t-bast]

I stopped reviewing this half-way, when I reached the incorrect statement that Alice encrypts the onion to (Bob', Carol', Dave'). As detailed in my comment below, this is not how it works, and leads me to think that this updated version is actually less clear than the previous one.

But I do see where the confusion comes from: there is a fundamental difference between onion messages and payments, because onion messages combine two blinded paths while payments only use the recipient path. That's why I think the way it is specified currently on master makes more sense: how you decrypt the onion only depends on whether you received a path key or not (if you receive a path key, the onion is encrypted for your blinded node_id, otherwise it's encrypted to your normal node_id). Then, once you have a decrypted onion, if it contains an encrypted_data_tlv, you will use the path key you must have received somehow to derive the shared secret to decrypt it. That is a pretty simple rule to follow, so the reader requirements should be very straightforward. We may be missing some detailed writer requirements however, which is where there are different cases for payments and onion messages.

I think we can create a smaller patch to clear up those confusions than what this PR does, I'd like to try a different approach for that.

I like the renaming of blinding points to path keys though!

[t-bast]

nit: it's a bit confusing to how path_key is mentioned here, it feels like we should include it in the associated_data (which is not the case), what about:

Suggested change

	- MUST decrypt `onion_routing_packet` as specified in [Onion Decryption](04-onion-routing.md#onion-decryption) using `payment_hash` as `associated_data` (and `path_key` if specified).
	- MUST decrypt `onion_routing_packet` as specified in [Onion Decryption](04-onion-routing.md#onion-decryption):
	- MUST use `path_key` if specified to derive the onion decryption key.
	- MUST use `payment_hash` as `associated_data`.

I've done that change in t-bast@c073ef8 to simplify updates.

[t-bast]

nit: indenting is wrong:

Suggested change

	- MUST follow the requirements for processing the payload under [Failure Messages](04-onion-routing.md#failure-messages)
	- MUST follow the requirements for processing the payload under [Failure Messages](04-onion-routing.md#failure-messages)

[t-bast]

When used for payments, Alice doesn't encrypt the onion to Bob', she encrypts it to Bob and includes the first_path_key in the onion. That's what allows nodes between Alice and Bob to be unaware that a blinded path is used downstream. If the onion was encrypted to Bob', Bob would have no way of obtaining the path_key to decrypt it.

Alice uses Bob' only when she combines a blinded path from herself to Bob to the blinded path from Bob to Dave (which is what is done for onion messages).

In both cases, Bob only uses that path key to decrypt the encrypted_data_tlv: it's only nodes that are not the introduction node that use the path key to decrypt the onion.

I clarified that in t-bast@c073ef8

[rustyrussell]

I agree on the details, just not the framing.

I think of the broad case which I'm describing here as "encrypt to the entire blinded chain", with a special exception for the payment case.

This is spelled out clearly in the new requirements (which I just noticed did not replace blinding so uses the old terminology):

The reader of the `blinded_path`:
- MUST create its own onion to reach the `first_node_id`
- For the first entry in `path`:
  - if it is sending a payment:
    - MAY encrypt the first blinded path onion to `first_node_id` and include `blinding` as `current_blinding_point`
  - if it does not do that:
    - MUST encrypt the first blinded path onion to the first `blinded_node_id`.
    - MUST set `next_blinding_override` in the prior onion payload to `blinding`.
  - MUST include the first `path` `encrypted_recipient_data` in each onion payload within the blinded path.

And in the rationale:

Note that there are two ways for the sender to reach the introduction
point: one is to create a normal (unblinded) payment, and place the
initial blinding point in `current_path_key` along with the
`encrypted_recipient_data` in the onion payload for the introduction point to
start the blinded path. The second way (which is the only way for
onion messages) is to create a blinded path to
the introduction point, set `next_path_key_override` inside the
`encrypted_data_tlv` on the hop prior to the introduction point to the
`first_path_key`, so it is sent to the introduction node.  However, this
only works if that prior node supports blinded paths.

[t-bast]

I think that one of the things that makes this PR hard to review is that 5983830 and 570f6fa are unrelated to route blinding and could be extracted to its own PR. It's easier to review on its own and we should reach agreement quickly on it and merge it as a first step.

[rustyrussell]

I think that one of the things that makes this PR hard to review is that 5983830 and 570f6fa are unrelated to route blinding and could be extracted to its own PR. It's easier to review on its own and we should reach agreement quickly on it and merge it as a first step.

Agreed. It's only when I started editing I found these.

I will open a new PR for those, then rebase on that.