Bug 1213630 - Webhook header needs to include cryptographic signature in header for identification.
Webhook header needs to include cryptographic signature in header for identif...
Status: CLOSED CURRENTRELEASE
Product: Zanata
Classification: Community
Component: Security (Show other bugs)
3.7
Unspecified Unspecified
high Severity high
: ---
: 3.8
Assigned To: Alex Eng
Damian Jansen
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-20 20:19 EDT by Alex Eng
Modified: 2016-01-20 20:40 EST (History)
7 users (show)

See Also:
Fixed In Version: 3.8.0-SNAPSHOT (git-jenkins-zanata-server-github-pull-requests-3971)
Doc Type: Bug Fix
Doc Text:
Story Points: 2
Clone Of:
Environment:
Last Closed: 2016-01-20 20:40:14 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Alex Eng 2015-04-20 20:19:04 EDT
Description of problem:

Current Webhook post data doesn't have any Zanata identification. This would potentially expose the listener of Zanata webhook to any anonymous POST. 

Zanata webhook should include cryptographic signature for security purposes.
Comment 1 Ralph Bean 2015-06-18 15:06:24 EDT
Just to clarify, we can't actually use the webhooks feature in Fedora Infrastructure unless this is implemented.

If anyone used the webhooks feature, there is no way currently to verify that the POST actually comes from zanata.org, and so they would be wide open to abuse.

It would be quite convenient if zanata signed their messages in the same way or in a similar way that github.com does.

https://developer.github.com/webhooks/securing/#validating-payloads-from-github
Comment 2 Alex Eng 2015-06-21 23:49:20 EDT
This bug should also make changes to the json output
- include URL for the related document in project-version

Current output:
{
    "eventType": "org.zanata.events.DocumentMilestoneEvent",
    "milestone": "100% Translated",
    "locale": "as",
    "docId": "anaconda",
    "version": "copy-7",
    "project": "anaconda"
}
Comment 3 David Mason 2015-07-08 00:03:50 EDT
 - Add a field in project -> settings -> webhooks for an *optional* secret key
 - Hash message body using sha1/HMAC using the secret key
   - will need to generate the JSON up-front to sign it
 - Send the hash with the webhook response in header "X-Zanata-Signature".
 - Do not include "X-Zanata-Signature" header if no secret key has been specified for the project.

Note: make sure we are setting the content type header to JSON.

Technical note: may be able to use some kind of RestEasy interceptor, or generate the body ourselves to have access to it.
Comment 4 Sean Flanigan 2015-07-08 21:34:14 EDT
Since our header doesn't have any historical baggage yet, rather than duplicate GitHub's header exactly, I suggest we make it a little stronger, and very similar to Trello.

According to the GitHub docs, use of the GitHub header may be vulnerable to a timing attack if the webhook receiver fails to use a constant-time string comparison.  Also, I think it may be vulnerable to another attack, if the same secret is used for multiple webhook URLs (replaying a webhook for one URL against the other).

The Trello header uses a double HMAC (to defeat the timing attack), plus the hash includes both the body and the URL (to defeat the replay attack).

Trello docs and sample implementation: https://trello.com/docs/gettingstarted/webhooks.html#triggering-webhooks

The timing attack: http://web.archive.org/web/20141016010907/https://www.isecpartners.com/blog/2011/february/double-hmac-verification.aspx

There are implementations which go further (including some of the HTTP headers in the hash), but I don't want to add too much to the complexity


Instead of "X-Zanata-Signature" (what sort of signature?), I suggest the header name "X-Zanata-Webhook".

I don't think we clarified this before, but I recommend we use Base64 (like Trello), not hexadecimal (like GitHub).

Ralph, would you say including the URL in the hash will make signature verification too complex at the receiver's end?
Comment 5 Ralph Bean 2015-07-09 10:52:05 EDT
No, that sounds fine.  It's not too much to ask the recipient to know where they live.  Thanks for the additions Sean.
Comment 6 Alex Eng 2015-07-10 01:37:59 EDT
Pull request:
https://github.com/zanata/zanata-server/pull/904
Comment 7 Damian Jansen 2015-07-15 21:26:20 EDT
Verified at 52366c98ca6e423cafad8d363d751fce4442b1a9
Comment 8 Damian Jansen 2015-07-21 00:08:12 EDT
Verified merge bd33f08c933c8572725d0d8f131cd654aaee5b50
Comment 9 Michelle Kim 2015-07-21 20:16:13 EDT
Documentation link:
http://docs.zanata.org/en/latest/user-guide/projects/projects-settings/#webhooks

Note You need to log in before you can comment on or make changes to this bug.