Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

add a method to perform a peer ID comparison on inbound connections #82

Closed
wants to merge 1 commit into from

Conversation

marten-seemann
Copy link
Contributor

We need this to check that we're connected to the expected peer when the underlying connection resulted from a TCP simultaneous open.

We need this to check that we're connected to the expected peer when the
underlying connection resulted from a TCP simultaneous open.
// full libp2p-transport connection.
// It verifies that the remote peer ID matches p.
// This is needed when upgrading a connection that resulted from a TCP simultaneous connect.
func (u *Upgrader) UpgradeInboundWithPeerCheck(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't do anything. Why do we need it?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. Is this really necessary? We can just as easily check after the fact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can do it either way. I don’t have a strong opinion on this, I just thought it would be nice to abort as early as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, how about exposing upgrade as Upgrade and deprecating the others?

NOTE: the current patch won't work because setupSecurity assumes that "no peer id" means "inbound".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. If we want to kill the connection on the TLS / noise level, this needs to be wired through the SecureTransport / SecureMuxer interfaces.

@marten-seemann
Copy link
Contributor Author

Closing this PR because it is broken, and doing this right will require a much larger change (see libp2p/go-libp2p-core#211 as a starting point).

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

Successfully merging this pull request may close these issues.

2 participants