Merge pull request #2389 from 2uasimojo/HIVE-2485/mce-2.6

Hive 2485/mce 2.6: Backport AssumeRole, credential_process, and kubeconfig exec fixes

contrib/pkg/utils/aws/aws.go

@@ -1,12 +1,14 @@
 package aws
 
 import (
+	"errors"
 	"fmt"
-	corev1 "k8s.io/api/core/v1"
 	"os"
 	"path/filepath"
 	"strings"
 
+	corev1 "k8s.io/api/core/v1"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 
@@ -242,6 +244,16 @@ func GetAWSCreds(credsFile, defaultCredsFile string) (string, string, error) {
 	return accessKeyIDValue.String(), secretAccessKeyValue.String(), nil
 }
 
+var awsConfigForbidCredentialProcess utils.ProjectToDirFileFilter = func(key string, contents []byte) (basename string, newContents []byte, err error) {
+	// First, only process aws_config
+	bn, newContents, err := utils.ProjectOnlyTheseKeys(constants.AWSConfigSecretKey)(key, contents)
+	// If that passed, scrub for credential_process
+	if err == nil && bn != "" && awsclient.ContainsCredentialProcess(newContents) {
+		return "", nil, errors.New("credential_process is insecure and thus forbidden")
+	}
+	return bn, newContents, err
+}
+
 // ConfigureCreds loads a secret designated by the environment variables CLUSTERDEPLOYMENT_NAMESPACE
 // and CREDS_SECRET_NAME and configures AWS credential environment variables and config files
 // accordingly.
@@ -258,11 +270,11 @@ func ConfigureCreds(c client.Client) {
 		os.Setenv("AWS_SECRET_ACCESS_KEY", secret)
 	}
 	if config := credsSecret.Data[constants.AWSConfigSecretKey]; len(config) != 0 {
-		// Lay this down as a file
-		utils.ProjectToDir(credsSecret, constants.AWSCredsMount, constants.AWSConfigSecretKey)
+		// Lay this down as a file, but forbid credential_process
+		utils.ProjectToDir(credsSecret, constants.AWSCredsMount, awsConfigForbidCredentialProcess)
 		os.Setenv("AWS_CONFIG_FILE", filepath.Join(constants.AWSCredsMount, constants.AWSConfigSecretKey))
 	}
-	// Allow credential_process in the config file
+	// This would normally allow credential_process in the config file, but we checked for that above.
 	os.Setenv("AWS_SDK_LOAD_CONFIG", "true")
 	// Install cluster proxy trusted CA bundle
 	utils.InstallCerts(constants.TrustedCABundleDir)

contrib/pkg/utils/azure/azure.go

@@ -36,7 +36,7 @@ func ConfigureCreds(c client.Client) {
 	if credsSecret == nil {
 		return
 	}
-	utils.ProjectToDir(credsSecret, constants.AzureCredentialsDir)
+	utils.ProjectToDir(credsSecret, constants.AzureCredentialsDir, nil)
 	os.Setenv(constants.AzureCredentialsEnvVar, constants.AzureCredentialsDir+"/"+constants.AzureCredentialsName)
 	// Install cluster proxy trusted CA bundle
 	utils.InstallCerts(constants.TrustedCABundleDir)

contrib/pkg/utils/gcp/gcp.go

@@ -37,7 +37,7 @@ func ConfigureCreds(c client.Client) {
 	if credsSecret == nil {
 		return
 	}
-	utils.ProjectToDir(credsSecret, constants.GCPCredentialsDir)
+	utils.ProjectToDir(credsSecret, constants.GCPCredentialsDir, nil)
 	os.Setenv("GOOGLE_CREDENTIALS", constants.GCPCredentialsDir+"/"+constants.GCPCredentialsName)
 	// Install cluster proxy trusted CA bundle
 	utils.InstallCerts(constants.TrustedCABundleDir)

contrib/pkg/utils/generic.go

@@ -150,16 +150,56 @@ func loadOrDie(c client.Client, nameEnvKey string, obj client.Object) bool {
 
 }
 
+// ProjectToDirFileFilter is run by ProjectToDir for each key found in the obj.
+// If the second return is an error, ProjectToDir will panic with it. Otherwise:
+// If ProjectToDir should create the file, the first return should be the base
+// name of the file in which the newContents should be written.
+// If the file should be skipped, the first return should be the empty string.
+type ProjectToDirFileFilter func(key string, contents []byte) (basename string, newContents []byte, err error)
+
+// projectDefault is a ProjectToDirFileFilter that causes ProjectToDir to create
+// files for all keys in the obj, naming each file the same as its key.
+var projectDefault ProjectToDirFileFilter = func(key string, contents []byte) (string, []byte, error) {
+	return key, contents, nil
+}
+
+// ProjectOnlyTheseKeys returns a ProjectToDirFileFilter that instructs ProjectToDir
+// to create only the files with the specified keys. Each file's name will be the
+// same as the key. The error return is always nil.
+func ProjectOnlyTheseKeys(keys ...string) ProjectToDirFileFilter {
+	return func(key string, contents []byte) (string, []byte, error) {
+		if len(keys) == 0 {
+			// Caller should use nil (projectDefault) instead, but meh.
+			return key, contents, nil
+		}
+		if slice.ContainsString(keys, key, nil) {
+			// A match, project the file with the key as the basename and unchanged contents
+			return key, contents, nil
+		}
+		// No match; skip this file
+		return "", nil, nil
+	}
+}
+
 // ProjectToDir simulates what happens when you mount a secret or configmap as a volume on a pod, creating
 // files named after each key under `dir` and populating them with the contents represented by the values.
-func ProjectToDir(obj client.Object, dir string, keys ...string) {
-	write := func(filename string, bytes []byte) {
-		if len(keys) != 0 && !slice.ContainsString(keys, filename, nil) {
+// This default behavior can be modified by specifying a non-nil filter to validate, skip, and/or rename
+// the file corresponding to each key.
+func ProjectToDir(obj client.Object, dir string, filter ProjectToDirFileFilter) {
+	write := func(key string, bytes []byte) {
+		if filter == nil {
+			filter = projectDefault
+		}
+		filename, newBytes, err := filter(key, bytes)
+		if err != nil {
+			panic(err)
+		}
+		if filename == "" {
 			// Skip this key
 			return
 		}
 		path := filepath.Join(dir, filename)
-		if err := os.WriteFile(path, bytes, 0400); err != nil {
+		if err := os.WriteFile(path, newBytes, 0400); err != nil {
 			log.WithError(err).WithField("path", path).Fatal("Failed to write file")
 		}
 	}

contrib/pkg/utils/openstack/openstack.go

@@ -39,10 +39,10 @@ func GetCreds(credsFile string) ([]byte, error) {
 // CREDS_SECRET_NAME, and CERTS_SECRET_NAME and configures OpenStack credential config files accordingly.
 func ConfigureCreds(c client.Client) {
 	if credsSecret := utils.LoadSecretOrDie(c, "CREDS_SECRET_NAME"); credsSecret != nil {
-		utils.ProjectToDir(credsSecret, constants.OpenStackCredentialsDir)
+		utils.ProjectToDir(credsSecret, constants.OpenStackCredentialsDir, nil)
 	}
 	if certsSecret := utils.LoadSecretOrDie(c, "CERTS_SECRET_NAME"); certsSecret != nil {
-		utils.ProjectToDir(certsSecret, constants.OpenStackCertificatesDir)
+		utils.ProjectToDir(certsSecret, constants.OpenStackCertificatesDir, nil)
 		utils.InstallCerts(constants.OpenStackCertificatesDir)
 	}
 	// Install cluster proxy trusted CA bundle

contrib/pkg/utils/ovirt/ovirt.go

@@ -36,11 +36,11 @@ func GetCreds(credsFile string) ([]byte, error) {
 // and config files accordingly.
 func ConfigureCreds(c client.Client) {
 	if credsSecret := utils.LoadSecretOrDie(c, "CREDS_SECRET_NAME"); credsSecret != nil {
-		utils.ProjectToDir(credsSecret, constants.OvirtCredentialsDir)
+		utils.ProjectToDir(credsSecret, constants.OvirtCredentialsDir, nil)
 		os.Setenv(constants.OvirtConfigEnvVar, constants.OvirtCredentialsDir+"/"+constants.OvirtCredentialsName)
 	}
 	if certsSecret := utils.LoadSecretOrDie(c, "CERTS_SECRET_NAME"); certsSecret != nil {
-		utils.ProjectToDir(certsSecret, constants.OvirtCertificatesDir)
+		utils.ProjectToDir(certsSecret, constants.OvirtCertificatesDir, nil)
 		utils.InstallCerts(constants.OvirtCertificatesDir)
 	}
 	// Install cluster proxy trusted CA bundle

contrib/pkg/utils/vsphere/vsphere.go

@@ -21,10 +21,10 @@ func ConfigureCreds(c client.Client) {
 		}
 		// NOTE: I think this is only used for terminateWhenFilesChange(), which we don't really
 		// care about anymore. Can we get rid of it?
-		utils.ProjectToDir(credsSecret, constants.VSphereCredentialsDir)
+		utils.ProjectToDir(credsSecret, constants.VSphereCredentialsDir, nil)
 	}
 	if certsSecret := utils.LoadSecretOrDie(c, "CERTS_SECRET_NAME"); certsSecret != nil {
-		utils.ProjectToDir(certsSecret, constants.VSphereCertificatesDir)
+		utils.ProjectToDir(certsSecret, constants.VSphereCertificatesDir, nil)
 		utils.InstallCerts(constants.VSphereCertificatesDir)
 	}
 	// Install cluster proxy trusted CA bundle

docs/awsassumerolecreds.md

@@ -26,7 +26,7 @@ Let's create a Secret in `hive` (target namespace in HiveConfig) with credential
 
 ```console
 $ cat aws-service-provider-config
-[default]
+[profile source]
 aws_access_key_id = XXXXXX
 aws_secret_access_key = XXXXX
 role_arn = arn:aws:iam::123456:role/hive-aws-service-provider

pkg/awsclient/client.go

@@ -5,6 +5,8 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"regexp"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
@@ -597,7 +599,10 @@ func NewSessionFromSecret(secret *corev1.Secret, region string) (*session.Sessio
 
 	// Special case to not use a secret to gather credentials.
 	if secret != nil {
-		config := awsCLIConfigFromSecret(secret)
+		config, err := awsCLIConfigFromSecret(secret)
+		if err != nil {
+			return nil, err
+		}
 		f, err := os.CreateTemp("", "hive-aws-config")
 		if err != nil {
 			return nil, err
@@ -626,18 +631,30 @@ func NewSessionFromSecret(secret *corev1.Secret, region string) (*session.Sessio
 	return s, nil
 }
 
+var credentialProcessRE = regexp.MustCompile(`(?i)\bcredential_process\b`)
+
+func ContainsCredentialProcess(config []byte) bool {
+	return len(credentialProcessRE.Find(config)) != 0
+}
+
 // awsCLIConfigFromSecret returns an AWS CLI config using the data available in the secret.
-func awsCLIConfigFromSecret(secret *corev1.Secret) []byte {
+func awsCLIConfigFromSecret(secret *corev1.Secret) ([]byte, error) {
 	if config, ok := secret.Data[constants.AWSConfigSecretKey]; ok {
-		return config
+		if ContainsCredentialProcess(config) {
+			return nil, errors.New("credential_process is insecure and thus forbidden")
+		}
+		return config, nil
 	}
 
 	buf := &bytes.Buffer{}
 	fmt.Fprint(buf, "[default]\n")
 	for k, v := range secret.Data {
+		if strings.ToLower(k) == "credential_process" {
+			return nil, errors.New("credential_process is insecure and thus forbidden")
+		}
 		fmt.Fprintf(buf, "%s = %s\n", k, v)
 	}
-	return buf.Bytes()
+	return buf.Bytes(), nil
 }
 
 func awsChinaEndpointResolver(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) {

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")
 
@@ -1268,7 +1272,7 @@ func newAWSClient(r *ReconcileAWSPrivateLink, cd *hivev1.ClusterDeployment) (*aw
 			},
 			AssumeRole: &awsclient.AssumeRoleCredentialsSource{
 				SecretRef: corev1.SecretReference{
-					Name:      os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar),
+					Name:      controllerutils.AWSServiceProviderSecretName(""),
 					Namespace: controllerutils.GetHiveNamespace(),
 				},
 				Role: cd.Spec.Platform.AWS.CredentialsAssumeRole,
@@ -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
 			}