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(autonat): reject inbound dial request from peer if its not connected #5597

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

Eligioo
Copy link
Contributor

@Eligioo Eligioo commented Sep 11, 2024

Description

As discovered and described in the issue below, there are situations where an incoming AutoNAT dial can come from a non-connected peer. However resolve_inbound_request expects that this situation cannot occur. This PR adds a check upfront and refuses the incoming dial when no connected peer is found.

Fixes #5570.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@Eligioo Eligioo changed the title AutoNAT V1: reject inbound dial request from peer if it is not connected fix(autonat v1): reject inbound dial request from peer if it is not connected Sep 11, 2024
@dariusc93 dariusc93 changed the title fix(autonat v1): reject inbound dial request from peer if it is not connected fix(autonat): reject inbound dial request from peer if it is not connected Sep 11, 2024
@dariusc93 dariusc93 changed the title fix(autonat): reject inbound dial request from peer if it is not connected fix(autonat): reject inbound dial request from peer if its not connected Sep 12, 2024
protocols/autonat/CHANGELOG.md Show resolved Hide resolved
protocols/autonat/src/v1/behaviour/as_server.rs Outdated Show resolved Hide resolved
protocols/autonat/src/v1/behaviour/as_server.rs Outdated Show resolved Hide resolved
@Eligioo
Copy link
Contributor Author

Eligioo commented Sep 13, 2024

@dariusc93 thanks for the review. Addressed the comments.

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

LGTM thanks! Will wait for Darius give it a look again

Copy link
Member

@dariusc93 dariusc93 left a comment

Choose a reason for hiding this comment

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

Can you also update the workspace Cargo.toml? Besides that everything LGTM :)

@dariusc93
Copy link
Member

Thanks @Eligioo!

@Eligioo
Copy link
Contributor Author

Eligioo commented Sep 13, 2024

@dariusc93 fixed the workspace Cargo.toml too.

Might be a small improvement to inform external contributors to also bump the various Cargo.toml files besides the changelog entry ;)

@dariusc93
Copy link
Member

@dariusc93 fixed the workspace Cargo.toml too.

Might be a small improvement to inform external contributors to also bump the various Cargo.toml files besides the changelog entry ;)

Thanks! We do try to if its needed and CI will inform us if they need to be updated as well :)

@jxs jxs added the send-it label Sep 13, 2024
@mergify mergify bot merged commit a2a2816 into libp2p:master Sep 13, 2024
70 of 72 checks passed
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
…ted (libp2p#5597)

## Description
As discovered and described in the issue below, there are situations
where an incoming AutoNAT dial can come from a non-connected peer.
However `resolve_inbound_request` expects that this situation cannot
occur. This PR adds a check upfront and refuses the incoming dial when
no connected peer is found.

Fixes libp2p#5570.
## Change checklist

- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] A changelog entry has been made in the appropriate crates

Co-authored-by: João Oliveira <[email protected]>
@Eligioo Eligioo deleted the fix/autonat-reject-peer-dial branch September 15, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AutoNAT V1 panic: Peer is connected
3 participants