Validate webhook's payload

readthedocs/integrations/migrations/0004_add_integration_secret.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-12-03 17:56
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import readthedocs.integrations.utils
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('integrations', '0003_add_missing_model_change_migrations'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='integration',
+            name='secret',
+            field=models.CharField(blank=True, default=readthedocs.integrations.utils.get_secret, help_text='Secret used to validate the payload of the webhook', max_length=255, null=True),
+        ),
+    ]

readthedocs/integrations/models.py

@@ -30,6 +30,7 @@
 from rest_framework import status
 
 from readthedocs.core.fields import default_token
+from readthedocs.integrations.utils import get_secret
 from readthedocs.projects.models import Project
 
 from .utils import normalize_request_payload
@@ -268,6 +269,13 @@ class Integration(models.Model):
         'HttpExchange',
         related_query_name='integrations',
     )
+    secret = models.CharField(
+        help_text='Secret used to validate the payload of the webhook',
+        max_length=255,
+        blank=True,
+        null=True,
+        default=get_secret
+    )
 
     objects = IntegrationQuerySet.as_manager()
 

readthedocs/integrations/utils.py

@@ -1,5 +1,10 @@
 """Integration utility functions."""
 
+from __future__ import division, print_function, unicode_literals
+
+import os
+import six
+
 
 def normalize_request_payload(request):
     """
@@ -21,3 +26,16 @@ def normalize_request_payload(request):
         except AttributeError:
             pass
     return request_payload
+
+
+def get_secret(size=64):
+    """
+    Get a random string of `size` bytes.
+
+    :param size: Number of bytes
+    """
+    secret = os.urandom(size)
+    if six.PY2:
+        # On python two os.urandom returns str instead of bytes
+        return secret.encode('hex')
+    return secret.hex()

readthedocs/oauth/services/github.py

@@ -170,6 +170,7 @@ def get_webhook_data(self, project, integration):
                                 'integration_pk': integration.pk}
                     )
                 ),
+                'secret': integration.secret,
                 'content_type': 'json',
             },
             'events': ['push', 'pull_request', 'create', 'delete'],

readthedocs/oauth/services/gitlab.py

@@ -250,6 +250,7 @@ def get_webhook_data(self, repo_id, project, integration):
                     },
                 ),
             ),
+            'token': integration.secret,
 
             # Optional
             'issues_events': False,

readthedocs/projects/forms.py

@@ -710,7 +710,7 @@ class IntegrationForm(forms.ModelForm):
 
     class Meta(object):
         model = Integration
-        exclude = ['provider_data', 'exchanges']  # pylint: disable=modelform-uses-exclude
+        exclude = ['provider_data', 'exchanges', 'secret']  # pylint: disable=modelform-uses-exclude
 
     def __init__(self, *args, **kwargs):
         self.project = kwargs.pop('project', None)

readthedocs/restapi/parsers.py

@@ -0,0 +1,55 @@
+from __future__ import division, print_function, unicode_literals
+
+import json
+
+from django.http import QueryDict
+from rest_framework.exceptions import ParseError
+from rest_framework.parsers import BaseParser
+
+
+class RawBodyParser(BaseParser):
+
+    """
+    Parse a request and expose the original payload.
+
+    DRF doesn't expose the request's body after using ``request.data``.
+    We are implementing a custom parser to exposed it as `raw_body`.
+    """
+
+    media_type = None
+
+    def parse(self, stream, media_type, parser_context):
+        request = parser_context['request']
+        raw_body = stream.read().decode()
+        setattr(request, 'raw_body', raw_body)
+        return raw_body
+
+
+class RawBodyJSONParser(RawBodyParser):
+
+    """Parser adapted from `rest_framework.parsers.JSONParser`."""
+
+    media_type = 'application/json'
+
+    def parse(self, stream, media_type, parser_context):
+        raw_body = super(RawBodyJSONParser, self).parse(
+            stream, media_type, parser_context
+        )
+        try:
+            return json.loads(raw_body)
+        except ValueError as exc:
+            raise ParseError('JSON parse error - %s' % str(exc))
+
+
+class RawBodyFormParser(RawBodyParser):
+
+    """Parser adapted from `rest_framework.parsers.FormParser`."""
+
+    media_type = 'application/x-www-form-urlencoded'
+
+    def parse(self, stream, media_type, parser_context):
+        raw_body = super(RawBodyFormParser, self).parse(
+            stream, media_type, parser_context
+        )
+        data = QueryDict(raw_body)
+        return dict(data.items())

readthedocs/restapi/views/integrations.py

@@ -7,14 +7,16 @@
     unicode_literals,
 )
 
+import hashlib
+import hmac
 import json
 import logging
 import re
 
 import six
 from django.shortcuts import get_object_or_404
 from rest_framework import permissions
-from rest_framework.exceptions import NotFound, ParseError
+from rest_framework.exceptions import AuthenticationFailed, NotFound, ParseError
 from rest_framework.renderers import JSONRenderer
 from rest_framework.response import Response
 from rest_framework.views import APIView
@@ -26,15 +28,17 @@
 )
 from readthedocs.core.views.hooks import build_branches, sync_versions
 from readthedocs.integrations.models import HttpExchange, Integration
-from readthedocs.integrations.utils import normalize_request_payload
 from readthedocs.projects.models import Project
+from readthedocs.restapi.parsers import RawBodyFormParser, RawBodyJSONParser
 
 log = logging.getLogger(__name__)
 
 GITHUB_EVENT_HEADER = 'HTTP_X_GITHUB_EVENT'
+GITHUB_SIGNATURE_HEADER = 'HTTP_X_HUB_SIGNATURE'
 GITHUB_PUSH = 'push'
 GITHUB_CREATE = 'create'
 GITHUB_DELETE = 'delete'
+GITLAB_TOKEN_HEADER = 'HTTP_X_GITLAB_TOKEN'
 GITLAB_PUSH = 'push'
 GITLAB_NULL_HASH = '0' * 40
 GITLAB_TAG_PUSH = 'tag_push'
@@ -47,6 +51,7 @@ class WebhookMixin(object):
     """Base class for Webhook mixins."""
 
     permission_classes = (permissions.AllowAny,)
+    parser_classes = (RawBodyJSONParser, RawBodyFormParser)
     renderer_classes = (JSONRenderer,)
     integration = None
     integration_type = None
@@ -60,6 +65,8 @@ def post(self, request, project_slug):
         except Project.DoesNotExist:
             raise NotFound('Project not found')
         self.data = self.get_data()
+        if not self.is_payload_valid():
+            raise AuthenticationFailed('Payload not valid')
         resp = self.handle_webhook()
         if resp is None:
             log.info('Unhandled webhook event')
@@ -82,13 +89,16 @@ def finalize_response(self, req, *args, **kwargs):
         return resp
 
     def get_data(self):
-        """Normalize posted data."""
-        return normalize_request_payload(self.request)
+        return self.request.data
 
     def handle_webhook(self):
         """Handle webhook payload."""
         raise NotImplementedError
 
+    def is_payload_valid(self):
+        """Validates the webhook's payload using the integration's secret."""
+        return True
+
     def get_integration(self):
         """
         Get or create an inbound webhook to track webhook requests.
@@ -104,10 +114,19 @@ def get_integration(self):
         # in `WebhookView`
         if self.integration is not None:
             return self.integration
-        integration, _ = Integration.objects.get_or_create(
-            project=self.project,
-            integration_type=self.integration_type,
-        )
+        try:
+            integration = Integration.objects.get(
+                project=self.project,
+                integration_type=self.integration_type,
+            )
+        except Integration.DoesNotExist:
+            integration = Integration.objects.create(
+                project=self.project,
+                integration_type=self.integration_type,
+                # If we didn't create the integration,
+                # we didn't set a secret.
+                secret='',
+            )
         return integration
 
     def get_response_push(self, project, branches):
@@ -178,6 +197,30 @@ def get_data(self):
                 pass
         return super(GitHubWebhookView, self).get_data()
 
+    def is_payload_valid(self):
+        """See https://developer.github.com/webhooks/securing/"""
+        signature = self.request.META.get(GITHUB_SIGNATURE_HEADER)
+        secret = self.get_integration().secret
+        if not secret:
+            log.info(
+                'Skipping payload validation for project: %s',
+                self.project.slug
+            )
+            return True
+        if not signature:
+            return False
+        msg = self.request.raw_body
+        digest = hmac.new(
+            secret.encode(),
+            msg=msg.encode(),
+            digestmod=hashlib.sha1
+        ).hexdigest()
+        result = hmac.compare_digest(
+            b'sha1=' + digest.encode(),
+            signature.encode()
+        )
+        return result
+
     def handle_webhook(self):
         # Get event and trigger other webhook events
         event = self.request.META.get(GITHUB_EVENT_HEADER, GITHUB_PUSH)
@@ -228,6 +271,21 @@ class GitLabWebhookView(WebhookMixin, APIView):
 
     integration_type = Integration.GITLAB_WEBHOOK
 
+    def is_payload_valid(self):
+        """GitLab only sends back the token."""
+        token = self.request.META.get(GITLAB_TOKEN_HEADER)
+        secret = self.get_integration().secret
+        if not secret:
+            log.info(
+                'Skipping payload validation for project: %s',
+                self.project.slug
+            )
+            return True
+        if not token:
+            return False
+        result = token == secret
+        return result
+
     def handle_webhook(self):
         """
         Handle GitLab events for push and tag_push.