From 99a85df29b5b226f4adfaa0baaf24b1245b8eca2 Mon Sep 17 00:00:00 2001 From: NHAS Date: Wed, 24 Jan 2024 22:07:55 +1300 Subject: [PATCH] #86 make rule parsing hitting one invalid rule not kill entire rule application, add validator to data.SetAcl --- internal/data/acls.go | 5 +++++ internal/router/bpf.go | 6 +++--- internal/routetypes/parser.go | 37 +++++++++++++++++++++++++---------- pkg/control/server/config.go | 7 +++++-- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/internal/data/acls.go b/internal/data/acls.go index 429a4fb4..1a6254ee 100644 --- a/internal/data/acls.go +++ b/internal/data/acls.go @@ -9,6 +9,7 @@ import ( "github.com/NHAS/wag/internal/acls" "github.com/NHAS/wag/internal/config" + "github.com/NHAS/wag/internal/routetypes" "github.com/NHAS/wag/pkg/control" clientv3 "go.etcd.io/etcd/client/v3" "go.etcd.io/etcd/client/v3/clientv3util" @@ -16,6 +17,10 @@ import ( func SetAcl(effects string, policy acls.Acl, overwrite bool) error { + if err := routetypes.ValidateRules(policy.Mfa, policy.Allow, policy.Deny); err != nil { + return err + } + policyJson, _ := json.Marshal(policy) if overwrite { diff --git a/internal/router/bpf.go b/internal/router/bpf.go index eeeff918..f3ce4eae 100644 --- a/internal/router/bpf.go +++ b/internal/router/bpf.go @@ -311,9 +311,9 @@ func SetLockAccount(username string, locked uint32) error { // Takes the LPM table and associates a route to a policy func xdpAddRoute(usersRouteTable *ebpf.Map, userAcls acls.Acl) error { - rules, err := routetypes.ParseRules(userAcls.Mfa, userAcls.Allow, userAcls.Deny) - if err != nil { - return err + rules, errs := routetypes.ParseRules(userAcls.Mfa, userAcls.Allow, userAcls.Deny) + if len(errs) != 0 { + log.Println("Parsing rules for user had errors: ", errs) } for _, rule := range rules { diff --git a/internal/routetypes/parser.go b/internal/routetypes/parser.go index 350888c1..6fc9f23d 100644 --- a/internal/routetypes/parser.go +++ b/internal/routetypes/parser.go @@ -52,7 +52,7 @@ func hash(mfa, public, deny []string) string { return hex.EncodeToString(result[:]) } -func ParseRules(mfa, public, deny []string) (result []Rule, err error) { +func ParseRules(mfa, public, deny []string) (result []Rule, errs []error) { cache := map[string]int{} @@ -72,7 +72,8 @@ func ParseRules(mfa, public, deny []string) (result []Rule, err error) { r, err := parseRule(0, rule) if err != nil { - return nil, err + errs = append(errs, err) + continue } for i := range r.Keys { @@ -91,7 +92,8 @@ func ParseRules(mfa, public, deny []string) (result []Rule, err error) { for _, rule := range public { r, err := parseRule(PUBLIC, rule) if err != nil { - return nil, err + errs = append(errs, err) + continue } for i := range r.Keys { @@ -110,7 +112,8 @@ func ParseRules(mfa, public, deny []string) (result []Rule, err error) { for _, rule := range deny { r, err := parseRule(DENY, rule) if err != nil { - return nil, err + errs = append(errs, err) + continue } for i := range r.Keys { @@ -128,7 +131,8 @@ func ParseRules(mfa, public, deny []string) (result []Rule, err error) { for i := range result { if len(result[i].Values) > MAX_POLICIES { - return nil, errors.New("number of policies defined was greather than max") + errs = append(errs, errors.New("number of policies defined was greather than max")) + return nil, errs } temp := make([]Policy, 0, MAX_POLICIES) @@ -139,9 +143,12 @@ func ParseRules(mfa, public, deny []string) (result []Rule, err error) { result[i].Values = temp[:cap(temp)] } - rwLock.Lock() - globalCache[parseKey] = result - rwLock.Unlock() + // Dont add a cache entry if there was an error parsing + if len(errs) == 0 { + rwLock.Lock() + globalCache[parseKey] = result + rwLock.Unlock() + } return } @@ -237,8 +244,18 @@ func parseKeys(address string) (keys []Key, err error) { } func ValidateRules(mfa, public, deny []string) error { - _, err := ParseRules(mfa, public, deny) - return err + _, errs := ParseRules(mfa, public, deny) + + if len(errs) == 0 { + return nil + } + + str := "" + for _, err := range errs { + str += err.Error() + "\n" + } + + return errors.New(str) } func parseService(service string) (Policy, error) { diff --git a/pkg/control/server/config.go b/pkg/control/server/config.go index 047fee6a..5cf9f38b 100644 --- a/pkg/control/server/config.go +++ b/pkg/control/server/config.go @@ -42,7 +42,8 @@ func newPolicy(w http.ResponseWriter, r *http.Request) { } - if err := data.SetAcl(acl.Effects, acls.Acl{Mfa: acl.MfaRoutes, Allow: acl.PublicRoutes}, false); err != nil { + if err := data.SetAcl(acl.Effects, acls.Acl{Mfa: acl.MfaRoutes, Allow: acl.PublicRoutes, Deny: acl.DenyRoutes}, false); err != nil { + log.Println("Unable to set acls: ", err) http.Error(w, err.Error(), 500) return } @@ -66,7 +67,8 @@ func editPolicy(w http.ResponseWriter, r *http.Request) { } - if err := data.SetAcl(polciyData.Effects, acls.Acl{Mfa: polciyData.MfaRoutes, Allow: polciyData.PublicRoutes}, true); err != nil { + if err := data.SetAcl(polciyData.Effects, acls.Acl{Mfa: polciyData.MfaRoutes, Allow: polciyData.PublicRoutes, Deny: polciyData.DenyRoutes}, true); err != nil { + log.Println("Unable to set acls: ", err) http.Error(w, err.Error(), 500) return } @@ -91,6 +93,7 @@ func deletePolicies(w http.ResponseWriter, r *http.Request) { for _, policyName := range policyNames { if err := data.RemoveAcl(policyName); err != nil { + log.Println("Unable to set remove policy: ", err) http.Error(w, err.Error(), 500) return }