async-aws / aws

AWS SDK with readable code and async responses
https://async-aws.com
MIT License
429 stars 129 forks source link

Suspected DST-sensitivity in credential expiration logic #1658

Open ldebrouwer opened 7 months ago

ldebrouwer commented 7 months ago

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;

stof commented 7 months ago

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 commented 7 months ago

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"
}

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'))).