Innablr / revolver

AWS Powercycle Facility
MIT License
2 stars 3 forks source link

Credentials cache does not consider region - and may provide invalid result? #312

Closed lyricnz closed 5 months ago

lyricnz commented 5 months ago

It looks like the code in connectTo() uses a cache, but this doesn't consider that multiple regions may be in use for the same role. This may result in a client being created with invalid credentials for the particular region.

https://github.com/Innablr/revolver/blob/develop/lib/assume.ts#L30 with problem areas highlighted.

  async connectTo(remoteRole: string, region?: string): Promise<Credentials | Provider<Credentials>> {
    logger.debug(`Requested connection via ${remoteRole}`);
    if (remoteRole === undefined || remoteRole.endsWith('/none')) {
      return fromNodeProviderChain();
    }

    if (remoteRole in this.creds) {
      if (this.creds[remoteRole].expiration > DateTime.now().setZone('UTC')) {
        logger.debug(`Role ${remoteRole} is cached, will expire at ${this.creds[remoteRole].expiration}`);
        return this.creds[remoteRole].creds; // <==========
      }
      logger.debug(
        `Cached role ${remoteRole} expired at ${this.creds[remoteRole].expiration}, requesting new creds...`,
      );
    }

    const awsConfig = getAwsConfig(region || 'ap-southeast-2'); // <==========
    logger.debug(`Assuming role ${remoteRole}...`);
    const sts = new STS(awsConfig); // <==========
    const creds = await sts
      .assumeRole({
        RoleArn: remoteRole,
        RoleSessionName: `Revolver_${dateTime.getTime().toFormat('yyyyLLddHHmmss')}`,
      })
      .then((r) => r.Credentials); // <==========
lyricnz commented 5 months ago

Is this real issue? STS is a global service, but it looks like it has a list of "enabled by default" regions (which doesn't include Melbourne)

https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp_enable-regions.html

If you use a regional endpoint (as recommended) then the credential is valid for that region only? So this bug is real?

lyricnz commented 5 months ago

I think this is a red herring. Certainly it's obsoleted by https://github.com/Innablr/revolver/pull/313