Forbid kubeconfig exec

pkg/controller/argocdregister/argocdregister_controller.go

@@ -211,7 +211,7 @@ func (r *ArgoCDRegisterController) reconcileCluster(cd *hivev1.ClusterDeployment
 		return reconcile.Result{}, nil
 	}
 
-	kubeConfigSecretName := cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name
+	kubeConfigSecretName := cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name // HIVE-2485 ✓
 	argoCDServerConfigBytes, err := r.generateArgoCDServerConfig(kubeConfigSecretName, cd.Namespace, argoCDNamespace, cdLog)
 	if err != nil {
 		return reconcile.Result{}, err
@@ -320,7 +320,7 @@ func (r *ArgoCDRegisterController) loadArgoCDServiceAccountToken(argoCDNamespace
 }
 
 func (r *ArgoCDRegisterController) generateArgoCDServerConfig(kubeconfigSecretName, kubeConfigSecretNamespace, argoCDNamespace string, cdLog log.FieldLogger) ([]byte, error) {
-	kubeconfig, err := r.loadSecretData(kubeconfigSecretName, kubeConfigSecretNamespace, adminKubeConfigKey)
+	kubeconfig, err := controllerutils.LoadSecretData(r.Client, kubeconfigSecretName, kubeConfigSecretNamespace, adminKubeConfigKey)
 	if err != nil {
 		cdLog.WithError(err).Error("unable to load cluster admin kubeconfig")
 		return nil, err
@@ -375,19 +375,6 @@ func tlsClientConfigBuilderFunc(kubeConfig clientcmd.ClientConfig, cdLog log.Fie
 	return tlsClientConfig, nil
 }
 
-func (r *ArgoCDRegisterController) loadSecretData(secretName, namespace, dataKey string) (string, error) {
-	s := &corev1.Secret{}
-	err := r.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, s)
-	if err != nil {
-		return "", err
-	}
-	retStr, ok := s.Data[dataKey]
-	if !ok {
-		return "", fmt.Errorf("secret %s did not contain key %s", secretName, dataKey)
-	}
-	return string(retStr), nil
-}
-
 // getPredictableSecretName generates a unique secret name by hashing the server API URL,
 // which is required as all cluster secrets land in the argocd namespace.
 // This code matches what is presently done in ArgoCD (util/db/cluster.go). However the actual

pkg/controller/awsprivatelink/awsprivatelink_controller.go

@@ -25,8 +25,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/util/flowcontrol"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/client-go/util/workqueue"
@@ -301,7 +299,13 @@ func (r *ReconcileAWSPrivateLink) Reconcile(ctx context.Context, request reconci
 		return reconcile.Result{}, nil
 	}
 
-	return r.reconcilePrivateLink(cd, &hivev1.ClusterMetadata{InfraID: *cp.Spec.InfraID, AdminKubeconfigSecretRef: *cp.Spec.AdminKubeconfigSecretRef}, logger)
+	return r.reconcilePrivateLink(
+		cd,
+		&hivev1.ClusterMetadata{
+			InfraID:                  *cp.Spec.InfraID,
+			AdminKubeconfigSecretRef: *cp.Spec.AdminKubeconfigSecretRef, // HIVE-2585: via ClusterMetadata
+		},
+		logger)
 }
 
 // shouldSync returns if we should sync the desired ClusterDeployment. If it returns false, it also returns
@@ -533,7 +537,7 @@ func (r *ReconcileAWSPrivateLink) reconcilePrivateLink(cd *hivev1.ClusterDeploym
 
 	// Figure out the API address for cluster.
 	apiDomain, err := initialURL(r.Client,
-		client.ObjectKey{Namespace: cd.Namespace, Name: clusterMetadata.AdminKubeconfigSecretRef.Name})
+		client.ObjectKey{Namespace: cd.Namespace, Name: clusterMetadata.AdminKubeconfigSecretRef.Name}) // HIVE-2485 ✓
 	if err != nil {
 		logger.WithError(err).Error("could not get API URL from kubeconfig")
 
@@ -1303,7 +1307,7 @@ func initialURL(c client.Client, key client.ObjectKey) (string, error) {
 	); err != nil {
 		return "", err
 	}
-	cfg, err := restConfigFromSecret(kubeconfigSecret)
+	cfg, err := controllerutils.RestConfigFromSecret(kubeconfigSecret, true)
 	if err != nil {
 		return "", errors.Wrap(err, "failed to load the kubeconfig")
 	}
@@ -1315,22 +1319,6 @@ func initialURL(c client.Client, key client.ObjectKey) (string, error) {
 	return strings.TrimSuffix(u.Hostname(), "."), nil
 }
 
-func restConfigFromSecret(kubeconfigSecret *corev1.Secret) (*rest.Config, error) {
-	kubeconfigData := kubeconfigSecret.Data[constants.RawKubeconfigSecretKey]
-	if len(kubeconfigData) == 0 {
-		kubeconfigData = kubeconfigSecret.Data[constants.KubeconfigSecretKey]
-	}
-	if len(kubeconfigData) == 0 {
-		return nil, errors.New("kubeconfig secret does not contain necessary data")
-	}
-	config, err := clientcmd.Load(kubeconfigData)
-	if err != nil {
-		return nil, err
-	}
-	kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{})
-	return kubeConfig.ClientConfig()
-}
-
 // awsErrCodeEquals returns true if the error matches all these conditions:
 //   - err is of type awserr.Error
 //   - Error.Code() equals code

pkg/controller/awsprivatelink/cleanup.go

@@ -61,7 +61,7 @@ func (r *ReconcileAWSPrivateLink) cleanupPreviousProvisionAttempt(cd *hivev1.Clu
 	}
 	metadata := &hivev1.ClusterMetadata{
 		InfraID:                  *cp.Spec.PrevInfraID,
-		AdminKubeconfigSecretRef: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef,
+		AdminKubeconfigSecretRef: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef, // HIVE-2485: via ClusterMetadata
 	}
 
 	if err := r.cleanupPrivateLink(cd, metadata, logger); err != nil {
@@ -138,7 +138,7 @@ func (r *ReconcileAWSPrivateLink) cleanupHostedZone(awsClient awsclient.Client,
 
 	if hzID == "" { // since we don't have the hz ID, we try to discover it to prevent leaks
 		apiDomain, err := initialURL(r.Client,
-			client.ObjectKey{Namespace: cd.Namespace, Name: metadata.AdminKubeconfigSecretRef.Name})
+			client.ObjectKey{Namespace: cd.Namespace, Name: metadata.AdminKubeconfigSecretRef.Name}) // HIVE-2485 ✓
 		if apierrors.IsNotFound(err) {
 			logger.Info("no hostedZoneID in status and admin kubeconfig does not exist, skipping hosted zone cleanup")
 			return nil

pkg/controller/clusterclaim/clusterclaim_controller.go

@@ -549,7 +549,7 @@ func (r *ReconcileClusterClaim) applyHiveClaimOwnerRole(claim *hivev1.ClusterCla
 				APIGroups: []string{corev1.GroupName},
 				Resources: []string{"secrets"},
 				ResourceNames: []string{
-					cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name,
+					cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name, // HIVE-2485: n/a
 					cd.Spec.ClusterMetadata.AdminPasswordSecretRef.Name,
 				},
 				Verbs: []string{"get"},

pkg/controller/clusterdeployment/clusterdeployment_controller.go

@@ -456,7 +456,14 @@ func (r *ReconcileClusterDeployment) addAdditionalKubeconfigCAs(cd *hivev1.Clust
 	cdLog log.FieldLogger) error {
 
 	adminKubeconfigSecret := &corev1.Secret{}
-	if err := r.Get(context.Background(), types.NamespacedName{Namespace: cd.Namespace, Name: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name}, adminKubeconfigSecret); err != nil {
+	if err := r.Get(
+		context.Background(),
+		types.NamespacedName{
+			Namespace: cd.Namespace,
+			// HIVE-2485: No need to scrub here
+			Name: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name,
+		},
+		adminKubeconfigSecret); err != nil {
 		cdLog.WithError(err).Error("failed to get admin kubeconfig secret")
 		return err
 	}
@@ -658,6 +665,7 @@ func (r *ReconcileClusterDeployment) reconcile(request reconcile.Request, cd *hi
 			}
 
 			// Add cluster deployment as additional owner reference to admin secrets
+			// HIVE-2485: No need to scrub here
 			if err := r.addOwnershipToSecret(cd, cdLog, cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name); err != nil {
 				return reconcile.Result{}, err
 			}

pkg/controller/clusterdeployment/clusterinstalls.go

@@ -54,7 +54,7 @@ func (r *ReconcileClusterDeployment) reconcileExistingInstallingClusterInstall(c
 	if met := ci.Spec.ClusterMetadata; met != nil &&
 		met.InfraID != "" &&
 		met.ClusterID != "" &&
-		met.AdminKubeconfigSecretRef.Name != "" &&
+		met.AdminKubeconfigSecretRef.Name != "" && // HIVE-2485: via ClusterMetadata
 		met.AdminPasswordSecretRef != nil &&
 		met.AdminPasswordSecretRef.Name != "" {
 		if !reflect.DeepEqual(cd.Spec.ClusterMetadata, ci.Spec.ClusterMetadata) {

pkg/controller/clusterdeployment/clusterprovisions.go

@@ -423,7 +423,7 @@ func (r *ReconcileClusterDeployment) reconcileExistingProvision(cd *hivev1.Clust
 			clusterMetadata.ClusterID = *provision.Spec.ClusterID
 		}
 		if provision.Spec.AdminKubeconfigSecretRef != nil {
-			clusterMetadata.AdminKubeconfigSecretRef = *provision.Spec.AdminKubeconfigSecretRef
+			clusterMetadata.AdminKubeconfigSecretRef = *provision.Spec.AdminKubeconfigSecretRef // HIVE-2485: via ClusterMetadata
 		}
 		if provision.Spec.AdminPasswordSecretRef != nil {
 			clusterMetadata.AdminPasswordSecretRef = provision.Spec.AdminPasswordSecretRef

pkg/controller/utils/secrets.go

@@ -2,23 +2,76 @@ package utils
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
+	"github.com/openshift/hive/pkg/constants"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/clientcmd"
+	"k8s.io/client-go/tools/clientcmd/api"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// LoadSecretData loads a given secret key and returns it's data as a string.
+// LoadSecretData loads a given secret key and returns its data as a string.
 func LoadSecretData(c client.Client, secretName, namespace, dataKey string) (string, error) {
 	s := &corev1.Secret{}
 	err := c.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, s)
 	if err != nil {
 		return "", err
 	}
-	retStr, ok := s.Data[dataKey]
+	retBytes, ok := s.Data[dataKey]
 	if !ok {
 		return "", fmt.Errorf("secret %s did not contain key %s", secretName, dataKey)
 	}
-	return string(retStr), nil
+	switch dataKey {
+	case constants.KubeconfigSecretKey, constants.RawKubeconfigSecretKey:
+		if _, err := validateKubeconfig(retBytes); err != nil {
+			return "", err
+		}
+	}
+	return string(retBytes), nil
+}
+
+// RestConfigFromSecret accepts a Secret containing `kubeconfig` and/or `raw-kubeconfig` keys
+// and returns a rest.Config loaded therefrom.
+// If tryRaw is true, we will look for `raw-kubeconfig` first and use it if present, falling
+// back to `kubeconfig` otherwise.
+// The error return is non-nil if:
+// - The Secret's Data does not contain the [raw-]kubeconfig key(s)
+// - The kubeconfig data cannot be Load()ed
+// - The kubeconfig is insecure
+func RestConfigFromSecret(kubeconfigSecret *corev1.Secret, tryRaw bool) (*rest.Config, error) {
+	var kubeconfigData []byte
+	if tryRaw {
+		kubeconfigData = kubeconfigSecret.Data[constants.RawKubeconfigSecretKey]
+	}
+	if len(kubeconfigData) == 0 {
+		kubeconfigData = kubeconfigSecret.Data[constants.KubeconfigSecretKey]
+	}
+	if len(kubeconfigData) == 0 {
+		return nil, errors.New("kubeconfig secret does not contain necessary data")
+	}
+	config, err := validateKubeconfig(kubeconfigData)
+	if err != nil {
+		return nil, err
+	}
+	kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{})
+	return kubeConfig.ClientConfig()
+}
+
+// validateKubeconfig ensures the kubeconfig represented by kc does not use insecure paths.
+// HIVE-2485
+func validateKubeconfig(kc []byte) (*api.Config, error) {
+	config, err := clientcmd.Load(kc)
+	if err != nil {
+		return nil, err
+	}
+	for k, ai := range config.AuthInfos {
+		if ai.Exec != nil {
+			return nil, fmt.Errorf("insecure exec in AuthInfos[%s]", k)
+		}
+	}
+	return config, nil
 }

pkg/remoteclient/kubeconfig.go

@@ -1,6 +1,7 @@
 package remoteclient
 
 import (
+	"github.com/openshift/hive/pkg/controller/utils"
 	"github.com/openshift/hive/pkg/util/scheme"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/discovery"
@@ -81,5 +82,5 @@ func (b *kubeconfigBuilder) UseSecondaryAPIURL() Builder {
 }
 
 func (b *kubeconfigBuilder) RESTConfig() (*rest.Config, error) {
-	return restConfigFromSecret(b.secret)
+	return utils.RestConfigFromSecret(b.secret, false)
 }

pkg/remoteclient/remoteclient.go

@@ -18,11 +18,9 @@ import (
 	kubeclient "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
-	"k8s.io/client-go/tools/clientcmd"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
-	"github.com/openshift/hive/pkg/constants"
 	"github.com/openshift/hive/pkg/controller/utils"
 	"github.com/openshift/hive/pkg/util/scheme"
 )
@@ -291,23 +289,11 @@ func unadulteratedRESTConfig(c client.Client, cd *hivev1.ClusterDeployment) (*re
 	kubeconfigSecret := &corev1.Secret{}
 	if err := c.Get(
 		context.Background(),
+		// HIVE-2485 ✓
 		client.ObjectKey{Namespace: cd.Namespace, Name: cd.Spec.ClusterMetadata.AdminKubeconfigSecretRef.Name},
 		kubeconfigSecret,
 	); err != nil {
 		return nil, errors.Wrap(err, "could not get admin kubeconfig secret")
 	}
-	return restConfigFromSecret(kubeconfigSecret)
-}
-
-func restConfigFromSecret(kubeconfigSecret *corev1.Secret) (*rest.Config, error) {
-	kubeconfigData, ok := kubeconfigSecret.Data[constants.KubeconfigSecretKey]
-	if !ok {
-		return nil, errors.Errorf("kubeconfig secret does not contain %q data", constants.KubeconfigSecretKey)
-	}
-	config, err := clientcmd.Load(kubeconfigData)
-	if err != nil {
-		return nil, err
-	}
-	kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{})
-	return kubeConfig.ClientConfig()
+	return utils.RestConfigFromSecret(kubeconfigSecret, false)
 }