diff --git a/contrib/pkg/utils/aws/aws.go b/contrib/pkg/utils/aws/aws.go index 8c4536ab642..b8de1633b74 100644 --- a/contrib/pkg/utils/aws/aws.go +++ b/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) diff --git a/contrib/pkg/utils/azure/azure.go b/contrib/pkg/utils/azure/azure.go index bf252533d97..c26ac7bc150 100644 --- a/contrib/pkg/utils/azure/azure.go +++ b/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) diff --git a/contrib/pkg/utils/gcp/gcp.go b/contrib/pkg/utils/gcp/gcp.go index 94693e1f980..b3be4616ad3 100644 --- a/contrib/pkg/utils/gcp/gcp.go +++ b/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) diff --git a/contrib/pkg/utils/generic.go b/contrib/pkg/utils/generic.go index 0a78efc6961..1fe68650814 100644 --- a/contrib/pkg/utils/generic.go +++ b/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") } } diff --git a/contrib/pkg/utils/openstack/openstack.go b/contrib/pkg/utils/openstack/openstack.go index 48c33bde3df..26e2d313700 100644 --- a/contrib/pkg/utils/openstack/openstack.go +++ b/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 diff --git a/contrib/pkg/utils/ovirt/ovirt.go b/contrib/pkg/utils/ovirt/ovirt.go index cc44a3feb59..e859220423a 100644 --- a/contrib/pkg/utils/ovirt/ovirt.go +++ b/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 diff --git a/contrib/pkg/utils/vsphere/vsphere.go b/contrib/pkg/utils/vsphere/vsphere.go index 2bc77a62691..accf3290442 100644 --- a/contrib/pkg/utils/vsphere/vsphere.go +++ b/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 diff --git a/docs/awsassumerolecreds.md b/docs/awsassumerolecreds.md index f25330be715..ea1195e0013 100644 --- a/docs/awsassumerolecreds.md +++ b/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 diff --git a/pkg/awsclient/client.go b/pkg/awsclient/client.go index 78d460e964f..c52ca98fec4 100644 --- a/pkg/awsclient/client.go +++ b/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) { diff --git a/pkg/controller/awsprivatelink/awsprivatelink_controller.go b/pkg/controller/awsprivatelink/awsprivatelink_controller.go index 04afd70b8b9..4cbad7abdde 100644 --- a/pkg/controller/awsprivatelink/awsprivatelink_controller.go +++ b/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, diff --git a/pkg/controller/clusterdeployment/clusterprovisions.go b/pkg/controller/clusterdeployment/clusterprovisions.go index 0d819224d61..32d6223965d 100644 --- a/pkg/controller/clusterdeployment/clusterprovisions.go +++ b/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 } @@ -774,7 +770,7 @@ func (r *ReconcileClusterDeployment) setupAWSCredentialForAssumeRole(cd *hivev1. return nil } - return install.AWSAssumeRoleCLIConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) + return install.AWSAssumeRoleConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) } func (r *ReconcileClusterDeployment) watchClusterProvisions(mgr manager.Manager, c controller.Controller) error { diff --git a/pkg/controller/clusterdeprovision/awsactuator.go b/pkg/controller/clusterdeprovision/awsactuator.go index b6c12229316..7786664a95a 100644 --- a/pkg/controller/clusterdeprovision/awsactuator.go +++ b/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, diff --git a/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go b/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go index 958919abfdc..dea6077535a 100644 --- a/pkg/controller/clusterdeprovision/clusterdeprovision_controller.go +++ b/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 } @@ -478,6 +474,6 @@ func (r *ReconcileClusterDeprovision) setupAWSCredentialForAssumeRole(cd *hivev1 return nil } - return install.AWSAssumeRoleCLIConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) + return install.AWSAssumeRoleConfig(r.Client, cd.Spec.Platform.AWS.CredentialsAssumeRole, install.AWSAssumeRoleSecretName(cd.Name), cd.Namespace, cd, r.scheme) } diff --git a/pkg/controller/dnszone/dnszone_controller.go b/pkg/controller/dnszone/dnszone_controller.go index dc1413473f0..14bdc8ce6a6 100644 --- a/pkg/controller/dnszone/dnszone_controller.go +++ b/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, }, diff --git a/pkg/controller/hibernation/aws_actuator.go b/pkg/controller/hibernation/aws_actuator.go index 67a64891f1b..2455a6d3974 100644 --- a/pkg/controller/hibernation/aws_actuator.go +++ b/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, diff --git a/pkg/controller/machinepool/machinepool_controller.go b/pkg/controller/machinepool/machinepool_controller.go index 517e91e1a0d..2355bd1795e 100644 --- a/pkg/controller/machinepool/machinepool_controller.go +++ b/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, }, diff --git a/pkg/controller/utils/secrets.go b/pkg/controller/utils/secrets.go index 45bd125eda8..c5ad9f5ac37 100644 --- a/pkg/controller/utils/secrets.go +++ b/pkg/controller/utils/secrets.go @@ -3,13 +3,15 @@ package utils import ( "context" "fmt" + "os" + "github.com/openshift/hive/pkg/constants" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "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) @@ -22,3 +24,20 @@ func LoadSecretData(c client.Client, secretName, namespace, dataKey string) (str } return string(retStr), nil } + +// Generate the name of the Secret containing AWS Service Provider configuration. The +// prefix should be specified when the env var is known to refer to the global (hive +// namespace) Secret to convert it to the local (CD-specific) name. +// Beware of recursion: we're overloading the env var to refer to both the secret in the +// hive namespace and that in the CD-specific namespace. +func AWSServiceProviderSecretName(prefix string) string { + spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) + // If no secret is given via env, this is n/a + if spSecretName == "" { + return "" + } + if prefix == "" { + return spSecretName + } + return prefix + "-" + spSecretName +} diff --git a/pkg/install/generate.go b/pkg/install/generate.go index b287085296f..484a469eba0 100644 --- a/pkg/install/generate.go +++ b/pkg/install/generate.go @@ -3,7 +3,7 @@ package install import ( "context" "fmt" - "os" + "reflect" "strconv" "strings" "time" @@ -15,6 +15,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -50,7 +51,7 @@ func AWSAssumeRoleSecretName(secretPrefix string) string { func CopyAWSServiceProviderSecret(client client.Client, destNamespace string, envVars []corev1.EnvVar, owner metav1.Object, scheme *runtime.Scheme) error { hiveNS := controllerutils.GetHiveNamespace() - spSecretName := os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) + spSecretName := controllerutils.AWSServiceProviderSecretName("") if spSecretName == "" { // If the src secret reference wasn't found, then don't attempt to copy the secret. return nil @@ -74,42 +75,79 @@ func CopyAWSServiceProviderSecret(client client.Client, destNamespace string, en return controllerutils.CopySecret(client, src, dest, owner, scheme) } -// AWSAssumeRoleCLIConfig creates a secret that can assume the role using the hiveutil -// credential_process helper. -func AWSAssumeRoleCLIConfig(client client.Client, role *hivev1aws.AssumeRole, secretName, secretNamespace string, owner metav1.Object, scheme *runtime.Scheme) error { - cmd := "/output/hiveutil" - args := []string{"install-manager", "aws-credentials"} - args = append(args, []string{"--namespace", secretNamespace}...) - args = append(args, []string{"--role-arn", role.RoleARN}...) - if role.ExternalID != "" { - args = append(args, []string{"--external-id", role.ExternalID}...) +// AWSAssumeRoleConfig creates or updates a secret with an AWS credentials file containing: +// - Role configuration for AssumeRole, pointing to... +// - A profile containing the source credentials for AssumeRole. +func AWSAssumeRoleConfig(client client.Client, role *hivev1aws.AssumeRole, secretName, secretNamespace string, owner metav1.Object, scheme *runtime.Scheme) error { + + // Credentials source + credsSecret := &corev1.Secret{} + credsSecretName := controllerutils.AWSServiceProviderSecretName(owner.GetName()) + if err := client.Get( + context.TODO(), + types.NamespacedName{ + Namespace: secretNamespace, + Name: credsSecretName, + }, + credsSecret); err != nil { + return errors.Wrapf(err, "failed to load credentials source secret %s", credsSecretName) } + // The old credential_process flow documented creating this with [default]. + // For backward compatibility, accept that, but convert to [profile source]. + sourceProfile := strings.Replace(string(credsSecret.Data[constants.AWSConfigSecretKey]), `[default]`, `[profile source]`, 1) - cmd = fmt.Sprintf("%s %s", cmd, strings.Join(args, " ")) - - template := `[default] -credential_process = %s -` - data := fmt.Sprintf(template, cmd) + extID := "" + if role.ExternalID != "" { + extID = fmt.Sprintf("external_id = %s\n", role.ExternalID) + } + + // Build the config file + configFile := fmt.Sprintf(`[default] +source_profile = source +role_arn = %s +%s +%s +`, + role.RoleARN, extID, sourceProfile) + + // Load the config secret if it already exists + configSecret := &corev1.Secret{} + if err := client.Get(context.TODO(), types.NamespacedName{Namespace: secretNamespace, Name: secretName}, configSecret); err != nil { + if !apierrors.IsNotFound(err) { + return errors.Wrapf(err, "failed to load config secret %s", secretName) + } + // Not found -- build it + configSecret = &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: secretNamespace, + Name: secretName, + }, + Data: map[string][]byte{ + constants.AWSConfigSecretKey: []byte(configFile), + }, + } + if err := controllerutil.SetOwnerReference(owner, configSecret, scheme); err != nil { + return err + } + return client.Create(context.TODO(), configSecret) + } - secret := &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Secret", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: secretNamespace, - Name: secretName, - }, - Data: map[string][]byte{ - constants.AWSConfigSecretKey: []byte(data), - }, + // Secret exists -- do we need to update it? Compare data and owner references. + origSecret := configSecret.DeepCopy() + configSecret.Data[constants.AWSConfigSecretKey] = []byte(configFile) + // SetOwnerReference is a no-op if the owner is already registered + if err := controllerutil.SetOwnerReference(owner, configSecret, scheme); err != nil { + return err } - if err := controllerutil.SetOwnerReference(owner, secret, scheme); err != nil { + if reflect.DeepEqual(origSecret, configSecret) { return nil } - return client.Create(context.TODO(), secret) + return client.Update(context.TODO(), configSecret) } // InstallerPodSpec generates a spec for an installer pod. diff --git a/pkg/installmanager/aws_credentials.go b/pkg/installmanager/aws_credentials.go deleted file mode 100644 index 3b1f497bd71..00000000000 --- a/pkg/installmanager/aws_credentials.go +++ /dev/null @@ -1,157 +0,0 @@ -package installmanager - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "os" - "time" - - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/credentials/stscreds" - "github.com/pkg/errors" - "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - contributils "github.com/openshift/hive/contrib/pkg/utils" - "github.com/openshift/hive/pkg/awsclient" - "github.com/openshift/hive/pkg/constants" -) - -// AWSCredentials is a supported external process credential provider as detailed in -// https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html -type AWSCredentials struct { - output io.WriteCloser - kubeClient client.Client - - ServiceProviderSecretName string - ServiceProviderSecretNamespace string - - RoleARN string - ExternalID string -} - -// NewInstallManagerAWSCredentials is the entrypoint to load credentials for AWS SDK -// using the service provider credentials. It supports the external process credential -// provider as mentioned in https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html -func NewInstallManagerAWSCredentials() *cobra.Command { - options := &AWSCredentials{} - cmd := &cobra.Command{ - Use: "aws-credentials", - Short: "Loads AWS credentials using the service provider credentials", - Long: "This loads the AWS credentials using the service provider credentials and then returns them in format defined in https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html", - Run: func(cmd *cobra.Command, args []string) { - if err := options.Complete(args); err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - - if err := options.Validate(); err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - - var err error - options.kubeClient, err = contributils.GetClient() - if err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - - if err := options.Run(); err != nil { - fmt.Fprint(os.Stderr, err.Error()) - os.Exit(1) - return - } - }, - } - flags := cmd.Flags() - flags.StringVar(&options.ServiceProviderSecretNamespace, "namespace", "", "The namespace where the service provider secret is stored") - cmd.MarkFlagRequired("namespace") - flags.StringVar(&options.RoleARN, "role-arn", "", "The IAM role that should be assumed") - cmd.MarkFlagRequired("role-arn") - flags.StringVar(&options.ExternalID, "external-id", "", "External identifier required to assume the role specified.") - - return cmd -} - -// Validate the options -func (options *AWSCredentials) Validate() error { return nil } - -// Complete the options using the args -func (options *AWSCredentials) Complete(args []string) error { - options.output = os.Stdout - options.ServiceProviderSecretName = os.Getenv(constants.HiveAWSServiceProviderCredentialsSecretRefEnvVar) - return nil -} - -// Run runs the command using the options. -func (options *AWSCredentials) Run() error { - var secret *corev1.Secret - if options.ServiceProviderSecretName != "" { - secret = &corev1.Secret{} - if err := options.kubeClient.Get(context.TODO(), - client.ObjectKey{Namespace: options.ServiceProviderSecretNamespace, Name: options.ServiceProviderSecretName}, - secret); err != nil { - return errors.Wrap(err, "failed to get the service provider secret") - } - } - - sess, err := awsclient.NewSessionFromSecret(secret, "") - if err != nil { - return errors.Wrap(err, "failed to create AWS session") - } - - duration := stscreds.DefaultDuration - creds := stscreds.NewCredentials(sess, options.RoleARN, func(p *stscreds.AssumeRoleProvider) { - p.Duration = duration - if options.ExternalID != "" { - p.ExternalID = &options.ExternalID - } - }) - v, err := creds.Get() - if err != nil { - return errors.Wrap(err, "failed to Assume the require role") - } - - resp, err := newCredentialProcessResponse(v, time.Now().Add(-1*time.Minute).Add(duration)) - if err != nil { - return errors.Wrap(err, "failed to create response for credential process") - } - _, err = fmt.Fprint(options.output, resp) - return err -} - -func newCredentialProcessResponse(v credentials.Value, expiry time.Time) (string, error) { - resp := &credentialProcessResponse{ - Version: 1, - AccessKeyID: v.AccessKeyID, - SecretAccessKey: v.SecretAccessKey, - SessionToken: v.SessionToken, - Expiration: &expiry, - } - outRaw, err := json.Marshal(resp) - if err != nil { - return "", errors.Wrap(err, "failed to create the credential process response") - } - outRawCompact := &bytes.Buffer{} - if err := json.Compact(outRawCompact, outRaw); err != nil { - return "", errors.Wrap(err, "failed to compact the JSON response") - } - - return outRawCompact.String(), nil -} - -type credentialProcessResponse struct { - Version int - AccessKeyID string `json:"AccessKeyId"` - SecretAccessKey string - SessionToken string - Expiration *time.Time -} diff --git a/pkg/installmanager/aws_credentials_test.go b/pkg/installmanager/aws_credentials_test.go deleted file mode 100644 index 9e3a141337b..00000000000 --- a/pkg/installmanager/aws_credentials_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package installmanager - -import ( - "testing" - "time" - - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestNewCredentialProcessResponse(t *testing.T) { - - cases := []struct { - name string - creds credentials.Value - resp string - }{{ - name: "valid credentials", - creds: credentials.Value{ - AccessKeyID: "ASX..ID...", - SecretAccessKey: "ASX..SECRET...", - SessionToken: "ASX..TOKEN...", - }, - resp: `{"Version":1,"AccessKeyId":"ASX..ID...","SecretAccessKey":"ASX..SECRET...","SessionToken":"ASX..TOKEN...","Expiration":"0001-01-01T00:00:00Z"}`, - }} - for _, test := range cases { - t.Run(test.name, func(t *testing.T) { - got, err := newCredentialProcessResponse(test.creds, time.Time{}) - require.NoError(t, err) - assert.Equal(t, test.resp, got) - }) - } -} diff --git a/pkg/installmanager/installmanager.go b/pkg/installmanager/installmanager.go index 4d1e0451706..0e2678876b0 100644 --- a/pkg/installmanager/installmanager.go +++ b/pkg/installmanager/installmanager.go @@ -205,7 +205,6 @@ SSH_PRIV_KEY_PATH: File system path of a file containing the SSH private key cor flags.StringVar(&im.WorkDir, "work-dir", "/output", "directory to use for all input and output") flags.StringVar(&im.LogsDir, "logs-dir", "/logs", "directory to use for all installer logs") - cmd.AddCommand(NewInstallManagerAWSCredentials()) return cmd } @@ -547,26 +546,28 @@ func loadSecrets(m *InstallManager, cd *hivev1.ClusterDeployment) { } // Load up the install config and pull secret. These env vars are required; else we'll panic. - contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "INSTALLCONFIG_SECRET_NAME"), "/installconfig") - contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "PULLSECRET_SECRET_NAME"), "/pullsecret") + contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "INSTALLCONFIG_SECRET_NAME"), "/installconfig", nil) + contributils.ProjectToDir(contributils.LoadSecretOrDie(m.DynamicClient, "PULLSECRET_SECRET_NAME"), "/pullsecret", nil) // Additional manifests? Could come in on a Secret or a ConfigMap if manSecret := contributils.LoadSecretOrDie(m.DynamicClient, "MANIFESTS_SECRET_NAME"); manSecret != nil { - contributils.ProjectToDir(manSecret, "/manifests") + contributils.ProjectToDir(manSecret, "/manifests", nil) } else if manCM := contributils.LoadConfigMapOrDie(m.DynamicClient, "MANIFESTS_CONFIGMAP_NAME"); manCM != nil { - contributils.ProjectToDir(manCM, "/manifests") + contributils.ProjectToDir(manCM, "/manifests", nil) } // Custom BoundServiceAccountSigningKey if bsask := contributils.LoadSecretOrDie(m.DynamicClient, "BOUND_TOKEN_SIGNING_KEY_SECRET_NAME"); bsask != nil { - contributils.ProjectToDir(bsask, constants.BoundServiceAccountSigningKeyDir, constants.BoundServiceAccountSigningKeyFile) + contributils.ProjectToDir( + bsask, constants.BoundServiceAccountSigningKeyDir, + contributils.ProjectOnlyTheseKeys(constants.BoundServiceAccountSigningKeyFile)) os.Setenv(constants.BoundServiceAccountSigningKeyEnvVar, constants.BoundServiceAccountSigningKeyDir+"/"+constants.BoundServiceAccountSigningKeyFile) } // SSH private key if sshkey := contributils.LoadSecretOrDie(m.DynamicClient, "SSH_PRIVATE_KEY_SECRET_PATH"); sshkey != nil { - contributils.ProjectToDir(sshkey, constants.SSHPrivateKeyDir) + contributils.ProjectToDir(sshkey, constants.SSHPrivateKeyDir, nil) // TODO: Collapse this in initSSHKey os.Setenv(constants.SSHPrivKeyPathEnvVar, constants.SSHPrivateKeyDir+"/"+constants.SSHPrivateKeySecretKey) @@ -574,7 +575,7 @@ func loadSecrets(m *InstallManager, cd *hivev1.ClusterDeployment) { // BareMetal Libvirt SSH private key if sshkey := contributils.LoadSecretOrDie(m.DynamicClient, "LIBVIRT_SSH_KEYS_SECRET_NAME"); sshkey != nil { - contributils.ProjectToDir(sshkey, constants.LibvirtSSHPrivateKeyDir) + contributils.ProjectToDir(sshkey, constants.LibvirtSSHPrivateKeyDir, nil) os.Setenv(constants.LibvirtSSHPrivKeyPathEnvVar, constants.LibvirtSSHPrivateKeyDir+"/"+constants.SSHPrivateKeySecretKey) }