From e5b1f724c4668bfa03e870bc2b06e7996b1aaba8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20B=C3=BCtler?= <flbuetle@ethz.ch>
Date: Fri, 12 Apr 2024 15:53:47 +0200
Subject: [PATCH] fix #4504: ensure acl default rule matches all for
 unmarshalled json and yaml acls

---
 private/path/pathpol/acl.go      | 21 +++++++++++++++++----
 private/path/pathpol/acl_test.go | 29 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/private/path/pathpol/acl.go b/private/path/pathpol/acl.go
index 11efbe74ac..9b167e6c1d 100644
--- a/private/path/pathpol/acl.go
+++ b/private/path/pathpol/acl.go
@@ -35,8 +35,8 @@ type ACL struct {
 
 // NewACL creates a new entry and checks for the presence of a default action
 func NewACL(entries ...*ACLEntry) (*ACL, error) {
-	if len(entries) == 0 || !entries[len(entries)-1].Rule.matchesAll() {
-		return nil, ErrNoDefault
+	if err := validateACL(entries); err != nil {
+		return nil, err
 	}
 	return &ACL{Entries: entries}, nil
 }
@@ -61,7 +61,10 @@ func (a *ACL) MarshalJSON() ([]byte, error) {
 }
 
 func (a *ACL) UnmarshalJSON(b []byte) error {
-	return json.Unmarshal(b, &a.Entries)
+	if err := json.Unmarshal(b, &a.Entries); err != nil {
+		return err
+	}
+	return validateACL(a.Entries)
 }
 
 func (a *ACL) MarshalYAML() (interface{}, error) {
@@ -69,7 +72,10 @@ func (a *ACL) MarshalYAML() (interface{}, error) {
 }
 
 func (a *ACL) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	return unmarshal(&a.Entries)
+	if err := unmarshal(&a.Entries); err != nil {
+		return err
+	}
+	return validateACL(a.Entries)
 }
 
 func (a *ACL) evalPath(pm *snet.PathMetadata) ACLAction {
@@ -90,6 +96,13 @@ func (a *ACL) evalInterface(iface snet.PathInterface, ingress bool) ACLAction {
 	panic("Default ACL action missing")
 }
 
+func validateACL(entries []*ACLEntry) error {
+	if len(entries) == 0 || !entries[len(entries)-1].Rule.matchesAll() {
+		return ErrNoDefault
+	}
+	return nil
+}
+
 type ACLEntry struct {
 	Action ACLAction
 	Rule   *HopPredicate
diff --git a/private/path/pathpol/acl_test.go b/private/path/pathpol/acl_test.go
index 080c145336..e733017eae 100644
--- a/private/path/pathpol/acl_test.go
+++ b/private/path/pathpol/acl_test.go
@@ -60,6 +60,35 @@ func TestNewACL(t *testing.T) {
 	}
 }
 
+func TestUnmarshalJSON(t *testing.T) {
+	tests := map[string]struct {
+		Input       string
+		ExpectedErr error
+	}{
+		"No entry": {
+			Input:       `[]`,
+			ExpectedErr: ErrNoDefault,
+		},
+		"No default entry": {
+			Input:       `["+ 42"]`,
+			ExpectedErr: ErrNoDefault,
+		},
+		"Entry without rule": {
+			Input: `["+"]`,
+		},
+		"Entry with hop predicates": {
+			Input: `["+ 42", "-"]`,
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			var acl ACL
+			err := acl.UnmarshalJSON([]byte(test.Input))
+			assert.ErrorIs(t, err, test.ExpectedErr)
+		})
+	}
+}
+
 func TestACLEntryLoadFromString(t *testing.T) {
 	tests := map[string]struct {
 		String         string