smite: add BOLT 4 onion routing (Sphinx) - #170
Conversation
|
#166 was merged, so I think this can be rebased on master now |
a3b3e6c to
a9c273b
Compare
Adds an `onion` module that builds and peels BOLT 4 packets. - `keys`: shared secrets and the rho/mu/um/pad/ammag/ammagext key derivation, plus the zero-nonce ChaCha20 stream. - `payload`: the `HopPayload` TLV (types 2, 4, 6, 8, 10, 12, 16, 18), with `forward()` and `receive()` constructors. - `packet`: `OnionBuilder` for the 1366-byte packet, and `peel` for the reverse direction. Verified against `onion-test.json` and `onion-error-test.json` from the BOLT repository, vendored verbatim: the reference packet is reproduced byte for byte, peels at every hop, and the derived shared secrets and keys match. What this enables today: constructing a spec-valid onion for any route and decrypting one addressed to us. Malformed onions are reachable through `OnionBuilder::raw_hop`, which accepts the reserved payload lengths 0 and 1, non-TLV bytes, and oversized payloads, and through the public fields of `OnionPacket`. Deferred to follow-ups: - Failure decryption. `KeyType::Ammag` and `Um` are derived here but unused; obfuscating a `reason`, recovering the erring hop, and the failure codes are not implemented, so `update_fail_htlc.reason` stays opaque. - Attribution data crypto. The wire type already exists in `bolt::attribution_data`; the `ammagext` stream and the 210 truncated HMACs over it do not. - Route blinding. `HopPayload` carries `encrypted_recipient_data` and `current_path_key`, and `peel` accepts a path key, but there is no blinded path builder or `encrypted_data` TLV to produce either side. - Onion messages (type 513), which use a variable-size packet.
a9c273b to
52e51eb
Compare
Thanks for the reminder. I've rebased it and dropped the old tu32/tu64 commit. |
|
|
||
| /// [`PAYMENT_HOP_PAYLOADS_SIZE`] as a `u64`, to compare against wire-declared | ||
| /// lengths. | ||
| const PAYMENT_HOP_PAYLOADS_CAPACITY: u64 = PAYMENT_HOP_PAYLOADS_SIZE as u64; |
There was a problem hiding this comment.
This wasn't used for any comparisons, but only for returning errors (OnionError::PayloadsTooLong), so we could use PAYMENT_HOP_PAYLOADS_SIZE as u64 in those places as well, similar to the needed as u64 used just before it
| impl OnionPacket { | ||
| /// Encodes the packet to its 1366-byte wire representation. | ||
| #[must_use] | ||
| pub fn encode(&self) -> Vec<u8> { |
There was a problem hiding this comment.
OnionPacket will only be used in UpdateAddHtlc, Considering that, do we need both encode/decode and a WireFormat impl for it?
Can we define only one of them? I think if we define this as a type in UpdateAddHtlc, then we could use WireFormat for consistency. But, I don't think defining a separate type is a good idea, since we'd then have to handle that type throughout the IR as well, when we could simply use Vec to send an arbitrary onion packet. So I think we'll only use this internally, using encode/decode makes more sense to me. WDYT?
There was a problem hiding this comment.
I think it doesn't make much sense to send an arbitrary onion packet with random bytes. I think a more appropriate approach would be:
If we chose to follow this, it would make sense to define OnionPacket as a type in UpdateAddHtlc.
There was a problem hiding this comment.
Apologies for not being clear, what I meant is that we definitely need OnionPacket as a struct to create a valid onion, but we definitely want to leave room for sending malformed onions as well (possibly using #178). So, in UpdateAddHtlc, we will keep the onion packet as a Vec only I think
What I meant earlier was that we now have two ways to encode/decode OnionPacket, and it would be better to have only one. Implementing encode/decode makes more sense to me than implementing WireFormat for it
| /// the packet that reaches hop `k` is fully determined by the shared secrets of | ||
| /// hops `0..k`. The sender must reproduce it or the HMACs will not match. | ||
| fn generate_filler(shared_secrets: &[SharedSecret], hops: &[Hop]) -> Vec<u8> { | ||
| debug_assert!(!hops.is_empty(), "build rejects empty routes"); |
There was a problem hiding this comment.
Caller already rejects empty hops before calling this, so do we really need an assert here? Also, debug_assert! won't be useful for us since we are running the campaign in a release build
There was a problem hiding this comment.
Right. The idea was to test the invariant when running the tests. However, I'll remove those since the unit tests should already cover it.
3 participants
Adds an
onionmodule that builds and peels BOLT 4 packets.keys: shared secrets and the rho/mu/um/pad/ammag/ammagext key derivation, plus the zero-nonceChaCha20stream.payload: theHopPayloadTLV (types 2, 4, 6, 8, 10, 12, 16, 18), withforward()andreceive()constructors.packet:OnionBuilderfor the 1366-byte packet, andpeelfor the reverse direction.Verified against
onion-test.jsonandonion-error-test.jsonfrom the BOLT repository: the reference packet is reproduced byte for byte, peels at every hop, and the derived shared secrets and keys match.What this enables today: constructing a spec-valid onion for any route and decrypting one addressed to us. Malformed onions are reachable through
OnionBuilder::raw_hop, which accepts the reserved payload lengths 0 and 1, non-TLV bytes, and oversized payloads, and through the public fields ofOnionPacket.Deferred to follow-ups:
KeyType::AmmagandUmare derived here but unused; obfuscating areason, recovering the erring hop, and the failure codes are not implemented, soupdate_fail_htlc.reasonstays opaque.bolt::attribution_data; theammagextstream and the 210 truncated HMACs over it do not.HopPayloadcarriesencrypted_recipient_dataandcurrent_path_key, andpeelaccepts a path key, but there is no blinded path builder orencrypted_dataTLV to produce either side.Depends on #166