Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
GarrettBlinkhorn opened this issue Jan 19, 2024 · 5 comments

Comments

@GarrettBlinkhorn
Copy link

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

  • If I set up a new profile that handles the role assumption itself and pass that new profile then the command executes properly. If I pass this new profile alongside the same assume_role_arn then the command executes properly as well, but credentials are being sourced from my local ~/.aws/credentials rather than the temporary ones that were fetched:
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
Copy link
Author

GarrettBlinkhorn commented Jan 19, 2024

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 added a commit that referenced this issue Jan 31, 2024
- FIX : Typo in `aws_cli_runner.sh` when running assuming a role. Thank you [Garrett Blinkhorn](#11).
@rquadling
Copy link
Member

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 added a commit that referenced this issue Jan 31, 2024
- FIX : If `var.profile` and `var.assume_role_arn` are used, then continuing to use `var.profile` invalidates the
  assumed role. The `aws_cli_runner.sh` now no longer uses `var.profile` when a role has been successfully assumed.
  Thank you [Garrett Blinkhorn](#11).
rquadling added a commit that referenced this issue Jan 31, 2024
- Added testing for Terraform 1.7+
- FIX : If `var.profile` and `var.assume_role_arn` are used, then continuing to use `var.profile` invalidates the
  assumed role. The `aws_cli_runner.sh` now no longer uses `var.profile` when a role has been successfully assumed.
  Thank you [Garrett Blinkhorn](#11).
@rquadling
Copy link
Member

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
Copy link
Author

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 #12 as well

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

@rquadling
Copy link
Member

rquadling commented Jan 31, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants