Forbid and convert credential_process

As a security measure, check AWS config/credential files for
`credential_process`, and explode if found.

We used to use `credential_process` deliberately to AssumeRole for STS
clusters. A prior commit switched this over to use a different
mechanism, but existing clusters in the field may still be configured
with the old mechanism in the relevant Secrets. Convert such Secrets to
use the new mechanism.

HIVE-2485

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,10 +270,12 @@ 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))
 	}
+	// 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

pkg/awsclient/client.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"regexp"
 
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
@@ -597,7 +598,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 +630,30 @@ func NewSessionFromSecret(secret *corev1.Secret, region string) (*session.Sessio
 	return s, nil
 }
 
+var credentialProcessRE = regexp.MustCompile(`\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 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/awsprivatelink/awsprivatelink_controller.go

@@ -1268,7 +1268,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,

pkg/controller/clusterdeployment/clusterprovisions.go

@@ -215,12 +215,8 @@ func (r *ReconcileClusterDeployment) startNewProvision(
 	}
 
 	if err := r.setupAWSCredentialForAssumeRole(cd); err != nil {
-		if !apierrors.IsAlreadyExists(err) {
-			// Couldn't create the assume role credential secret for a reason other than it already exists.
-			// If the secret already exists, then we should just use that secret.
-			logger.WithError(err).Error("could not create AWS assume role credential secret")
-			return reconcile.Result{}, err
-		}
+		logger.WithError(err).Error("could not create or update AWS assume role credential secret")
+		return reconcile.Result{}, err
 	}
 
 	r.expectations.ExpectCreations(types.NamespacedName{Namespace: cd.Namespace, Name: cd.Name}.String(), 1)
@@ -750,7 +746,7 @@ func getInstallLogEnvVars(secretPrefix string) ([]corev1.EnvVar, error) {
 
 func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeployment, secretPrefix string) []corev1.EnvVar {
 	var extraEnvVars []corev1.EnvVar
-	spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar)
+	spSecretName := controllerutils.AWSServiceProviderSecretName(secretPrefix)
 	if spSecretName == "" {
 		return extraEnvVars
 	}
@@ -761,7 +757,7 @@ func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeployment, secretPrefix str
 
 	extraEnvVars = append(extraEnvVars, corev1.EnvVar{
 		Name:  constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar,
-		Value: secretPrefix + "-" + spSecretName,
+		Value: spSecretName,
 	})
 	return extraEnvVars
 }

pkg/controller/clusterdeprovision/awsactuator.go

@@ -1,8 +1,6 @@
 package clusterdeprovision
 
 import (
-	"os"
-
 	log "github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -11,7 +9,6 @@ import (
 
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	awsclient "github.com/openshift/hive/pkg/awsclient"
-	"github.com/openshift/hive/pkg/constants"
 	controllerutils "github.com/openshift/hive/pkg/controller/utils"
 )
 
@@ -61,7 +58,7 @@ func getAWSClient(cd *hivev1.ClusterDeprovision, c client.Client, logger log.Fie
 			},
 			AssumeRole: &awsclient.AssumeRoleCredentialsSource{
 				SecretRef: corev1.SecretReference{
-					Name:      os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar),
+					Name:      controllerutils.AWSServiceProviderSecretName(""),
 					Namespace: controllerutils.GetHiveNamespace(),
 				},
 				Role: cd.Spec.Platform.AWS.CredentialsAssumeRole,

pkg/controller/clusterdeprovision/clusterdeprovision_controller.go

@@ -291,12 +291,8 @@ func (r *ReconcileClusterDeprovision) Reconcile(ctx context.Context, request rec
 	}
 
 	if err := r.setupAWSCredentialForAssumeRole(instance); err != nil {
-		if !errors.IsAlreadyExists(err) {
-			// Couldn't create the assume role credentials secret for a reason other than it already exists.
-			// If the secret already exists, then we should just use that secret.
-			rLog.WithError(err).Error("could not create assume role AWS secret")
-			return reconcile.Result{}, err
-		}
+		rLog.WithError(err).Error("could not create or update assume role AWS secret")
+		return reconcile.Result{}, err
 	}
 
 	if err := controllerutils.SetupClusterUninstallServiceAccount(r, cd.Namespace, rLog); err != nil {
@@ -454,7 +450,7 @@ func (r *ReconcileClusterDeprovision) getActuator(cd *hivev1.ClusterDeprovision)
 
 func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeprovision, secretPrefix string) []corev1.EnvVar {
 	var extraEnvVars []corev1.EnvVar
-	spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar)
+	spSecretName := controllerutils.AWSServiceProviderSecretName(secretPrefix)
 	if spSecretName == "" {
 		return extraEnvVars
 	}
@@ -465,7 +461,7 @@ func getAWSServiceProviderEnvVars(cd *hivev1.ClusterDeprovision, secretPrefix st
 
 	extraEnvVars = append(extraEnvVars, corev1.EnvVar{
 		Name:  constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar,
-		Value: secretPrefix + "-" + spSecretName,
+		Value: spSecretName,
 	})
 	return extraEnvVars
 }

pkg/controller/dnszone/dnszone_controller.go

@@ -16,7 +16,6 @@ import (
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	awsclient "github.com/openshift/hive/pkg/awsclient"
 	"github.com/openshift/hive/pkg/azureclient"
-	"github.com/openshift/hive/pkg/constants"
 	hivemetrics "github.com/openshift/hive/pkg/controller/metrics"
 	controllerutils "github.com/openshift/hive/pkg/controller/utils"
 	gcpclient "github.com/openshift/hive/pkg/gcpclient"
@@ -399,7 +398,7 @@ func (r *ReconcileDNSZone) getActuator(dnsZone *hivev1.DNSZone, dnsLog log.Field
 			AssumeRole: &awsclient.AssumeRoleCredentialsSource{
 				SecretRef: corev1.SecretReference{
 					Namespace: controllerutils.GetHiveNamespace(),
-					Name:      os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar),
+					Name:      controllerutils.AWSServiceProviderSecretName(""),
 				},
 				Role: dnsZone.Spec.AWS.CredentialsAssumeRole,
 			},

pkg/controller/hibernation/aws_actuator.go

@@ -3,7 +3,6 @@ package hibernation
 import (
 	"context"
 	"fmt"
-	"os"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -18,7 +17,6 @@ import (
 
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	awsclient "github.com/openshift/hive/pkg/awsclient"
-	"github.com/openshift/hive/pkg/constants"
 	controllerutils "github.com/openshift/hive/pkg/controller/utils"
 )
 
@@ -261,7 +259,7 @@ func getAWSClient(cd *hivev1.ClusterDeployment, c client.Client, logger log.Fiel
 			},
 			AssumeRole: &awsclient.AssumeRoleCredentialsSource{
 				SecretRef: corev1.SecretReference{
-					Name:      os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar),
+					Name:      controllerutils.AWSServiceProviderSecretName(""),
 					Namespace: controllerutils.GetHiveNamespace(),
 				},
 				Role: cd.Spec.Platform.AWS.CredentialsAssumeRole,

pkg/controller/machinepool/machinepool_controller.go

@@ -1215,7 +1215,7 @@ func (r *ReconcileMachinePool) createActuator(
 			AssumeRole: &awsclient.AssumeRoleCredentialsSource{
 				SecretRef: corev1.SecretReference{
 					Namespace: controllerutils.GetHiveNamespace(),
-					Name:      os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar),
+					Name:      controllerutils.AWSServiceProviderSecretName(""),
 				},
 				Role: cd.Spec.Platform.AWS.CredentialsAssumeRole,
 			},