[Draft]: pod restart annotation name conflict vulnerability

[whg517]

background

After the pod uses tls to assign the certificate, secret-csi updates the expiration time of the certificate to the pod annotation. commons-operator's pod-restarter coordinator deletes the pod when it detects that time has expired. The workload controller automatically creates new Pods, and the corresponding tls is regenerated.

desc

The current implementation of the expiration time storage policy is to set the expiration time in the tls volume pod annotation, the key value of the annotation is restarter.zncdata.dev/expires-at.<volume hash>. The policy is to remove the 16-bit length of secrets.zncdata.dev/volume: and volumeID after sha256, then take the first 16 digits

Since the k8s annotation key length is 63, the above strategy is adopted to ensure the readability of the key value and its correlation with the volume. But that led to a crash hole.

// updatePod updates the pod annotation with the secret expiration time.
// The volume ID is hashed using sha256, and the first 16 bytes are used as the volume tag.
// Then, the expiration time is written to the pod annotation with the key "secrets.zncdata.dev/restarter-expires-at:<volume_tag>".
//
// Considering the length limitation of Kubernetes annotations, we hash the volume ID to maintain the readability of the annotation
// and its association with the volume. However, truncating the hash to the first 16 bytes may introduce collision risks.
func (n *NodeServer) updatePod(ctx context.Context, pod *corev1.Pod, volume_id string, expiresTime *time.Time) error {
	if pod.Annotations == nil {
		pod.Annotations = make(map[string]string)
	}
	patch := client.MergeFrom(pod.DeepCopy())
	if expiresTime == nil {
		logger.V(5).Info("Expiration time is nil, skip update pod annotation", "pod", pod.Name)
		return nil
	}

	volume_tag_hash := sha256.New()
	volume_tag_hash.Write([]byte("secrets.zncdata.dev/volume:"))
	volume_tag_hash.Write([]byte(volume_id))
	volume_tag := volume_tag_hash.Sum(nil)
	// get 16 bytes of volume tag, but it maybe cause collision vulnerability
	volume_tag = volume_tag[:16]

	annotationexpiresName := constants.PrefixLabelRestarterExpiresAt + string(volume_tag)
	expiresTimeStr := expiresTime.Format(time.RFC3339)

	pod.Annotations[annotationexpiresName] = expiresTimeStr

	if err := n.client.Patch(ctx, pod, patch); err != nil {
		return err
	}
	logger.V(5).Info("Pod patched", "pod", pod.Name)
	return nil
}

suggest

• Consider using fixed keys to compare the time before each annotation update, refreshing the value if the expiration time to be updated is earlier than the existing expiration time, and not updating otherwise. eg: restarter.zncdata.dev/expires: 2006-01-02T15:04:05Z07:00
• Using fixed keys, the volume and the corresponding expiration time are stored as values in a json data structure or base64 serialized. eg: field.cattle.io/publicEndpoints: >- [{"addresses":["192.168.9.200"],"port":80,"protocol":"HTTP","serviceName":"zncdata-devops-mgmt:alist","ingressName":"zncdata-devops-mgmt:alist","hostname":"alist.zncdata.net","path":"/","allNodes":false}]

[whg517]

@lwpk110 @tutunannan please review it