Skip to content

Commit 06a1f14

Browse files
committed
✨Remove the use of the cryptographically weak MD5 hash
1 parent d0c68a3 commit 06a1f14

3 files changed

Lines changed: 130 additions & 6 deletions

File tree

hack/images/ci/conformance.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,12 @@ export TF_VAR_vsphere_network="sddc-cgw-network-3"
8181

8282
# The cluster name is a combination of the build ID and the first seven
8383
# characters of a hash of the job ID.
84-
CLUSTER_NAME="prow-$(echo "${BUILD_ID:-1}-${PROW_JOB_ID:-$(date +%s)}" | { md5sum 2>/dev/null || md5; } | awk '{print $1}' | cut -c-7)"
84+
if command -v python3 >/dev/null 2>&1; then
85+
SUFFIX=$(python3 -c "h = 2166136261; [h := ((h ^ b) * 16777619) & 0xffffffff for b in b\"${BUILD_ID:-1}-${PROW_JOB_ID:-$(date +%s)}\"]; print(f\"{h:08x}\")" | cut -c-7)
86+
else
87+
SUFFIX=$(echo "${BUILD_ID:-1}-${PROW_JOB_ID:-$(date +%s)}" | { sha256sum 2>/dev/null || shasum -a 256; } | awk '{print $1}' | cut -c-7)
88+
fi
89+
CLUSTER_NAME="prow-${SUFFIX}"
8590

8691
# Write information about the build out to disk.
8792
cat <<EOF >"${ARTIFACTS-}/build-info.json"

pkg/cloudprovider/vsphereparavirtual/vmservice/vmservice.go

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package vmservice
1818

1919
import (
2020
"context"
21-
"crypto/md5" // #nosec
21+
"crypto/sha256"
2222
"encoding/hex"
2323
"fmt"
2424
"reflect"
@@ -29,6 +29,7 @@ import (
2929
v1 "k8s.io/api/core/v1"
3030
apierrors "k8s.io/apimachinery/pkg/api/errors"
3131
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32+
"k8s.io/apimachinery/pkg/labels"
3233

3334
vmop "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmoperator"
3435
vmoptypes "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmoperator/types"
@@ -106,15 +107,50 @@ func NewVMService(vmClient vmop.Interface, ns string, ownerRef *metav1.OwnerRefe
106107
}
107108

108109
func (s *vmService) hashString(str string) string {
109-
// #nosec
110-
hash := md5.New()
110+
// SHA-256 is used as a FIPS-approved, well-distributed hash to derive a
111+
// deterministic name suffix. The output is later truncated to
112+
// MaxCheckSumLen; this is not a security-sensitive use.
113+
hash := sha256.New()
111114
if _, err := hash.Write([]byte(str)); err != nil {
112115
log.Error(err, "create hash string failed")
113116
}
114117

115118
return hex.EncodeToString(hash.Sum(nil))
116119
}
117120

121+
// findLegacyVMService locates a VirtualMachineService that was created by an
122+
// older release using a different name-hashing scheme. It matches on the
123+
// identifying labels that the CPI has always stamped on every
124+
// VirtualMachineService, so the lookup is independent of how the name was
125+
// generated. Returns nil when no matching resource exists.
126+
//
127+
// Any List error (including a missing "list" RBAC permission) is logged and
128+
// treated as "no legacy resource found" so that the primary, name-based flow is
129+
// never blocked. In particular, a freshly created Service has no
130+
// VirtualMachineService yet and must still be able to proceed to Create.
131+
func (s *vmService) findLegacyVMService(ctx context.Context, service *v1.Service, clusterName string) *vmoptypes.VirtualMachineServiceInfo {
132+
logger := log.WithValues("name", service.Name, "namespace", service.Namespace)
133+
134+
selector := labels.SelectorFromSet(labels.Set{
135+
LabelClusterNameKey: clusterName,
136+
LabelServiceNameKey: service.Name,
137+
LabelServiceNameSpaceKey: service.Namespace,
138+
}).String()
139+
140+
list, err := s.vmClient.VirtualMachineServices().List(ctx, s.namespace, vmoptypes.ListOptions{LabelSelector: selector})
141+
if err != nil {
142+
logger.Error(err, "failed to list VirtualMachineServices for legacy lookup; treating as not found")
143+
return nil
144+
}
145+
if len(list) == 0 {
146+
return nil
147+
}
148+
if len(list) > 1 {
149+
logger.V(2).Info("multiple VirtualMachineServices matched the legacy label lookup; using the first", "count", len(list))
150+
}
151+
return list[0]
152+
}
153+
118154
// GetVMServiceName returns VirtualMachineService name for a lb type of service
119155
func (s *vmService) GetVMServiceName(service *v1.Service, clusterName string) string {
120156
suffix := s.hashString(service.Name + "." + service.Namespace)
@@ -133,9 +169,17 @@ func (s *vmService) Get(ctx context.Context, service *v1.Service, clusterName st
133169
logger := log.WithValues("name", service.Name, "namespace", service.Namespace)
134170
logger.V(2).Info("Attempting to get VirtualMachineService")
135171

136-
vmService, err := s.vmClient.VirtualMachineServices().Get(ctx, s.namespace, s.GetVMServiceName(service, clusterName))
172+
name := s.GetVMServiceName(service, clusterName)
173+
vmService, err := s.vmClient.VirtualMachineServices().Get(ctx, s.namespace, name)
137174
if err != nil {
138175
if apierrors.IsNotFound(err) {
176+
// Fallback for resources created by an older release that used a
177+
// different name-hashing scheme. Located via the always-present
178+
// identifying labels rather than by recomputing the legacy name.
179+
if legacy := s.findLegacyVMService(ctx, service, clusterName); legacy != nil {
180+
logger.V(2).Info("VirtualMachineService not found by name; found legacy resource via labels", "legacyName", legacy.Name)
181+
return legacy, nil
182+
}
139183
return nil, nil
140184
}
141185
logger.Error(ErrGetVMService, fmt.Sprintf("%v", err))
@@ -304,8 +348,27 @@ func (s *vmService) Delete(ctx context.Context, service *v1.Service, clusterName
304348
logger := log.WithValues("name", service.Name, "namespace", service.Namespace)
305349
logger.V(2).Info("Attempting to delete VirtualMachineService")
306350

307-
err := s.vmClient.VirtualMachineServices().Delete(ctx, s.namespace, s.GetVMServiceName(service, clusterName))
351+
name := s.GetVMServiceName(service, clusterName)
352+
err := s.vmClient.VirtualMachineServices().Delete(ctx, s.namespace, name)
308353
if err != nil {
354+
if apierrors.IsNotFound(err) {
355+
// Fallback: a resource created by an older release lives under a
356+
// different name. Locate it via the identifying labels and delete it.
357+
legacy := s.findLegacyVMService(ctx, service, clusterName)
358+
if legacy == nil {
359+
return nil
360+
}
361+
logger.V(2).Info("VirtualMachineService not found by name; deleting legacy resource found via labels", "legacyName", legacy.Name)
362+
if derr := s.vmClient.VirtualMachineServices().Delete(ctx, s.namespace, legacy.Name); derr != nil {
363+
if apierrors.IsNotFound(derr) {
364+
return nil
365+
}
366+
logger.Error(ErrDeleteVMService, fmt.Sprintf("failed to delete legacy VirtualMachineService: %v", derr))
367+
return derr
368+
}
369+
logger.V(2).Info("Successfully deleted legacy VirtualMachineService")
370+
return nil
371+
}
309372
logger.Error(ErrDeleteVMService, fmt.Sprintf("%v", err))
310373
return err
311374
}

pkg/cloudprovider/vsphereparavirtual/vmservice/vmservice_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,62 @@ func TestGetVMService(t *testing.T) {
176176
assert.NoError(t, err)
177177
}
178178

179+
func TestGetVMService_LegacyFallback(t *testing.T) {
180+
testK8sService, vms, _ := initTest(testServiceAnnotationPropagationEnabled)
181+
k8sService := &v1.Service{
182+
ObjectMeta: metav1.ObjectMeta{
183+
Name: testK8sServiceName,
184+
Namespace: testK8sServiceNameSpace,
185+
},
186+
}
187+
188+
// Simulate a VirtualMachineService created by an older release: it lives
189+
// under a name that does NOT match the current (SHA-256-based) scheme, but it
190+
// carries the identifying labels that the CPI has always stamped on every
191+
// VirtualMachineService.
192+
legacyName := "legacy-" + testClustername + "-deadbeef"
193+
ports, _ := findPorts(testK8sService)
194+
legacyInfo := &vmoptypes.VirtualMachineServiceInfo{
195+
Name: legacyName,
196+
Namespace: testClusterNameSpace,
197+
Labels: map[string]string{
198+
LabelClusterNameKey: testClustername,
199+
LabelServiceNameKey: testK8sServiceName,
200+
LabelServiceNameSpaceKey: testK8sServiceNameSpace,
201+
},
202+
Spec: vmoptypes.VirtualMachineServiceSpec{
203+
Type: vmoptypes.VirtualMachineServiceTypeLoadBalancer,
204+
Ports: ports,
205+
Selector: map[string]string{
206+
ClusterSelectorKey: testClustername,
207+
NodeSelectorKey: NodeRole,
208+
},
209+
},
210+
}
211+
_, err := vms.(*vmService).vmClient.VirtualMachineServices().Create(context.Background(), legacyInfo)
212+
assert.NoError(t, err)
213+
214+
// Sanity check: the legacy resource is not reachable by the current name.
215+
currentName := vms.GetVMServiceName(k8sService, testClustername)
216+
assert.NotEqual(t, legacyName, currentName)
217+
218+
// Get must fall back to the label-based lookup and find the legacy resource.
219+
vmServiceObj, err := vms.Get(context.Background(), k8sService, testClustername)
220+
assert.NoError(t, err)
221+
assert.NotNil(t, vmServiceObj)
222+
assert.Equal(t, legacyName, vmServiceObj.Name)
223+
224+
// Delete must likewise fall back and remove the legacy resource.
225+
err = vms.Delete(context.Background(), k8sService, testClustername)
226+
assert.NoError(t, err)
227+
228+
// Verify it is indeed deleted.
229+
deletedObj, err := vms.(*vmService).vmClient.VirtualMachineServices().Get(context.Background(), testClusterNameSpace, legacyName)
230+
assert.Error(t, err)
231+
assert.True(t, apierrors.IsNotFound(err))
232+
assert.Nil(t, deletedObj)
233+
}
234+
179235
func TestCreateVMService(t *testing.T) {
180236
testK8sService, vms, _ := initTest(testServiceAnnotationPropagationEnabled)
181237
ports, _ := findPorts(testK8sService)

0 commit comments

Comments
 (0)