From 983d7bafbededabcd4f05c0fe01e59962d2f92ec Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Tue, 4 Jun 2024 17:04:50 +0200 Subject: [PATCH] Remove unused variables from peer conn (#2074) Remove unused variables from peer conn --- client/internal/engine.go | 5 -- client/internal/peer/conn.go | 109 +++++++++++++++-------------------- signal/client/client.go | 17 ------ signal/client/client_test.go | 36 ------------ 4 files changed, 48 insertions(+), 119 deletions(-) diff --git a/client/internal/engine.go b/client/internal/engine.go index 83e6928f4bc..b0923571492 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -983,7 +983,6 @@ func (e *Engine) createPeerConn(pubKey string, allowedIPs string) (*peer.Conn, e WgConfig: wgConfig, LocalWgPort: e.config.WgPort, NATExternalIPs: e.parseNATExternalIPMappings(), - UserspaceBind: e.wgInterface.IsUserspaceBind(), RosenpassPubKey: e.getRosenpassPubKey(), RosenpassAddr: e.getRosenpassAddr(), } @@ -1046,8 +1045,6 @@ func (e *Engine) receiveSignalEvents() { return err } - conn.RegisterProtoSupportMeta(msg.Body.GetFeaturesSupported()) - var rosenpassPubKey []byte rosenpassAddr := "" if msg.GetBody().GetRosenpassConfig() != nil { @@ -1070,8 +1067,6 @@ func (e *Engine) receiveSignalEvents() { return err } - conn.RegisterProtoSupportMeta(msg.GetBody().GetFeaturesSupported()) - var rosenpassPubKey []byte rosenpassAddr := "" if msg.GetBody().GetRosenpassConfig() != nil { diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index 5f329bf7f12..c64c074a759 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -20,7 +20,6 @@ import ( "github.com/netbirdio/netbird/iface" "github.com/netbirdio/netbird/iface/bind" "github.com/netbirdio/netbird/route" - signal "github.com/netbirdio/netbird/signal/client" sProto "github.com/netbirdio/netbird/signal/proto" nbnet "github.com/netbirdio/netbird/util/net" "github.com/netbirdio/netbird/version" @@ -70,9 +69,6 @@ type ConnConfig struct { NATExternalIPs []string - // UsesBind indicates whether the WireGuard interface is userspace and uses bind.ICEBind - UserspaceBind bool - // RosenpassPubKey is this peer's Rosenpass public key RosenpassPubKey []byte // RosenpassPubKey is this peer's RosenpassAddr server address (IP:port) @@ -135,32 +131,15 @@ type Conn struct { wgProxyFactory *wgproxy.Factory wgProxy wgproxy.Proxy - remoteModeCh chan ModeMessage - meta meta - adapter iface.TunAdapter iFaceDiscover stdnet.ExternalIFaceDiscover sentExtraSrflx bool - remoteEndpoint *net.UDPAddr - remoteConn *ice.Conn - connID nbnet.ConnectionID beforeAddPeerHooks []BeforeAddPeerHookFunc afterRemovePeerHooks []AfterRemovePeerHookFunc } -// meta holds meta information about a connection -type meta struct { - protoSupport signal.FeaturesSupport -} - -// ModeMessage represents a connection mode chosen by the peer -type ModeMessage struct { - // Direct indicates that it decided to use a direct connection - Direct bool -} - // GetConf returns the connection config func (conn *Conn) GetConf() ConnConfig { return conn.config @@ -187,7 +166,6 @@ func NewConn(config ConnConfig, statusRecorder *Status, wgProxyFactory *wgproxy. remoteOffersCh: make(chan OfferAnswer), remoteAnswerCh: make(chan OfferAnswer), statusRecorder: statusRecorder, - remoteModeCh: make(chan ModeMessage, 1), wgProxyFactory: wgProxyFactory, adapter: adapter, iFaceDiscover: iFaceDiscover, @@ -378,8 +356,6 @@ func (conn *Conn) Open(ctx context.Context) error { remoteWgPort = remoteOfferAnswer.WgListenPort } - conn.remoteConn = remoteConn - // the ice connection has been established successfully so we are ready to start the proxy remoteAddr, err := conn.configureConnection(remoteConn, remoteWgPort, remoteOfferAnswer.RosenpassPubKey, remoteOfferAnswer.RosenpassAddr) @@ -437,7 +413,6 @@ func (conn *Conn) configureConnection(remoteConn net.Conn, remoteWgPort int, rem } endpointUdpAddr, _ := net.ResolveUDPAddr(endpoint.Network(), endpoint.String()) - conn.remoteEndpoint = endpointUdpAddr log.Debugf("Conn resolved IP for %s: %s", endpoint, endpointUdpAddr.IP) conn.connID = nbnet.GenerateConnID() @@ -623,40 +598,39 @@ func (conn *Conn) SetSendSignalMessage(handler func(message *sProto.Message) err // onICECandidate is a callback attached to an ICE Agent to receive new local connection candidates // and then signals them to the remote peer func (conn *Conn) onICECandidate(candidate ice.Candidate) { - if candidate != nil { - // TODO: reported port is incorrect for CandidateTypeHost, makes understanding ICE use via logs confusing as port is ignored - log.Debugf("discovered local candidate %s", candidate.String()) - go func() { - err := conn.signalCandidate(candidate) - if err != nil { - log.Errorf("failed signaling candidate to the remote peer %s %s", conn.config.Key, err) - } + // nil means candidate gathering has been ended + if candidate == nil { + return + } - // sends an extra server reflexive candidate to the remote peer with our related port (usually the wireguard port) - // this is useful when network has an existing port forwarding rule for the wireguard port and this peer - if !conn.sentExtraSrflx && candidate.Type() == ice.CandidateTypeServerReflexive && candidate.Port() != candidate.RelatedAddress().Port { - relatedAdd := candidate.RelatedAddress() - extraSrflx, err := ice.NewCandidateServerReflexive(&ice.CandidateServerReflexiveConfig{ - Network: candidate.NetworkType().String(), - Address: candidate.Address(), - Port: relatedAdd.Port, - Component: candidate.Component(), - RelAddr: relatedAdd.Address, - RelPort: relatedAdd.Port, - }) - if err != nil { - log.Errorf("failed creating extra server reflexive candidate %s", err) - return - } - err = conn.signalCandidate(extraSrflx) - if err != nil { - log.Errorf("failed signaling the extra server reflexive candidate to the remote peer %s: %s", conn.config.Key, err) - return - } - conn.sentExtraSrflx = true - } - }() + // TODO: reported port is incorrect for CandidateTypeHost, makes understanding ICE use via logs confusing as port is ignored + log.Debugf("discovered local candidate %s", candidate.String()) + go func() { + err := conn.signalCandidate(candidate) + if err != nil { + log.Errorf("failed signaling candidate to the remote peer %s %s", conn.config.Key, err) + } + }() + + if !conn.shouldSendExtraSrflxCandidate(candidate) { + return + } + + // sends an extra server reflexive candidate to the remote peer with our related port (usually the wireguard port) + // this is useful when network has an existing port forwarding rule for the wireguard port and this peer + extraSrflx, err := extraSrflxCandidate(candidate) + if err != nil { + log.Errorf("failed creating extra server reflexive candidate %s", err) + return } + conn.sentExtraSrflx = true + + go func() { + err = conn.signalCandidate(extraSrflx) + if err != nil { + log.Errorf("failed signaling the extra server reflexive candidate to the remote peer %s: %s", conn.config.Key, err) + } + }() } func (conn *Conn) onICESelectedCandidatePair(c1 ice.Candidate, c2 ice.Candidate) { @@ -807,10 +781,23 @@ func (conn *Conn) GetKey() string { return conn.config.Key } -// RegisterProtoSupportMeta register supported proto message in the connection metadata -func (conn *Conn) RegisterProtoSupportMeta(support []uint32) { - protoSupport := signal.ParseFeaturesSupported(support) - conn.meta.protoSupport = protoSupport +func (conn *Conn) shouldSendExtraSrflxCandidate(candidate ice.Candidate) bool { + if !conn.sentExtraSrflx && candidate.Type() == ice.CandidateTypeServerReflexive && candidate.Port() != candidate.RelatedAddress().Port { + return true + } + return false +} + +func extraSrflxCandidate(candidate ice.Candidate) (*ice.CandidateServerReflexive, error) { + relatedAdd := candidate.RelatedAddress() + return ice.NewCandidateServerReflexive(&ice.CandidateServerReflexiveConfig{ + Network: candidate.NetworkType().String(), + Address: candidate.Address(), + Port: relatedAdd.Port, + Component: candidate.Component(), + RelAddr: relatedAdd.Address, + RelPort: relatedAdd.Port, + }) } func candidateViaRoutes(candidate ice.Candidate, clientRoutes route.HAMap) bool { diff --git a/signal/client/client.go b/signal/client/client.go index e4d9d74b33e..9d99b367766 100644 --- a/signal/client/client.go +++ b/signal/client/client.go @@ -25,11 +25,6 @@ const ( DirectCheck uint32 = 1 ) -// FeaturesSupport register protocol supported features -type FeaturesSupport struct { - DirectCheck bool -} - type Client interface { io.Closer StreamConnected() bool @@ -79,15 +74,3 @@ type Credential struct { UFrag string Pwd string } - -// ParseFeaturesSupported parses a slice of supported features into FeaturesSupport -func ParseFeaturesSupported(featuresMessage []uint32) FeaturesSupport { - var protoSupport FeaturesSupport - for _, feature := range featuresMessage { - if feature == DirectCheck { - protoSupport.DirectCheck = true - return protoSupport - } - } - return protoSupport -} diff --git a/signal/client/client_test.go b/signal/client/client_test.go index 8dea535f2b9..3ad348b6fec 100644 --- a/signal/client/client_test.go +++ b/signal/client/client_test.go @@ -4,7 +4,6 @@ import ( "context" "net" "sync" - "testing" "time" . "github.com/onsi/ginkgo" @@ -168,41 +167,6 @@ var _ = Describe("GrpcClient", func() { }) -func TestParseFeaturesSupported(t *testing.T) { - expectedOnEmptyOrUnsupported := FeaturesSupport{DirectCheck: false} - expectedWithDirectCheck := FeaturesSupport{DirectCheck: true} - testCases := []struct { - name string - input []uint32 - expected FeaturesSupport - }{ - { - name: "Should Return DirectCheck Supported", - input: []uint32{DirectCheck}, - expected: expectedWithDirectCheck, - }, - { - name: "Should Return DirectCheck Unsupported When Nil", - input: nil, - expected: expectedOnEmptyOrUnsupported, - }, - { - name: "Should Return DirectCheck Unsupported When Not Known Feature", - input: []uint32{9999}, - expected: expectedOnEmptyOrUnsupported, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - result := ParseFeaturesSupported(testCase.input) - if result.DirectCheck != testCase.expected.DirectCheck { - t.Errorf("Direct check feature should match: Expected: %t, Got: %t", testCase.expected.DirectCheck, result.DirectCheck) - } - }) - } -} - func createSignalClient(addr string, key wgtypes.Key) *GrpcClient { var sigTLSEnabled = false client, err := NewClient(context.Background(), addr, key, sigTLSEnabled)