Blinded Paths are probe-able

[TheBlueMatt]

See #3085 (comment) We probably need to get this done ASAP in the next release since this will invalidate all existing offers.

[jkczyz]

We discussed offline possible including a 16-byte nonce in each BlindedPath and using this when deriving Offer::signing_pubkey. However, we already include an encrypted nonce in Offer::metadata. If we include the same nonce in each BlindedPath, we could simply check that this nonce matches the nonce in the metadata (after decrypting it) when handling an InvoiceRequest. @TheBlueMatt Is this sufficient?

[TheBlueMatt]

Isn't the metadata in Offer visible to senders? They'd be able to use that to build a blinded path to candidate nodes and probe still, I think.

[jkczyz]

Oh, actually the Nonce isn't encrypted. I was confusing that with the PaymentId in the payer metadata. But we use the nonce when encrypting that.

[jkczyz]

I had been thinking if it were encrypted with the ExpandedKey then we could check against it... but we'd still need some nonce for doing the actual encryption, so I guess that wouldn't work.

[TheBlueMatt]

Is there some world where we could drop the metadata from Offer and use the one in the blinded path instead?

[jkczyz]

We use the one in the metadata to verify the offer, and computing it requires knowing all the fields (including the paths) in advance. So we'd need to exclude the paths from verification and only construct them when calling OfferBuilder::build, which would entail having the user pass a MessageRouter or not expose the builder at all in the ChannelManager usage.

Hmm... might also be a problem for offers with no amount and description since no other fields would be set? Though maybe having the unique nonce is sufficient?

There's also the case where no paths are given (i.e., deriving metadata for verification and signing using the node id), so we'd need to conditionally set the metadata then.

[TheBlueMatt]

I was thinking basically we'd put a nonce in the blinded path (and in the offer if there's no BP, I guess), then do basically H(offer || nonce) as the key derivation secret.

[jkczyz]

Ah, so IIUC we are essentially doing H(offer || nonce) already:

rust-lightning/lightning/src/offers/signer.rs

Line 174 in 87fc324

hmac: expanded_key.hmac_for_offer(nonce, iv_bytes),

And deriving the secret after including the offer TLVs in the HMAC:

rust-lightning/lightning/src/offers/signer.rs

Lines 112 to 117 in 87fc324

	Metadata::DerivedSigningPubkey(mut metadata_material) => {
	tlv_stream.write(&mut metadata_material.hmac).unwrap();
	let secp_ctx = secp_ctx.unwrap();
	let (metadata, keys) = metadata_material.derive_metadata_and_keys(secp_ctx);
	(Metadata::Bytes(metadata), Some(keys))
	},

rust-lightning/lightning/src/offers/signer.rs

Lines 198 to 200 in 87fc324

	let hmac = Hmac::from_engine(self.hmac);
	let privkey = SecretKey::from_slice(hmac.as_byte_array()).unwrap();
	let keys = Keypair::from_secret_key(secp_ctx, &privkey);

So we'd simply keep the metadata empty and use the same nonce in the blinded paths. When handling a request, we'd pass the received nonce to InvoiceRequest::verify instead of using the now-empty metadata. Do I have that right?

[TheBlueMatt]

I think that works, yea. Its even backwards compatible (if we want to bother) because we can keep accepting our old offers by accepting the metadata but we wont generate new offers with it.