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

feat(iroh)!: Remove access to local and remote IP addresses #3148

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

flub
Copy link
Contributor

@flub flub commented Jan 23, 2025

Description

The Connecting and Connection structs exposed IP addresses, these are
probably the NodeIdMappedAddresses rather than real ones. We address
based on NodeId, so let's remove this stuff.

Breaking Changes

iroh

  • Connecting::local_ip is removed.
  • Connecting::remote_address is removed.
  • Connection::local_ip is removed.
  • Connection::remote_address is removed.

Notes & open questions

I didn't think long about this. Getting this PR out so someone else
can think carefully about this. :)

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

The Connecting and Connection structs exposed IP addresses, these are
probably the NodeIdMappedAddresses rather than real ones.  We address
based on NodeId, so let's remove this stuff.
Copy link

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3148/docs/iroh/

Last updated: 2025-01-23T14:59:07Z

Copy link

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 23f3b87

@flub flub changed the title feat(iroh): Remove access to local and remote IP addresses feat(iroh)!: Remove access to local and remote IP addresses Jan 23, 2025
Copy link
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

Agreed we should remove the current version.

At some point it might be nice to add a version back that allows denylisting incoming connections by IP addr, if they misbehave often enough.
For that, we'd need the non-mapped addresses though. And also would need more design (unsure about how to realize this with relays).

@flub
Copy link
Contributor Author

flub commented Jan 24, 2025

Agreed we should remove the current version.

At some point it might be nice to add a version back that allows denylisting incoming connections by IP addr, if they misbehave often enough. For that, we'd need the non-mapped addresses though. And also would need more design (unsure about how to realize this with relays).

Yes, note that Incoming is the first struct you get on the accept side and this still has the local and remote IP methods. Though they are probably also the wrong IPs and the thing does need more design.

@flub flub added this pull request to the merge queue Jan 24, 2025
Merged via the queue into main with commit 08bd2a1 Jan 24, 2025
26 of 27 checks passed
@flub flub deleted the flub/no-pub-ip-addrs branch January 24, 2025 09:06
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.

2 participants