Merge pull request kubernetes#133511 from BenTheElder/no-md5

k8s-ci-robot · web-flow · commit 60e1f61a6361 · 2025-10-24T10:59:35.000-07:00
eliminate md5 usage, block new usage
diff --git a/cmd/kubeadm/app/util/staticpod/utils.go b/cmd/kubeadm/app/util/staticpod/utils.go
@@ -18,9 +18,9 @@ package staticpod
 
 import (
 	"bytes"
-	"crypto/md5"
 	"fmt"
 	"hash"
+	"hash/fnv"
 	"io"
 	"math"
 	"net/url"
@@ -373,7 +373,7 @@ func ManifestFilesAreEqual(path1, path2 string) (bool, string, error) {
 		return false, "", err
 	}
 
-	hasher := md5.New()
+	hasher := fnv.New128a()
 	DeepHashObject(hasher, pod1)
 	hash1 := hasher.Sum(nil)[0:]
 	DeepHashObject(hasher, pod2)
diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml
@@ -385,6 +385,9 @@ linters:
     forbidigo:
       analyze-types: true
       forbid:
+        - pattern: md5\.*
+          # https://github.com/kubernetes/kubernetes/issues/129652
+          msg: md5 is oudated, insecure, prefer a secure hash (such as sha256) or a non-cryptographic hash
         - pattern: ^managedfields\.ExtractInto$
           pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$
           msg: should not be used because managedFields was removed
diff --git a/hack/golangci.yaml b/hack/golangci.yaml
@@ -394,6 +394,9 @@ linters:
     forbidigo:
       analyze-types: true
       forbid:
+        - pattern: md5\.*
+          # https://github.com/kubernetes/kubernetes/issues/129652
+          msg: md5 is oudated, insecure, prefer a secure hash (such as sha256) or a non-cryptographic hash
         - pattern: ^managedfields\.ExtractInto$
           pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$
           msg: should not be used because managedFields was removed
diff --git a/hack/golangci.yaml.in b/hack/golangci.yaml.in
@@ -214,6 +214,9 @@ linters:
     forbidigo:
       analyze-types: true
       forbid:
+        - pattern: md5\.*
+          # https://github.com/kubernetes/kubernetes/issues/129652
+          msg: md5 is oudated, insecure, prefer a secure hash (such as sha256) or a non-cryptographic hash
         - pattern: ^managedfields\.ExtractInto$
           pkg: ^k8s\.io/apimachinery/pkg/util/managedfields$
           msg: should not be used because managedFields was removed
diff --git a/pkg/api/v1/endpoints/util.go b/pkg/api/v1/endpoints/util.go
@@ -18,12 +18,12 @@ package endpoints
 
 import (
 	"bytes"
-	"crypto/md5"
 	"encoding/hex"
 	"hash"
+	"hash/fnv"
 	"sort"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	hashutil "k8s.io/kubernetes/pkg/util/hash"
 )
@@ -154,7 +154,7 @@ func hashAddresses(addrs addressSet) string {
 		slice = append(slice, addrReady{k, ready})
 	}
 	sort.Sort(addrsReady(slice))
-	hasher := md5.New()
+	hasher := fnv.New128a()
 	hashutil.DeepHashObject(hasher, slice)
 	return hex.EncodeToString(hasher.Sum(nil)[0:])
 }
@@ -210,7 +210,7 @@ type subsetsByHash []v1.EndpointSubset
 func (sl subsetsByHash) Len() int      { return len(sl) }
 func (sl subsetsByHash) Swap(i, j int) { sl[i], sl[j] = sl[j], sl[i] }
 func (sl subsetsByHash) Less(i, j int) bool {
-	hasher := md5.New()
+	hasher := fnv.New128a()
 	h1 := hashObject(hasher, sl[i])
 	h2 := hashObject(hasher, sl[j])
 	return bytes.Compare(h1, h2) < 0
@@ -229,7 +229,7 @@ type portsByHash []v1.EndpointPort
 func (sl portsByHash) Len() int      { return len(sl) }
 func (sl portsByHash) Swap(i, j int) { sl[i], sl[j] = sl[j], sl[i] }
 func (sl portsByHash) Less(i, j int) bool {
-	hasher := md5.New()
+	hasher := fnv.New128a()
 	h1 := hashObject(hasher, sl[i])
 	h2 := hashObject(hasher, sl[j])
 	return bytes.Compare(h1, h2) < 0
diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go
@@ -452,7 +452,10 @@ func (e *Controller) syncService(ctx context.Context, key string) error {
 	// When comparing the subsets, we ignore the difference in ResourceVersion of Pod to avoid unnecessary Endpoints
 	// updates caused by Pod updates that we don't care, e.g. annotation update.
 	if !createEndpoints &&
-		endpointSubsetsEqualIgnoreResourceVersion(currentEndpoints.Subsets, subsets) &&
+		(endpointSubsetsEqualIgnoreResourceVersion(currentEndpoints.Subsets, subsets) ||
+			// If the comparison fails, try again after repacking, it may be a difference in the hash algorithm
+			// For more context: https://github.com/kubernetes/kubernetes/issues/129652#issuecomment-3264035333
+			endpointSubsetsEqualIgnoreResourceVersion(endpoints.RepackSubsets(currentEndpoints.Subsets), subsets)) &&
 		labelsCorrectForEndpoints(currentEndpoints.Labels, service.Labels) &&
 		capacityAnnotationSetCorrectly(currentEndpoints.Annotations, currentEndpoints.Subsets) {
 		logger.V(5).Info("endpoints are equal, skipping update", "service", klog.KObj(service))
diff --git a/pkg/controller/endpoint/endpoints_controller_test.go b/pkg/controller/endpoint/endpoints_controller_test.go
@@ -1008,6 +1008,63 @@ func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) {
 	endpointsHandler.ValidateRequestCount(t, 0)
 }
 
+func TestSyncEndpointsItemsPreexistingIdenticalUnsorted(t *testing.T) {
+	ns := metav1.NamespaceDefault
+	testServer, endpointsHandler := makeTestServer(t, ns)
+	defer testServer.Close()
+	tCtx := ktesting.Init(t)
+	endpoints := newController(tCtx, testServer.URL, 0*time.Second)
+	pod0 := testPod(ns, 0, 1, true, ipv4only)
+	pod1 := testPod(ns, 1, 1, true, ipv4only)
+
+	subsets := []v1.EndpointSubset{{
+		Addresses: []v1.EndpointAddress{
+			// known to not be packed correctly according to the current hash
+			{IP: pod1.Status.PodIPs[0].IP, NodeName: &emptyNodeName, TargetRef: &v1.ObjectReference{Kind: "Pod", Name: pod1.Name, Namespace: ns}},
+			{IP: pod0.Status.PodIPs[0].IP, NodeName: &emptyNodeName, TargetRef: &v1.ObjectReference{Kind: "Pod", Name: pod0.Name, Namespace: ns}},
+		},
+		Ports: []v1.EndpointPort{{Port: 8080, Protocol: "TCP"}},
+	}}
+
+	// Assert that endpoints are not repacked correctly according to the current hash.
+	// We want to prove that this does not cause a re-sync.
+	repacked := endptspkg.RepackSubsets(subsets)
+	if reflect.DeepEqual(subsets, repacked) {
+		t.Errorf("subsets were already in sorted order")
+	}
+
+	endpoints.endpointsStore.Add(&v1.Endpoints{
+		ObjectMeta: metav1.ObjectMeta{
+			ResourceVersion: "1",
+			Name:            "foo",
+			Namespace:       ns,
+			Labels: map[string]string{
+				LabelManagedBy: ControllerName,
+			},
+		},
+		Subsets: subsets,
+	})
+
+	endpoints.podStore.Add(pod0)
+	endpoints.podStore.Add(pod1)
+
+	endpoints.serviceStore.Add(&v1.Service{
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: metav1.NamespaceDefault},
+		Spec: v1.ServiceSpec{
+			Selector:   map[string]string{"foo": "bar"},
+			Ports:      []v1.ServicePort{{Port: 80, Protocol: "TCP", TargetPort: intstr.FromInt32(8080)}},
+			IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
+		},
+	})
+	err := endpoints.syncService(tCtx, ns+"/foo")
+	if err != nil {
+		t.Errorf("Unexpected error syncing service %v", err)
+	}
+
+	// syncing should've been a no-op
+	endpointsHandler.ValidateRequestCount(t, 0)
+}
+
 func TestSyncEndpointsItems(t *testing.T) {
 	ns := "other"
 	testServer, endpointsHandler := makeTestServer(t, ns)
diff --git a/pkg/kubelet/config/common.go b/pkg/kubelet/config/common.go
@@ -17,10 +17,10 @@ limitations under the License.
 package config
 
 import (
-	"crypto/md5"
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"hash/fnv"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
@@ -59,7 +59,7 @@ func generatePodName(name string, nodeName types.NodeName) string {
 
 func applyDefaults(logger klog.Logger, pod *api.Pod, source string, isFile bool, nodeName types.NodeName) error {
 	if len(pod.UID) == 0 {
-		hasher := md5.New()
+		hasher := fnv.New128a()
 		hash.DeepHashObject(hasher, pod)
 		// DeepHashObject resets the hash, so we should write the pod source
 		// information AFTER it.
diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go
@@ -18,8 +18,8 @@ package reconciler
 
 import (
 	"context"
-	"crypto/md5"
 	"fmt"
+	"hash/fnv"
 	"path/filepath"
 	"sync"
 	"testing"
@@ -1523,13 +1523,14 @@ func Test_UncertainDeviceGlobalMounts(t *testing.T) {
 
 	modes := []v1.PersistentVolumeMode{v1.PersistentVolumeBlock, v1.PersistentVolumeFilesystem}
 
+	hasher := fnv.New128a()
 	for modeIndex := range modes {
 		for tcIndex := range tests {
 			mode := modes[modeIndex]
 			tc := tests[tcIndex]
 			testName := fmt.Sprintf("%s [%s]", tc.name, mode)
 			uniqueTestString := fmt.Sprintf("global-mount-%s", testName)
-			uniquePodDir := fmt.Sprintf("%s-%x", kubeletPodsDir, md5.Sum([]byte(uniqueTestString)))
+			uniquePodDir := fmt.Sprintf("%s-%x", kubeletPodsDir, hasher.Sum([]byte(uniqueTestString)))
 			t.Run(testName+"[", func(t *testing.T) {
 				t.Parallel()
 				pv := &v1.PersistentVolume{
@@ -1736,13 +1737,14 @@ func Test_UncertainVolumeMountState(t *testing.T) {
 	}
 	modes := []v1.PersistentVolumeMode{v1.PersistentVolumeBlock, v1.PersistentVolumeFilesystem}
 
+	hasher := fnv.New128a()
 	for modeIndex := range modes {
 		for tcIndex := range tests {
 			mode := modes[modeIndex]
 			tc := tests[tcIndex]
 			testName := fmt.Sprintf("%s [%s]", tc.name, mode)
 			uniqueTestString := fmt.Sprintf("local-mount-%s", testName)
-			uniquePodDir := fmt.Sprintf("%s-%x", kubeletPodsDir, md5.Sum([]byte(uniqueTestString)))
+			uniquePodDir := fmt.Sprintf("%s-%x", kubeletPodsDir, hasher.Sum([]byte(uniqueTestString)))
 			t.Run(testName, func(t *testing.T) {
 				t.Parallel()
 				pv := &v1.PersistentVolume{
diff --git a/staging/src/k8s.io/endpointslice/util/controller_utils.go b/staging/src/k8s.io/endpointslice/util/controller_utils.go
@@ -17,10 +17,10 @@ limitations under the License.
 package util
 
 import (
-	"crypto/md5"
 	"encoding/hex"
 	"fmt"
 	"hash"
+	"hash/fnv"
 	"reflect"
 	"sort"
 
@@ -165,7 +165,7 @@ func NewPortMapKey(endpointPorts []discovery.EndpointPort) PortMapKey {
 
 // deepHashObjectToString creates a unique hash string from a go object.
 func deepHashObjectToString(objectToWrite interface{}) string {
-	hasher := md5.New()
+	hasher := fnv.New128a()
 	deepHashObject(hasher, objectToWrite)
 	return hex.EncodeToString(hasher.Sum(nil)[0:])
 }
diff --git a/staging/src/k8s.io/kubectl/pkg/util/util.go b/staging/src/k8s.io/kubectl/pkg/util/util.go
@@ -17,7 +17,6 @@ limitations under the License.
 package util
 
 import (
-	"crypto/md5"
 	"errors"
 	"fmt"
 	"path"
@@ -26,7 +25,6 @@ import (
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 )
 
 // ParseRFC3339 parses an RFC3339 date in either RFC3339Nano or RFC3339 format.
@@ -41,15 +39,6 @@ func ParseRFC3339(s string, nowFn func() metav1.Time) (metav1.Time, error) {
 	return metav1.Time{Time: t}, nil
 }
 
-// HashObject returns the hash of a Object hash by a Codec
-func HashObject(obj runtime.Object, codec runtime.Codec) (string, error) {
-	data, err := runtime.Encode(codec, obj)
-	if err != nil {
-		return "", err
-	}
-	return fmt.Sprintf("%x", md5.Sum(data)), nil
-}
-
 // ParseFileSource parses the source given.
 //
 //	Acceptable formats include:
diff --git a/test/e2e/storage/testsuites/volume_io.go b/test/e2e/storage/testsuites/volume_io.go
@@ -44,13 +44,13 @@ import (
 	admissionapi "k8s.io/pod-security-admission/api"
 )
 
-// MD5 hashes of the test file corresponding to each file size.
+// sha256 hashes of the test file corresponding to each file size.
 // Test files are generated in testVolumeIO()
 // If test file generation algorithm changes, these must be recomputed.
-var md5hashes = map[int64]string{
-	storageframework.FileSizeSmall:  "5c34c2813223a7ca05a3c2f38c0d1710",
-	storageframework.FileSizeMedium: "f2fa202b1ffeedda5f3a58bd1ae81104",
-	storageframework.FileSizeLarge:  "8d763edc71bd16217664793b5a15e403",
+var sha256sum = map[int64]string{
+	storageframework.FileSizeSmall:  "aa39c2cbd49a28425c977d229b43e1ff9579ecd98cea7b466982bab04753f2ab",
+	storageframework.FileSizeMedium: "5f718f3b7788352e1450c6c14c6051f0298d49ac6af18c5a1ce3cd3b5947de24",
+	storageframework.FileSizeLarge:  "11b058d55b088031fbdec33e3e896c032550ff20ca0ed37f76ad1d5d155c5f50",
 }
 
 const mountPath = "/opt"
@@ -270,18 +270,18 @@ func verifyFile(ctx context.Context, f *framework.Framework, pod *v1.Pod, fpath
 	}
 
 	ginkgo.By("verifying file hash")
-	rtnstr, stderr, err = e2epod.ExecShellInPodWithFullOutput(ctx, f, pod.Name, fmt.Sprintf("md5sum %s | cut -d' ' -f1", fpath))
+	rtnstr, stderr, err = e2epod.ExecShellInPodWithFullOutput(ctx, f, pod.Name, fmt.Sprintf("sha256sum %s | cut -d' ' -f1", fpath))
 	if err != nil {
-		return fmt.Errorf("unable to test file hash via `md5sum %s`: %v\nstdout: %s\nstderr: %s", fpath, err, rtnstr, stderr)
+		return fmt.Errorf("unable to test file hash via `sha256sum %s`: %v\nstdout: %s\nstderr: %s", fpath, err, rtnstr, stderr)
 	}
 	actualHash := strings.TrimSuffix(rtnstr, "\n")
-	expectedHash, ok := md5hashes[expectSize]
+	expectedHash, ok := sha256sum[expectSize]
 	if !ok {
 		return fmt.Errorf("file hash is unknown for file size %d. Was a new file size added to the test suite?",
 			expectSize)
 	}
 	if actualHash != expectedHash {
-		return fmt.Errorf("MD5 hash is incorrect for file %s with size %d. Expected: `%s`; Actual: `%s`",
+		return fmt.Errorf("SHA256 hash is incorrect for file %s with size %d. Expected: `%s`; Actual: `%s`",
 			fpath, expectSize, expectedHash, actualHash)
 	}
 
