From e7ee0c1da4d5dc38e7f973166d6f4fe814dd49cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Wei=C3=9Fe?=
 <66256922+daniel-weisse@users.noreply.github.com>
Date: Mon, 16 Dec 2024 16:40:21 +0100
Subject: [PATCH] coordinator: fix equality checks for manifest properties
 (#777)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix DisableSecretBinding ignored in Marble.Equal

* fix AcceptedTCBStatuses and AcceptedAdvisories ignored in PackageProperties.Equal

---------

Signed-off-by: Daniel Weiße <dw@edgeless.systems>
Co-authored-by: Thomas Tendyck <tt@edgeless.systems>
---
 coordinator/manifest/manifest.go | 19 ++++------------
 coordinator/quote/ert.go         |  6 ++++++
 util/util.go                     | 15 +++++++++++++
 util/util_test.go                | 37 ++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/coordinator/manifest/manifest.go b/coordinator/manifest/manifest.go
index 7291d5c1..58795359 100644
--- a/coordinator/manifest/manifest.go
+++ b/coordinator/manifest/manifest.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/edgelesssys/marblerun/coordinator/quote"
 	"github.com/edgelesssys/marblerun/coordinator/user"
+	"github.com/edgelesssys/marblerun/util"
 	"go.uber.org/zap"
 )
 
@@ -94,26 +95,14 @@ type Marble struct {
 
 // Equal returns true if two Marble definitions are equal.
 func (m Marble) Equal(other Marble) bool {
-	if len(m.TLS) != len(other.TLS) {
+	if !util.SliceEqualElements(m.TLS, other.TLS) {
 		return false
 	}
 
-	mTLS := make([]string, len(m.TLS))
-	copy(mTLS, m.TLS)
-	otherTLS := make([]string, len(other.TLS))
-	copy(otherTLS, other.TLS)
-
-	sort.Strings(mTLS)
-	sort.Strings(otherTLS)
-	for i := range mTLS {
-		if mTLS[i] != otherTLS[i] {
-			return false
-		}
-	}
-
 	return m.Package == other.Package &&
 		m.MaxActivations == other.MaxActivations &&
-		m.Parameters.Equal(other.Parameters)
+		m.Parameters.Equal(other.Parameters) &&
+		m.DisableSecretBinding == other.DisableSecretBinding
 }
 
 // Parameters contains lists for files, environment variables and commandline arguments that should be passed to an application.
diff --git a/coordinator/quote/ert.go b/coordinator/quote/ert.go
index fdb75da1..055a5a08 100644
--- a/coordinator/quote/ert.go
+++ b/coordinator/quote/ert.go
@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/edgelesssys/marblerun/util"
 	"github.com/google/go-cmp/cmp"
 )
 
@@ -68,6 +69,11 @@ func (p PackageProperties) Equal(other PackageProperties) bool {
 		return false
 	}
 
+	if !util.SliceEqualElements(p.AcceptedAdvisories, other.AcceptedAdvisories) ||
+		!util.SliceEqualElements(p.AcceptedTCBStatuses, other.AcceptedTCBStatuses) {
+		return false
+	}
+
 	return true
 }
 
diff --git a/util/util.go b/util/util.go
index 326eec28..bf0df30c 100644
--- a/util/util.go
+++ b/util/util.go
@@ -7,6 +7,7 @@ SPDX-License-Identifier: BUSL-1.1
 package util
 
 import (
+	"cmp"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/sha256"
@@ -19,6 +20,7 @@ import (
 	"log"
 	"net"
 	"os"
+	"slices"
 
 	"golang.org/x/crypto/hkdf"
 )
@@ -181,3 +183,16 @@ func IsRawSGXQuote(quote []byte) bool {
 
 	return true
 }
+
+// SliceEqualElements checks if a slice contains the same elements as another slice.
+// Order of elements does not matter.
+// Elements must be of type [cmp.Ordered].
+func SliceEqualElements[T cmp.Ordered](a, b []T) bool {
+	aCopy := make([]T, len(a))
+	bCopy := make([]T, len(b))
+	copy(aCopy, a)
+	copy(bCopy, b)
+	slices.Sort(aCopy)
+	slices.Sort(bCopy)
+	return slices.Equal(aCopy, bCopy)
+}
diff --git a/util/util_test.go b/util/util_test.go
index d6a8e25d..ed556e17 100644
--- a/util/util_test.go
+++ b/util/util_test.go
@@ -150,3 +150,40 @@ func TestIsRawSGXQuote(t *testing.T) {
 		})
 	}
 }
+
+func TestSliceEqualElements(t *testing.T) {
+	testCases := map[string]struct {
+		sliceA, sliceB []string
+		want           bool
+	}{
+		"empty slices": {
+			sliceA: []string{},
+			sliceB: []string{},
+			want:   true,
+		},
+		"one empty slice": {
+			sliceA: []string{"foo"},
+			sliceB: []string{},
+			want:   false,
+		},
+		"equal slices": {
+			sliceA: []string{"foo", "bar"},
+			sliceB: []string{"foo", "bar"},
+			want:   true,
+		},
+		"element order doesn't matter": {
+			sliceA: []string{"foo", "bar"},
+			sliceB: []string{"bar", "foo"},
+			want:   true,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			assert.Equal(tc.want, SliceEqualElements(tc.sliceA, tc.sliceB))
+			assert.Equal(tc.want, SliceEqualElements(tc.sliceB, tc.sliceA))
+		})
+	}
+}