Merge pull request #1676 from aramase/aramase/b/sha_logic

fix: update the hash generation logic

Author: k8s-ci-robot

pkg/util/secretutil/secret.go

@@ -23,15 +23,16 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
-	"io"
+	"math"
 	"os"
-	"sort"
 	"strings"
 
 	secretsstorev1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1"
 
+	"golang.org/x/crypto/cryptobyte"
 	"golang.org/x/crypto/pkcs12"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 const (
@@ -217,23 +218,33 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType
 
 // GetSHAFromSecret gets SHA for the secret data
 func GetSHAFromSecret(data map[string][]byte) (string, error) {
-	var values []string
-	for k, v := range data {
-		values = append(values, k+"="+string(v))
-	}
-	// sort the values to always obtain a deterministic SHA for
-	// same content in different order
-	sort.Strings(values)
-	return generateSHA(strings.Join(values, ";"))
-}
+	if len(data) == 0 {
+		return "", nil
+	}
+
+	b := cryptobyte.NewBuilder(nil)
+	if len(data) > math.MaxUint32 {
+		return "", fmt.Errorf("data too large: length exceeds uint32 max")
+	}
+	// we are checking the length of the data to be less than uint32 max
+	// so we can safely cast it to uint32 without worrying about overflow
+	b.AddUint32(uint32(len(data))) // nolint:gosec
 
-// generateSHA generates SHA from string
-func generateSHA(data string) (string, error) {
-	hasher := sha256.New()
-	_, err := io.WriteString(hasher, data)
+	keys := sets.StringKeySet(data).List()
+
+	for _, k := range keys {
+		b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+			b.AddBytes([]byte(k))
+		})
+		b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+			b.AddBytes(data[k])
+		})
+	}
+
+	hashData, err := b.Bytes()
 	if err != nil {
 		return "", err
 	}
-	sha := hasher.Sum(nil)
-	return fmt.Sprintf("%x", sha), nil
+
+	return fmt.Sprintf("%x", sha256.Sum256(hashData)), nil
 }

pkg/util/secretutil/secret_test.go

@@ -412,6 +412,16 @@ func TestGenerateSHAFromSecret(t *testing.T) {
 			data2:            map[string][]byte{"key2": []byte("value2"), "key1": []byte("value1")},
 			expectedSHAMatch: true,
 		},
+		{
+			name: "different keys with the same concatenated result but should not match",
+			data1: map[string][]byte{
+				"key1": []byte("=value1"),
+			},
+			data2: map[string][]byte{
+				"key1=": []byte("value1"),
+			},
+			expectedSHAMatch: false,
+		},
 	}
 
 	for _, test := range tests {