From 57b1d012c011089f8dae601cefe9991d62c45f0c Mon Sep 17 00:00:00 2001 From: matthiasL-scality Date: Fri, 17 Jul 2026 11:58:51 +0200 Subject: [PATCH 1/2] Fix GCS presigned URL signature for multipart part uploads GCS has two deviations from standard AWS S3 V2 signing for presigned URLs: 1. ':' in the object key is normalised to '%3A' in the canonical resource 2. ?partNumber=N&uploadId=X is NOT included in the canonical resource for presigned part PUTs (unlike the AWS S3 V2 spec) Both mismatches caused SignatureDoesNotMatch (403) on every part upload, making all multipart uploads fail. Validated directly against GCS before this fix by generating presigned part URLs and PUTting 5 MB parts. Also applies the ':' fix to PRESIGN_PUT for consistency. Add two e2e tests that check the presigned URL path contains '%3A' instead of raw ':', catching this class of bug in CI against cloudserver. Co-Authored-By: Claude Sonnet 4.6 --- lua/compute_aws_s3_signature.lua | 22 +++++++++------- tests/end2end/test_presign_upload.py | 39 +++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/lua/compute_aws_s3_signature.lua b/lua/compute_aws_s3_signature.lua index d4603e3..84cf8de 100644 --- a/lua/compute_aws_s3_signature.lua +++ b/lua/compute_aws_s3_signature.lua @@ -344,7 +344,10 @@ elseif signature_mode == "PRESIGN_PUT" then local expires = ngx.time() + 3600 local aws_secret_key = os.getenv('AWS_SECRET_ACCESS_KEY') - local canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. ngx.var.encoded_key + -- GCS normalises ':' to '%3A' when computing StringToSign for presigned URLs; + -- encode it here so the signature matches what GCS will verify. + local url_safe_key = ngx.var.encoded_key:gsub(':', '%%3A') + local canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key -- Sign as PUT with no Content-MD5 and no Content-Type (presigned, not proxied). local string_to_sign = "PUT\n\n\n" .. expires .. "\n" .. canonicalized_resource local aws_signature = ngx.encode_base64(ngx.hmac_sha1(aws_secret_key, string_to_sign)) @@ -373,17 +376,18 @@ elseif signature_mode == "PRESIGN_PART" then end local expires = ngx.time() + 3600 local aws_secret_key = os.getenv('AWS_SECRET_ACCESS_KEY') - -- GCS V2 requires URL-encoded values in the canonical resource (unlike AWS V2 which uses raw values). - -- See: https://cloud.google.com/storage/docs/access-control/signed-urls-v2 - -- ngx.var.arg_* may return a raw (already percent-encoded) or decoded value depending on the nginx - -- version. Normalize by unescaping first, then re-escaping to avoid double-encoding (%2B → %252B). + -- GCS normalises ':' to '%3A' when computing StringToSign for presigned URLs; + -- encode it here so the signature matches what GCS will verify. + local url_safe_key = ngx.var.encoded_key:gsub(':', '%%3A') + -- Normalise uploadId: ngx.var.arg_* may be pre-encoded or raw depending on the nginx + -- version; unescape then re-escape to avoid double-encoding (%2B → %252B). local escaped_upload_id = ngx.escape_uri(ngx.unescape_uri(upload_id)) - -- subresources must appear in canonical resource (alphabetical: partNumber < uploadId) - local canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. ngx.var.encoded_key .. - "?partNumber=" .. part_number .. "&uploadId=" .. escaped_upload_id + -- GCS does NOT include ?partNumber=N&uploadId=X in the canonical resource + -- for presigned part PUTs (unlike standard AWS S3 V2 spec). + local canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key local string_to_sign = "PUT\n\n\n" .. expires .. "\n" .. canonicalized_resource local aws_signature = ngx.encode_base64(ngx.hmac_sha1(aws_secret_key, string_to_sign)) - local presigned_url = ngx.var.redirect_endpoint .. "/" .. ngx.var.aws_tgt_bucket .. "/" .. ngx.var.encoded_key .. + local presigned_url = ngx.var.redirect_endpoint .. "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key .. "?partNumber=" .. part_number .. "&uploadId=" .. escaped_upload_id .. "&AWSAccessKeyId=" .. ngx.escape_uri(ngx.var.aws_access_key) .. diff --git a/tests/end2end/test_presign_upload.py b/tests/end2end/test_presign_upload.py index 242c066..27b7b3f 100644 --- a/tests/end2end/test_presign_upload.py +++ b/tests/end2end/test_presign_upload.py @@ -8,6 +8,7 @@ """ import hashlib +from urllib.parse import urlparse import requests @@ -55,6 +56,17 @@ def test_presign_upload_file_reachable_after_direct_put(session, artifacts_url): assert dl.content == data +def test_presign_upload_encodes_colons_in_url(session, artifacts_url): + """Presigned URL path must use %3A for ':' — GCS normalises ':' to '%3A' + when computing StringToSign, so a literal ':' causes SignatureDoesNotMatch.""" + resp = session.get(f'{artifacts_url}/presign-upload/{STAGING_BUILD}/colon-check.txt') + assert resp.status_code == 200 + url = resp.text.strip() + path = urlparse(url).path + assert '%3A' in path, f"Expected '%3A' in presigned URL path, got: {path!r}" + assert ':' not in path, f"Raw ':' in presigned URL path causes SignatureDoesNotMatch on GCS" + + def test_presign_upload_rejects_non_get(session, artifacts_url): """Only GET is allowed on /presign-upload/; other methods return 400.""" url = f'{artifacts_url}/presign-upload/{STAGING_BUILD}/file.txt' @@ -94,10 +106,6 @@ def test_presign_upload_part_returns_url(session, artifacts_url): def test_presign_upload_part_encodes_special_chars_in_upload_id(session, artifacts_url): """uploadId containing +, / and = (GCS-style base64) is percent-encoded in the presigned URL.""" - # Use a synthetic uploadId with base64 special characters. The PRESIGN_PART - # endpoint does not validate that the uploadId corresponds to a real upload, - # so we can inject one directly to verify the encoding behaviour without - # needing a backend that generates such IDs. special_upload_id = 'abc+def/ghi=jkl' resp = session.get( f'{artifacts_url}/presign-upload-part/{STAGING_BUILD}/presign/encoding-test.bin', @@ -105,13 +113,32 @@ def test_presign_upload_part_encodes_special_chars_in_upload_id(session, artifac ) assert resp.status_code == 200, f'{resp.status_code} {resp.text}' url = resp.text.strip() - # The uploadId must appear percent-encoded in the presigned URL so that GCS - # can reconstruct the canonical resource from the URL literally (GCS V2 spec). assert 'uploadId=abc%2Bdef%2Fghi%3Djkl' in url, ( f'Expected uploadId to be percent-encoded in presigned URL, got: {url!r}' ) +def test_presign_upload_part_encodes_colons_in_url(session, artifacts_url): + """Presigned part URL path must use %3A for ':' — GCS normalises ':' to '%3A' + when computing StringToSign, so a literal ':' causes SignatureDoesNotMatch.""" + upload_id = multipart_initiate(session, artifacts_url, STAGING_BUILD, 'presign/colon-part.bin') + try: + resp = session.get( + f'{artifacts_url}/presign-upload-part/{STAGING_BUILD}/presign/colon-part.bin', + params={'partNumber': 1, 'uploadId': upload_id}, + ) + assert resp.status_code == 200, f'{resp.status_code} {resp.text}' + url = resp.text.strip() + path = urlparse(url).path + assert '%3A' in path, f"Expected '%3A' in presigned part URL path, got: {path!r}" + assert ':' not in path, f"Raw ':' in presigned part URL path causes SignatureDoesNotMatch on GCS" + finally: + session.delete( + f'{artifacts_url}/upload-multipart/abort/{STAGING_BUILD}/presign/colon-part.bin', + params={'uploadId': upload_id}, + ) + + def test_presign_multipart_full_round_trip(session, artifacts_url): """Initiate via nginx, upload parts directly to S3, complete via nginx.""" build = STAGING_BUILD From 08d997eaaea06b72e9696b8b2feb871bbe8c4a09 Mon Sep 17 00:00:00 2001 From: matthiasL-scality Date: Fri, 17 Jul 2026 12:09:23 +0200 Subject: [PATCH 2/2] Fix PRESIGN_PART canonical resource to handle GCS vs AWS S3 backends GCS does not include ?partNumber=N&uploadId=X in the canonical resource when verifying presigned part PUT signatures (unlike standard AWS S3 V2). cloudserver and other AWS-compatible backends do include them. Detect GCS from ENDPOINT_URL and omit subresources only for GCS, keeping the standard behaviour for all other backends. Co-Authored-By: Claude Sonnet 4.6 --- lua/compute_aws_s3_signature.lua | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lua/compute_aws_s3_signature.lua b/lua/compute_aws_s3_signature.lua index 84cf8de..c5b795d 100644 --- a/lua/compute_aws_s3_signature.lua +++ b/lua/compute_aws_s3_signature.lua @@ -382,9 +382,17 @@ elseif signature_mode == "PRESIGN_PART" then -- Normalise uploadId: ngx.var.arg_* may be pre-encoded or raw depending on the nginx -- version; unescape then re-escape to avoid double-encoding (%2B → %252B). local escaped_upload_id = ngx.escape_uri(ngx.unescape_uri(upload_id)) - -- GCS does NOT include ?partNumber=N&uploadId=X in the canonical resource - -- for presigned part PUTs (unlike standard AWS S3 V2 spec). - local canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key + -- GCS does NOT include ?partNumber=N&uploadId=X in the canonical resource for presigned + -- part PUTs. Standard AWS S3-compatible backends (cloudserver, Scaleway S3) do include + -- them per the V2 spec. Detect GCS from ENDPOINT_URL to pick the right behaviour. + local endpoint_url = os.getenv('ENDPOINT_URL') or '' + local canonicalized_resource + if endpoint_url:find('googleapis', 1, true) then + canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key + else + canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key .. + "?partNumber=" .. part_number .. "&uploadId=" .. escaped_upload_id + end local string_to_sign = "PUT\n\n\n" .. expires .. "\n" .. canonicalized_resource local aws_signature = ngx.encode_base64(ngx.hmac_sha1(aws_secret_key, string_to_sign)) local presigned_url = ngx.var.redirect_endpoint .. "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key ..