Skip to content

fix(s3-events): implement OSS callback signature verification#163

Open
Kartalops wants to merge 1 commit into
Ontos-AI:mainfrom
Kartalops:fix/arvuno/oss-callback-signature-verify
Open

fix(s3-events): implement OSS callback signature verification#163
Kartalops wants to merge 1 commit into
Ontos-AI:mainfrom
Kartalops:fix/arvuno/oss-callback-signature-verify

Conversation

@Kartalops

Copy link
Copy Markdown

Replaces the TODO stub in verify_oss_signature with a real HMAC-SHA1 verification using the OSS v1 string-to-sign layout (method, content-md5, content-type, date, canonicalized resource). The function previously returned True unconditionally after reading OSS_EVENT_VERIFY_SIGNATURE and OSS_EVENT_CALLBACK_KEY from settings.

Behavior:

  • If OSS_EVENT_VERIFY_SIGNATURE is disabled, skip (unchanged).
  • If callback key is not set, skip with a warning (unchanged).
  • If public key is not set, skip with a warning (new; allows staging before keys are wired up).
  • Otherwise parse the Authorization: OSS <key>:<sig> header, build the canonical string, compute HMAC-SHA1(callback_key, string_to_sign), base64-encode, and constant-time-compare against the provided signature.
  • Any malformed header or comparison mismatch returns False and logs the reason.

The change is contained to the single TODO block. No new dependencies — hashlib, hmac, base64, urllib.parse are all stdlib.

Risk: low. Behavior is gated by OSS_EVENT_VERIFY_SIGNATURE and OSS_EVENT_CALLBACK_KEY — both already required by the existing flow. Anyone who relied on the stub returning True while these were set will now correctly receive rejections.

Follow-ups for a future PR (not in scope here):

  • A unit test file. The existing tests in this repo use pytest with a tests/ directory; the new test would need the actual callback key to be configured in fixtures.
  • Wire OSS_EVENT_PUBLIC_KEY into shared.core.config.settings (it is read with getattr(..., default="") here to keep the change local).

Replaces the TODO stub in verify_oss_signature with a real HMAC-SHA1
verification using the OSS v1 string-to-sign layout (method, content-md5,
content-type, date, canonicalized resource). Reads OSS_EVENT_PUBLIC_KEY
from settings and requires the OSS_EVENT_CALLBACK_KEY already in use.

Behavior:
- If OSS_EVENT_VERIFY_SIGNATURE is disabled, skip (unchanged).
- If callback key is not set, skip with a warning (unchanged).
- If public key is not set, skip with a warning (new; allows staging).
- Otherwise validate the Authorization header (OSS <key>:<sig>) and
  compare the HMAC-SHA1 of the canonicalized string with the shared key.

Falls back to logging and returning False on any parse failure.
@suguanYang

Copy link
Copy Markdown
Contributor

Thanks for the patch, The current implementation uses a local HMAC-style check, but Aliyun OSS callbacks require public-key/RSA verification over the callback request path/query/body. Please refer to Aliyun’s official “Verify the request signature” section here: https://help.aliyun.com/en/oss/developer-reference/callback

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.

3 participants