From 64eb7cfa8eead3601b1e99e236db5873387774c8 Mon Sep 17 00:00:00 2001 From: Tuomo Tanskanen Date: Fri, 23 Aug 2024 10:48:59 +0300 Subject: [PATCH] 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 Co-Authored-By: Dmitry Tantsur Signed-off-by: Tuomo Tanskanen --- .../baremetalhost_controller_test.go | 4 ++ controllers/metal3.io/host_config_data.go | 5 ++ .../metal3.io/host_config_data_test.go | 50 ++++++++++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go index cba36eb4ea..e0b5eba0d8 100644 --- a/controllers/metal3.io/baremetalhost_controller_test.go +++ b/controllers/metal3.io/baremetalhost_controller_test.go @@ -37,6 +37,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.go b/controllers/metal3.io/host_config_data.go index 3fd92dada0..037a0372c4 100644 --- a/controllers/metal3.io/host_config_data.go +++ b/controllers/metal3.io/host_config_data.go @@ -4,6 +4,7 @@ import ( "github.com/go-logr/logr" 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" ) @@ -20,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, diff --git a/controllers/metal3.io/host_config_data_test.go b/controllers/metal3.io/host_config_data_test.go index dd3e4ac925..f7d9ac4a58 100644 --- a/controllers/metal3.io/host_config_data_test.go +++ b/controllers/metal3.io/host_config_data_test.go @@ -323,6 +323,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 { @@ -378,7 +426,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)) } }) }