From 00db8c6359f950087827b6e7390c1a973b718f98 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Sat, 31 May 2025 04:56:54 +0800 Subject: [PATCH] lnd: move peer perms assignment into `peerConnected` When the callback is called in `scheduledPeerConnection`, it is referencing the old `access` variable which was created when the peer was first connected. However, if this peer opens a channel with us and goes offline, or another inbound connection is made from this peer, we may still use the old `access` value. To fix it, we need to make sure we always get the fresh perm by calling `assignPeerPerms` inside `peerConnected`. --- server.go | 68 ++++++++++++++++++++++--------------------------------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/server.go b/server.go index 5d12e3e2d..0aec863ee 100644 --- a/server.go +++ b/server.go @@ -4002,22 +4002,6 @@ func (s *server) InboundPeerConnected(conn net.Conn) { s.mu.Lock() defer s.mu.Unlock() - // If the remote node's public key is banned, drop the connection. - access, err := s.peerAccessMan.assignPeerPerms(nodePub) - if err != nil { - // Clean up the persistent peer maps if we're dropping this - // connection. - s.bannedPersistentPeerConnection(pubStr) - - srvrLog.Debugf("Dropping connection for %x since we are out "+ - "of restricted-access connection slots: %v.", pubSer, - err) - - conn.Close() - - return - } - // If we already have an outbound connection to this peer, then ignore // this new connection. if p, ok := s.outboundPeers[pubStr]; ok { @@ -4052,7 +4036,7 @@ func (s *server) InboundPeerConnected(conn net.Conn) { // We were unable to locate an existing connection with the // target peer, proceed to connect. s.cancelConnReqs(pubStr, nil) - s.peerConnected(conn, nil, true, access) + s.peerConnected(conn, nil, true) case nil: // We already have a connection with the incoming peer. If the @@ -4084,7 +4068,7 @@ func (s *server) InboundPeerConnected(conn net.Conn) { s.removePeer(connectedPeer) s.ignorePeerTermination[connectedPeer] = struct{}{} s.scheduledPeerConnection[pubStr] = func() { - s.peerConnected(conn, nil, true, access) + s.peerConnected(conn, nil, true) } } } @@ -4109,25 +4093,6 @@ func (s *server) OutboundPeerConnected(connReq *connmgr.ConnReq, conn net.Conn) s.mu.Lock() defer s.mu.Unlock() - access, err := s.peerAccessMan.assignPeerPerms(nodePub) - if err != nil { - // Clean up the persistent peer maps if we're dropping this - // connection. - s.bannedPersistentPeerConnection(pubStr) - - srvrLog.Debugf("Dropping connection for %x since we are out "+ - "of restricted-access connection slots: %v.", pubSer, - err) - - if connReq != nil { - s.connMgr.Remove(connReq.ID()) - } - - conn.Close() - - return - } - // If we already have an inbound connection to this peer, then ignore // this new connection. if p, ok := s.inboundPeers[pubStr]; ok { @@ -4162,7 +4127,7 @@ func (s *server) OutboundPeerConnected(connReq *connmgr.ConnReq, conn net.Conn) return } - srvrLog.Infof("Established connection to: %x@%v", pubStr, + srvrLog.Infof("Established outbound connection to: %x@%v", pubStr, conn.RemoteAddr()) if connReq != nil { @@ -4186,7 +4151,7 @@ func (s *server) OutboundPeerConnected(connReq *connmgr.ConnReq, conn net.Conn) case ErrPeerNotConnected: // We were unable to locate an existing connection with the // target peer, proceed to connect. - s.peerConnected(conn, connReq, false, access) + s.peerConnected(conn, connReq, false) case nil: // We already have a connection with the incoming peer. If the @@ -4220,7 +4185,7 @@ func (s *server) OutboundPeerConnected(connReq *connmgr.ConnReq, conn net.Conn) s.removePeer(connectedPeer) s.ignorePeerTermination[connectedPeer] = struct{}{} s.scheduledPeerConnection[pubStr] = func() { - s.peerConnected(conn, connReq, false, access) + s.peerConnected(conn, connReq, false) } } } @@ -4353,12 +4318,33 @@ func (s *server) notifyFundingTimeoutPeerEvent(op wire.OutPoint, // starting all the goroutines the peer needs to function properly. The inbound // boolean should be true if the peer initiated the connection to us. func (s *server) peerConnected(conn net.Conn, connReq *connmgr.ConnReq, - inbound bool, access peerAccessStatus) { + inbound bool) { brontideConn := conn.(*brontide.Conn) addr := conn.RemoteAddr() pubKey := brontideConn.RemotePub() + // If the remote node's public key is banned, drop the connection. + // + // TODO(yy): Consider perform this check in + // `peerAccessMan.addPeerAccess`. + access, err := s.peerAccessMan.assignPeerPerms(pubKey) + if err != nil { + pubSer := pubKey.SerializeCompressed() + + // Clean up the persistent peer maps if we're dropping this + // connection. + s.bannedPersistentPeerConnection(string(pubSer)) + + srvrLog.Debugf("Dropping connection for %x since we are out "+ + "of restricted-access connection slots: %v.", pubSer, + err) + + conn.Close() + + return + } + srvrLog.Infof("Finalizing connection to %x@%s, inbound=%v", pubKey.SerializeCompressed(), addr, inbound)