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

Webhook signature verification fails #71025

Open
quintasda opened this issue May 16, 2024 · 9 comments · Fixed by #71310
Open

Webhook signature verification fails #71025

quintasda opened this issue May 16, 2024 · 9 comments · Fixed by #71310

Comments

@quintasda
Copy link

Environment

SaaS (https://sentry.io/)

Steps to Reproduce

Verify the Signature

Customer case

Expected Result

The signature matches

Actual Result

The signature doesn't match in ~10% of the messages

Product Area

Settings - Integrations

Link

No response

DSN

No response

Version

No response

@getsantry
Copy link
Contributor

getsantry bot commented May 16, 2024

Assigning to @getsentry/support for routing ⏲️

@getsantry
Copy link
Contributor

getsantry bot commented May 16, 2024

Routing to @getsentry/product-owners-settings-integrations for triage ⏲️

@getsantry getsantry bot moved this from Waiting for: Support to Waiting for: Product Owner in GitHub Issues with 👀 3 May 16, 2024
@Dhrumil-Sentry
Copy link

Thanks for this report, we will investigate this cc @sentaur-athena

@sentaur-athena
Copy link
Member

This issue is fixed now. We investigated and the reason was a new json serializer we started using since May 9. We reverted the changes to the previous serializer and looking at dashboards the 400 and 401 responses are eliminated.

@sentaur-athena
Copy link
Member

@quintasda as we're adding back the new serializer wanted to make sure we're not breaking customer's experience. Can you please provide me with code snippet of how you're validating signature?

You mentioned Verify the Signature from our docs but I was wondering if you're using the python or js example? Also what you use to dump json.

@jasonyonker
Copy link

@sentaur-athena I'm having issues with this using the exact code from the Sentry docs:

    const hmac = crypto.createHmac('sha256', clientSecret);
    hmac.update(JSON.stringify(body), 'utf8');
    const digest = hmac.digest('hex');
    return digest === signature;

This is on NodeJS 19.

Frankly, I don't think this solution is technically sound. JSON.stringify does not produce stable, deterministic output across different Node versions and even if it did there's no guarantee that serializers for Python, Ruby, Go, etc. will produce the same output. There are dedicated libraries that attempt to do this but I don't know of any standard that is widely supported on different platforms.

See the discussion here: nodejs/node#15628

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Oct 24, 2024
@sentaur-athena
Copy link
Member

@jasonyonker I agree with you. We should look into something more stable here. Let me look into it and get back to you.

@sentaur-athena
Copy link
Member

@jasonyonker I read the discussion you sent and I don't think any string serializer would work here since any customer might use a different language and framework. I'll update the docs and suggest to do the verification on raw bytes of body. The catch with that is that some frameworks don't provide raw body bytes at all. Looks like fastify and express might be some examples.

Can you try with hmac.update(raw_request_body)?

@Christinarlong
Copy link
Contributor

Christinarlong commented Dec 11, 2024

Update: We'll update the docs to bring awareness to this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

5 participants