Skip to content

fix: use gmdate to format x-amz-date with UTC irrespective of timezone#540

Merged
vishwarajanand merged 1 commit intogoogleapis:mainfrom
yfuruyama:fix/aws_amz_date_timezone
Mar 7, 2024
Merged

fix: use gmdate to format x-amz-date with UTC irrespective of timezone#540
vishwarajanand merged 1 commit intogoogleapis:mainfrom
yfuruyama:fix/aws_amz_date_timezone

Conversation

@yfuruyama
Copy link
Copy Markdown
Contributor

@yfuruyama yfuruyama commented Mar 5, 2024

Issue

Currently the value for x-amz-date is generated from date function, but if user sets a different timezone other than UTC, the generated timestamp has an invalid timestamp.

This causes the following SignatureDoesNotMatch error when authenticating with Workload Identity Federation:

When the timezone is older than UTC:

{"error":"invalid_grant","error_description":"The given AWS signed request is expired."}

When the timezone is later than UTC:

{"error":"invalid_grant","error_description":"Received invalid AWS response of type SignatureDoesNotMatch with error message: Signature not yet current: 20240305T152423Z is still later than 20240305T063924Z (20240305T062424Z + 15 min.)"}

This happens because date('Ymd\THis\Z') produces timezone-aware timestamp, but the suffix is always Z (UTC).

Reproduce

We can easily reproduce this issue just adding date_default_timezone_set('America/Los_Angeles'); to the code.

Also the following code illustrates how date('Ymd\THis\Z') produces invalid timestamp.

$ date
Tue Mar  5 06:37:39 UTC 2024

$ php -a
php > echo date('Ymd\THis\Z');
20240305T063748Z

php > date_default_timezone_set('America/Los_Angeles');

php > echo date('Ymd\THis\Z');
20240304T223756Z

How to fix this

I changed the code to use gmdate instead of date function irrespective of the timezone the user sets.

I locally changed the code and confirmed that this fix works even if a different timezone is set in the code.

@yfuruyama yfuruyama requested a review from a team March 5, 2024 06:40
@yfuruyama yfuruyama force-pushed the fix/aws_amz_date_timezone branch from 2d6582b to 597b6ea Compare March 5, 2024 06:42
@yfuruyama yfuruyama changed the title Use gmdate to format x-amz-date with UTC irrespective of timezone fix: use gmdate to format x-amz-date with UTC irrespective of timezone Mar 5, 2024
@yfuruyama
Copy link
Copy Markdown
Contributor Author

@bshaffer Hi Brent, could you review this pull request?

@vishwarajanand
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

@vishwarajanand vishwarajanand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good to me.

Per Aws guide on creating signed request, it appears that datetime is required in UTC time in ISO 8601.

I did some testing in my local and confirmed that this change is scoped and looks compatible with existing users php.ini configs.

@vishwarajanand vishwarajanand merged commit 3031d2c into googleapis:main Mar 7, 2024
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

Successfully merging this pull request may close these issues.

2 participants