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

implement connection gating support: intercept peer, address dials, upgraded conns #201

Merged
merged 13 commits into from
May 15, 2020

Conversation

aarshkshah1992
Copy link
Collaborator

@aarshkshah1992 aarshkshah1992 commented Apr 7, 2020

For libp2p/go-libp2p#872.

Core PR at libp2p/go-libp2p-core#139.
Upgrader PR at libp2p/go-libp2p-transport-upgrader#55.

This PR uses the connection lifecycle state based Connection Gating interface to gate connections.

swarm.go Outdated
if s.Filters.AddrBlocked(raddr) {
tc.Close()
return nil, ErrAddrFiltered
func (s *Swarm) denyConn(tc transport.CapableConn, dir network.Direction) (deny bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

given that there's only one caller of this function, and the way that the ConnGater is getting invoked is slighty different based on if we know the specific address or just the peer, i wonder if this function is useful, or if we should just directly check the ConnGater in swarm_listen, like we do when dialing.

Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

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

This looks good; my main gripe is that we're duplicating concerns across layers.

swarm.go Outdated
rejectConnection = true
}
case network.DirOutbound:
if !s.ConnGater.InterceptPeerAddrDial(p, tc.RemoteMultiaddr()) || !s.ConnGater.InterceptPeerDial(p) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we intercept this after the connection has been established? Can't we intercept it before we make the connection attempt (since we already know the target peer ID and the multiaddr we're about to dial)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has been removed.

swarm.go Outdated Show resolved Hide resolved
swarm_dial.go Outdated
@@ -409,7 +418,9 @@ func (s *Swarm) filterKnownUndialables(addrs []ma.Multiaddr) []ma.Multiaddr {
s.canDial,
// TODO: Consider allowing link-local addresses
addrutil.AddrOverNonLocalIP,
addrutil.FilterNeg(s.Filters.AddrBlocked),
func(addr ma.Multiaddr) bool {
return s.ConnGater == nil || s.ConnGater.InterceptPeerAddrDial(p, addr)
Copy link
Member

Choose a reason for hiding this comment

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

Why do this here and on addConn?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has been removed from addConn.

swarm_dial.go Outdated Show resolved Hide resolved
swarm_test.go Outdated Show resolved Hide resolved
@aarshkshah1992
Copy link
Collaborator Author

@raulk Have changed the imports for libp2p-core & libp2p-upgrader & have addressed your changes. Let me know what you think.

@raulk raulk changed the title Connection Gating in Swarm implement connection gating: intercept peer, address dials, upgraded outbound conns May 15, 2020
@raulk raulk changed the title implement connection gating: intercept peer, address dials, upgraded outbound conns implement connection gating: intercept peer, address dials, upgraded conns May 15, 2020
@raulk raulk changed the title implement connection gating: intercept peer, address dials, upgraded conns implement connection gating support: intercept peer, address dials, upgraded conns May 15, 2020
swarm.go Outdated Show resolved Hide resolved
@raulk raulk merged commit 34b2b47 into master May 15, 2020
@raulk raulk deleted the feat/connection-gating branch May 15, 2020 13:26
@Stebalien Stebalien mentioned this pull request May 26, 2020
77 tasks
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.

3 participants