Skip to content
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

fix(iroh-net): prevent adding addressing info that points back to us #2333

Merged
merged 15 commits into from
Jun 18, 2024

Conversation

divagant-martian
Copy link
Contributor

@divagant-martian divagant-martian commented May 28, 2024

Description

Failed disco boxes for which the sender is our own node is are because we tried to contact another node for which the addressing information we have points back to us. We are the sender (this explains the source of the disco box being ourselves) but we are not the meant recipient (thus the failed decryption - we should always be able to decrypt a message meant to ourselves - ).

This is of course undesirable. This Pr should mostly mitigate this issue, but a failed disco box to ourselves can happen in other less likely ways.

Breaking Changes

n/a

Notes & open questions

n/a

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@divagant-martian divagant-martian added this to the v0.18.0 milestone May 28, 2024
@divagant-martian divagant-martian self-assigned this May 28, 2024
iroh-net/src/endpoint.rs Outdated Show resolved Hide resolved
@dignifiedquire
Copy link
Contributor

@divagant-martian what's missing from this to be ready?

@dignifiedquire dignifiedquire modified the milestones: v0.18.0, v0.19.0 Jun 7, 2024
@divagant-martian divagant-martian marked this pull request as ready for review June 11, 2024 15:34
@divagant-martian divagant-martian linked an issue Jun 12, 2024 that may be closed by this pull request
Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I have mostly minor nits, now that I care more about public api docs :)

iroh-net/src/endpoint.rs Outdated Show resolved Hide resolved
iroh-net/src/endpoint.rs Show resolved Hide resolved
pub fn add_node_addr(&self, addr: NodeAddr) {
self.node_map.add_node_addr(addr);
pub fn add_node_addr(&self, mut addr: NodeAddr, source: node_map::Source) -> Result<()> {
let my_addresses = self.endpoints.get().last_endpoints;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you felt like renaming self.endpoints here to self.direct_addresses or something i would not object... :) But I can also do this separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it out. It feels out of scope since I'm not changing the field, simply using it here

iroh-net/src/magicsock.rs Outdated Show resolved Hide resolved
iroh-net/src/magicsock.rs Outdated Show resolved Hide resolved
iroh-net/src/magicsock.rs Outdated Show resolved Hide resolved
iroh-net/src/endpoint.rs Show resolved Hide resolved
iroh-net/src/magicsock/node_map.rs Outdated Show resolved Hide resolved
iroh-net/src/magicsock/node_map.rs Outdated Show resolved Hide resolved
iroh-net/src/magicsock/node_map/node_state.rs Outdated Show resolved Hide resolved
@divagant-martian divagant-martian added this pull request to the merge queue Jun 18, 2024
Merged via the queue into n0-computer:main with commit b2e8557 Jun 18, 2024
25 checks passed
ppodolsky pushed a commit to izihawa/iroh that referenced this pull request Jun 22, 2024
…0-computer#2333)

## Description

Failed disco boxes for which the sender is our own node is are because
we tried to contact another node for which the addressing information we
have points back to us. We are the sender (this explains the source of
the disco box being ourselves) but we are not the meant recipient (thus
the failed decryption - we should always be able to decrypt a message
meant to ourselves - ).

This is of course undesirable. This Pr should mostly mitigate this
issue, but a failed disco box to ourselves can happen in other less
likely ways.

## Breaking Changes

n/a

## Notes & open questions

n/a

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [ ] ~Tests if relevant.~
- [ ] ~All breaking changes documented.~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Too many failed to open disco box in logs
3 participants