SonarQube Vulnerability Critical Fixes

conf/parser.py

@@ -179,7 +179,10 @@ def parse(self, ifaces):
                     "/tmp/notifycp"
                 )
             )
+        self._parse_post_unix_config()
 
+    def _parse_post_unix_config(self):
+        """Parses Unix socket paths, feature flags, table sizes, and monitoring config."""
         # UnixPort Paths
         try:
             self.endmarker_sockaddr = self.conf["endmarker_sockaddr"]

conf/ports.py

@@ -409,4 +409,4 @@ def setup_port(
         merge.connect(update)
 
         if self.mode == "sim":
-            self.rtr = merge
+            self.rtr = merge

conf/test_route_control.py

@@ -12,7 +12,7 @@
                                 fetch_mac, mac_to_hex, mac_to_int,
                                 validate_ipv4)
 
-
+DEFAULT_MAC_ADDRESS  = "00:1a:2b:3c:4d:5e"
 class BessControllerMock(object):
     """Mock of BessController to avoid using BESS from pybess.bess"""
 
@@ -71,7 +71,7 @@ def test_given_invalid_ip_when_validate_ipv6_then_returns_false(self):
         self.assertFalse(validate_ipv4(""))
 
     def test_given_valid_mac_when_mac_to_int_then_returns_int_representation(self):
-        self.assertEqual(mac_to_int("00:1a:2b:3c:4d:5e"), 112394521950)
+        self.assertEqual(mac_to_int(DEFAULT_MAC_ADDRESS ), 112394521950)
 
     def test_given_invalid_mac_when_mac_to_int_then_raises_exception(self):
         with self.assertRaises(ValueError):
@@ -80,17 +80,17 @@ def test_given_invalid_mac_when_mac_to_int_then_raises_exception(self):
     def test_given_valid_mac_when_mac_to_hex_then_return_hex_string_representation(
         self,
     ):
-        self.assertEqual(mac_to_hex("00:1a:2b:3c:4d:5e"), "001A2B3C4D5E")
+        self.assertEqual(mac_to_hex(DEFAULT_MAC_ADDRESS ), "001A2B3C4D5E")
 
     def test_given_known_destination_when_fetch_mac_then_returns_mac(self):
         ndb = Mock()
         kwargs = {
             "ifindex": 1,
             "dst": "192.168.1.1",
-            "lladdr": "00:1a:2b:3c:4d:5e"
+            "lladdr": DEFAULT_MAC_ADDRESS 
         }
         ndb.neighbours.dump.return_value = [kwargs]
-        self.assertEqual(fetch_mac(ndb, "192.168.1.1"), "00:1a:2b:3c:4d:5e")
+        self.assertEqual(fetch_mac(ndb, "192.168.1.1"), DEFAULT_MAC_ADDRESS)
 
     def test_given_unknown_destination_when_fetch_mac_then_returns_none(self):
         ndb = Mock()
@@ -132,13 +132,13 @@ def add_route_entry(
         kwargs = {
             "ifindex": 1,
             "dst": "192.168.1.1",
-            "lladdr": "00:1a:2b:3c:4d:5e"
+            "lladdr": DEFAULT_MAC_ADDRESS 
         }
         self.ndb.neighbours.dump.return_value = [kwargs]
         mock_get_update_module_name.return_value = "merge_module"
         mock_get_route_module_name.return_value = "route_module"
         mock_get_merge_module_name.return_value = "update_module"
-        mock_fetch_mac.return_value = "00:1a:2b:3c:4d:5e"
+        mock_fetch_mac.return_value = DEFAULT_MAC_ADDRESS 
         self.route_controller.add_new_route_entry(route_entry)
         return route_entry
 
@@ -257,7 +257,7 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_then_route
         kwargs = {
             "ifindex": 1,
             "dst": "192.168.1.1",
-            "lladdr": "00:1a:2b:3c:4d:5e"
+            "lladdr": DEFAULT_MAC_ADDRESS 
         }
         self.ndb.neighbours.dump.return_value = [kwargs]
         mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}]
@@ -279,7 +279,7 @@ def test_given_valid_new_route_when_add_new_route_entry_and_mac_known_and_neighb
         kwargs = {
             "ifindex": 1,
             "dst": "1.2.3.4",
-            "lladdr": "00:1a:2b:3c:4d:5e"
+            "lladdr": DEFAULT_MAC_ADDRESS 
         }
         self.ndb.neighbours.dump.return_value = [kwargs]
         mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}]
@@ -481,13 +481,13 @@ def test_given_new_neighbor_in_unresolved_when_add_unresolved_new_neighbor_then_
         kwargs = {
             "ifindex": 1,
             "dst": "192.168.1.1",
-            "lladdr": "00:1a:2b:3c:4d:5e"
+            "lladdr": DEFAULT_MAC_ADDRESS 
         }
         self.ndb.neighbours.dump.return_value = [kwargs]
         mock_netlink_msg = {
             "attrs": {
                 "NDA_DST": "1.2.3.4",
-                "NDA_LLADDR": "00:1a:2b:3c:4d:5e",
+                "NDA_LLADDR": DEFAULT_MAC_ADDRESS ,
             }
         }
         mock_routes = [{"event": "RTM_NEWROUTE"}, {"event": "OTHER_ACTION"}]

pfcpiface/config.go

@@ -4,16 +4,15 @@
 package pfcpiface
 
 import (
-	"github.com/omec-project/upf-epc/internal/p4constants"
-	"go.uber.org/zap"
-	"go.uber.org/zap/zapcore"
-
-	"net"
-	"time"
-
 	"encoding/json"
+	"net"
 	"os"
 	"regexp"
+	"time"
+
+	"github.com/omec-project/upf-epc/internal/p4constants"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 )
 
 const (
@@ -148,6 +147,18 @@ func validateConf(conf Conf) error {
 		}
 	}
 
+	if err := validateUEIPPoolAndPeers(conf); err != nil {
+		return err
+	}
+
+	if err := validateTimeouts(conf); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateUEIPPoolAndPeers(conf Conf) error {
 	if conf.CPIface.EnableUeIPAlloc {
 		for _, dnn := range conf.CPIface.DnnList {
 			_, _, err := net.ParseCIDR(dnn.UEIPPool)
@@ -163,7 +174,10 @@ func validateConf(conf Conf) error {
 			return ErrInvalidArgumentWithReason("conf.CPIface.Peers", peer, "invalid IP")
 		}
 	}
+	return nil
+}
 
+func validateTimeouts(conf Conf) error {
 	if _, err := time.ParseDuration(conf.RespTimeout); err != nil {
 		return ErrInvalidArgumentWithReason("conf.RespTimeout", conf.RespTimeout, "invalid duration")
 	}

pfcpiface/conn.go

@@ -122,7 +122,7 @@ func (node *PFCPNode) NewPFCPConn(lAddr, rAddr string, buf []byte) *PFCPConn {
 
 	rng := rand.New(rand.NewSource(time.Now().UnixNano())) // #nosec G404
 
-	var p = &PFCPConn{
+	p := &PFCPConn{
 		ctx:            node.ctx,
 		Conn:           conn,
 		ts:             ts,
@@ -213,7 +213,10 @@ func (pConn *PFCPConn) Serve() {
 	}(connTimeout)
 
 	// TODO: Sender goroutine
+	pConn.waitForShutdown(connTimeout)
+}
 
+func (pConn *PFCPConn) waitForShutdown(connTimeout chan struct{}) {
 	for {
 		select {
 		case <-connTimeout:

pfcpiface/ip_pool_test.go

@@ -4,22 +4,24 @@
 package pfcpiface
 
 import (
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
 	"math"
 	"net"
 	"sync"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+const ipSubnetCIDR = "10.0.0.0/24"
+
 func TestNewIPPool(t *testing.T) {
 	tests := []struct {
 		name       string
 		poolSubnet string
 		wantErr    bool
 	}{
-		{name: "normal pool", poolSubnet: "10.0.0.0/24", wantErr: false},
+		{name: "normal pool", poolSubnet: ipSubnetCIDR, wantErr: false},
 		{name: "smallest allowed pool", poolSubnet: "10.0.0.0/30", wantErr: false},
 		{name: "IPv6 pool", poolSubnet: "2001::/124", wantErr: false},
 		{name: "too small pool", poolSubnet: "10.0.0.0/32", wantErr: true},
@@ -53,7 +55,7 @@ func TestIPPool_LookupOrAllocIP(t *testing.T) {
 	})
 
 	t.Run("repeated SEID lookups return same IP", func(t *testing.T) {
-		const poolSubnet = "10.0.0.0/24"
+		const poolSubnet = ipSubnetCIDR
 		const seid = 1234
 		pool, err := NewIPPool(poolSubnet)
 		require.NoError(t, err)
@@ -66,7 +68,7 @@ func TestIPPool_LookupOrAllocIP(t *testing.T) {
 	})
 
 	t.Run("full subnet allocation", func(t *testing.T) {
-		const poolSubnet = "10.0.0.0/24"
+		const poolSubnet = ipSubnetCIDR
 		const usableAddresses = 256 - 2 // Account for network and broadcast addresses
 		const baseSeid = 1000
 		_, ipnet, err := net.ParseCIDR(poolSubnet)
@@ -118,7 +120,7 @@ func TestIPPool_LookupOrAllocIP(t *testing.T) {
 
 func TestIPPool_DeallocIP(t *testing.T) {
 	t.Run("plain alloc into dealloc", func(t *testing.T) {
-		const poolSubnet = "10.0.0.0/24"
+		const poolSubnet = ipSubnetCIDR
 		const seid = 1234
 		pool, err := NewIPPool(poolSubnet)
 		require.NoError(t, err)
@@ -129,7 +131,7 @@ func TestIPPool_DeallocIP(t *testing.T) {
 	})
 
 	t.Run("dealloc non-existent SEIDs fails", func(t *testing.T) {
-		pool, err := NewIPPool("10.0.0.0/24")
+		pool, err := NewIPPool(ipSubnetCIDR)
 		require.NoError(t, err)
 		err = pool.DeallocIP(1234)
 		assert.Error(t, err)

pfcpiface/messages_conn.go

@@ -12,9 +12,13 @@ import (
 	"github.com/wmnsk/go-pfcp/message"
 )
 
-var errFlowDescAbsent = errors.New("flow description not present")
-var errDatapathDown = errors.New("datapath down")
-var errReqRejected = errors.New("request rejected")
+const msgAssociationSetupResponseFrom = "association Setup Response from"
+
+var (
+	errFlowDescAbsent = errors.New("flow description not present")
+	errDatapathDown   = errors.New("datapath down")
+	errReqRejected    = errors.New("request rejected")
+)
 
 func (pConn *PFCPConn) sendAssociationRequest() {
 	// Build request message
@@ -189,7 +193,7 @@ func (pConn *PFCPConn) handleAssociationSetupResponse(msg message.Message) error
 	}
 
 	if cause != ie.CauseRequestAccepted {
-		logger.PfcpLog.Errorln("association Setup Response from", addr,
+		logger.PfcpLog.Errorln(msgAssociationSetupResponseFrom, addr,
 			"with Cause:", cause)
 		return errReqRejected
 	}
@@ -206,12 +210,12 @@ func (pConn *PFCPConn) handleAssociationSetupResponse(msg message.Message) error
 
 	if pConn.ts.remote.IsZero() {
 		pConn.ts.remote = ts
-		logger.PfcpLog.Infoln("association Setup Response from", addr,
+		logger.PfcpLog.Infoln(msgAssociationSetupResponseFrom, addr,
 			"with recovery timestamp:", ts)
 	} else if ts.After(pConn.ts.remote) {
 		old := pConn.ts.remote
 		pConn.ts.remote = ts
-		logger.PfcpLog.Warnln("association Setup Response from", addr,
+		logger.PfcpLog.Warnln(msgAssociationSetupResponseFrom, addr,
 			"with newer recovery timestamp:", ts, "older:", old)
 	}
 

pfcpiface/node.go

@@ -15,6 +15,8 @@ import (
 	"github.com/omec-project/upf-epc/pfcpiface/metrics"
 )
 
+const msgRemovedConnectionTo = "removed connection to"
+
 // PFCPNode represents a PFCP endpoint of the UPF.
 type PFCPNode struct {
 	ctx    context.Context
@@ -126,7 +128,7 @@ func (node *PFCPNode) Serve() {
 			})
 		case rAddr := <-node.pConnDone:
 			node.pConns.Delete(rAddr)
-			logger.PfcpLog.Infoln("removed connection to", rAddr)
+			logger.PfcpLog.Infoln(msgRemovedConnectionTo, rAddr)
 		case <-node.ctx.Done():
 			shutdown = true
 
@@ -148,7 +150,7 @@ func (node *PFCPNode) Serve() {
 							break clearLoop
 						}
 						node.pConns.Delete(rAddr)
-						logger.PfcpLog.Infoln("removed connection to", rAddr)
+						logger.PfcpLog.Infoln(msgRemovedConnectionTo, rAddr)
 					}
 				default:
 					// nothing to read from channel
@@ -159,7 +161,7 @@ func (node *PFCPNode) Serve() {
 			if len(node.pConnDone) > 0 {
 				for rAddr := range node.pConnDone {
 					node.pConns.Delete(rAddr)
-					logger.PfcpLog.Infoln("removed connection to", rAddr)
+					logger.PfcpLog.Infoln(msgRemovedConnectionTo, rAddr)
 				}
 			}