From cbeaa365e86c1d6a741339840b329be5512702cf 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 | 10 ++-- .../metal3.io/host_config_data_test.go | 50 ++++++++++++++++++- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go index 52c3515179..4dc0655d09 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.go b/controllers/metal3.io/host_config_data.go index 0a298e24d9..037a0372c4 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, diff --git a/controllers/metal3.io/host_config_data_test.go b/controllers/metal3.io/host_config_data_test.go index f763a6e8a7..4ecc5a6d68 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)) } }) }