distributhor/workflow-webhook

Prevent replay attacks

johannes-huther opened this issue · 1 comments

As mentioned in #21 a lot of information is published using --verbose/-v by default.

This includes the headers sent, including X-Hub-Signature and X-Hub-Signature-256. If I do understand the algorithm correctly this signature is used to verify the request body only.

Since the default payload is pretty predictable (source: README.md) :

{
    "event": "push",
    "repository": "owner/project",
    "commit": "a636b6f0861bbee98039bf3df66ee13d8fbc9c74",
    "ref": "refs/heads/master",
    "head": "",
    "workflow": "Build and deploy"
}

and the signature/ hash is logged publically, I do believe that it would be an easy one to replay this request. The server (at least my current implementation) would think this is a genuine request from GitHub.

There is no way to inject malicious data or similar, since that would change and therefore invalidate the signature, but this could make it way easier to DDoS my service that recreates a docker container. Just sending unauthorized requests should have minimal performance impact, but if a genuine message is replayed, my service would try to recreate the container over and over, which would hit really badly performance wise.

This is not too bad at the moment as not the complete URL is logged, but I do believe that many people use URLs similar to https://test.com/webhooks or similar predictable URLs. If #21 is taken care of and --verbose is not the default anymore, the issue would only affect people explicitly using the verbose: true option and their old workflow logs (but I believe those are deleted anyway after 3 months).

There are multiple ways to address this issue:

  1. Send a salt with each request:
    To prevent this problem from affecting me I will use an additional salt that is stored as a secret and is therefore private. This makes it impossible (or at least way harder) to replay the request, as part of the body is unknown.
        - name: Invoke deployment hook
          uses: distributhor/workflow-webhook@v1     
          env:
            webhook_url: ${{ secrets.WEBHOOK_URL }}
            webhook_secret: ${{ secrets.WEBHOOK_SECRET }}
            data: '{ "salt": "${{ secrets.WEBHOOK_SALT }}" }'
    Similarly adding other data, that is not publically available or predictable makes this attack way harder and therefore unreasonable. You could also add some sort of unique request ID (e.g. a random UUID) that is sent with each request.
  2. Verify that each request is only processed once on the receiving side:
    If the receiving side stores some identifier (e.g. the commit SHA, or even better aforementioned unique request ID) and makes sure that each request is only handled once, replay attacks are not useful anymore. It might be a good idea to state this somehere in README.md.

Didn't know that merging my own PR in my fork closes the issue. Reopening!