diff --git a/reviewboard/hostingsvcs/forms.py b/reviewboard/hostingsvcs/forms.py index 3617f3775eb63b2bf799aa77987d8ec2704d653b..a80ba851506edeb2059c910b6add2aed1bc4bc3b 100644 --- a/reviewboard/hostingsvcs/forms.py +++ b/reviewboard/hostingsvcs/forms.py @@ -9,6 +9,7 @@ from django.utils.translation import ugettext_lazy as _, ugettext from reviewboard.hostingsvcs.errors import (AuthorizationError, TwoFactorAuthCodeRequiredError) from reviewboard.hostingsvcs.models import HostingServiceAccount +from reviewboard.scmtools.errors import UnverifiedCertificateError class HostingServiceAuthForm(forms.Form): @@ -243,7 +244,7 @@ class HostingServiceAuthForm(forms.Form): return credentials def save(self, allow_authorize=True, force_authorize=False, - extra_authorize_kwargs=None, save=True): + extra_authorize_kwargs=None, trust_host=False, save=True): """Save the hosting account and authorize against the service. This will create or update a hosting account, based on the information @@ -266,6 +267,10 @@ class HostingServiceAuthForm(forms.Form): ` call. + trust_host (bool, optional): + Whether to trust the given host, even if the linked certificate + is invalid or self-signed. + save (bool, optional): Whether or not the created account should be saved. @@ -349,46 +354,87 @@ class HostingServiceAuthForm(forms.Form): password = credentials.get('password') two_factor_auth_code = credentials.get('two_factor_auth_code') + authorize_kwargs = dict({ + 'username': username, + 'password': password, + 'hosting_url': hosting_url, + 'two_factor_auth_code': two_factor_auth_code, + 'local_site_name': local_site_name, + 'credentials': credentials, + }, **extra_authorize_kwargs) + try: - hosting_account.service.authorize( - username=username, - password=password, - hosting_url=hosting_url, - two_factor_auth_code=two_factor_auth_code, - local_site_name=local_site_name, - credentials=credentials, - **extra_authorize_kwargs) - except TwoFactorAuthCodeRequiredError: - # Mark this asrequired for the next form render. - self.fields['hosting_account_two_factor_auth_code']\ - .required = True - - # Re-raise the error. - raise - except AuthorizationError: - logging.exception('Authorization error linking hosting ' - 'account ID=%r for hosting service=%r, ' - 'username=%r, LocalSite=%r', - hosting_account.pk, hosting_service_id, - username, local_site_name) - - # Re-raise the error. - raise - except Exception: - logging.exception('Unknown error linking hosting account ' - 'ID=%r for hosting service=%r, ' - 'username=%r, LocalSite=%r', - hosting_account.pk, hosting_service_id, - username, local_site_name) - - # Re-raise the error. - raise + self.authorize(hosting_account, hosting_service_id, + **authorize_kwargs) + except UnverifiedCertificateError as e: + if trust_host: + hosting_account.accept_certificate(e.certificate) + self.authorize(hosting_account, hosting_service_id, + **authorize_kwargs) + else: + raise if save: hosting_account.save() return hosting_account + def authorize(self, hosting_account, hosting_service_id, + username=None, local_site_name=None, **kwargs): + """Authorize the service. + + Args: + hosting_account (reviewboard.hostingsvcs.models. + HostingServiceAccount): + The hosting service account. + + hosting_service_id (unicode): + The ID of the hosting service. + + username (unicode): + The username for the account. + + local_site_name (unicode, optional): + The Local Site name, if any, that the account should be + bound to. + + **kwargs (dict): + Keyword arguments to pass into the service authorize function. + """ + try: + hosting_account.service.authorize(username=username, + local_site_name=local_site_name, + **kwargs) + except TwoFactorAuthCodeRequiredError: + # Mark this as required for the next form render. + self.fields['hosting_account_two_factor_auth_code']\ + .required = True + + # Re-raise the error. + raise + except AuthorizationError: + logging.exception('Authorization error linking hosting ' + 'account ID=%r for hosting service=%r, ' + 'username=%r, LocalSite=%r', + hosting_account.pk, hosting_service_id, + username, local_site_name) + + # Re-raise the error. + raise + except UnverifiedCertificateError: + # Re-raise the error so the user will see the "I trust this + # host" prompt. + raise + except Exception: + logging.exception('Unknown error linking hosting account ' + 'ID=%r for hosting service=%r, ' + 'username=%r, LocalSite=%r', + hosting_account.pk, hosting_service_id, + username, local_site_name) + + # Re-raise the error. + raise + def clean_hosting_url(self): """Clean the hosting URL field. diff --git a/reviewboard/hostingsvcs/models.py b/reviewboard/hostingsvcs/models.py index f4044d8f270612a04cf110ca1c97c154a32f7fdc..e038389c2dc1e50a884893e307dc27e006d9314e 100644 --- a/reviewboard/hostingsvcs/models.py +++ b/reviewboard/hostingsvcs/models.py @@ -73,6 +73,23 @@ class HostingServiceAccount(models.Model): return user.has_perm('hostingsvcs.change_hostingserviceaccount', self.local_site) + def accept_certificate(self, certificate): + """Accept the SSL certificate for the linked hosting URL. + + Args: + certificate (reviewboard.scmtools.certs.Certificate): + The certificate to accept. + + Raises: + ValueError: + The certificate data did not include required fields. + """ + if not certificate.pem_data: + raise ValueError('The certificate does not include a PEM-encoded ' + 'representation.') + + self.data['ssl_cert'] = certificate.pem_data + class Meta: db_table = 'hostingsvcs_hostingserviceaccount' verbose_name = _('Hosting Service Account') diff --git a/reviewboard/hostingsvcs/rbgateway.py b/reviewboard/hostingsvcs/rbgateway.py index 7406dae605ca9c783536249709e3a4e2ae474b3b..4b3658088a776438d016d52cc7f7fb0e5a56ea67 100644 --- a/reviewboard/hostingsvcs/rbgateway.py +++ b/reviewboard/hostingsvcs/rbgateway.py @@ -12,12 +12,15 @@ from reviewboard.hostingsvcs.errors import (AuthorizationError, HostingServiceError) from reviewboard.hostingsvcs.forms import HostingServiceForm from reviewboard.hostingsvcs.service import HostingService +from reviewboard.scmtools.core import Branch, Commit from reviewboard.scmtools.crypto_utils import (decrypt_password, encrypt_password) -from reviewboard.scmtools.core import Branch, Commit from reviewboard.scmtools.errors import FileNotFoundError, SCMError +logger = logging.getLogger(__name__) + + class ReviewBoardGatewayForm(HostingServiceForm): """Hosting service form for Review Board Gateway. @@ -74,17 +77,17 @@ class ReviewBoardGateway(HostingService): username=username, password=password) except HTTPError as e: - if e.code == 404: + if e.code == 401: + raise AuthorizationError( + ugettext('The username or password is incorrect.')) + elif e.code == 404: raise HostingServiceError( ugettext('A Review Board Gateway server was not found at ' 'the provided URL.')) - elif e.code == 401: - raise AuthorizationError( - ugettext('The username or password is incorrect.')) else: - logging.warning('Failed authorization at %s: %s', - hosting_url + '/session', e, exc_info=1) - raise + logger.exception('Failed authorization at %s/session: %s', + hosting_url, e) + raise self.account.data['private_token'] = \ encrypt_password(data['private_token']) @@ -113,8 +116,7 @@ class ReviewBoardGateway(HostingService): if e.code == 404: raise FileNotFoundError(path, revision) else: - logging.warning('Failed to get file from %s: %s', - url, e, exc_info=1) + logger.exception('Failed to get file from %s: %s', url, e) raise SCMError(six.text_type(e)) def get_file_exists(self, repository, path, revision, base_commit_id, @@ -132,8 +134,8 @@ class ReviewBoardGateway(HostingService): if e.code == 404: return False else: - logging.warning('Failed to get file exists from %s: %s', - url, e, exc_info=1) + logger.exception('Failed to get file exists from %s: %s', + url, e) raise SCMError(six.text_type(e)) def get_branches(self, repository): @@ -154,8 +156,7 @@ class ReviewBoardGateway(HostingService): return results except Exception as e: - logging.warning('Failed to get branches from %s: %s', - url, e, exc_info=1) + logger.exception('Failed to get branches from %s: %s', url, e) raise SCMError(six.text_type(e)) def get_commits(self, repository, branch=None, start=None): @@ -186,8 +187,7 @@ class ReviewBoardGateway(HostingService): return results except Exception as e: - logging.warning('Failed to fetch commits from %s: %s', - url, e, exc_info=1) + logger.exception('Failed to fetch commits from %s: %s', url, e) raise SCMError(six.text_type(e)) def get_change(self, repository, revision): @@ -208,8 +208,8 @@ class ReviewBoardGateway(HostingService): diff=commit['diff']) except Exception as e: - logging.warning('Failed to fetch commit change from %s: %s', - url, e, exc_info=1) + logger.exception('Failed to fetch commit change from %s: %s', + url, e) raise SCMError(six.text_type(e)) def _get_file_url(self, repository, revision, base_commit_id=None, @@ -249,12 +249,12 @@ class ReviewBoardGateway(HostingService): except HTTPError as e: if e.code == 401: raise AuthorizationError( - ugettext('The login or password is incorrect.')) + ugettext('The username or password is incorrect.')) elif e.code == 404: raise else: - logging.warning('Failed to execute a GET request at %s: %s', - url, e, exc_info=1) + logger.exception('Failed to execute a GET request at %s: %s', + url, e) raise def _api_head(self, url): @@ -275,12 +275,12 @@ class ReviewBoardGateway(HostingService): except HTTPError as e: if e.code == 401: raise AuthorizationError( - ugettext('The login or password is incorrect.')) + ugettext('The username or password is incorrect.')) elif e.code == 404: raise else: - logging.warning('Failed to execute a HEAD request at %s: %s', - url, e, exc_info=1) + logger.exception('Failed to execute a HEAD request at %s: %s', + url, e) raise def _get_private_token(self): diff --git a/reviewboard/hostingsvcs/service.py b/reviewboard/hostingsvcs/service.py index 5e9d94d64ff702e836cdacc55982074e7a530fa5..311d7cfb9edd5f59bc9e41c294b4121159e9f90f 100644 --- a/reviewboard/hostingsvcs/service.py +++ b/reviewboard/hostingsvcs/service.py @@ -3,14 +3,19 @@ from __future__ import unicode_literals import base64 +import hashlib import json import logging import mimetools import re +import ssl +from cryptography import x509 +from cryptography.hazmat.backends import default_backend from django.conf.urls import include, url from django.dispatch import receiver from django.utils import six +from django.utils.six.moves.urllib.error import URLError from django.utils.six.moves.urllib.parse import urlparse from django.utils.six.moves.urllib.request import (Request as BaseURLRequest, HTTPBasicAuthHandler, @@ -22,6 +27,8 @@ from djblets.registries.registry import (ALREADY_REGISTERED, LOAD_ENTRY_POINT, import reviewboard.hostingsvcs.urls as hostingsvcs_urls from reviewboard.registries.registry import EntryPointRegistry +from reviewboard.scmtools.certs import Certificate +from reviewboard.scmtools.errors import UnverifiedCertificateError from reviewboard.signals import initializing @@ -105,7 +112,7 @@ class HostingServiceClient(object): hosting_service (HostingService): The hosting service that is using this client. """ - pass + self.hosting_service = hosting_service # # HTTP utility methods @@ -294,9 +301,49 @@ class HostingServiceClient(object): if username is not None and password is not None: request.add_basic_auth(username, password) - response = urlopen(request) - - return response.read(), response.headers + try: + if (self.hosting_service and + 'ssl_cert' in self.hosting_service.account.data): + # create_default_context only exists in Python 2.7.9+. Using it + # here should be fine, however, because accepting invalid or + # self-signed certificates is only possible when running + # against versions that have this (see the check for + # create_default_context below). + context = ssl.create_default_context() + context.load_verify_locations( + cadata=self.hosting_service.account.data['ssl_cert']) + context.check_hostname = False + else: + context = None + + response = urlopen(request, context=context) + return response.read(), response.headers + except URLError as e: + if ('CERTIFICATE_VERIFY_FAILED' not in six.text_type(e) or + not hasattr(ssl, 'create_default_context')): + raise + + parts = urlparse(url) + port = parts.port or 443 + + cert_pem = ssl.get_server_certificate((parts.hostname, port)) + cert_der = ssl.PEM_cert_to_DER_cert(cert_pem) + + cert = x509.load_pem_x509_certificate(cert_pem.encode('ascii'), + default_backend()) + issuer = cert.issuer.get_attributes_for_oid( + x509.oid.NameOID.COMMON_NAME)[0].value + subject = cert.subject.get_attributes_for_oid( + x509.oid.NameOID.COMMON_NAME)[0].value + + raise UnverifiedCertificateError( + Certificate( + pem_data=cert_pem, + valid_from=cert.not_valid_before.isoformat(), + valid_until=cert.not_valid_after.isoformat(), + issuer=issuer, + hostname=subject, + fingerprint=hashlib.sha256(cert_der).hexdigest())) # # JSON utility methods diff --git a/reviewboard/scmtools/certs.py b/reviewboard/scmtools/certs.py index c0cdd70a6912cbc7a81f2a6209465619f48fe375..b363202e10e5352c6a0d1d3abdc62cc39b7b25e5 100644 --- a/reviewboard/scmtools/certs.py +++ b/reviewboard/scmtools/certs.py @@ -3,8 +3,42 @@ from __future__ import unicode_literals class Certificate(object): """A representation of an HTTPS certificate.""" - def __init__(self, valid_from='', valid_until='', hostname='', realm='', - fingerprint='', issuer='', failures=[]): + + def __init__(self, pem_data='', valid_from='', valid_until='', hostname='', + realm='', fingerprint='', issuer='', failures=[]): + """Initialize the certificate. + + Args: + pem_data (unicode): + The PEM-encoded certificate, if available. + + valid_from (unicode): + A user-readable representation of the initiation date of the + certificate. + + valid_until (unicode): + A user-readable representation of the expiration date of the + certificate. + + hostname (unicode): + The hostname that this certificate is tied to. + + realm (unicode): + An authentication realm (used with SVN). + + fingerprint (unicode): + The fingerprint of the certificate. This can be in various + formats depending on the backend which is dealing with the + certificate, but ideally should be a SHA256-sum of the + DER-encoded certificate. + + issuer (unicode): + The common name of the issuer of the certificate. + + failures (list of unicode): + A list of the verification failures, if available. + """ + self.pem_data = pem_data self.valid_from = valid_from self.valid_until = valid_until self.hostname = hostname diff --git a/reviewboard/scmtools/forms.py b/reviewboard/scmtools/forms.py index b79964141c79e16b82f10c370f3625462ebd8c50..2e3b005a57fc0262a2cdc7b42b1c7b57560a6d8a 100644 --- a/reviewboard/scmtools/forms.py +++ b/reviewboard/scmtools/forms.py @@ -669,7 +669,8 @@ class RepositoryForm(forms.ModelForm): try: hosting_account = auth_form.save( extra_authorize_kwargs=repository_extra_data, - force_authorize=force_authorize) + force_authorize=force_authorize, + trust_host=self.cleaned_data['trust_host']) except ValueError as e: # There was an error with a value provided to the form from # The user. Bubble this up. @@ -687,16 +688,24 @@ class RepositoryForm(forms.ModelForm): _('Unable to link the account: %s') % e, ]) return + except UnverifiedCertificateError as e: + self.certerror = e + return except Exception as e: + error = six.text_type(e) + + if error.endswith('.'): + error = error[:-1] + logging.exception('Unexpected error when linking hosting ' 'service account on %s: %s', - hosting_type, e) + hosting_type, error) self.errors['hosting_account'] = self.error_class([ _('Unknown error when linking the account: %s. The ' 'details of the failure are in the Review Board log ' 'file.') - % e, + % error, ]) return @@ -971,6 +980,18 @@ class RepositoryForm(forms.ModelForm): self._clean_ssh_key_association() + if self.certerror: + # In the case where there's a certificate error on a hosting + # service, we'll bail out of the validation process before + # computing any of the derived fields (like path). This results in + # the "I trust this host" prompt being shown at the top, but a + # spurious "Please correct the error below" error shown when no + # errors are visible. We therefore want to clear out the errors and + # let the certificate error show on its own. If the user then + # chooses to trust the cert, the regular verification will run its + # course. + self.errors.clear() + return super(RepositoryForm, self).clean() def _clean_ssh_key_association(self): diff --git a/reviewboard/templates/admin/repository_fields.js b/reviewboard/templates/admin/repository_fields.js index a1fe0e820942610427c8a534e0a6118e5d6e89b9..7bf7e7a1947c74b4e2a810ae1f8fe6e3f2cd64c5 100644 --- a/reviewboard/templates/admin/repository_fields.js +++ b/reviewboard/templates/admin/repository_fields.js @@ -4,14 +4,15 @@ var HOSTING_SERVICES = {{form.hosting_service_info|json_dumps:2}}, TOOLS_INFO = {{form.tool_info|json_dumps:2}}; {% if form.hostkeyerror or form.certerror or adminform.userkeyerror %} -$(document).ready(function() { - var inputs = $("fieldset, .submit-row").find("input, select"); +$(function() { + var $inputs = $('fieldset, .submit-row') + .find('input, select') + .prop('disabled', true); - inputs.prop("disabled", true); - $(".confirmation input") - .prop("disabled", false); - .click(function() { - inputs.prop("disabled", false); - }); + $('.confirmation input') + .prop('disabled', false) + .click(function() { + $inputs.prop('disabled', false); + }); }); {% endif %}