Add ecs and eks imds ip adresses to greengrass list

jgrigg commented 1 month ago

The AWS Profile EcsContainer credential provider is broken for application code running within the EKS environment using the AWS Pod Identity Service for credentials. This is because the AWS JSv3 SDK (using this library) configures itself using the fromContainerMetadata method which has an allow-list to only the ECS service (or localhost). The EcsContainer credential provider is intended to be used with either ECS or EKS credential services (see https://docs.aws.amazon.com/sdkref/latest/guide/feature-container-credentials.html). This library restricts this provider to usage only with the ECS credential service. This PR allow-lists the EKS credential service in addition to the ECS service to match the supported use-cases when configuring from the AWS profile config. This brings the AWS JSv3 SDK (which is supported) which uses this library back into feature-parity with the other SDKs supporting the EKS Pod Identity Service such as the Go SDK. It’s been noted in other PRs/issues around this issue that the fromHttp method can be used for the EKS Pod Identity Service. This is correct however this is for programmatic configuration whereas this PR fix is about fixing the supported use-case of configuring the SDK via the AWS Profile Config.

Note1: Equivalent PR from November 2023 in the Go SDK

Note2: this is effectively reopening https://github.com/smithy-lang/smithy-typescript/pull/1144

kuhe commented 1 month ago

what are the ENV vars and config file data needed to reproduce the issue?

kuhe commented 1 month ago

please provide, in addition to the environment variables and config file data, a code sample used to initialize an SDK client and perform the first request

jgrigg commented 1 month ago

Thanks George

ENV (those that are relevant):

        - name: AWS_CONFIG_FILE
          value: /workdir/.aws/config
        - name: AWS_STS_REGIONAL_ENDPOINTS
          value: regional
        - name: AWS_DEFAULT_REGION
          value: ap-southeast-2
        - name: AWS_REGION
          value: ap-southeast-2
        - name: AWS_CONTAINER_CREDENTIALS_FULL_URI
          value: 'http://169.254.170.23/v1/credentials'
        - name: AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE
          value: >-
            /var/run/secrets/pods.eks.amazonaws.com/serviceaccount/eks-pod-identity-token

/workdir/.aws/config

    # Content of the AWS Config file
    [default]
    source_profile = cluster_role
    role_arn = arn:aws:iam::1234567890:role/some-role

    [profile cluster_role]
    credential_source=EcsContainer

getSSMParameter.ts

import { GetParameterCommand, SSMClient } from '@aws-sdk/client-ssm';

export const getSSMParameter = async () => {
  const client = new SSMClient({}); // Default credential chain!

  const command = new GetParameterCommand({
    Name: 'my-param',
  });
  const response = await client.send(command);

  return response.Parameter?.Value;
};

kuhe commented 1 month ago

It looks like you want this sequence to happen based on that information:

merge the source_profile options into the default profile indicating the credential_source is EcsContainer
get initial credentials from AWS_CONTAINER_CREDENTIALS_FULL_URI
make an assume role call with STS to the default profile's role_arn
- use these secondary credentials in the client for further requests like GetParameter

is that correct?

Secondly, can you provide the output of using the default credential chain (except explicitly) and providing it a logger?

import { STS } from "@aws-sdk/client-sts";
import { defaultProvider } from "@aws-sdk/credential-provider-node";

const client = new STS({
  credentials: defaultProvider({
    logger: console,
  }),
});

await client.getCallerIdentity();

The output should look something like

@aws-sdk/credential-provider-node defaultProvider::fromEnv
@aws-sdk/credential-provider-env fromEnv
@aws-sdk/credential-provider-node defaultProvider::fromSSO
@aws-sdk/credential-provider-node defaultProvider::fromIni
@aws-sdk/credential-provider-ini fromIni
@aws-sdk/credential-provider-ini resolveAssumeRoleCredentials (STS)
@aws-sdk/credential-provider-node defaultProvider::fromProcess
@aws-sdk/credential-provider-process fromProcess
@aws-sdk/credential-provider-node defaultProvider::fromTokenFile
@aws-sdk/credential-provider-web-identity fromTokenFile
@aws-sdk/credential-provider-node defaultProvider::remoteProvider
@aws-sdk/credential-provider-node remoteProvider::fromHttp/fromContainerMetadata
@aws-sdk/credential-provider-http fromHttp

and it indicates the order of credential providers attempted in the chain.

jgrigg commented 1 month ago

Yes that's exactly what I'm trying to do :)

Logs as requested (baseline):

@aws-sdk/credential-provider-node defaultProvider::fromEnv
@aws-sdk/credential-provider-env fromEnv
@aws-sdk/credential-provider-node defaultProvider::fromSSO
@aws-sdk/credential-provider-node defaultProvider::fromIni
@aws-sdk/credential-provider-ini fromIni
@aws-sdk/credential-provider-ini resolveAssumeRoleCredentials (STS)
@aws-sdk/credential-provider-ini resolveAssumeRoleCredentials (STS)
[2024-04-15T23:57:51.447] [ERROR] default - {
  clientName: 'SSMClient',
  commandName: 'GetParameterCommand',
  input: { Name: 'AUTOMAT_REFERENCE_APP_TEST_PARAMETER' },
  error: CredentialsProviderError: 169.254.170.23 is not a valid container metadata service hostname
      at getCmdsUri (/workdir/node_modules/@smithy/credential-provider-imds/dist-cjs/index.js:158:13)
      at /workdir/node_modules/@smithy/credential-provider-imds/dist-cjs/index.js:118:34
      at retry (/workdir/node_modules/@smithy/credential-provider-imds/dist-cjs/index.js:104:17)
      at /workdir/node_modules/@smithy/credential-provider-imds/dist-cjs/index.js:117:16
      at resolveAssumeRoleCredentials (/workdir/node_modules/@aws-sdk/credential-provider-ini/dist-cjs/index.js:108:85)
      at async /workdir/node_modules/@smithy/property-provider/dist-cjs/index.js:79:27
      at async coalesceProvider (/workdir/node_modules/@smithy/property-provider/dist-cjs/index.js:106:18)
      at async /workdir/node_modules/@smithy/property-provider/dist-cjs/index.js:124:18
      at async /workdir/node_modules/@smithy/core/dist-cjs/index.js:82:17
      at async /workdir/node_modules/@aws-sdk/middleware-logger/dist-cjs/index.js:33:22 {
    tryNextLink: false
  },
  metadata: undefined
}

After applying this patch:

@aws-sdk/credential-provider-node defaultProvider::fromEnv
@aws-sdk/credential-provider-env fromEnv
@aws-sdk/credential-provider-node defaultProvider::fromSSO
@aws-sdk/credential-provider-node defaultProvider::fromIni
@aws-sdk/credential-provider-ini fromIni
@aws-sdk/credential-provider-ini resolveAssumeRoleCredentials (STS)
@aws-sdk/credential-provider-node defaultProvider::fromProcess
@aws-sdk/credential-provider-process fromProcess
@aws-sdk/credential-provider-node defaultProvider::fromTokenFile
@aws-sdk/credential-provider-web-identity fromTokenFile
@aws-sdk/credential-provider-node defaultProvider::remoteProvider
@aws-sdk/credential-provider-node remoteProvider::fromHttp/fromContainerMetadata
@aws-sdk/credential-provider-http fromHttp

☝️ Succeeds to get creds but not assume the role in the config file

In other news I've had a small win. I've managed to patch both your change here https://github.com/aws/aws-sdk-js-v3/pull/6132 AND the changes in this PR into my test harness. The role assumption is successful when i use the following config (the one i quoted above using source_profile still fails):

    # Content of the AWS Config file. This works!!!
    [default]
    role_arn = arn:aws:iam::1234567890:role/some-role
    credential_source=EcsContainer

...and the logs to go with it:

@aws-sdk/credential-provider-node defaultProvider::fromEnv
@aws-sdk/credential-provider-env fromEnv
@aws-sdk/credential-provider-node defaultProvider::fromSSO
@aws-sdk/credential-provider-node defaultProvider::fromIni
@aws-sdk/credential-provider-ini fromIni
@aws-sdk/credential-provider-ini resolveAssumeRoleCredentials (STS)
@aws-sdk/credential-provider-ini credential_source EcsContainer
@aws-sdk/credential-provider-http fromHttp

kuhe commented 1 month ago

I think I have a full picture of the issue now, and fixes will be applied in the linked PRs.

Here is a summary:

CredentialsProviderError: 169.254.170.23 is not a valid container metadata service hostname this is the superficial error, which makes it look like we need to add more hosts to the hardcoded allowlist within packages/credential-provider-imds/src/fromContainerMetadata.ts
- however, this is misleading, since the default credential provider chain in AWS SDKs for Node.js has many steps, and only reports the final terminal exception. There are actually many thrown & caught exceptions along the way as each credential provider within the chain is attempted.
- as a fix, we will improve the logging of the AWS SDK's node default credential chain, https://github.com/smithy-lang/smithy-typescript/pull/1290 & downstream in aws-sdk-js-v3 PR 6132.
the real root cause has two parts
1. the @aws-sdk/credential-provider-ini pkg, which is part of the default chain and is responsible for file-configured assumeRole credential resolution, does not route to the newer fromHttp provider, only the older fromContainerMetadata provider. The fix PR will add this.
2. the same package, when merging a source_profile, expects the source_profile to also have a role_arn. This is a separate bug, and I believe this isn't consistent with our docs at https://docs.aws.amazon.com/sdkref/latest/guide/feature-assume-role-credentials.html. It is not the source profile that should have a role_arn, it is the root profile.

Both bugs are planned to be addressed by https://github.com/aws/aws-sdk-js-v3/pull/6132. Release is pending review and testing.

After the patch the credential debug output should look like this:

@aws-sdk/credential-provider-node defaultProvider::fromEnv
@aws-sdk/credential-provider-env - fromEnv
@smithy/property-provider -> Unable to find environment variable credentials.
@aws-sdk/credential-provider-node defaultProvider::fromSSO
@smithy/property-provider -> Skipping SSO provider in default chain (inputs do not include SSO fields).
@aws-sdk/credential-provider-node defaultProvider::fromIni
@aws-sdk/credential-provider-ini - fromIni
    default isAssumeRoleWithSourceProfile source_profile=cluster_role
@aws-sdk/credential-provider-ini - resolveAssumeRoleCredentials (STS)
@aws-sdk/credential-provider-ini - finding credential resolver using source_profile=[cluster_role]
    cluster_role isCredentialSourceProfile credential_source=EcsContainer
@aws-sdk/credential-provider-ini - resolveAssumeRoleCredentials (STS)
@aws-sdk/credential-provider-ini - finding credential resolver using profile=[cluster_role]
@aws-sdk/credential-provider-ini - credential_source is EcsContainer
@aws-sdk/credential-provider-http - fromHttp
@aws-sdk/client-sts::resolveRegion accepting first of: undefined (provider) <<YOUR_REGION>> (parent client) us-east-1 (STS default)

That is, it should go directly from resolveAssumeRoleCredentials (STS) to fromHttp. With your PR's change, it looks like it is exiting the ini file-based provider and hitting the fromHttp/fromContainerMetadata providers later in the chain. In that scenario it is only seeing the environment variables and resolving container credentials in a terminal way without seeing the config file and its role_arn.

jgrigg commented 1 month ago

That all makes sense George thank you for following up on this 🙏

smithy-lang / smithy-typescript

Add ecs and eks imds ip adresses to greengrass list #1288