ARSN-560: return AccessDenied error if x-amz-timestamp values before epoch#2599
Conversation
Hello leif-scality,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/8.2 #2599 +/- ##
===================================================
+ Coverage 71.96% 71.98% +0.01%
===================================================
Files 222 222
Lines 17940 17954 +14
Branches 3734 3738 +4
===================================================
+ Hits 12911 12924 +13
- Misses 5025 5026 +1
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fredmnl
left a comment
There was a problem hiding this comment.
Two comments come to mind but I'm not coming with a solution, so apologies for that. Other than that, I'm all good with the analysis and the intent of the fix.
lib/auth/v4/timeUtils.ts
Outdated
| * isValidISO8601Compact('20160208T251405Z'); // false (25 hours invalid) | ||
| * isValidISO8601Compact('2016-02-08T20:14:05Z'); // false (wrong format) | ||
| * isValidISO8601Compact('abcd0208T201405Z'); // false (contains letters) | ||
| * isValidISO8601Compact('19500707T215304Z'); // false (pre-Unix epoch) |
There was a problem hiding this comment.
Major. Semantically checking for < 1970 in isValidISO8601Compact isn't great because we are now denying valid ISO8601 inputs. 19500707T215304Z is actually a valid ISO8601 compact time
There was a problem hiding this comment.
replaced isValidISO8601Compact with ParseISO8601Compact that returns a Date object if it managed to parse the string, the epoch check is done outside the function using date.getTime() < 0
lib/auth/v4/timeUtils.ts
Outdated
|
|
||
| const [, year, month, day, hour, minute, second] = match; | ||
|
|
||
| if (parseInt(year, 10) < 1970) { |
There was a problem hiding this comment.
Nit. If it's easy to do, I would prefer unix < 0 rather than year < 1970 which would be a bit more meaningful. If it's adding more than a single line of code, I'm fine with keeping it written that way.
There was a problem hiding this comment.
replaced isValidISO8601Compact with ParseISO8601Compact that returns a Date object if it managed to parse the string, the epoch check is done outside the function using date.getTime() < 0
0724e6e to
13381ac
Compare
Branches have divergedThis pull request's source branch To avoid any integration risks, please re-synchronize them using one of the
Note: If you choose to rebase, you may have to ask me to rebuild |
c30114c to
8f6cb9f
Compare
|
8f6cb9f to
ed3fa7a
Compare
|
ed3fa7a to
4c683ec
Compare
|
lib/auth/v4/headerAuthCheck.ts
Outdated
| 'x-amz-date header') }; | ||
| return { | ||
| err: errorInstances.AccessDenied. | ||
| customizeDescription('Authentication requires a valid Date or ' + 'x-amz-date header') |
There was a problem hiding this comment.
| customizeDescription('Authentication requires a valid Date or ' + 'x-amz-date header') | |
| customizeDescription('Authentication requires a valid Date or x-amz-date header') |
…isValidISO8601Compact with ParseISO8601Compact
4c683ec to
362f790
Compare
|
ping |
|
LGTM |
|
ping |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
|
/create_integration_branches |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: git fetch
git checkout -B w/8.3/bugfix/ARSN-560-error-with-date-before-epoch origin/development/8.3
git merge origin/bugfix/ARSN-560-error-with-date-before-epoch
# <intense conflict resolution>
git commit
git push -u origin w/8.3/bugfix/ARSN-560-error-with-date-before-epochThe following options are set: create_integration_branches |
|
/create_pull_requests |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
Follow integration pull requests if you would like to be notified of The following options are set: create_pull_requests, create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_pull_requests, create_integration_branches |
|
/approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue ARSN-560. Goodbye leif-scality. The following options are set: approve, create_pull_requests, create_integration_branches |
Issue
The ceph s3-tests test_object_create_bad_amz_date_before_epoch_aws4 and test_bucket_create_bad_amz_date_before_epoch_aws4 are failing in CI. These tests send an AWS4-signed request with X-Amz-Date: 19500707T215304Z (pre-Unix epoch, 1950) and expect a 403 with error code AccessDenied or SignatureDoesNotMatch.
Cloudserver returns RequestTimeTooSkewed instead, causing an AssertionError.
Root cause
The regression was introduced in arsenal commit e3223a7 (ARSN-552, 2026-02-11). The old headerAuthCheck.ts had an inline check that explicitly rejected pre-1970 dates: Number.parseInt(xAmzDateArr[0], 10) > 19700101
ARSN-552 replaced this with a call to the new isValidISO8601Compact function, which no longer rejects pre-1970 dates — JavaScript's Date handles them fine as negative Unix timestamps, so the round-trip validation passes for 1950-07-07.
With a valid timestamp, execution reaches validateCredentials, which compares the credential scope date (20260323) against the x-amz-date timestamp date (19500707), finds a mismatch, and returns RequestTimeTooSkewed — the wrong error.
Fix
Add a pre-1970 guard in isValidISO8601Compact in arsenal/lib/auth/v4/timeUtils.ts, restoring the original behavior:
With this fix, isValidISO8601Compact('19500707T215304Z') returns false, timestamp stays undefined, and headerAuthCheck returns AccessDenied — matching the test expectation and the pre-ARSN-552 behavior.