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

autonat/README: Document DOS attack prevention #369

Merged
merged 2 commits into from
Oct 12, 2021

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Oct 6, 2021

Document that peers MUST NOT dial addresses that are not based on the IP
addresses the requesting node is observed as.

Corresponding logic in Golang implementation:

https://github.com/libp2p/go-libp2p-autonat/blob/1247ac6d9fa798e7032127878a6f3d0b9eb487c6/svc.go#L133-L147

(Pointed out by @marten-seemann.)

Document that peers MUST NOT dial addresses that are not based on the IP
addresses the requesting node is observed as.

Corresponding logic in Golang implementation:

https://github.com/libp2p/go-libp2p-autonat/blob/1247ac6d9fa798e7032127878a6f3d0b9eb487c6/svc.go#L133-L147
@@ -52,6 +52,23 @@ Upon receiving this message, the peer starts to dial these addresses. It MAY
dial all of them in parallel. The peer MAY use a different IP and peer ID than
it uses for its regular libp2p connection to perform these dial backs.

In order to prevent attacks like the one described below, implementations
MUST NOT dial any multiaddress unless it is based on the IP address the
requesting node is observed as.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
requesting node is observed as.
requesting node is observed as, as described in [RFC 3489, Section 12.1.1](https://www.rfc-editor.org/rfc/rfc3489#section-12.1.1):

Copy link
Member Author

Choose a reason for hiding this comment

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

I am a bit confused. Is the full citation of the paragraph + the link below not enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the confusion. I'd prefer to have the link above, not below. If you accept this suggestion, please remove the link below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 314ae0e with a slight rewording. Let me know what you think @marten-seemann.

@mxinden
Copy link
Member Author

mxinden commented Oct 12, 2021

Sorry for the force-push. I missed that the autonat branch is behind current master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants