From feba51a28f64c219eb4ab9aa43ca8d0fcaf04e5f Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 21 Dec 2024 21:01:31 +0100 Subject: [PATCH 1/4] Add Equal method for CrossSigningKey --- fclient/crosssigning.go | 45 ++++++++++++++++- fclient/crosssigning_test.go | 95 ++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 fclient/crosssigning_test.go diff --git a/fclient/crosssigning.go b/fclient/crosssigning.go index e709697c..1738263c 100644 --- a/fclient/crosssigning.go +++ b/fclient/crosssigning.go @@ -15,8 +15,8 @@ package fclient import ( + "bytes" "encoding/json" - "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" "github.com/tidwall/gjson" @@ -46,6 +46,49 @@ type CrossSigningKey struct { func (s *CrossSigningKey) isCrossSigningBody() {} // implements CrossSigningBody +func (s *CrossSigningKey) Equal(other *CrossSigningKey) bool { + if s == nil || other == nil { + return false + } + if s.UserID != other.UserID { + return false + } + if len(s.Usage) != len(other.Usage) { + return false + } + for i := range s.Usage { + if s.Usage[i] != other.Usage[i] { + return false + } + } + if len(s.Keys) != len(other.Keys) { + return false + } + for k, v := range s.Keys { + if !bytes.Equal(other.Keys[k], v) { + return false + } + } + if len(s.Signatures) != len(other.Signatures) { + return false + } + for k, v := range s.Signatures { + otherV, ok := other.Signatures[k] + if !ok { + return false + } + if len(v) != len(otherV) { + return false + } + for k2, v2 := range v { + if !bytes.Equal(otherV[k2], v2) { + return false + } + } + } + return true +} + type CrossSigningBody interface { isCrossSigningBody() } diff --git a/fclient/crosssigning_test.go b/fclient/crosssigning_test.go new file mode 100644 index 00000000..76b0a6e4 --- /dev/null +++ b/fclient/crosssigning_test.go @@ -0,0 +1,95 @@ +package fclient + +import ( + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/gomatrixserverlib/spec" + "testing" +) + +func TestCrossSigningKeyEqual(t *testing.T) { + tests := []struct { + name string + s *CrossSigningKey + other *CrossSigningKey + expect bool + }{ + { + name: "NilReceiver_ReturnsFalse", + s: nil, + other: &CrossSigningKey{}, + expect: false, + }, + { + name: "NilOther_ReturnsFalse", + s: &CrossSigningKey{}, + other: nil, + expect: false, + }, + { + name: "DifferentUserID_ReturnsFalse", + s: &CrossSigningKey{UserID: "user1"}, + other: &CrossSigningKey{UserID: "user2"}, + expect: false, + }, + { + name: "DifferentUsageLength_ReturnsFalse", + s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}}, + other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster, CrossSigningKeyPurposeSelfSigning}}, + expect: false, + }, + { + name: "DifferentUsageValues_ReturnsFalse", + s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}}, + other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeSelfSigning}}, + expect: false, + }, + { + name: "DifferentKeysLength_ReturnsFalse", + s: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}}, + other: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}, "key2": {}}}, + expect: false, + }, + { + name: "DifferentKeysValues_ReturnsFalse", + s: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}}, + other: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {1}}}, + expect: false, + }, + { + name: "DifferentSignaturesLength_ReturnsFalse", + s: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}}, + other: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}, "sig2": {"key2": {}}}}, + expect: false, + }, + { + name: "DifferentSignaturesValues_ReturnsFalse", + s: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}}, + other: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {1}}}}, + expect: false, + }, + { + name: "IdenticalKeys_ReturnsTrue", + s: &CrossSigningKey{ + UserID: "user1", + Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}, + Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}, + Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}, + }, + other: &CrossSigningKey{ + UserID: "user1", + Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}, + Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}, + Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}, + }, + expect: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.s.Equal(tt.other); got != tt.expect { + t.Errorf("Equal() = %v, want %v", got, tt.expect) + } + }) + } +} From 756fb1477388721f99120ad8dde29f9e0b760550 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 21 Dec 2024 21:19:43 +0100 Subject: [PATCH 2/4] Fix used repository --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index aa1e73d0..1fa751d8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -87,7 +87,7 @@ jobs: steps: - uses: actions/checkout@v3 with: - repository: "matrix-org/dendrite" + repository: "element-hq/dendrite" - name: Install libolm run: sudo apt-get install libolm-dev libolm3 - name: Setup go From 7a74c731a3cf337b7f2582439e82c9e372030d33 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Sat, 21 Dec 2024 21:21:24 +0100 Subject: [PATCH 3/4] Run goimports --- fclient/crosssigning.go | 1 + fclient/crosssigning_test.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/fclient/crosssigning.go b/fclient/crosssigning.go index 1738263c..3670a330 100644 --- a/fclient/crosssigning.go +++ b/fclient/crosssigning.go @@ -17,6 +17,7 @@ package fclient import ( "bytes" "encoding/json" + "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" "github.com/tidwall/gjson" diff --git a/fclient/crosssigning_test.go b/fclient/crosssigning_test.go index 76b0a6e4..63ea3216 100644 --- a/fclient/crosssigning_test.go +++ b/fclient/crosssigning_test.go @@ -1,9 +1,10 @@ package fclient import ( + "testing" + "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" - "testing" ) func TestCrossSigningKeyEqual(t *testing.T) { From 30cbdb4f140c61ae8a5c233bbe869aeb61610d27 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:56:26 +0100 Subject: [PATCH 4/4] Make sure slices are sorted --- fclient/crosssigning.go | 9 ++ fclient/crosssigning_test.go | 175 ++++++++++++++++++++--------------- 2 files changed, 108 insertions(+), 76 deletions(-) diff --git a/fclient/crosssigning.go b/fclient/crosssigning.go index 3670a330..daf3c725 100644 --- a/fclient/crosssigning.go +++ b/fclient/crosssigning.go @@ -17,6 +17,7 @@ package fclient import ( "bytes" "encoding/json" + "slices" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" @@ -57,6 +58,14 @@ func (s *CrossSigningKey) Equal(other *CrossSigningKey) bool { if len(s.Usage) != len(other.Usage) { return false } + + // Make sure the slices are sorted before we compare them. + if !slices.IsSorted(s.Usage) { + slices.Sort(s.Usage) + } + if !slices.IsSorted(other.Usage) { + slices.Sort(other.Usage) + } for i := range s.Usage { if s.Usage[i] != other.Usage[i] { return false diff --git a/fclient/crosssigning_test.go b/fclient/crosssigning_test.go index 63ea3216..59ef6203 100644 --- a/fclient/crosssigning_test.go +++ b/fclient/crosssigning_test.go @@ -7,85 +7,97 @@ import ( "github.com/matrix-org/gomatrixserverlib/spec" ) -func TestCrossSigningKeyEqual(t *testing.T) { - tests := []struct { - name string - s *CrossSigningKey - other *CrossSigningKey - expect bool - }{ - { - name: "NilReceiver_ReturnsFalse", - s: nil, - other: &CrossSigningKey{}, - expect: false, - }, - { - name: "NilOther_ReturnsFalse", - s: &CrossSigningKey{}, - other: nil, - expect: false, - }, - { - name: "DifferentUserID_ReturnsFalse", - s: &CrossSigningKey{UserID: "user1"}, - other: &CrossSigningKey{UserID: "user2"}, - expect: false, - }, - { - name: "DifferentUsageLength_ReturnsFalse", - s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}}, - other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster, CrossSigningKeyPurposeSelfSigning}}, - expect: false, - }, - { - name: "DifferentUsageValues_ReturnsFalse", - s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}}, - other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeSelfSigning}}, - expect: false, - }, - { - name: "DifferentKeysLength_ReturnsFalse", - s: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}}, - other: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}, "key2": {}}}, - expect: false, - }, - { - name: "DifferentKeysValues_ReturnsFalse", - s: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}}, - other: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {1}}}, - expect: false, - }, - { - name: "DifferentSignaturesLength_ReturnsFalse", - s: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}}, - other: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}, "sig2": {"key2": {}}}}, - expect: false, - }, - { - name: "DifferentSignaturesValues_ReturnsFalse", - s: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}}, - other: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {1}}}}, - expect: false, +var tests = []struct { + name string + s *CrossSigningKey + other *CrossSigningKey + expect bool +}{ + { + name: "NilReceiver_ReturnsFalse", + s: nil, + other: &CrossSigningKey{}, + expect: false, + }, + { + name: "NilOther_ReturnsFalse", + s: &CrossSigningKey{}, + other: nil, + expect: false, + }, + { + name: "DifferentUserID_ReturnsFalse", + s: &CrossSigningKey{UserID: "user1"}, + other: &CrossSigningKey{UserID: "user2"}, + expect: false, + }, + { + name: "DifferentUsageLength_ReturnsFalse", + s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}}, + other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster, CrossSigningKeyPurposeSelfSigning}}, + expect: false, + }, + { + name: "UnsortedUsages_ReturnsTrue", + s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeSelfSigning, CrossSigningKeyPurposeMaster}}, + other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster, CrossSigningKeyPurposeSelfSigning}}, + expect: true, + }, + { + name: "UnsortedUsages_ReturnsTrue", + s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeSelfSigning, CrossSigningKeyPurposeMaster}}, + other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeSelfSigning, CrossSigningKeyPurposeMaster}}, + expect: true, + }, + { + name: "DifferentUsageValues_ReturnsFalse", + s: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}}, + other: &CrossSigningKey{Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeSelfSigning}}, + expect: false, + }, + { + name: "DifferentKeysLength_ReturnsFalse", + s: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}}, + other: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}, "key2": {}}}, + expect: false, + }, + { + name: "DifferentKeysValues_ReturnsFalse", + s: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}}, + other: &CrossSigningKey{Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {1}}}, + expect: false, + }, + { + name: "DifferentSignaturesLength_ReturnsFalse", + s: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}}, + other: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}, "sig2": {"key2": {}}}}, + expect: false, + }, + { + name: "DifferentSignaturesValues_ReturnsFalse", + s: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}}, + other: &CrossSigningKey{Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {1}}}}, + expect: false, + }, + { + name: "IdenticalKeys_ReturnsTrue", + s: &CrossSigningKey{ + UserID: "user1", + Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}, + Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}, + Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}, }, - { - name: "IdenticalKeys_ReturnsTrue", - s: &CrossSigningKey{ - UserID: "user1", - Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}, - Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}, - Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}, - }, - other: &CrossSigningKey{ - UserID: "user1", - Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}, - Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}, - Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}, - }, - expect: true, + other: &CrossSigningKey{ + UserID: "user1", + Usage: []CrossSigningKeyPurpose{CrossSigningKeyPurposeMaster}, + Keys: map[gomatrixserverlib.KeyID]spec.Base64Bytes{"key1": {}}, + Signatures: map[string]map[gomatrixserverlib.KeyID]spec.Base64Bytes{"sig1": {"key1": {}}}, }, - } + expect: true, + }, +} +func TestCrossSigningKeyEqual(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if got := tt.s.Equal(tt.other); got != tt.expect { @@ -94,3 +106,14 @@ func TestCrossSigningKeyEqual(t *testing.T) { }) } } + +func BenchmarkEqual(b *testing.B) { + + for i := 0; i < b.N; i++ { + for _, tt := range tests { + if !tt.s.Equal(tt.other) && tt.expect { + b.Fatal(tt.name, tt.s) + } + } + } +}