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

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

Merged

Conversation

yfuruyama
Copy link
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 as a code owner 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
Contributor Author

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

@vishwarajanand
Copy link
Contributor

Copy link
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
12 checks passed
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