multi: Route blinding #57

ellemouton · 2022-06-30T20:04:01Z

This PR adds the changes that higher layers will need in order to take advantage of route blinding as defined in the spec proposal

Note that callers of this code will need to know how to extract the next_blinding_point (if the caller is the introduction node in the route) and next_blinding_override points from the onion packet payloads.

carlaKC

very high level pass, just submitting in case GH loses my comments

blinding.go

crypto.go

carlaKC · 2022-07-05T11:06:14Z

Also a meta thing that I noticed while using this PR. When this is merged I think that it would make sense to make two tags: one on the blinded route commit, and one on the final merge/full PR.

Reason being that code that imports this change and lnd requires lnd to be upgraded to use the new arg in ProcessOnionPacket (which requires lnd to update its dependency as well). Adding an extra tag to easily allow code to just point to the blinded paths stuff could be useful in avoiding modhell/ requiring lnd to update its dep prematurely. Just a note for future-selves.

sputn1ck

Very cool PR. Have not tested it yet and want to compare it to other implementations.

blinding.go

blinding_test.go

calvinrzachman

This PR makes really good use of the existing functionality, nicely leveraging the similarities between route blinding and normal sphinx processing. Props for not getting lost in the “blinding” sauce (route blinding, blinding factor, blinding point, etc.)! I also like how the implementation shields any complexity of the route blinding protocol from callers. They just proceed like normal and leave the nuances of decrypting onions in a blinded route to the library. I think the effort there will help with integrating this into the Channel Link.

A few of my comments are notes. Let me know if our understandings match and feel free to resolve them so they don’t add clutter! My favorite PR - only to be superseded by the PR which adds this to LND :) Thanks for working on this!

blinding.go

calvinrzachman · 2022-07-14T15:09:38Z

crypto.go

+	var blindingFactor btcec.ModNScalar
+	blindingFactor.SetBytes(&blindingFactorBytes)
+
+	ephemeral := blindGroupElement(dhKey, blindingFactor)


Subtle Note: Rather than directly computing the blinded private key, b(i), we follow this section of the route blinding proposal document. This is ephemeral = blinding_factor*E_sender

yes indeed :) i have added a comment to point this out

Nit: I haven't seen this definition in the spec, especially the context you are usingblinding_factor here @calvinrzachman ?

Apologies for any confusion, Ziggie. It's been a while since I've looked at this, but I'm pretty sure that when I say blinding_factor here I am referring to Elle's variable blindingFactor := HMAC256("blinded_node_id", ss(i)). Which you are correct does differ from how the term is used in the spec document. Brought that up with Elle briefly here

Also, looks like my link may have gotten rug pulled by a change to t-bast's underlying spec PR. I usually pick an older, non-master release or tag so that links point to the expected line, but I don't know how to do that for an actively changing PR :/ In any case, the section this used to link to is now here.

calvinrzachman · 2022-07-14T17:27:40Z

crypto.go

+	blindingFactor.SetBytes(&blindingFactorBytes)
+
+	ephemeral := blindGroupElement(dhKey, blindingFactor)
+	return sharedSecret(r.onionKey, ephemeral)


This step mixes the blinding factor and node ID private key, k(i), implicitly computing the blinded node ID private key, b(i), and is equivalent to using b(i) in an ECDH with the sender's usual ephemeral key from the onion, E_sender.

ECDH(b(i), E_sender) is not done explicitly. Rather we have ECDH(k(i), blinding_factor*E_sender), which correctly returns a shared secret between onion encryptor and routing node:

ss_sender/node = k*blinding_factor*E_sender = (blinding_factor*k)*E_sender = b(i)*E_sender = ECDH(b(i), E_sender)

From here the caller is free to use rho = HMAC-256("rho", ss_sender/node) to decrypt the onion like normal.

In the line for ss_sender/node you're missing hash functions, thanks for the comment, this helped me 🙏.

Thanks so much, very useful comment, we need this definitely in the codebase too.

In the line for ss_sender/node you're missing hash functions, ...

You are quite right. Glad the message still came through!

cool - added some more comments to help describe this

blinding.go

ellemouton

Thanks for the reviews y'all!! updated 🔥

blinding.go

blinding_test.go

crypto.go

ellemouton · 2022-07-29T04:01:36Z

crypto.go

+	var blindingFactor btcec.ModNScalar
+	blindingFactor.SetBytes(&blindingFactorBytes)
+
+	ephemeral := blindGroupElement(dhKey, blindingFactor)


yes indeed :) i have added a comment to point this out

ellemouton · 2022-08-04T08:20:01Z

making a note here for myself to update the PR to point to the latest state of the spec PR & its test vectors

ellemouton · 2022-08-04T08:28:50Z

ah ok, looks like the updated test vectors still pass with the existing code. I think the main changes were related to the tlv fields/types which does not affect this pr since pr assumes the tlvs are already encoded. Will double check this later though

lightninglabs-deploy · 2022-08-11T09:07:39Z

@ellemouton, remember to re-request review from reviewers when ready

ellemouton · 2022-08-11T09:53:07Z

!lightninglabs-deploy mute

blinding.go

obfuscation.go

carlaKC

LGTM! Just needs rebase on ca23184 - when I import this to LND now some tests fail.

I've used this for onion messagaing and have interop with CL + LDK, and everything works as advertized in a WIP branch for forwarding blinded routes for LND.

blinding.go

carlaKC · 2023-01-25T15:41:28Z

Just needs rebase on ca23184 - when I import this to LND now some tests fail.

Rebase-beg when you have a minute!

ellemouton · 2023-01-26T07:10:31Z

done! cc @carlaKC

bitromortac

Cool work, this looks really nice 👍. Looking forward to explore the usage of this fundamental implementation 🎉

cmd/main.go

testdata/route-blinding-test.json

blinding_test.go

sphinx.go

blinding.go

ziggie1984

Impressive work ⚡️ :)
Took me a a bit to go through and understand the big picture here 😅, especially the blinding variables where tricky for me, so I left some question regarding the whole topic, apart from that I had only some nits.

Keep up the very good work 🚀

ziggie1984 · 2023-05-18T09:00:18Z

blinding.go

+	BlindedHops []*BlindedHopInfo
+}
+
+// hopInfo represents a single hop in a blinded path. The BlindedHopInfo wrapper


Q: should this note be the other way around, shouldn't we use the BlindedHopInfo wrapper for hops with the blinded nodeId?

ziggie1984 · 2023-05-21T08:41:45Z

blinding.go

+	// point that the introduction node will use in order to create a shared
+	// secret with the builder of the blinded route. This point will need
+	// to be communicated to the introduction node by the sender in some
+	// way.


Q: What are the ways how this introduction point could be delivered to the First node in the blinded path? Do you refer in this comment to concatenate blinded paths for example ?

What are the ways how this introduction point could be delivered to the First node in the blinded path?

it will be communicated via update_add_htlc

Do you refer in this comment to concatenate blinded paths for example ?

not sure what you mean?

crypto.go

ziggie1984 · 2023-05-21T14:05:14Z

crypto.go

+	var blindingFactor btcec.ModNScalar
+	blindingFactor.SetBytes(&blindingFactorBytes)
+
+	ephemeral := blindGroupElement(dhKey, blindingFactor)


Nit: I haven't seen this definition in the spec, especially the context you are usingblinding_factor here @calvinrzachman ?

ziggie1984 · 2023-05-21T14:06:43Z

crypto.go

+	blindingFactor.SetBytes(&blindingFactorBytes)
+
+	ephemeral := blindGroupElement(dhKey, blindingFactor)
+	return sharedSecret(r.onionKey, ephemeral)


Thanks so much, very useful comment, we need this definitely in the codebase too.

ziggie1984 · 2023-05-23T11:06:05Z

blinding.go

@@ -0,0 +1,169 @@
+package sphinx


Nit: Update Commit BlindBlindedRoute => BuildBlindedRoute

DecryptBlindedData would need an update as well

blinding.go

ziggie1984 · 2023-05-23T12:09:51Z

blinding.go

+		return nil, err
+	}
+
+	blindingFactor := computeBlindingFactor(ephemPub, ss[:])


Hmm, so we have blindingFactor but also blindingFactorBytes (in this PR). The first one adheres with the definition in the specification the other one not. I agree both are blinding scalars but this confused me, maybe adding some comments where we diverge ?

ziggie1984 · 2023-05-23T12:25:34Z

blinding_test.go

+	pathED, err := BuildBlindedPath(eveSessKey, eveDavePath)
+	require.NoError(t, err)
+
+	// At this point, Eve will give her blinded path to Bob who will then


Q: I do not quite understand what sense it makes for Bob to use a blinded Path to Carol when it's only one Hop from Bob => Carol, does it make sense in the real world?

no it probably doesnt make sense in the real world. This is just to mimic the scenario in the spec test vector

bitromortac

LGTM 🔥 🚀 again, very nice work!

cmd/main.go

bitromortac · 2023-05-25T13:28:07Z

blinding.go

@@ -0,0 +1,169 @@
+package sphinx


DecryptBlindedData would need an update as well

blinding_test.go

cmd/main.go

Update the TestVariablePayloadOnion test to make use of the `require` package.

Add a TestTLVPayloadOnion test that tests the newer spec test vector which tests the construction of an onion in the case where all the hops are using the TLV version payload.

This commit updates the cli tool to use the urfave library. This makes things easier to read and will make it easier to extend the tool in future. A `genKeys` function is also added as a helper for quickly generating key pairs.

This commit is a pure refactor and move commit. It moves the HopPayload methods out from the `path.go` file and into a dedicated `payload.go` file in order to prepare for a follow up commit that will add a blinded path struct to the `path.go` file. The opportunity is also taken to separate out the tlv vs legacy encoding of the hop payload a bit (seprate constructors for example) in order to prepare for a future where the legacy encoding will be removed.

crypto.go

Roasbeef · 2023-08-17T00:47:24Z

crypto.go

+	}
+
+	// Encrypt the message and append the ciphertext to the nonce.
+	return aead.Seal(plainTxt[:0], chaChaPolyZeroNonce, plainTxt, nil), nil


Is plainTxt actually always appropriately sized? IIUC this is only useful when you have a buffer that is already pre-allocated, as then you end up with plaintext || ciphertext after the routine ends. In that case, you don't even need the return value though.

the comment for Seal says:

// To reuse plaintext's storage for the encrypted output, use plaintext[:0] // as dst. Otherwise, the remaining capacity of dst must not overlap plaintext.

so kinda just went with that suggestion

path.go

crypto.go

obfuscation.go

sphinx.go

This commit introduces a new `BlindedPath` struct which holds all the information defining a blinded route. The commit also includes the addition of various helper methods to blind and unblind node IDs and hop data. Test vectors from the spec are also added.

In this commit, ProcessOnionPacket is updated to take in a blinding key as a parameter. This key is then used in order to determine the blinding factor necessary to decrypt the onion. This change is accompanied with a test that tests this change against a test vector from the route blinding spec PR.

In this commit, DecryptBlindHopData and NextEphemeral methods are added to Router which makes use of the Router's onion key in order to unblind blinded data and compute the next ephermal key respectively.

Add a blinded-key option to the parse command so that it can be used to test parsing of an onion for hops in a blinded route. Also add a helper nextephemeral command.

ellemouton

Thanks for the review @Roasbeef 🙏 ready for another round!

crypto.go

ellemouton · 2023-08-17T07:20:17Z

crypto.go

+	}
+
+	// Encrypt the message and append the ciphertext to the nonce.
+	return aead.Seal(plainTxt[:0], chaChaPolyZeroNonce, plainTxt, nil), nil


the comment for Seal says:

// To reuse plaintext's storage for the encrypted output, use plaintext[:0] // as dst. Otherwise, the remaining capacity of dst must not overlap plaintext.

crypto.go

path.go

testdata/route-blinding-test.json

path.go

crypto.go

path.go

ellemouton · 2023-08-17T09:30:53Z

crypto.go

+	}
+
+	// Encrypt the message and append the ciphertext to the nonce.
+	return aead.Seal(plainTxt[:0], chaChaPolyZeroNonce, plainTxt, nil), nil


so kinda just went with that suggestion

Roasbeef

LGTM ⛷️

This was referenced Jun 30, 2022

BOLT12 lightningnetwork/lnd#5594

Open

routing+htlcswitch: add support for selecting, and encoding+decoding blinded paths lightningnetwork/lnd#6690

Closed

carlaKC reviewed Jul 5, 2022

View reviewed changes

ellemouton force-pushed the routeBlinding branch 2 times, most recently from e552258 to 91a33c8 Compare July 5, 2022 12:14

carlaKC mentioned this pull request Jul 8, 2022

mod: Unmerged dependencies carlaKC/boltnd#18

Open

calvinrzachman mentioned this pull request Jul 21, 2022

lightning-onion: route blinding implementation PR review notes calvinrzachman/lightning-onion#1

Open

sputn1ck reviewed Jul 28, 2022

View reviewed changes

blinding.go Outdated Show resolved Hide resolved

blinding_test.go Outdated Show resolved Hide resolved

calvinrzachman reviewed Jul 28, 2022

View reviewed changes

ellemouton force-pushed the routeBlinding branch from 91a33c8 to 462661d Compare July 29, 2022 04:03

ellemouton commented Jul 29, 2022

View reviewed changes

ellemouton force-pushed the routeBlinding branch from 462661d to d06c852 Compare August 4, 2022 08:26

carlaKC reviewed Sep 7, 2022

View reviewed changes

blinding.go Outdated Show resolved Hide resolved

carlaKC reviewed Sep 9, 2022

View reviewed changes

blinding.go Outdated Show resolved Hide resolved

ellemouton force-pushed the routeBlinding branch from d06c852 to 654a7f6 Compare October 13, 2022 09:16

carlaKC reviewed Nov 21, 2022

View reviewed changes

blinding.go Outdated Show resolved Hide resolved

carlaKC reviewed Nov 22, 2022

View reviewed changes

obfuscation.go Outdated Show resolved Hide resolved

ellemouton force-pushed the routeBlinding branch from 654a7f6 to 4bf61b8 Compare December 20, 2022 09:25

ellemouton requested a review from carlaKC December 20, 2022 09:27

carlaKC mentioned this pull request Dec 20, 2022

routing: Support Pathfinding to Blinded Routes lightningnetwork/lnd#7267

Merged

ellemouton force-pushed the routeBlinding branch from 4bf61b8 to 4bf26cb Compare December 22, 2022 07:52

carlaKC approved these changes Dec 30, 2022

View reviewed changes

blinding.go Outdated Show resolved Hide resolved

blinding.go Outdated Show resolved Hide resolved

blinding.go Outdated Show resolved Hide resolved

blinding.go Outdated Show resolved Hide resolved

carlaKC mentioned this pull request Jan 6, 2023

[feature]: Blinded Forwarding in LND lightningnetwork/lnd#7298

Closed

ellemouton force-pushed the routeBlinding branch from 4bf26cb to d0b521d Compare January 26, 2023 07:10

bitromortac reviewed May 5, 2023

View reviewed changes

ellemouton force-pushed the routeBlinding branch 4 times, most recently from f3212d5 to f050833 Compare May 10, 2023 09:44

ellemouton requested a review from bitromortac May 16, 2023 17:06

ziggie1984 reviewed May 23, 2023

View reviewed changes

bitromortac approved these changes May 25, 2023

View reviewed changes

ellemouton force-pushed the routeBlinding branch from f050833 to 531080e Compare August 3, 2023 11:33

ellemouton added 5 commits August 3, 2023 20:57

go.mod: update go version to 1.19

921494f

go.mod+test: modernise the TestVariablePayloadOnion test

f30c0a7

Update the TestVariablePayloadOnion test to make use of the `require` package.

testdata+test: add pure tlv onion test

7f38495

Add a TestTLVPayloadOnion test that tests the newer spec test vector which tests the construction of an onion in the case where all the hops are using the TLV version payload.

cmd+go.mod: upgrade cli tool to use urfave lib

258028a

This commit updates the cli tool to use the urfave library. This makes things easier to read and will make it easier to extend the tool in future. A `genKeys` function is also added as a helper for quickly generating key pairs.

ellemouton force-pushed the routeBlinding branch from 531080e to 5a8aff7 Compare August 8, 2023 19:11

Roasbeef requested changes Aug 17, 2023

View reviewed changes

ellemouton force-pushed the routeBlinding branch from 5a8aff7 to e8adea9 Compare August 17, 2023 09:23

ellemouton added 3 commits August 17, 2023 11:28

sphinx: add route blinding helper methods to Router

a800c9f

In this commit, DecryptBlindHopData and NextEphemeral methods are added to Router which makes use of the Router's onion key in order to unblind blinded data and compute the next ephermal key respectively.

cmd: add blinded path helper commands

f971c75

Add a blinded-key option to the parse command so that it can be used to test parsing of an onion for hops in a blinded route. Also add a helper nextephemeral command.

ellemouton force-pushed the routeBlinding branch from e8adea9 to f971c75 Compare August 17, 2023 09:28

ellemouton commented Aug 17, 2023

View reviewed changes

ellemouton mentioned this pull request Aug 17, 2023

multi: update lightning-onion version lightningnetwork/lnd#7877

Merged

ellemouton requested a review from Roasbeef August 17, 2023 18:11

Roasbeef approved these changes Aug 23, 2023

View reviewed changes

Roasbeef merged commit 06182b1 into lightningnetwork:master Aug 23, 2023

ellemouton deleted the routeBlinding branch August 23, 2023 10:18

multi: Route blinding #57

multi: Route blinding #57

Conversation

ellemouton commented Jun 30, 2022

carlaKC left a comment

Choose a reason for hiding this comment

carlaKC commented Jul 5, 2022

sputn1ck left a comment

Choose a reason for hiding this comment

calvinrzachman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellemouton commented Aug 4, 2022

ellemouton commented Aug 4, 2022

lightninglabs-deploy commented Aug 11, 2022

ellemouton commented Aug 11, 2022

carlaKC left a comment

Choose a reason for hiding this comment

carlaKC commented Jan 25, 2023

ellemouton commented Jan 26, 2023

bitromortac left a comment

Choose a reason for hiding this comment

ziggie1984 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitromortac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment