Check Address and LocalAddress instead of RouterId

osrg · Oct 15, 2023 · 419c50d · 419c50d
1 parent 16a9c95
commit 419c50d
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 11 deletions.
diff --git a/pkg/packet/bgp/validate.go b/pkg/packet/bgp/validate.go
@@ -9,7 +9,7 @@ import (
 )
 
 // Validator for BGPUpdate
-func ValidateUpdateMsg(m *BGPUpdate, rfs map[RouteFamily]BGPAddPathMode, isEBGP bool, isConfed bool, loopbackAllowed bool) (bool, error) {
+func ValidateUpdateMsg(m *BGPUpdate, rfs map[RouteFamily]BGPAddPathMode, isEBGP bool, isConfed bool, loopbackNextHopAllowed bool) (bool, error) {
 	var strongestError error
 
 	eCode := uint8(BGP_ERROR_UPDATE_MESSAGE_ERROR)
@@ -31,7 +31,7 @@ func ValidateUpdateMsg(m *BGPUpdate, rfs map[RouteFamily]BGPAddPathMode, isEBGP
 			seen[a.GetType()] = a
 			newAttrs = append(newAttrs, a)
 			//check specific path attribute
-			ok, err := ValidateAttribute(a, rfs, isEBGP, isConfed, loopbackAllowed)
+			ok, err := ValidateAttribute(a, rfs, isEBGP, isConfed, loopbackNextHopAllowed)
 			if !ok {
 				msgErr := err.(*MessageError)
 				if msgErr.ErrorHandling == ERROR_HANDLING_SESSION_RESET {
@@ -81,7 +81,7 @@ func ValidateUpdateMsg(m *BGPUpdate, rfs map[RouteFamily]BGPAddPathMode, isEBGP
 	return strongestError == nil, strongestError
 }
 
-func ValidateAttribute(a PathAttributeInterface, rfs map[RouteFamily]BGPAddPathMode, isEBGP bool, isConfed bool, loopbackAllowed bool) (bool, error) {
+func ValidateAttribute(a PathAttributeInterface, rfs map[RouteFamily]BGPAddPathMode, isEBGP bool, isConfed bool, loopbackNextHopAllowed bool) (bool, error) {
 	var strongestError error
 
 	eCode := uint8(BGP_ERROR_UPDATE_MESSAGE_ERROR)
@@ -169,7 +169,7 @@ func ValidateAttribute(a PathAttributeInterface, rfs map[RouteFamily]BGPAddPathM
 		}
 
 		//check IP address represents host address
-		if (!loopbackAllowed && p.Value.IsLoopback()) || isZero(p.Value) || isClassDorE(p.Value) {
+		if (!loopbackNextHopAllowed && p.Value.IsLoopback()) || isZero(p.Value) || isClassDorE(p.Value) {
 			eMsg := "invalid nexthop address"
 			data, _ := a.Serialize()
 			e := NewMessageErrorWithErrorHandling(eCode, eSubCodeBadNextHop, data, getErrorHandlingFromPathAttribute(p.GetType()), nil, eMsg)

diff --git a/pkg/server/fsm.go b/pkg/server/fsm.go
@@ -21,7 +21,6 @@ import (
 	"io"
 	"math/rand"
 	"net"
-	"net/netip"
 	"os"
 	"strconv"
 	"sync"
@@ -1076,13 +1075,12 @@ func (h *fsmHandler) recvMessageWithError() (*fsmMsg, error) {
 				rfMap := h.fsm.rfMap
 				h.fsm.lock.RUnlock()
 
-				// Allow updates from loopback addresses if the GoBGP instance
-				// itself is assigned to 127.0.0.0/8, since this can happen when
-				// testing, where multiple GoBGP instances might be created within
-				// 127.0.0.0/8.
+				// Allow updates from host loopback addresses if the BGP connection
+				// with the neighbour is both dialed and received on loopback
+				// addresses.
 				var allowLoopback bool
-				if routerIDAddr, err := netip.ParseAddr(h.fsm.gConf.Config.RouterId); err == nil && routerIDAddr.Is4() {
-					allowLoopback = routerIDAddr.IsLoopback()
+				if localAddr, peerAddr := h.fsm.peerInfo.LocalAddress, h.fsm.peerInfo.Address; localAddr.To4() != nil && peerAddr.To4() != nil {
+					allowLoopback = localAddr.IsLoopback() && peerAddr.IsLoopback()
 				}
 				ok, err := bgp.ValidateUpdateMsg(body, rfMap, isEBGP, isConfed, allowLoopback)
 				if !ok {