-
Notifications
You must be signed in to change notification settings - Fork 31
Fix SURB: only hand out the innermost payload key, not every hop's #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -367,11 +367,31 @@ impl BuiltHeader { | |
| .collect() | ||
| } | ||
|
|
||
| pub(crate) fn payload_key_seeds(&self) -> Vec<PayloadKeySeed> { | ||
| /// Returns the key used to add the innermost layer of payload encryption, | ||
| /// i.e. the layer belonging to the last hop of the route. | ||
| /// | ||
| /// This is the only payload key material a SURB may hand out to whoever uses it: the | ||
| /// remaining layers get added by each hop processing, and | ||
| /// only the SURB's original creator - who retains every derived key locally - can remove | ||
| /// them all again. Handing out every layer's key would let the SURB user precompute every | ||
| /// intermediate ciphertext and, by colluding with the last hop, deanonymize the reply route. | ||
| pub(crate) fn legacy_first_layer_payload_key(&self) -> PayloadKey { | ||
| *self.last_hop_secret().legacy_payload_key() | ||
| } | ||
|
|
||
| /// Seed variant of [`Self::legacy_first_layer_payload_key`]. | ||
| pub(crate) fn first_layer_payload_key_seed(&self) -> PayloadKeySeed { | ||
| *self.last_hop_secret().payload_key_seed() | ||
| } | ||
|
|
||
| #[allow(clippy::expect_used)] | ||
| fn last_hop_secret(&self) -> &ExpandedSharedSecret { | ||
| // `build_header` already panics on an empty route (slicing `expanded_shared_secrets` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's quite a bad justification from Claude. it panics elsewhere so panic here is fine. ideally we shouldn't allow panics anywhere by guarding against degenerative cases |
||
| // by `route.len() - 1` to build the filler), so a `BuiltHeader` is never constructed | ||
| // with an empty `expanded_secrets` in the first place | ||
| self.expanded_secrets | ||
| .iter() | ||
| .map(|s| *s.payload_key_seed()) | ||
| .collect() | ||
| .last() | ||
| .expect("BuiltHeader is always constructed with a non-empty route") | ||
| } | ||
|
|
||
| pub(crate) fn into_header(self) -> SphinxHeader { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,7 +108,14 @@ impl Payload { | |
| } | ||
|
|
||
| /// Tries to add an additional layer of encryption onto self. | ||
| fn add_encryption_layer<P: Borrow<PayloadKey>>(mut self, payload_key: P) -> Result<Self> { | ||
| /// | ||
| /// Besides being used internally by [`Self::encapsulate_message`], this is also what lets a | ||
| /// SURB's original creator undo the layers each mix node added to a SURB reply's payload as | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it though? wouldn't the SURB's original creator need to call |
||
| /// it transited the network: since every hop always *removes* a layer with its own key | ||
| /// regardless of whether the packet is a forward packet or a SURB reply, re-*adding* those | ||
| /// same layers (in the same, per-hop order) with the independently re-derived hop keys | ||
| /// exactly reverses that transit-time processing. | ||
| pub fn add_encryption_layer<P: Borrow<PayloadKey>>(mut self, payload_key: P) -> Result<Self> { | ||
| let lioness_cipher = NymLioness::new(payload_key.borrow().into()); | ||
|
|
||
| if let Err(err) = lioness_cipher.encrypt_block(&mut self.0) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it the case that if we use the seed variant (i.e. the "legacy" mode), we need all seeds for compatibility?