)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"ddd4262721dcb01d498d59faf8cf51657c261d3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f38d50fe_1aec5c45","updated":"2026-09-18 12:03:47.000000000","message":"Thanks for the review - you\u0027re right on the decoder mismatch, line-index assumptions, and the fail-open path (including an unsigned `token` field on EC2).\nI re-ran this against Swift s3token on DevStack rather than relying on static analysis alone. Findings:\n  - PS5\u0027s STS extraction did fire for SigV2 Date, but then failed because\n    `parse_isotime` cannot parse RFC 1123 HTTP-date → fresh s3token got 401.\n  - The fail-open `return` when `credentials[\u0027token\u0027]` was set was also a\n    real bypass for EC2 (unsigned token field).\n  This patch set follows the shape you suggested:\n  1. Shared base: require explicit Timestamp / X-Amz-Date; no token exception.\n  2. Move STS extraction into S3Resource (after signature verification), using\n     urlsafe_b64decode, with SigV2 (Date / x-amz-date / Expires) and SigV4\n     (line 1) parsing, plus RFC 1123 via parsedate_to_datetime; fail closed.\n  3. Tests covering the EC2 unsigned-token bypass and the s3token v2/v4 matrix.\nDevstack: fresh/expired SigV2+V4 s3tokens and EC2 behave as expected; end-to-end Swift S3 PUT through s3token middleware succeeds with a correctly signed STS.","commit_id":"c90b6edf0399b15ebf0a58118e29e1bc2c8f978a"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"f913ccdb2159b809e871ed4ede77434decdc5033","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5071169f_4ebcd3c5","updated":"2026-09-16 16:32:30.000000000","message":"review partially produced by AI agents and re-verified with other agents:\n\n- s3tokens.py\u0027s _check_signature decodes the token \n  with base64.urlsafe_b64decode, while the new _check_timestamp code \n  added in the commit uses base64.b64decode on the same field - verified \n  this mismatch empirically: urlsafe-encoded bytes containing -/_ \n  either raise binascii.Error (caught -\u003e falls into the fail-open \n  \"skip enforcement\" branch) or, worse, silently decode to different \n  bytes when non-alphabet chars are stripped by binascii.a2b_base64. \n  Reproduced both outcomes with os.urandom data in this session. \n- the existing test_good_response fixture in \n  test_contrib_s3_core.py (sts \u003d \u0027string to sign\u0027, base64-encoded) \n  has no newlines, so sts.split(\u0027\\n\u0027)[3] raises IndexError -\u003e caught -\u003e\n  fail-open return. This is why CI is green despite the extraction \n  logic never firing for that \"realistic-looking\" fixture - it\u0027s actually \n  an opaque non-representative string, not a real signed request. \n- Not independently verifiable here: the specific claim about what real \n  Swift\u0027s s3token middleware sends ({access, signature, token} only, \n  no Timestamp/X-Amz-Date) and the exact string-to-sign line layout per \n  client (s3cmd Date header vs boto x-amz-date vs v2 presigned Expires vs \n  v4) — this repo has no Swift source to check against. This matches \n  known AWS S3 SigV2/SigV4 string-to-sign layout from general knowledge \n  (V2: Verb\\nContent-MD5\\nContent-Type\\nDate\\nCanonicalizedAmzHeaders\\n CanonicalizedResource, with Date line blank and date embedded in\n  x-amz-date when that header is used instead; V4: \n  AWS4-HMAC-SHA256\\n\u003cISO8601-basic-timestamp\u003e\\n\u003cscope\u003e\\n\u003chash\u003e), which \n  is consistent with the reviewer\u0027s description, but flagged as \n  \"plausible / consistent with AWS spec, not independently reproduced \n  against live Swift code\" in the report rather than \"confirmed\".\n  \n- base64.b64decode vs urlsafe_b64decode mismatch — s3tokens.py:86 verifies the signature with urlsafe_b64decode; the new extraction uses b64decode. Swift sends urlsafe encoding; when the alphabet contains -/_, b64decode drops them → \"Incorrect padding\" → caught → fail-open bypass (reproduced: a perfectly valid presigned-STS token hit this). Use urlsafe_b64decode for consistency with the signature check.\n- Line index 3 is S3-v2-header-auth-only. v4 date is line 1 (YYYYMMDDTHHMMSSZ — parseable by parse_isotime); v2 query-auth line 3 is an epoch Expires; v2 x-amz-date puts the date in the canonicalized-amz-headers section. The parser needs to branch on AWS4 prefix and handle the x-amz-headers section (or the middleware should forward the date explicitly).\n- Fail closed: replace the return at EC2_S3_Resource.py:70-74 with Unauthorized. If you can\u0027t extract a date, deny.\n- Tests: no coverage of the extraction path at all; the five test-helper edits only add Timestamp so pre-existing EC2 tests survive the new 401. Add: EC2 + unsigned token must 401 (this would have caught finding 1), s3token v2-Date / v2-x-amz-date / v4 end-to-end cases.\n- except (base64.binascii.Error, ...)[EC2_S3_Resource.py:66] - binascii.Error is a ValueError subclass; except (ValueError, IndexError) is simpler.\n\nSuggested shape of the fix\n- In the shared base: require timestamp presence, raise Unauthorized if absent — no token exception at all.\n- Move the STS extraction into S3Resource (where token is actually signature-verified), using urlsafe_b64decode, with per-version parsing (v2: line 3 or the x-amz-date: line, accepting RFC 1123 via a second parse format; v4: line 1), failing closed on anything unexpected.\n- Add the EC2 \"unsigned token field\" regression test and the s3token matrix tests.","commit_id":"c90b6edf0399b15ebf0a58118e29e1bc2c8f978a"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"77293878629500200423df2d59d6f826e94012af","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"41de26ef_4f0b6e0f","updated":"2026-09-17 10:00:16.000000000","message":"I really have a problem remembering now, because it\u0027s a change I sent out for review 3 months ago. But I remember it coming out of a real tests done together with swift. The comments from your agent look like its all static analysis, I\u0027m not sure if it also analyzed swift or not. I\u0027ll try to re-run the tests and re-learn what this change was all about.","commit_id":"4b06312d6c9bf08fda3e78139f167db57452b1fe"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"851e8bb780dd8a1bf8749543861e4e8dbcd8e68b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"28645b98_0ef89e64","in_reply_to":"41de26ef_4f0b6e0f","updated":"2026-09-17 10:44:09.000000000","message":"it was a static analysis including the swift code base. But I surely understand what you mean","commit_id":"4b06312d6c9bf08fda3e78139f167db57452b1fe"}],"keystone/api/_shared/EC2_S3_Resource.py":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"f913ccdb2159b809e871ed4ede77434decdc5033","unresolved":true,"context_lines":[{"line_number":57,"context_line":"        # For callers such as Swift\u0027s s3token middleware the timestamp is"},{"line_number":58,"context_line":"        # embedded inside the string-to-sign rather than sent as a separate"},{"line_number":59,"context_line":"        # field.  Extract it from there so auth_ttl is enforced consistently."},{"line_number":60,"context_line":"        if not timestamp and credentials.get(\u0027token\u0027):"},{"line_number":61,"context_line":"            try:"},{"line_number":62,"context_line":"                sts \u003d base64.b64decode(credentials[\u0027token\u0027]).decode(\u0027utf-8\u0027)"},{"line_number":63,"context_line":"                date_field \u003d sts.split(\u0027\\n\u0027)[3].strip()"}],"source_content_type":"text/x-python","patch_set":5,"id":"13164fd5_9adc7d9c","line":60,"updated":"2026-09-16 16:32:30.000000000","message":"handle_authenticate calls _check_signature (ec2tokens.py:169-ish, actually shared EC2_S3_Resource.py:169) before _check_timestamp (EC2_S3_Resource.py:186). keystoneclient\u0027s Ec2Signer._calc_signature_2 only signs params/verb/host/path. A token field added to the EC2 JSON body is never covered by the signature. _check_timestamp (EC2_S3_Resource.py:60-74 on HEAD) treats credentials.get(\u0027token\u0027) truthy as \"s3token caller, skip enforcement\" - this is exactly the fail-open branch the commit message describes. Any validly-signed EC2 request with an added unsigned token field bypasses TTL entirely, either by failing extraction (IndexError on sts.split(\u0027\\n\u0027)[3]) or by crafting a token whose decoded line 3 is an attacker-chosen fresh date.","commit_id":"c90b6edf0399b15ebf0a58118e29e1bc2c8f978a"}],"keystone/tests/unit/test_v3_trust.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"557b4fb5953ad761a81ff0e70d60a878468e8745","unresolved":false,"context_lines":[{"line_number":24,"context_line":"from keystone.api import trusts as trusts_api"},{"line_number":25,"context_line":"from keystone.common import authorization"},{"line_number":26,"context_line":"from keystone.common import utils"},{"line_number":27,"context_line":"from keystone.common import context"},{"line_number":28,"context_line":"from keystone.common import provider_api"},{"line_number":29,"context_line":"import keystone.conf"},{"line_number":30,"context_line":"from keystone.credential.providers import fernet as credential_fernet"}],"source_content_type":"text/x-python","patch_set":4,"id":"f3eb6e52_255fbf9c","line":27,"updated":"2026-09-16 14:26:08.000000000","message":"pep8: H306: imports not in alphabetical order (keystone.common.utils, keystone.common.context)","commit_id":"896f6541dafb876a74de5535c636297da4f8be8c"}]}