service: add extra validation for global services

[ upstream commit fc969f6 ]

Add a validation function to prevent unmarshalling invalid
global service objects retrieved from the kvstore, which
could then possibly trigger unexpected behavior. Currently,
we check that the cluster ID is in the valid range, and
that IP addresses are valid.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>

pkg/service/store/store.go

@@ -5,13 +5,16 @@ package store
 
 import (
 	"encoding/json"
+	"net/netip"
 	"path"
 
+	cmtypes "github.com/cilium/cilium/pkg/clustermesh/types"
 	"github.com/cilium/cilium/pkg/kvstore"
 	"github.com/cilium/cilium/pkg/kvstore/store"
 	"github.com/cilium/cilium/pkg/loadbalancer"
 	"github.com/cilium/cilium/pkg/lock"
 	"github.com/cilium/cilium/pkg/logging/logfields"
+	"github.com/cilium/cilium/pkg/option"
 )
 
 var (
@@ -111,11 +114,39 @@ func (s *ClusterService) Unmarshal(_ string, data []byte) error {
 		return err
 	}
 
+	if err := newService.validate(); err != nil {
+		return err
+	}
+
 	*s = newService
 
 	return nil
 }
 
+func (s *ClusterService) validate() error {
+	// Skip the ClusterID check if it matches the local one, as we assume that
+	// it has already been validated, and to allow it to be zero.
+	if s.ClusterID != option.Config.ClusterID {
+		if err := cmtypes.ValidateClusterID(s.ClusterID); err != nil {
+			return err
+		}
+	}
+
+	for address := range s.Frontends {
+		if _, err := netip.ParseAddr(address); err != nil {
+			return err
+		}
+	}
+
+	for address := range s.Backends {
+		if _, err := netip.ParseAddr(address); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // NewClusterService returns a new cluster service definition
 func NewClusterService(name, namespace string) ClusterService {
 	return ClusterService{

pkg/service/store/store_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	check "github.com/cilium/checkmate"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/cilium/cilium/pkg/checker"
 	"github.com/cilium/cilium/pkg/loadbalancer"
@@ -100,3 +101,47 @@ func (s *ServiceGenericSuite) TestPortConfigurationDeepEqual(c *check.C) {
 		}
 	}
 }
+
+func TestClusterServiceValidate(t *testing.T) {
+	tests := []struct {
+		name   string
+		svc    ClusterService
+		assert assert.ErrorAssertionFunc
+	}{
+		{
+			name:   "empty",
+			svc:    ClusterService{},
+			assert: assert.NoError,
+		},
+		{
+			name: "valid",
+			svc: ClusterService{
+				ClusterID: 99,
+				Frontends: map[string]PortConfiguration{"10.1.2.3": {}, "abcd::0001": {}},
+				Backends:  map[string]PortConfiguration{"10.3.2.1": {}, "dcba::0001": {}},
+			},
+			assert: assert.NoError,
+		},
+		{
+			name:   "invalid cluster ID",
+			svc:    ClusterService{ClusterID: 260},
+			assert: assert.Error,
+		},
+		{
+			name:   "invalid frontend IP",
+			svc:    ClusterService{Frontends: map[string]PortConfiguration{"10.1.2.3": {}, "invalid": {}}},
+			assert: assert.Error,
+		},
+		{
+			name:   "invalid backend IP",
+			svc:    ClusterService{Backends: map[string]PortConfiguration{"invalid": {}, "dcba::0001": {}}},
+			assert: assert.Error,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.assert(t, tt.svc.validate())
+		})
+	}
+}