digitickets / terraform-aws-cli

Run the AWS CLI, with the ability to run under an assumed role, to access resources and properties missing from the Terraform AWS Provider.
26 stars 11 forks source link

`profile` Parameter Prevents the Use of Temp. Creds from `assume_role_arn` #11

Closed GarrettBlinkhorn closed 5 months ago

GarrettBlinkhorn commented 6 months ago

Versions

Terraform v1.6.5 on darwin_arm64
provider registry.terraform.io/hashicorp/aws v5.33.0
digitickets/cli/aws version = "5.2.0"

Expected Behavior

If I supply both a profile and an assume_role_arn to the module, then the module should use profile to retrieve a temporary set of credentials for assume_role_arn, then execute the desired query using the temporary credentials that were retrieved, not the profile itself.

In my case, I have a profile configured for the account where I store my Terraform state/resources and I want to make a cross-account role assumption to assume_role_arn so that I can use this module to read some data from the target account.

In aws_cli_runner.sh, if the user passes the assume_role_arn then the script uses the profile param alongside the assume_role_arn to retrieve a set of temporary credentials and store them as AWS_ENV_VARS.

# Do we need to assume a role?
if [ -n "${ASSUME_ROLE_ARN}" ]; then

  # Do we have an external ID?
  if [ -n "${EXTERNAL_ID}" ]; then
    AWS_CLI_EXTERNAL_ID_PARAM="--external-id '${EXTERNAL_ID}'"
  fi

  TEMP_ROLE=$(aws sts assume-role ${AWS_CLI_PROFILE_PARAM:-} ${AWS_CLI_REGION_PARAM:-} --output json --role-arn "${ASSUME_ROLE_ARN}" ${AWS_CLI_EXTERNAL_ID_PARAM:-} --role-session-name "${ROLE_SESSION_NAME:-AssumingRole}")
  export AWS_ACCESS_KEY_ID=$(echo "${TEMP_ROLE}" | jq -r '.Credentials.AccessKeyId')
  export AWS_SECRET_ACCESS_KEY=$(echo "${TEMP_ROLE}" | jq -r '.Credentials.SecretAccessKey')
  export AWS_SESSION_TOKEN=$(echo "${TEMP_ROLE}" | jq -r '.Credentials.SessionToken')
fi

Problem

The aws command which executes the aws_cli_commands also includes the AWS_CLI_PROFILE_PARAM if profile is provided, which it is in this case:

# Run the AWS_CLI command, exiting with a non zero exit code if required.
if ! eval "aws ${AWS_CLI_COMMANDS} ${AWS_CLI_PROFILE_PARAM:-} ${AWS_CLI_REGION_PARAM:-} ${AWS_CLI_QUERY_PARAM:-} --output json ${AWS_DEBUG_OPTION:-}" >"${OUTPUT_FILE}" ; then
  echo "Error: aws failed."
  exit 1
fi

This prevents the temporary credentials that were fetched earlier from actually being used, meaning that if both profile and assume_role_arn are passed, then its not actually possible to use the credentials that were fetched to run the aws command using the assume_role_arn.

When you run AWS CLI commands, the AWS CLI looks for credentials in a specific order—first in environment variables and then in the configuration file. Therefore, after you've put the temporary credentials into environment variables, the AWS CLI uses those credentials by default. (If you specify a profile parameter in the command, the AWS CLI skips the environment variables. Instead, the AWS CLI looks in the configuration file, which lets you override the credentials in the environment variables if you need to.) - Using temporary security credentials with the AWS CLI

Additional Findings

2024-01-19 11:40:07,666 - MainThread - botocore.credentials - DEBUG - Looking for credentials via: assume-role
2024-01-19 11:40:07,666 - MainThread - botocore.credentials - DEBUG - Looking for credentials via: assume-role-with-web-identity
2024-01-19 11:40:07,666 - MainThread - botocore.credentials - DEBUG - Looking for credentials via: sso
2024-01-19 11:40:07,666 - MainThread - botocore.credentials - DEBUG - Looking for credentials via: shared-credentials-file
2024-01-19 11:40:07,666 - MainThread - botocore.credentials - INFO - Found credentials in shared credentials file: ~/.aws/credentials

Desired Outcome

Remediate the above so that when both a profile and an assume_role_arn are passed, the script uses the profile to fetch the temporary credentials necessary to assume_role_arn then uses said credentials to execute the cli_query.

In my case, I have data in Account A that I would like make read-only for other teams in other accounts. Those teams have unique aws profiles that they use inside of their own accounts. I want to create a ReadOnly role in Account A, then allow other teams to use this role to read the data in Account A.

I would grant AssumeRole rights on my ReadOnly role to the unique roles that pertain to each different team's specific profiles, so that they can use this module and those profiles to assume the ReadOnly role and execute the query in Account A.

In this way, I can control Read access to Account A simply by adding and removing AssumeRole ARNs to my ReadOnly role policy. Then consuming teams can use our existing IAM role structure and this module to read the relevant data.

GarrettBlinkhorn commented 6 months ago

I went ahead and upgraded my "digitickets/cli/aws" to 6.0.1 just to rule out any possible differences there. I can see that the formatting of the script has changed but the fundamental issue described above still persists.

Additionally, I've discovered a new bug in the latest version when making the STS call for temporary creds:

### Get temporary credentials from AWS STS
  if ! eval "aws sts assume-role \
    --role-arn ${ASSUME_ROLE_ARN} \
    ${AWS_CLI_EXTERNAL_ID_PARAM:-} \
    --role-session-name ${ROLE_SESSION_NAME:-AssumingRole} \
    --output json \
    --debug
    ${AWS_CLI_PROFILE_PARAM:-} \
    ${AWS_CLI_REGION_PARAM:-} \
    " \
    >"${AWS_STS_JSON}" \
    2>"${AWS_STS_ERROR_LOG}"; then
      echo '{"error":"The call to AWS to get temporary credentials failed"}' >&2
      exit 4
    fi

The --debug is missing its \ which causes this command to fail and throw the error message described. Adding the \ locally resolved the issue here, though the core issue described above continues to persist.

rquadling commented 5 months ago

Hi. Thanks for this (and sorry for the delay).

I've just released. v6.0.2 for the silly bug fix on the debug.

Just looking at the logic for profile with assume role. It makes perfect sense to only use the profile once within the script. Either to assume a supplied role, or to use as part of the AWS call. Using it for both is sort of daft! So, yeah, sorry about that.

Will do my usual reviews and get it sorted.

rquadling commented 5 months ago

V6.1.0 has just been released. The approach I've used is to remove the profile variable once it has been successfully used to assume a role. If there was no role to assume, then the profile variable remains in place and will then be used against the AWS CLI call as before.

Hopefully this covers it.

GarrettBlinkhorn commented 5 months ago

Thanks for taking a look - I've tested v6.1.0 on my end and can confirm that it resolved the issue described above. Unsetting the profile variable is a much cleaner solution than the if-else solution I had originally proposed in https://github.com/digitickets/terraform-aws-cli/pull/12 as well

I'm closing this issue since the latest release resolved it.

rquadling commented 5 months ago

Happy to have helped.

On Wed, 31 Jan 2024, 13:36 Garrett Blinkhorn, @.***> wrote:

Closed #11 https://github.com/digitickets/terraform-aws-cli/issues/11 as completed.

— Reply to this email directly, view it on GitHub https://github.com/digitickets/terraform-aws-cli/issues/11#event-11655873497, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADEAIYSNO7BU2X7MFV46DYRJCF5AVCNFSM6AAAAABCCIFNUWVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGY2TKOBXGM2DSNY . You are receiving this because you commented.Message ID: @.*** com>