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

Suspected DST-sensitivity in credential expiration logic #1658

Open
ldebrouwer opened this issue Feb 2, 2024 · 2 comments
Open

Suspected DST-sensitivity in credential expiration logic #1658

ldebrouwer opened this issue Feb 2, 2024 · 2 comments

Comments

@ldebrouwer
Copy link

I stumbled across an issue during the last DST change of 2023, that I believe may be related to how this package determines the expiry moment for its credentials, or potentially the caching strategy around it.

We witnessed several K8s pods crashing the hour after the clocks went back by an hour in 2023, with the error message thrown by this package; The security token included in the request is expired.

It's my hypothesis that this might have happened because the expiry moment was determined prior to the DST-change moment, and that timezone information might be mishandled.

We also use the official AWS PHP SDK in the same application, and code using this package was unaffected.

The main difference I can spot between this package and the official AWS PHP SDK is that the latter converts the expiry moment to a Unix timestamp pretty early on and does its comparison against that. Whilst async-aws/core creates and uses a DateTimeImmutable for this.

Additional information;

  • We're in the Europe/London timezone.
  • We use the WebIdentity (K8s pod IAM web session identity) credential provider.
@stof
Copy link
Member

stof commented Feb 2, 2024

Maybe the expiration logic in credential providers should enforce using the UTC timezone instead of using the default timezone from php.ini

@jderusse what do you think ?

@ldebrouwer
Copy link
Author

Either that, or using a Unix timestamp like aws/aws-sdk-php would be the way forward I think. Currently the credential provider retrieves the expiration timestamp (represented in Zulu time format). When this is passed into a DateTimeImmutable, the resulting timezone type will be 2. E.g.;

object(DateTimeImmutable)#1 (3) {
  ["date"]=>
  string(26) "2024-02-03 08:55:05.000000"
  ["timezone_type"]=>
  int(2)
  ["timezone"]=>
  string(1) "Z"
}
  • Timezone type 1 indicates that the object was created from a timestamp with a UTC offset, for example; 2024-02-03T08:55:05+00:00.
  • Timezone type 2 indicates that the object was created from a timestamp with a timezone abbreviation, like GMT or Z in the example above.
  • Timezone type 3 indicates that an actual timezone identifier was used; new DateTimeImmutable('2024-02-03 08:55:05', new DateTimeZone('UTC')).

AFAIK only a DateTimeImmutable object with timezone type 3 allows for proper handling of DST. That said, as you can see, I had to omit the Z in the example of timezone type 3 to achieve this correct behaviour. So to achieve something similar you'd have to opt for; DateTimeImmutable::createFromFormat('Y-m-d\TH:i:s\Z', '2024-02-03T08:55:05Z', new DateTimeZone('UTC'))).

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

No branches or pull requests

2 participants