Bug 1213630 - Webhook header needs to include cryptographic signature in header for identification.
Summary: Webhook header needs to include cryptographic signature in header for identif...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Zanata
Classification: Retired
Component: Security
Version: 3.7
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ---
: 3.8
Assignee: Alex Eng
QA Contact: Damian Jansen
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-04-21 00:19 UTC by Alex Eng
Modified: 2016-01-21 01:40 UTC (History)
7 users (show)

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-21 01:40:14 UTC
Embargoed:


Attachments (Terms of Use)

Description Alex Eng 2015-04-21 00:19:04 UTC
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 19:06:24 UTC
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-22 03:49:20 UTC
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 04:03:50 UTC
 - 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-09 01:34:14 UTC
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 14:52:05 UTC
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 05:37:59 UTC
Pull request:
https://github.com/zanata/zanata-server/pull/904

Comment 7 Damian Jansen 2015-07-16 01:26:20 UTC
Verified at 52366c98ca6e423cafad8d363d751fce4442b1a9

Comment 8 Damian Jansen 2015-07-21 04:08:12 UTC
Verified merge bd33f08c933c8572725d0d8f131cd654aaee5b50

Comment 9 Michelle Kim 2015-07-22 00:16:13 UTC
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.