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>
Co-Authored-By: Dmitry Tantsur <dtantsur@protonmail.com>

Signed-off-by: Tuomo Tanskanen <tuomo.tanskanen@est.tech>

Author: tuminoid

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)))

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,

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))
 			}
 		})
 	}