#86 make rule parsing hitting one invalid rule not kill entire rule a…

…pplication, add validator to data.SetAcl
NHAS · Jan 24, 2024 · 99a85df · 99a85df
1 parent 7d09d13
commit 99a85df
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 15 deletions.
diff --git a/internal/data/acls.go b/internal/data/acls.go
@@ -9,13 +9,18 @@ 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"
 )
 
 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
@@ -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
@@ -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
@@ -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
 		}