Skip to content

Commit

Permalink
sort certificates in bundles to ensure deterministic behaviour
Browse files Browse the repository at this point in the history 
Signed-off-by: Jan Kantert <[email protected]>
  • Loading branch information
@jabdoa2
jabdoa2 committed Jul 9, 2024
1 parent ee6582a commit 8a9ce5f
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 21 deletions.
12 changes: 6 additions & 6 deletions pkg/bundle/bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1007,9 +1007,9 @@ func Test_Reconcile(t *testing.T) {
expResult: ctrl.Result{},
expError: false,
expPatches: []interface{}{
configMapPatch(baseBundle.Name, trustNamespace, map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2, dummy.TestCertificate3, dummy.TestCertificate5)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-1", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2, dummy.TestCertificate3, dummy.TestCertificate5)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-2", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2, dummy.TestCertificate3, dummy.TestCertificate5)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, trustNamespace, map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3, dummy.TestCertificate5)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-1", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3, dummy.TestCertificate5)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-2", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3, dummy.TestCertificate5)}, nil, ptr.To(targetKey)),
},
expBundlePatch: &trustapi.BundleStatus{
Conditions: []trustapi.BundleCondition{
Expand Down Expand Up @@ -1266,9 +1266,9 @@ func Test_Reconcile(t *testing.T) {
expResult: ctrl.Result{},
expError: false,
expPatches: []interface{}{
configMapPatch(baseBundle.Name, trustNamespace, map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2, dummy.TestCertificate3)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-1", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2, dummy.TestCertificate3)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-2", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2, dummy.TestCertificate3)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, trustNamespace, map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-1", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3)}, nil, ptr.To(targetKey)),
configMapPatch(baseBundle.Name, "ns-2", map[string]string{targetKey: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3)}, nil, ptr.To(targetKey)),
},
},
}
Expand Down
43 changes: 35 additions & 8 deletions pkg/bundle/source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"encoding/hex"
"encoding/pem"
"fmt"
"sort"
"strings"

jks "github.com/pavlo-v-chernykh/keystore-go/v4"
Expand Down Expand Up @@ -114,7 +115,7 @@ func (b *bundle) buildSourceBundle(ctx context.Context, sources []trustapi.Bundl
return bundleData{}, fmt.Errorf("couldn't find any valid certificates in bundle")
}

deduplicatedBundles, err := deduplicateBundles(bundles)
deduplicatedBundles, err := deduplicateAndSortBundles(bundles)
if err != nil {
return bundleData{}, err
}
Expand Down Expand Up @@ -343,12 +344,29 @@ func (b *bundleData) populateData(bundles []string, formats *trustapi.Additional
return nil
}

// remove duplicate certificates from bundles
func deduplicateBundles(bundles []string) ([]string, error) {
// ByHash is a type alias for sorting the slice of [32]byte arrays
type ByHash [][32]byte

// Len returns the length of the slice
func (a ByHash) Len() int {
return len(a)
}

// Swap swaps two elements in the slice
func (a ByHash) Swap(i, j int) {
a[i], a[j] = a[j], a[i]
}

// Less compares two elements in the slice
func (a ByHash) Less(i, j int) bool {
return bytes.Compare(a[i][:], a[j][:]) == -1
}

// remove duplicate certificates from bundles and sort certificates by hash
func deduplicateAndSortBundles(bundles []string) ([]string, error) {
var block *pem.Block

var certificatesHashes = make(map[[32]byte]struct{})
var dedupCerts []string
var certificatesHashes = make(map[[32]byte]string)

for _, cert := range bundles {
certBytes := []byte(cert)
Expand All @@ -369,12 +387,21 @@ func deduplicateBundles(bundles []string) ([]string, error) {
// check existence of the hash
if _, ok := certificatesHashes[hash]; !ok {
// neew to trim a newline which is added by Encoder
dedupCerts = append(dedupCerts, string(bytes.Trim(pem.EncodeToMemory(block), "\n")))
certificatesHashes[hash] = struct{}{}
certificatesHashes[hash] = string(bytes.Trim(pem.EncodeToMemory(block), "\n"))
}
}
}

var orderedKeys [][32]byte
for key := range certificatesHashes {
orderedKeys = append(orderedKeys, key)
}
sort.Sort(ByHash(orderedKeys))

var sortedDeduplicatedCerts []string
for _, key := range orderedKeys {
sortedDeduplicatedCerts = append(sortedDeduplicatedCerts, certificatesHashes[key])
}

return dedupCerts, nil
return sortedDeduplicatedCerts, nil
}
25 changes: 19 additions & 6 deletions pkg/bundle/source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func Test_buildSourceBundle(t *testing.T) {
{InLine: ptr.To(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2 + "\n\n")},
},
objects: []runtime.Object{},
expData: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2),
expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1),
expError: false,
expNotFoundError: false,
},
Expand Down Expand Up @@ -98,7 +98,20 @@ func Test_buildSourceBundle(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
Data: map[string]string{"key": dummy.TestCertificate1 + "\n" + dummy.TestCertificate2},
}},
expData: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2),
expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1),
expError: false,
expNotFoundError: false,
},
"if single ConfigMap source, return data even when order changes": {
// Test uses the same data as the previous one but with different order
sources: []trustapi.BundleSource{
{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
},
objects: []runtime.Object{&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
Data: map[string]string{"key": dummy.TestCertificate2 + "\n" + dummy.TestCertificate1},
}},
expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1),
expError: false,
expNotFoundError: false,
},
Expand All @@ -111,7 +124,7 @@ func Test_buildSourceBundle(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
Data: map[string]string{"key": dummy.TestCertificate1},
}},
expData: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2),
expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1),
expError: false,
expNotFoundError: false,
},
Expand Down Expand Up @@ -141,7 +154,7 @@ func Test_buildSourceBundle(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "secret"},
Data: map[string][]byte{"key": []byte(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2)},
}},
expData: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2),
expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1),
expError: false,
expNotFoundError: false,
},
Expand Down Expand Up @@ -174,7 +187,7 @@ func Test_buildSourceBundle(t *testing.T) {
Data: map[string][]byte{"key": []byte(dummy.TestCertificate2)},
},
},
expData: dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate3, dummy.TestCertificate2),
expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3),
expError: false,
expNotFoundError: false,
},
Expand Down Expand Up @@ -499,7 +512,7 @@ func TestBundlesDeduplication(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

resultBundle, err := deduplicateBundles(test.bundle)
resultBundle, err := deduplicateAndSortBundles(test.bundle)

assert.Nil(t, err)

Expand Down
2 changes: 1 addition & 1 deletion test/dummy/certificates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,8 @@ z40l74JcR+GvcFZWz7/jmJq95YMZ7LawLAr1CaAXxCwsoLbJpbgg4lVo6odACzY=

func DefaultJoinedCerts() string {
return JoinCerts(
TestCertificate1,
TestCertificate2,
TestCertificate1,
TestCertificate3,
)
}
Expand Down

0 comments on commit 8a9ce5f

Please sign in to comment.