Repository: metal3-io/baremetal-operator
Pull request #1931 (2024-09-03 05:36:39)

🐛 Disallow fetching secrets from namespaces different from the host's one
The BareMetalHost CRD allows the UserData, MetaData, and NetworkData for the provisioned host to be specified as links to k8s Secrets. There are fields for both the Name and Namespace of the Secret, meaning that the baremetal-operator will read a Secret from any namespace. If a Secret contains the key "value" (or "userData", "metaData", or "networkData"), its corresponding value can be exfiltrated by a user provisioning a Host pointing to that Secret, then retrieving that data from the provisioned host.

Authored-by: Zane Bitter <zbitter@redhat.com>



Patch:
diff --git a/controllers/metal3.io/host_config_data.go b/controllers/metal3.io/host_config_data.go
index 0a298e24..037a0372 100644
--- a/controllers/metal3.io/host_config_data.go
+++ b/controllers/metal3.io/host_config_data.go
@@ -2,11 +2,11 @@ package controllers
 
 import (
 	"github.com/go-logr/logr"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
-
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	"github.com/metal3-io/baremetal-operator/pkg/secretutils"
+	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 // hostConfigData is an implementation of host configuration data interface.
@@ -21,6 +21,10 @@ type hostConfigData struct {
 // parameter to detirmine which data to return in case secret contins multiple
 // keys.
 func (hcd *hostConfigData) getSecretData(name, namespace, dataKey string) (string, error) {
+	if namespace != hcd.host.Namespace {
+		return "", errors.Errorf("%s secret must be in same namespace as host %s/%s", dataKey, hcd.host.Namespace, hcd.host.Name)
+	}
+
 	key := types.NamespacedName{
 		Name:      name,
 		Namespace: namespace,


Test patch:
diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go
index 52c35151..4dc0655d 100644
--- a/controllers/metal3.io/baremetalhost_controller_test.go
+++ b/controllers/metal3.io/baremetalhost_controller_test.go
@@ -41,6 +41,10 @@ const (
 )
 
 func newSecret(name string, data map[string]string) *corev1.Secret {
+	return newSecretInNamespace(name, namespace, data)
+}
+
+func newSecretInNamespace(name, namespace string, data map[string]string) *corev1.Secret {
 	secretData := make(map[string][]byte)
 	for k, v := range data {
 		secretData[k] = []byte(base64.StdEncoding.EncodeToString([]byte(v)))
diff --git a/controllers/metal3.io/host_config_data_test.go b/controllers/metal3.io/host_config_data_test.go
index f763a6e8..4ecc5a6d 100644
--- a/controllers/metal3.io/host_config_data_test.go
+++ b/controllers/metal3.io/host_config_data_test.go
@@ -324,6 +324,54 @@ func TestProvisionWithHostConfig(t *testing.T) {
 			ExpectedNetworkData: "",
 			ErrNetworkData:      true,
 		},
+		{
+			Scenario: "user-data secret in different namespace",
+			Host: newHost("host-user-data",
+				&metal3api.BareMetalHostSpec{
+					BMC: metal3api.BMCDetails{
+						Address:         "ipmi://192.168.122.1:6233",
+						CredentialsName: defaultSecretName,
+					},
+					UserData: &corev1.SecretReference{
+						Name:      "user-data",
+						Namespace: "other-namespace",
+					},
+				}),
+			UserDataSecret: newSecretInNamespace("user-data", "other-namespace", map[string]string{"userData": "somedata"}),
+			ErrUserData:    true,
+		},
+		{
+			Scenario: "meta-data secret in different namespace",
+			Host: newHost("host-user-data",
+				&metal3api.BareMetalHostSpec{
+					BMC: metal3api.BMCDetails{
+						Address:         "ipmi://192.168.122.1:6233",
+						CredentialsName: defaultSecretName,
+					},
+					MetaData: &corev1.SecretReference{
+						Name:      "meta-data",
+						Namespace: "other-namespace",
+					},
+				}),
+			NetworkDataSecret: newSecretInNamespace("meta-data", "other-namespace", map[string]string{"metaData": "key: value"}),
+			ErrMetaData:       true,
+		},
+		{
+			Scenario: "network-data secret in different namespace",
+			Host: newHost("host-user-data",
+				&metal3api.BareMetalHostSpec{
+					BMC: metal3api.BMCDetails{
+						Address:         "ipmi://192.168.122.1:6233",
+						CredentialsName: defaultSecretName,
+					},
+					NetworkData: &corev1.SecretReference{
+						Name:      "net-data",
+						Namespace: "other-namespace",
+					},
+				}),
+			NetworkDataSecret: newSecretInNamespace("net-data", "other-namespace", map[string]string{"networkData": "key: value"}),
+			ErrNetworkData:    true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -379,7 +427,7 @@ func TestProvisionWithHostConfig(t *testing.T) {
 			}
 
 			if actualMetaData != tc.ExpectedMetaData {
-				t.Fatal(fmt.Errorf("Failed to assert MetaData. Expected '%s' got '%s'", actualMetaData, tc.ExpectedMetaData))
+				t.Fatal(fmt.Errorf("Failed to assert MetaData. Expected '%s' got '%s'", tc.ExpectedMetaData, actualMetaData))
 			}
 		})
 	}