Add validation against cross-namespace secret references

Add to BMH creation a validation, which checks if any secret references is from namespaces other than BMH's. This is to prevent the secret leakage when a user references secrets from other ns just to get the secrets out. Signed-off-by: Huy Mai <[email protected]>
metal3-io · Jan 21, 2025 · 505a3f5 · 505a3f5
1 parent af771b5
commit 505a3f5
Show file tree

Hide file tree

Showing 2 changed files with 133 additions and 0 deletions.
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_validation.go b/apis/metal3.io/v1alpha1/baremetalhost_validation.go
@@ -12,6 +12,9 @@ import (
 
 	"github.com/google/uuid"
 	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
+	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 var (
@@ -34,6 +37,8 @@ func (host *BareMetalHost) validateHost() []error {
 		}
 	}
 
+	errs = append(errs, host.validateCrossNamespaceSecretReferences()...)
+
 	if raidErrors := validateRAID(host.Spec.RAID); raidErrors != nil {
 		errs = append(errs, raidErrors...)
 	}
@@ -309,3 +314,39 @@ func validateRebootAnnotation(rebootAnnotation string) error {
 
 	return nil
 }
+
+// validateCrossNamespaceSecretReferences validates that a SecretReference does not refer to a Secret
+// in a different namespace than the host resource.
+func validateCrossNamespaceSecretReferences(hostNamespace, hostName, fieldName string, ref *corev1.SecretReference) error {
+	if ref != nil &&
+		ref.Namespace != "" &&
+		ref.Namespace != hostNamespace {
+		return k8serrors.NewForbidden(
+			schema.GroupResource{
+				Group:    "metal3.io",
+				Resource: "baremetalhosts",
+			},
+			hostName,
+			fmt.Errorf("%s: cross-namespace Secret references are not allowed", fieldName),
+		)
+	}
+	return nil
+}
+
+// validateCrossNamespaceSecretReferences checks all Secret references in the BareMetalHost spec
+// to ensure they do not reference Secrets from other namespaces. This includes userData,
+// networkData, and metaData Secret references.
+func (host *BareMetalHost) validateCrossNamespaceSecretReferences() []error {
+	secretRefs := map[*corev1.SecretReference]string{
+		host.Spec.UserData:    "userData",
+		host.Spec.NetworkData: "networkData",
+		host.Spec.MetaData:    "metaData",
+	}
+	errs := []error{}
+	for ref, fieldName := range secretRefs {
+		if err := validateCrossNamespaceSecretReferences(host.Namespace, host.Name, fieldName, ref); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	return errs
+}
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go b/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -678,6 +679,97 @@ func TestValidateCreate(t *testing.T) {
 			oldBMH:    nil,
 			wantedErr: "invalid value for the inspect.metal3.io annotation, allowed are \"disabled\" or \"\"",
 		},
+		{
+			name: "crossNamespaceUserData",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					UserData: &corev1.SecretReference{
+						Name:      "test-secret",
+						Namespace: "different-namespace", // Different from host's namespace
+					},
+				},
+			},
+			oldBMH:    nil,
+			wantedErr: "baremetalhosts.metal3.io \"test\" is forbidden: userData: cross-namespace Secret references are not allowed",
+		},
+		{
+			name: "crossNamespaceNetworkData",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					NetworkData: &corev1.SecretReference{
+						Name:      "test-secret",
+						Namespace: "different-namespace", // Different from host's namespace
+					},
+				},
+			},
+			oldBMH:    nil,
+			wantedErr: "baremetalhosts.metal3.io \"test\" is forbidden: networkData: cross-namespace Secret references are not allowed",
+		},
+		{
+			name: "crossNamespaceMetaData",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					MetaData: &corev1.SecretReference{
+						Name:      "test-secret",
+						Namespace: "different-namespace", // Different from host's namespace
+					},
+				},
+			},
+			oldBMH:    nil,
+			wantedErr: "baremetalhosts.metal3.io \"test\" is forbidden: metaData: cross-namespace Secret references are not allowed",
+		},
+		{
+			name: "multipleSecretsCrossNamespace",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					UserData: &corev1.SecretReference{
+						Name:      "test-secret1",
+						Namespace: "different-namespace1",
+					},
+					NetworkData: &corev1.SecretReference{
+						Name:      "test-secret2",
+						Namespace: "different-namespace2",
+					},
+					MetaData: &corev1.SecretReference{
+						Name:      "test-secret3",
+						Namespace: "different-namespace3",
+					},
+				},
+			},
+			oldBMH:    nil,
+			wantedErr: "baremetalhosts.metal3.io \"test\" is forbidden: userData: cross-namespace Secret references are not allowed", // Should catch at least one error
+		},
+		{
+			name: "sameNamespaceSecrets",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om, // namespace is "test-namespace"
+				Spec: BareMetalHostSpec{
+					UserData: &corev1.SecretReference{
+						Name:      "test-secret1",
+						Namespace: "test-namespace", // Same as host's namespace
+					},
+					NetworkData: &corev1.SecretReference{
+						Name:      "test-secret2",
+						Namespace: "test-namespace", // Same as host's namespace
+					},
+					MetaData: &corev1.SecretReference{
+						Name:      "test-secret3",
+						Namespace: "test-namespace", // Same as host's namespace
+					},
+				},
+			},
+			oldBMH:    nil,
+			wantedErr: "", // Should be valid
+		},
 	}
 
 	for _, tt := range tests {