From 8214de9e51b63ed4d811b580894f79c245503279 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 14 Mar 2024 17:58:26 +0300 Subject: [PATCH 1/4] Add posture checks validation --- management/server/posture/checks.go | 70 ++++++++++++----------- management/server/posture/geo_location.go | 26 +++++++++ management/server/posture/nb_version.go | 12 ++++ management/server/posture/network.go | 17 ++++++ management/server/posture/os_version.go | 38 +++++++++++- management/server/posture/process.go | 13 +++++ 6 files changed, 141 insertions(+), 35 deletions(-) diff --git a/management/server/posture/checks.go b/management/server/posture/checks.go index 073dc44606c..3876dd0c5e4 100644 --- a/management/server/posture/checks.go +++ b/management/server/posture/checks.go @@ -1,8 +1,9 @@ package posture import ( - "fmt" + "errors" "net/netip" + "regexp" "github.com/hashicorp/go-version" @@ -20,10 +21,15 @@ const ( CheckActionDeny string = "deny" ) +var ( + countryCodeRegex = regexp.MustCompile("^[a-zA-Z]{2}$") +) + // Check represents an interface for performing a check on a peer. type Check interface { - Check(peer nbpeer.Peer) (bool, error) Name() string + Check(peer nbpeer.Peer) (bool, error) + Validate() error } type Checks struct { @@ -148,47 +154,43 @@ func (pc *Checks) GetChecks() []Check { return checks } +// Validate checks the validity of a posture checks. func (pc *Checks) Validate() error { - if check := pc.Checks.NBVersionCheck; check != nil { - if !isVersionValid(check.MinVersion) { - return fmt.Errorf("%s version: %s is not valid", check.Name(), check.MinVersion) - } + if pc.Name == "" { + return errors.New("posture checks name shouldn't be empty") } - if osCheck := pc.Checks.OSVersionCheck; osCheck != nil { - if osCheck.Android != nil { - if !isVersionValid(osCheck.Android.MinVersion) { - return fmt.Errorf("%s android version: %s is not valid", osCheck.Name(), osCheck.Android.MinVersion) - } - } + // posture check should contain at least one check + if pc.Checks.NBVersionCheck == nil && pc.Checks.OSVersionCheck == nil && + pc.Checks.GeoLocationCheck == nil && pc.Checks.PeerNetworkRangeCheck == nil && pc.Checks.ProcessCheck == nil { + return errors.New("posture checks shouldn't be empty") + } - if osCheck.Ios != nil { - if !isVersionValid(osCheck.Ios.MinVersion) { - return fmt.Errorf("%s ios version: %s is not valid", osCheck.Name(), osCheck.Ios.MinVersion) - } + if pc.Checks.NBVersionCheck != nil { + if err := pc.Checks.NBVersionCheck.Validate(); err != nil { + return err } - - if osCheck.Darwin != nil { - if !isVersionValid(osCheck.Darwin.MinVersion) { - return fmt.Errorf("%s darwin version: %s is not valid", osCheck.Name(), osCheck.Darwin.MinVersion) - } + } + if pc.Checks.OSVersionCheck != nil { + if err := pc.Checks.OSVersionCheck.Validate(); err != nil { + return err } - - if osCheck.Linux != nil { - if !isVersionValid(osCheck.Linux.MinKernelVersion) { - return fmt.Errorf("%s linux kernel version: %s is not valid", osCheck.Name(), - osCheck.Linux.MinKernelVersion) - } + } + if pc.Checks.GeoLocationCheck != nil { + if err := pc.Checks.GeoLocationCheck.Validate(); err != nil { + return err } - - if osCheck.Windows != nil { - if !isVersionValid(osCheck.Windows.MinKernelVersion) { - return fmt.Errorf("%s windows kernel version: %s is not valid", osCheck.Name(), - osCheck.Windows.MinKernelVersion) - } + } + if pc.Checks.PeerNetworkRangeCheck != nil { + if err := pc.Checks.PeerNetworkRangeCheck.Validate(); err != nil { + return err + } + } + if pc.Checks.ProcessCheck != nil { + if err := pc.Checks.ProcessCheck.Validate(); err != nil { + return err } } - return nil } diff --git a/management/server/posture/geo_location.go b/management/server/posture/geo_location.go index 856913a7a48..b51f8051993 100644 --- a/management/server/posture/geo_location.go +++ b/management/server/posture/geo_location.go @@ -2,6 +2,7 @@ package posture import ( "fmt" + "slices" nbpeer "github.com/netbirdio/netbird/management/server/peer" ) @@ -60,3 +61,28 @@ func (g *GeoLocationCheck) Check(peer nbpeer.Peer) (bool, error) { func (g *GeoLocationCheck) Name() string { return GeoLocationCheckName } + +func (g *GeoLocationCheck) Validate() error { + if g.Action == "" { + return fmt.Errorf("%s action shouldn't be empty", g.Name()) + } + + allowedActions := []string{CheckActionAllow, CheckActionDeny} + if !slices.Contains(allowedActions, g.Action) { + return fmt.Errorf("%s action is not valid", g.Name()) + } + + if len(g.Locations) == 0 { + return fmt.Errorf("%s locations shouldn't be empty", g.Name()) + } + + for _, loc := range g.Locations { + if loc.CountryCode == "" { + return fmt.Errorf("%s country code shouldn't be empty", g.Name()) + } + if !countryCodeRegex.MatchString(loc.CountryCode) { + return fmt.Errorf("%s country code must be 2 letters (ISO 3166-1 alpha-2 format)", g.Name()) + } + } + return nil +} diff --git a/management/server/posture/nb_version.go b/management/server/posture/nb_version.go index 0645b8f73e0..62a6e268c07 100644 --- a/management/server/posture/nb_version.go +++ b/management/server/posture/nb_version.go @@ -1,6 +1,8 @@ package posture import ( + "fmt" + "github.com/hashicorp/go-version" log "github.com/sirupsen/logrus" @@ -37,3 +39,13 @@ func (n *NBVersionCheck) Check(peer nbpeer.Peer) (bool, error) { func (n *NBVersionCheck) Name() string { return NBVersionCheckName } + +func (n *NBVersionCheck) Validate() error { + if n.MinVersion == "" { + return fmt.Errorf("%s minimum version shouldn't be empty", n.Name()) + } + if !isVersionValid(n.MinVersion) { + return fmt.Errorf("%s version: %s is not valid", n.Name(), n.MinVersion) + } + return nil +} diff --git a/management/server/posture/network.go b/management/server/posture/network.go index 9bf969f4c36..ed90ea91bac 100644 --- a/management/server/posture/network.go +++ b/management/server/posture/network.go @@ -6,6 +6,7 @@ import ( "slices" nbpeer "github.com/netbirdio/netbird/management/server/peer" + "github.com/netbirdio/netbird/management/server/status" ) type PeerNetworkRangeCheck struct { @@ -52,3 +53,19 @@ func (p *PeerNetworkRangeCheck) Check(peer nbpeer.Peer) (bool, error) { func (p *PeerNetworkRangeCheck) Name() string { return PeerNetworkRangeCheckName } + +func (p *PeerNetworkRangeCheck) Validate() error { + if p.Action == "" { + return status.Errorf(status.InvalidArgument, "action for peer network range check shouldn't be empty") + } + + allowedActions := []string{CheckActionAllow, CheckActionDeny} + if !slices.Contains(allowedActions, p.Action) { + return fmt.Errorf("%s action is not valid", p.Name()) + } + + if len(p.Ranges) == 0 { + return fmt.Errorf("%s network ranges shouldn't be empty", p.Name()) + } + return nil +} diff --git a/management/server/posture/os_version.go b/management/server/posture/os_version.go index 4c311f01b94..acd0337e94c 100644 --- a/management/server/posture/os_version.go +++ b/management/server/posture/os_version.go @@ -1,11 +1,13 @@ package posture import ( + "fmt" "strings" "github.com/hashicorp/go-version" - nbpeer "github.com/netbirdio/netbird/management/server/peer" log "github.com/sirupsen/logrus" + + nbpeer "github.com/netbirdio/netbird/management/server/peer" ) type MinVersionCheck struct { @@ -48,6 +50,40 @@ func (c *OSVersionCheck) Name() string { return OSVersionCheckName } +func (c *OSVersionCheck) Validate() error { + emptyOS := c.Android == nil && c.Darwin == nil && c.Ios == nil && + c.Linux == nil && c.Windows == nil + emptyMinVersion := c.Android != nil && c.Android.MinVersion == "" || c.Darwin != nil && c.Darwin.MinVersion == "" || + c.Ios != nil && c.Ios.MinVersion == "" || c.Linux != nil && c.Linux.MinKernelVersion == "" || c.Windows != nil && + c.Windows.MinKernelVersion == "" + if emptyOS || emptyMinVersion { + return fmt.Errorf("%s minimum version for at least one OS shouldn't be empty", c.Name()) + } + + if c.Android != nil && !isVersionValid(c.Android.MinVersion) { + return fmt.Errorf("%s android version: %s is not valid", c.Name(), c.Android.MinVersion) + } + + if c.Ios != nil && !isVersionValid(c.Ios.MinVersion) { + return fmt.Errorf("%s ios version: %s is not valid", c.Name(), c.Ios.MinVersion) + } + + if c.Darwin != nil && !isVersionValid(c.Darwin.MinVersion) { + return fmt.Errorf("%s darwin version: %s is not valid", c.Name(), c.Darwin.MinVersion) + } + + if c.Linux != nil && !isVersionValid(c.Linux.MinKernelVersion) { + return fmt.Errorf("%s linux kernel version: %s is not valid", c.Name(), + c.Linux.MinKernelVersion) + } + + if c.Windows != nil && !isVersionValid(c.Windows.MinKernelVersion) { + return fmt.Errorf("%s windows kernel version: %s is not valid", c.Name(), + c.Windows.MinKernelVersion) + } + return nil +} + func checkMinVersion(peerGoOS, peerVersion string, check *MinVersionCheck) (bool, error) { if check == nil { log.Debugf("peer %s OS is not allowed in the check", peerGoOS) diff --git a/management/server/posture/process.go b/management/server/posture/process.go index b8a2be40666..11d2cf68f4e 100644 --- a/management/server/posture/process.go +++ b/management/server/posture/process.go @@ -47,3 +47,16 @@ func (p *ProcessCheck) Check(peer nbpeer.Peer) (bool, error) { func (p *ProcessCheck) Name() string { return ProcessCheckName } + +func (p *ProcessCheck) Validate() error { + if len(p.Processes) == 0 { + return fmt.Errorf("%s processes shouldn't be empty", p.Name()) + } + + for _, process := range p.Processes { + if process.Path == "" && process.WindowsPath == "" { + return fmt.Errorf("%s path shouldn't be empty", p.Name()) + } + } + return nil +} From 11774bf6810635783f1bc841a9004eeacecf8851 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 14 Mar 2024 17:59:26 +0300 Subject: [PATCH 2/4] Refactor code to incorporate posture checks validation directly into management. --- .../server/http/geolocations_handler.go | 9 +- .../server/http/posture_checks_handler.go | 101 +---------- .../http/posture_checks_handler_test.go | 164 ++---------------- management/server/posture_checks.go | 2 +- 4 files changed, 28 insertions(+), 248 deletions(-) diff --git a/management/server/http/geolocations_handler.go b/management/server/http/geolocations_handler.go index 070aa6350a5..cf961267b58 100644 --- a/management/server/http/geolocations_handler.go +++ b/management/server/http/geolocations_handler.go @@ -2,6 +2,7 @@ package http import ( "net/http" + "regexp" "github.com/gorilla/mux" @@ -13,6 +14,10 @@ import ( "github.com/netbirdio/netbird/management/server/status" ) +var ( + countryCodeRegex = regexp.MustCompile("^[a-zA-Z]{2}$") +) + // GeolocationsHandler is a handler that returns locations. type GeolocationsHandler struct { accountManager server.AccountManager @@ -73,8 +78,8 @@ func (l *GeolocationsHandler) GetCitiesByCountry(w http.ResponseWriter, r *http. } if l.geolocationManager == nil { - // TODO: update error message to include geo db self hosted doc link when ready - util.WriteError(status.Errorf(status.PreconditionFailed, "Geo location database is not initialized"), w) + util.WriteError(status.Errorf(status.PreconditionFailed, "Geo location database is not initialized. "+ + "Check the self-hosted Geo database documentation at https://docs.netbird.io/selfhosted/geo-support"), w) return } diff --git a/management/server/http/posture_checks_handler.go b/management/server/http/posture_checks_handler.go index 6ea84ea26fe..4245f8fbab1 100644 --- a/management/server/http/posture_checks_handler.go +++ b/management/server/http/posture_checks_handler.go @@ -4,8 +4,6 @@ import ( "encoding/json" "net/http" "net/netip" - "regexp" - "slices" "github.com/gorilla/mux" "github.com/rs/xid" @@ -19,10 +17,6 @@ import ( "github.com/netbirdio/netbird/management/server/status" ) -var ( - countryCodeRegex = regexp.MustCompile("^[a-zA-Z]{2}$") -) - // PostureChecksHandler is a handler that returns posture checks of the account. type PostureChecksHandler struct { accountManager server.AccountManager @@ -165,19 +159,16 @@ func (p *PostureChecksHandler) savePostureChecks( user *server.User, postureChecksID string, ) { + var ( + err error + req api.PostureCheckUpdate + ) - var req api.PostureCheckUpdate - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err = json.NewDecoder(r.Body).Decode(&req); err != nil { util.WriteErrorResponse("couldn't parse JSON request", http.StatusBadRequest, w) return } - err := validatePostureChecksUpdate(req) - if err != nil { - util.WriteErrorResponse(err.Error(), http.StatusBadRequest, w) - return - } - if postureChecksID == "" { postureChecksID = xid.New().String() } @@ -206,8 +197,8 @@ func (p *PostureChecksHandler) savePostureChecks( if geoLocationCheck := req.Checks.GeoLocationCheck; geoLocationCheck != nil { if p.geolocationManager == nil { - // TODO: update error message to include geo db self hosted doc link when ready - util.WriteError(status.Errorf(status.PreconditionFailed, "Geo location database is not initialized"), w) + util.WriteError(status.Errorf(status.PreconditionFailed, "Geo location database is not initialized. "+ + "Check the self-hosted Geo database documentation at https://docs.netbird.io/selfhosted/geo-support"), w) return } postureChecks.Checks.GeoLocationCheck = toPostureGeoLocationCheck(geoLocationCheck) @@ -233,84 +224,6 @@ func (p *PostureChecksHandler) savePostureChecks( util.WriteJSONObject(w, toPostureChecksResponse(&postureChecks)) } -func validatePostureChecksUpdate(req api.PostureCheckUpdate) error { - if req.Name == "" { - return status.Errorf(status.InvalidArgument, "posture checks name shouldn't be empty") - } - - if req.Checks == nil || (req.Checks.NbVersionCheck == nil && req.Checks.OsVersionCheck == nil && - req.Checks.GeoLocationCheck == nil && req.Checks.PeerNetworkRangeCheck == nil && req.Checks.ProcessCheck == nil) { - return status.Errorf(status.InvalidArgument, "posture checks shouldn't be empty") - } - - if req.Checks.NbVersionCheck != nil && req.Checks.NbVersionCheck.MinVersion == "" { - return status.Errorf(status.InvalidArgument, "minimum version for NetBird's version check shouldn't be empty") - } - - if osVersionCheck := req.Checks.OsVersionCheck; osVersionCheck != nil { - emptyOS := osVersionCheck.Android == nil && osVersionCheck.Darwin == nil && osVersionCheck.Ios == nil && - osVersionCheck.Linux == nil && osVersionCheck.Windows == nil - emptyMinVersion := osVersionCheck.Android != nil && osVersionCheck.Android.MinVersion == "" || - osVersionCheck.Darwin != nil && osVersionCheck.Darwin.MinVersion == "" || - osVersionCheck.Ios != nil && osVersionCheck.Ios.MinVersion == "" || - osVersionCheck.Linux != nil && osVersionCheck.Linux.MinKernelVersion == "" || - osVersionCheck.Windows != nil && osVersionCheck.Windows.MinKernelVersion == "" - if emptyOS || emptyMinVersion { - return status.Errorf(status.InvalidArgument, - "minimum version for at least one OS in the OS version check shouldn't be empty") - } - } - - if geoLocationCheck := req.Checks.GeoLocationCheck; geoLocationCheck != nil { - if geoLocationCheck.Action == "" { - return status.Errorf(status.InvalidArgument, "action for geolocation check shouldn't be empty") - } - allowedActions := []api.GeoLocationCheckAction{api.GeoLocationCheckActionAllow, api.GeoLocationCheckActionDeny} - if !slices.Contains(allowedActions, geoLocationCheck.Action) { - return status.Errorf(status.InvalidArgument, "action for geolocation check is not valid value") - } - if len(geoLocationCheck.Locations) == 0 { - return status.Errorf(status.InvalidArgument, "locations for geolocation check shouldn't be empty") - } - for _, loc := range geoLocationCheck.Locations { - if loc.CountryCode == "" { - return status.Errorf(status.InvalidArgument, "country code for geolocation check shouldn't be empty") - } - if !countryCodeRegex.MatchString(loc.CountryCode) { - return status.Errorf(status.InvalidArgument, "country code must be 2 letters (ISO 3166-1 alpha-2 format)") - } - } - } - - if peerNetworkRangeCheck := req.Checks.PeerNetworkRangeCheck; peerNetworkRangeCheck != nil { - if peerNetworkRangeCheck.Action == "" { - return status.Errorf(status.InvalidArgument, "action for peer network range check shouldn't be empty") - } - - allowedActions := []api.PeerNetworkRangeCheckAction{api.PeerNetworkRangeCheckActionAllow, api.PeerNetworkRangeCheckActionDeny} - if !slices.Contains(allowedActions, peerNetworkRangeCheck.Action) { - return status.Errorf(status.InvalidArgument, "action for peer network range check is not valid value") - } - if len(peerNetworkRangeCheck.Ranges) == 0 { - return status.Errorf(status.InvalidArgument, "network ranges for peer network range check shouldn't be empty") - } - } - - if processCheck := req.Checks.ProcessCheck; processCheck != nil { - if len(processCheck.Processes) == 0 { - return status.Errorf(status.InvalidArgument, "processes for process check shouldn't be empty") - } - - for _, process := range processCheck.Processes { - if process.Path == nil && process.WindowsPath == nil { - return status.Errorf(status.InvalidArgument, "path for process check shouldn't be empty") - } - } - } - - return nil -} - func toPostureChecksResponse(postureChecks *posture.Checks) *api.PostureCheck { var checks api.Checks diff --git a/management/server/http/posture_checks_handler_test.go b/management/server/http/posture_checks_handler_test.go index f312ba96118..733fdf7d2ae 100644 --- a/management/server/http/posture_checks_handler_test.go +++ b/management/server/http/posture_checks_handler_test.go @@ -43,6 +43,11 @@ func initPostureChecksTestData(postureChecks ...*posture.Checks) *PostureChecksH SavePostureChecksFunc: func(accountID, userID string, postureChecks *posture.Checks) error { postureChecks.ID = "postureCheck" testPostureChecks[postureChecks.ID] = postureChecks + + if err := postureChecks.Validate(); err != nil { + return status.Errorf(status.InvalidArgument, err.Error()) + } + return nil }, DeletePostureChecksFunc: func(accountID, postureChecksID, userID string) error { @@ -483,7 +488,7 @@ func TestPostureCheckUpdate(t *testing.T) { } } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -498,7 +503,7 @@ func TestPostureCheckUpdate(t *testing.T) { } } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -512,7 +517,7 @@ func TestPostureCheckUpdate(t *testing.T) { "nb_version_check": {} } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -526,7 +531,7 @@ func TestPostureCheckUpdate(t *testing.T) { "geo_location_check": {} } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -700,11 +705,8 @@ func TestPostureCheckUpdate(t *testing.T) { } } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, - setupHandlerFunc: func(handler *PostureChecksHandler) { - handler.geolocationManager = nil - }, }, { name: "Update Posture Checks Invalid Check", @@ -719,7 +721,7 @@ func TestPostureCheckUpdate(t *testing.T) { } } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -734,7 +736,7 @@ func TestPostureCheckUpdate(t *testing.T) { } } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -748,7 +750,7 @@ func TestPostureCheckUpdate(t *testing.T) { "nb_version_check": {} } }`)), - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -878,143 +880,3 @@ func TestPostureCheckUpdate(t *testing.T) { }) } } - -func TestPostureCheck_validatePostureChecksUpdate(t *testing.T) { - str := func(s string) *string { return &s } - - // empty name - err := validatePostureChecksUpdate(api.PostureCheckUpdate{}) - assert.Error(t, err) - - // empty checks - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default"}) - assert.Error(t, err) - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{}}) - assert.Error(t, err) - - // not valid NbVersionCheck - nbVersionCheck := api.NBVersionCheck{} - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{NbVersionCheck: &nbVersionCheck}}) - assert.Error(t, err) - - // valid NbVersionCheck - nbVersionCheck = api.NBVersionCheck{MinVersion: "1.0"} - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{NbVersionCheck: &nbVersionCheck}}) - assert.NoError(t, err) - - // not valid OsVersionCheck - osVersionCheck := api.OSVersionCheck{} - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{OsVersionCheck: &osVersionCheck}}) - assert.Error(t, err) - - // not valid OsVersionCheck - osVersionCheck = api.OSVersionCheck{Linux: &api.MinKernelVersionCheck{}} - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{OsVersionCheck: &osVersionCheck}}) - assert.Error(t, err) - - // not valid OsVersionCheck - osVersionCheck = api.OSVersionCheck{Linux: &api.MinKernelVersionCheck{}, Darwin: &api.MinVersionCheck{MinVersion: "14.2"}} - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{OsVersionCheck: &osVersionCheck}}) - assert.Error(t, err) - - // valid OsVersionCheck - osVersionCheck = api.OSVersionCheck{Linux: &api.MinKernelVersionCheck{MinKernelVersion: "6.0"}} - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{OsVersionCheck: &osVersionCheck}}) - assert.NoError(t, err) - - // valid OsVersionCheck - osVersionCheck = api.OSVersionCheck{ - Linux: &api.MinKernelVersionCheck{MinKernelVersion: "6.0"}, - Darwin: &api.MinVersionCheck{MinVersion: "14.2"}, - } - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{OsVersionCheck: &osVersionCheck}}) - assert.NoError(t, err) - - // valid peer network range check - peerNetworkRangeCheck := api.PeerNetworkRangeCheck{ - Action: api.PeerNetworkRangeCheckActionAllow, - Ranges: []string{ - "192.168.1.0/24", "10.0.0.0/8", - }, - } - err = validatePostureChecksUpdate( - api.PostureCheckUpdate{ - Name: "Default", - Checks: &api.Checks{ - PeerNetworkRangeCheck: &peerNetworkRangeCheck, - }, - }, - ) - assert.NoError(t, err) - - // invalid peer network range check - peerNetworkRangeCheck = api.PeerNetworkRangeCheck{ - Action: api.PeerNetworkRangeCheckActionDeny, - Ranges: []string{}, - } - err = validatePostureChecksUpdate( - api.PostureCheckUpdate{ - Name: "Default", - Checks: &api.Checks{ - PeerNetworkRangeCheck: &peerNetworkRangeCheck, - }, - }, - ) - assert.Error(t, err) - - // invalid peer network range check - peerNetworkRangeCheck = api.PeerNetworkRangeCheck{ - Action: "unknownAction", - Ranges: []string{}, - } - err = validatePostureChecksUpdate( - api.PostureCheckUpdate{ - Name: "Default", - Checks: &api.Checks{ - PeerNetworkRangeCheck: &peerNetworkRangeCheck, - }, - }, - ) - assert.Error(t, err) - - // valid process check - processCheck := api.ProcessCheck{ - Processes: []api.Process{ - { - Path: str("/usr/local/bin/netbird"), - WindowsPath: str("C:\\ProgramData\\NetBird\\netbird.exe"), - }, - }, - } - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{ProcessCheck: &processCheck}}) - assert.NoError(t, err) - - // invalid process check - processCheck = api.ProcessCheck{ - Processes: make([]api.Process, 0), - } - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{ProcessCheck: &processCheck}}) - assert.Error(t, err) - - // invalid process check - processCheck = api.ProcessCheck{ - Processes: []api.Process{ - { - Path: str("/usr/local/bin/netbird"), - }, - }, - } - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{ProcessCheck: &processCheck}}) - assert.Error(t, err) - - // invalid process check - processCheck = api.ProcessCheck{ - Processes: []api.Process{ - { - WindowsPath: str("C:\\ProgramData\\NetBird\\netbird.exe"), - }, - }, - } - err = validatePostureChecksUpdate(api.PostureCheckUpdate{Name: "Default", Checks: &api.Checks{ProcessCheck: &processCheck}}) - assert.Error(t, err) -} diff --git a/management/server/posture_checks.go b/management/server/posture_checks.go index 7e654b5fb7c..c6b0b2c77ba 100644 --- a/management/server/posture_checks.go +++ b/management/server/posture_checks.go @@ -52,7 +52,7 @@ func (am *DefaultAccountManager) SavePostureChecks(accountID, userID string, pos } if err := postureChecks.Validate(); err != nil { - return status.Errorf(status.BadRequest, err.Error()) + return status.Errorf(status.InvalidArgument, err.Error()) } exists, uniqName := am.savePostureChecks(account, postureChecks) From 3ed356e23717cfd0c16ab302b323a2286c060411 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 14 Mar 2024 19:26:23 +0300 Subject: [PATCH 3/4] Add posture checks validation for geolocation, OS version, network, process, and NB-version --- management/server/posture/checks_test.go | 13 ++ .../server/posture/geo_location_test.go | 78 ++++++++++ management/server/posture/nb_version_test.go | 30 ++++ management/server/posture/network_test.go | 49 +++++++ management/server/posture/os_version_test.go | 76 ++++++++++ management/server/posture/process_test.go | 135 +++++++++++++----- 6 files changed, 344 insertions(+), 37 deletions(-) diff --git a/management/server/posture/checks_test.go b/management/server/posture/checks_test.go index 41e1c2d784f..26b057b70b4 100644 --- a/management/server/posture/checks_test.go +++ b/management/server/posture/checks_test.go @@ -150,6 +150,19 @@ func TestChecks_Validate(t *testing.T) { checks Checks expectedError bool }{ + { + name: "Empty name", + checks: Checks{}, + expectedError: true, + }, + { + name: "Empty checks", + checks: Checks{ + Name: "Default", + Checks: ChecksDefinition{}, + }, + expectedError: true, + }, { name: "Valid checks version", checks: Checks{ diff --git a/management/server/posture/geo_location_test.go b/management/server/posture/geo_location_test.go index 267bbe0f2ed..a92732c5390 100644 --- a/management/server/posture/geo_location_test.go +++ b/management/server/posture/geo_location_test.go @@ -236,3 +236,81 @@ func TestGeoLocationCheck_Check(t *testing.T) { }) } } + +func TestGeoLocationCheck_Validate(t *testing.T) { + testCases := []struct { + name string + check GeoLocationCheck + expectedError bool + }{ + { + name: "Valid location list", + check: GeoLocationCheck{ + Action: CheckActionAllow, + Locations: []Location{ + { + CountryCode: "DE", + CityName: "Berlin", + }, + }, + }, + expectedError: false, + }, + { + name: "Invalid empty location list", + check: GeoLocationCheck{ + Action: CheckActionDeny, + Locations: []Location{}, + }, + expectedError: true, + }, + { + name: "Invalid empty country name", + check: GeoLocationCheck{ + Action: CheckActionDeny, + Locations: []Location{ + { + CityName: "Los Angeles", + }, + }, + }, + expectedError: true, + }, + { + name: "Invalid check action", + check: GeoLocationCheck{ + Action: "unknownAction", + Locations: []Location{ + { + CountryCode: "DE", + CityName: "Berlin", + }, + }, + }, + expectedError: true, + }, + { + name: "Invalid country code", + check: GeoLocationCheck{ + Action: CheckActionAllow, + Locations: []Location{ + { + CountryCode: "USA", + }, + }, + }, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.check.Validate() + if tc.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/management/server/posture/nb_version_test.go b/management/server/posture/nb_version_test.go index de51c2283b1..fbe24aa1670 100644 --- a/management/server/posture/nb_version_test.go +++ b/management/server/posture/nb_version_test.go @@ -108,3 +108,33 @@ func TestNBVersionCheck_Check(t *testing.T) { }) } } + +func TestNBVersionCheck_Validate(t *testing.T) { + testCases := []struct { + name string + check NBVersionCheck + expectedError bool + }{ + { + name: "Valid NBVersionCheck", + check: NBVersionCheck{MinVersion: "1.0"}, + expectedError: false, + }, + { + name: "Invalid NBVersionCheck", + check: NBVersionCheck{}, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.check.Validate() + if tc.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/management/server/posture/network_test.go b/management/server/posture/network_test.go index 36ead46602c..6242ece9971 100644 --- a/management/server/posture/network_test.go +++ b/management/server/posture/network_test.go @@ -147,3 +147,52 @@ func TestPeerNetworkRangeCheck_Check(t *testing.T) { }) } } + +func TestNetworkCheck_Validate(t *testing.T) { + testCases := []struct { + name string + check PeerNetworkRangeCheck + expectedError bool + }{ + { + name: "Valid network range", + check: PeerNetworkRangeCheck{ + Action: CheckActionAllow, + Ranges: []netip.Prefix{ + netip.MustParsePrefix("192.168.1.0/24"), + netip.MustParsePrefix("10.0.0.0/8"), + }, + }, + expectedError: false, + }, + { + name: "Invalid empty network range", + check: PeerNetworkRangeCheck{ + Action: CheckActionDeny, + Ranges: []netip.Prefix{}, + }, + expectedError: true, + }, + { + name: "Invalid check action", + check: PeerNetworkRangeCheck{ + Action: "unknownAction", + Ranges: []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/8"), + }, + }, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.check.Validate() + if tc.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/management/server/posture/os_version_test.go b/management/server/posture/os_version_test.go index 32bf5266091..845e703cf66 100644 --- a/management/server/posture/os_version_test.go +++ b/management/server/posture/os_version_test.go @@ -150,3 +150,79 @@ func TestOSVersionCheck_Check(t *testing.T) { }) } } + +func TestOSVersionCheck_Validate(t *testing.T) { + testCases := []struct { + name string + check OSVersionCheck + expectedError bool + }{ + { + name: "Valid linux kernel version", + check: OSVersionCheck{ + Linux: &MinKernelVersionCheck{MinKernelVersion: "6.0"}, + }, + expectedError: false, + }, + { + name: "Valid linux and darwin version", + check: OSVersionCheck{ + Linux: &MinKernelVersionCheck{MinKernelVersion: "6.0"}, + Darwin: &MinVersionCheck{MinVersion: "14.2"}, + }, + expectedError: false, + }, + { + name: "Invalid empty check", + check: OSVersionCheck{}, + expectedError: true, + }, + { + name: "Invalid empty linux kernel version", + check: OSVersionCheck{ + Linux: &MinKernelVersionCheck{}, + }, + expectedError: true, + }, + { + name: "Invalid empty linux kernel version with correct darwin version", + check: OSVersionCheck{ + Linux: &MinKernelVersionCheck{}, + Darwin: &MinVersionCheck{MinVersion: "14.2"}, + }, + expectedError: true, + }, + { + name: "Valid windows kernel version", + check: OSVersionCheck{ + Windows: &MinKernelVersionCheck{MinKernelVersion: "10.0"}, + }, + expectedError: false, + }, + { + name: "Valid ios minimum version", + check: OSVersionCheck{ + Ios: &MinVersionCheck{MinVersion: "13.0"}, + }, + expectedError: false, + }, + { + name: "Invalid empty window version with valid ios minimum version", + check: OSVersionCheck{ + Windows: &MinKernelVersionCheck{}, + Ios: &MinVersionCheck{MinVersion: "13.0"}, + }, + expectedError: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.check.Validate() + if tc.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/management/server/posture/process_test.go b/management/server/posture/process_test.go index 92b32cddc38..957235b1cf1 100644 --- a/management/server/posture/process_test.go +++ b/management/server/posture/process_test.go @@ -22,14 +22,14 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "darwin", Processes: []peer.Process{ - {Path: "process1"}, - {Path: "process2"}}, + {Path: "/Applications/process1.app"}, + {Path: "/Applications/process2.app"}}, }, }, check: ProcessCheck{ Processes: []Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/Applications/process1.app"}, + {Path: "/Applications/process2.app"}, }, }, wantErr: false, @@ -41,15 +41,15 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "darwin", Processes: []peer.Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/Applications/process1.app"}, + {Path: "/Applications/process2.app"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {WindowsPath: "process1"}, - {WindowsPath: "process2"}, + {WindowsPath: "C:\\Program Files\\process1.exe"}, + {WindowsPath: "C:\\Program Files\\process2.exe"}, }, }, wantErr: false, @@ -61,15 +61,15 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "linux", Processes: []peer.Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/usr/bin/process1"}, + {Path: "/usr/bin/process2"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/usr/bin/process1"}, + {Path: "/usr/bin/process2"}, }, }, wantErr: false, @@ -81,15 +81,15 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "linux", Processes: []peer.Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/usr/bin/process1"}, + {Path: "/usr/bin/process2"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {WindowsPath: "process1"}, - {WindowsPath: "process2"}, + {WindowsPath: "C:\\Program Files\\process1.exe"}, + {WindowsPath: "C:\\Program Files\\process2.exe"}, }, }, wantErr: false, @@ -101,15 +101,15 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "linux", Processes: []peer.Process{ - {Path: "process3"}, - {Path: "process4"}, + {Path: "/usr/bin/process3"}, + {Path: "/usr/bin/process4"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/usr/bin/process1"}, + {Path: "/usr/bin/process2"}, }, }, wantErr: false, @@ -121,15 +121,15 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "windows", Processes: []peer.Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "C:\\Program Files\\process1.exe"}, + {Path: "C:\\Program Files\\process1.exe"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {WindowsPath: "process1"}, - {WindowsPath: "process2"}, + {WindowsPath: "C:\\Program Files\\process1.exe"}, + {WindowsPath: "C:\\Program Files\\process1.exe"}, }, }, wantErr: false, @@ -141,15 +141,15 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "windows", Processes: []peer.Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "C:\\Program Files\\process1.exe"}, + {Path: "C:\\Program Files\\process1.exe"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/Applications/process1.app"}, + {Path: "/Applications/process2.app"}, }, }, wantErr: false, @@ -161,22 +161,22 @@ func TestProcessCheck_Check(t *testing.T) { Meta: peer.PeerSystemMeta{ GoOS: "windows", Processes: []peer.Process{ - {Path: "process3"}, - {Path: "process4"}, + {Path: "C:\\Program Files\\process3.exe"}, + {Path: "C:\\Program Files\\process4.exe"}, }, }, }, check: ProcessCheck{ Processes: []Process{ - {WindowsPath: "process1"}, - {WindowsPath: "process2"}, + {WindowsPath: "C:\\Program Files\\process1.exe"}, + {WindowsPath: "C:\\Program Files\\process2.exe"}, }, }, wantErr: false, isValid: false, }, { - name: "unsupported ios operating system with matching processes", + name: "unsupported ios operating system", input: peer.Peer{ Meta: peer.PeerSystemMeta{ GoOS: "ios", @@ -184,8 +184,8 @@ func TestProcessCheck_Check(t *testing.T) { }, check: ProcessCheck{ Processes: []Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "C:\\Program Files\\process1.exe"}, + {Path: "C:\\Program Files\\process2.exe"}, }, }, wantErr: true, @@ -200,8 +200,8 @@ func TestProcessCheck_Check(t *testing.T) { }, check: ProcessCheck{ Processes: []Process{ - {Path: "process1"}, - {Path: "process2"}, + {Path: "/usr/bin/process1"}, + {Path: "/usr/bin/process2"}, }, }, wantErr: true, @@ -221,3 +221,64 @@ func TestProcessCheck_Check(t *testing.T) { }) } } + +func TestProcessCheck_Validate(t *testing.T) { + testCases := []struct { + name string + check ProcessCheck + expectedError bool + }{ + { + name: "Valid unix and windows processes", + check: ProcessCheck{ + Processes: []Process{ + { + Path: "/usr/local/bin/netbird", + WindowsPath: "C:\\ProgramData\\NetBird\\netbird.exe", + }, + }, + }, + expectedError: false, + }, + { + name: "Valid unix process", + check: ProcessCheck{ + Processes: []Process{ + { + Path: "/usr/local/bin/netbird", + }, + }, + }, + expectedError: false, + }, + { + name: "Valid windows process", + check: ProcessCheck{ + Processes: []Process{ + { + WindowsPath: "C:\\ProgramData\\NetBird\\netbird.exe", + }, + }, + }, + expectedError: false, + }, + { + name: "Invalid empty processes", + check: ProcessCheck{ + Processes: []Process{}, + }, + expectedError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.check.Validate() + if tc.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} From 83c2ba285f042879fc9ea341474ddaee04dbc828 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Thu, 14 Mar 2024 20:55:03 +0300 Subject: [PATCH 4/4] Fix tests --- management/server/posture/checks_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/management/server/posture/checks_test.go b/management/server/posture/checks_test.go index 26b057b70b4..875f0f1e98b 100644 --- a/management/server/posture/checks_test.go +++ b/management/server/posture/checks_test.go @@ -166,6 +166,7 @@ func TestChecks_Validate(t *testing.T) { { name: "Valid checks version", checks: Checks{ + Name: "default", Checks: ChecksDefinition{ NBVersionCheck: &NBVersionCheck{ MinVersion: "0.25.0",