launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24390
[Merge] ~pappacena/launchpad:https-mirrors-2 into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:https-mirrors-2 into launchpad:master.
Commit message:
Undoing git revert on: Allowing to register HTTPS mirrors (both for CD images and archives).
This MP is undoing the git revert of HTTPS mirrors feature on master (https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/379387). This feature got reverted because the database patch was not merged yet, so the code should wait for it's deployment to production before being landed.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/379913
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:https-mirrors-2 into launchpad:master.
diff --git a/lib/lp/registry/browser/distributionmirror.py b/lib/lp/registry/browser/distributionmirror.py
index fd671a0..6ae1eb7 100644
--- a/lib/lp/registry/browser/distributionmirror.py
+++ b/lib/lp/registry/browser/distributionmirror.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -207,9 +207,9 @@ class DistributionMirrorDeleteView(LaunchpadFormView):
class DistributionMirrorAddView(LaunchpadFormView):
schema = IDistributionMirror
field_names = [
- "display_name", "description", "whiteboard", "http_base_url",
- "ftp_base_url", "rsync_base_url", "speed", "country", "content",
- "official_candidate",
+ "display_name", "description", "whiteboard", "https_base_url",
+ "http_base_url", "ftp_base_url", "rsync_base_url", "speed", "country",
+ "content", "official_candidate",
]
invariant_context = None
@@ -235,6 +235,7 @@ class DistributionMirrorAddView(LaunchpadFormView):
content=data['content'], display_name=data['display_name'],
description=data['description'],
whiteboard=data['whiteboard'],
+ https_base_url=data['https_base_url'],
http_base_url=data['http_base_url'],
ftp_base_url=data['ftp_base_url'],
rsync_base_url=data['rsync_base_url'],
@@ -279,8 +280,8 @@ class DistributionMirrorEditView(LaunchpadEditFormView):
schema = IDistributionMirror
field_names = [
"name", "display_name", "description", "whiteboard",
- "http_base_url", "ftp_base_url", "rsync_base_url", "speed",
- "country", "content", "official_candidate",
+ "https_base_url", "http_base_url", "ftp_base_url", "rsync_base_url",
+ "speed", "country", "content", "official_candidate",
]
@property
diff --git a/lib/lp/registry/browser/tests/distributionmirror-views.txt b/lib/lp/registry/browser/tests/distributionmirror-views.txt
index e11dbd9..f8fe4c5 100644
--- a/lib/lp/registry/browser/tests/distributionmirror-views.txt
+++ b/lib/lp/registry/browser/tests/distributionmirror-views.txt
@@ -44,18 +44,19 @@ The view provides a label, page_title, and cancel_url
>>> print view.cancel_url
http://launchpad.test/ubuntu
-A HTTP or FTP URL is required to register a mirror.
+A HTTP, HTTPS or FTP URL is required to register a mirror.
>>> view.field_names
- ['display_name', 'description', 'whiteboard', 'http_base_url',
- 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
- 'official_candidate']
+ ['display_name', 'description', 'whiteboard', 'https_base_url',
+ 'http_base_url', 'ftp_base_url', 'rsync_base_url', 'speed', 'country',
+ 'content', 'official_candidate']
>>> form = {
... 'field.display_name': 'Illuminati',
... 'field.description': 'description',
... 'field.whiteboard': 'whiteboard',
... 'field.http_base_url': 'http://secret.me/',
+ ... 'field.https_base_url': '',
... 'field.ftp_base_url': '',
... 'field.rsync_base_url': '',
... 'field.speed': 'S128K',
@@ -93,6 +94,7 @@ not significant).
The same is true for a FTP URL.
>>> mirror.ftp_base_url = 'ftp://now-here.me/'
+ >>> bad_form['field.https_base_url'] = ''
>>> bad_form['field.http_base_url'] = ''
>>> bad_form['field.ftp_base_url'] = 'ftp://now-here.me'
>>> view = create_initialized_view(ubuntu, '+newmirror', form=bad_form)
@@ -110,14 +112,15 @@ The same is true for a rsync URL.
... print error[2]
The distribution mirror ... is already registered with this URL.
-A mirror must have an ftp or http URL.
+A mirror must have an ftp, HTTPS or http URL.
+ >>> bad_form['field.https_base_url'] = ''
>>> bad_form['field.http_base_url'] = ''
>>> bad_form['field.ftp_base_url'] = ''
>>> view = create_initialized_view(ubuntu, '+newmirror', form=bad_form)
>>> for message in view.errors:
... print message
- A mirror must have at least an HTTP or FTP URL.
+ A mirror must have at least an HTTP(S) or FTP URL.
The URL cannot contain a fragment.
@@ -135,6 +138,34 @@ The URL cannot contain a query string.
... print error[2]
URIs with query strings are not allowed.
+The HTTPS URL may not have an HTTP scheme.
+
+ >>> bad_form['field.http_base_url'] = ''
+ >>> bad_form['field.https_base_url'] = 'http://secret.me/#fragement'
+ >>> view = create_initialized_view(ubuntu, '+newmirror', form=bad_form)
+ >>> for error in view.errors:
+ ... print error[2]
+ The URI scheme "http" is not allowed.
+ Only URIs with the following schemes may be used: https
+
+The HTTPS URL cannot contain a fragment.
+
+ >>> bad_form['field.http_base_url'] = ''
+ >>> bad_form['field.https_base_url'] = 'https://secret.me/#fragement'
+ >>> view = create_initialized_view(ubuntu, '+newmirror', form=bad_form)
+ >>> for error in view.errors:
+ ... print error[2]
+ URIs with fragment identifiers are not allowed.
+
+The URL cannot contain a query string.
+
+ >>> bad_form['field.http_base_url'] = ''
+ >>> bad_form['field.https_base_url'] = 'https://secret.me/?query=string'
+ >>> view = create_initialized_view(ubuntu, '+newmirror', form=bad_form)
+ >>> for error in view.errors:
+ ... print error[2]
+ URIs with query strings are not allowed.
+
Reviewing a distribution mirror
-------------------------------
@@ -239,9 +270,9 @@ The +edit view provides a label, page_title, and cancel_url.
The user can edit the mirror fields.
>>> view.field_names
- ['name', 'display_name', 'description', 'whiteboard', 'http_base_url',
- 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
- 'official_candidate']
+ ['name', 'display_name', 'description', 'whiteboard', 'https_base_url',
+ 'http_base_url', 'ftp_base_url', 'rsync_base_url', 'speed', 'country',
+ 'content', 'official_candidate']
>>> print mirror.ftp_base_url
None
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index 9be68e0..d5875c5 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -2046,6 +2046,7 @@
description
distribution
http_base_url
+ https_base_url
ftp_base_url
rsync_base_url
enabled
@@ -2084,8 +2085,9 @@
<require
permission="launchpad.Edit"
set_attributes="name display_name description whiteboard
- http_base_url ftp_base_url rsync_base_url enabled
- speed country content official_candidate owner"
+ http_base_url https_base_url ftp_base_url
+ rsync_base_url enabled speed country content
+ official_candidate owner"
attributes="official_candidate whiteboard resubmitForReview" />
<require
permission="launchpad.Admin"
diff --git a/lib/lp/registry/interfaces/distribution.py b/lib/lp/registry/interfaces/distribution.py
index c7ddb95..a7106f6 100644
--- a/lib/lp/registry/interfaces/distribution.py
+++ b/lib/lp/registry/interfaces/distribution.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces including and related to IDistribution."""
@@ -491,13 +491,13 @@ class IDistributionPublic(
"""Return the country DNS mirror for a country and content type."""
def newMirror(owner, speed, country, content, display_name=None,
- description=None, http_base_url=None,
+ description=None, http_base_url=None, https_base_url=None,
ftp_base_url=None, rsync_base_url=None, enabled=False,
official_candidate=False, whiteboard=None):
"""Create a new DistributionMirror for this distribution.
- At least one of http_base_url or ftp_base_url must be provided in
- order to create a mirror.
+ At least one of {http,https,ftp}_base_url must be provided in order to
+ create a mirror.
"""
def getOCIProject(name):
diff --git a/lib/lp/registry/interfaces/distributionmirror.py b/lib/lp/registry/interfaces/distributionmirror.py
index ce6720e..4849e7a 100644
--- a/lib/lp/registry/interfaces/distributionmirror.py
+++ b/lib/lp/registry/interfaces/distributionmirror.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -302,6 +302,12 @@ class DistroMirrorHTTPURIField(DistroMirrorURIField):
return getUtility(IDistributionMirrorSet).getByHttpUrl(url)
+class DistroMirrorHTTPSURIField(DistroMirrorURIField):
+
+ def getMirrorByURI(self, url):
+ return getUtility(IDistributionMirrorSet).getByHttpsUrl(url)
+
+
class DistroMirrorFTPURIField(DistroMirrorURIField):
def getMirrorByURI(self, url):
@@ -349,6 +355,14 @@ class IDistributionMirror(Interface):
allowed_schemes=['http'], allow_userinfo=False,
allow_query=False, allow_fragment=False, trailing_slash=True,
description=_('e.g.: http://archive.ubuntu.com/ubuntu/')))
+ https_base_url = exported(DistroMirrorHTTPSURIField(
+ title=_('HTTPS URL'), required=False, readonly=False,
+ allowed_schemes=['https'], allow_userinfo=False,
+ allow_query=False, allow_fragment=False, trailing_slash=True,
+ # XXX: pappacena 2020-02-21: Add description field with a more
+ # suitable example once we have https for archive.ubuntu.com, like:
+ # description=_('e.g.: http://archive.ubuntu.com/ubuntu/')
+ ))
ftp_base_url = exported(DistroMirrorFTPURIField(
title=_('FTP URL'), required=False, readonly=False,
allowed_schemes=['ftp'], allow_userinfo=False,
@@ -435,8 +449,9 @@ class IDistributionMirror(Interface):
@invariant
def mirrorMustHaveHTTPOrFTPURL(mirror):
- if not (mirror.http_base_url or mirror.ftp_base_url):
- raise Invalid('A mirror must have at least an HTTP or FTP URL.')
+ if not (mirror.http_base_url or mirror.https_base_url or
+ mirror.ftp_base_url):
+ raise Invalid('A mirror must have at least an HTTP(S) or FTP URL.')
def getSummarizedMirroredSourceSeries():
"""Return a summarized list of this distribution_mirror's
@@ -614,6 +629,9 @@ class IDistributionMirrorSet(Interface):
def getByHttpUrl(url):
"""Return the mirror with the given HTTP URL or None."""
+ def getByHttpsUrl(url):
+ """Return the mirror with the given HTTPS URL or None."""
+
def getByFtpUrl(url):
"""Return the mirror with the given FTP URL or None."""
diff --git a/lib/lp/registry/model/distribution.py b/lib/lp/registry/model/distribution.py
index 2288d0e..f78908b 100644
--- a/lib/lp/registry/model/distribution.py
+++ b/lib/lp/registry/model/distribution.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database classes for implementing distribution items."""
@@ -709,7 +709,7 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
country_dns_mirror=True).one()
def newMirror(self, owner, speed, country, content, display_name=None,
- description=None, http_base_url=None,
+ description=None, http_base_url=None, https_base_url=None,
ftp_base_url=None, rsync_base_url=None,
official_candidate=False, enabled=False,
whiteboard=None):
@@ -722,15 +722,17 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
return None
urls = {'http_base_url': http_base_url,
+ 'https_base_url': https_base_url,
'ftp_base_url': ftp_base_url,
'rsync_base_url': rsync_base_url}
for name, value in urls.items():
if value is not None:
urls[name] = IDistributionMirror[name].normalize(value)
- url = urls['http_base_url'] or urls['ftp_base_url']
+ url = (urls['https_base_url'] or urls['http_base_url'] or
+ urls['ftp_base_url'])
assert url is not None, (
- "A mirror must provide either an HTTP or FTP URL (or both).")
+ "A mirror must provide at least one HTTP/HTTPS/FTP URL.")
dummy, host, dummy, dummy, dummy, dummy = urlparse(url)
name = sanitize_name('%s-%s' % (host, content.name.lower()))
@@ -744,6 +746,7 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
distribution=self, owner=owner, name=name, speed=speed,
country=country, content=content, display_name=display_name,
description=description, http_base_url=urls['http_base_url'],
+ https_base_url=urls['https_base_url'],
ftp_base_url=urls['ftp_base_url'],
rsync_base_url=urls['rsync_base_url'],
official_candidate=official_candidate, enabled=enabled,
diff --git a/lib/lp/registry/model/distributionmirror.py b/lib/lp/registry/model/distributionmirror.py
index 747a2b0..323fb26 100644
--- a/lib/lp/registry/model/distributionmirror.py
+++ b/lib/lp/registry/model/distributionmirror.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Module docstring goes here."""
@@ -129,6 +129,8 @@ class DistributionMirror(SQLBase):
notNull=False, default=None)
http_base_url = StringCol(
notNull=False, default=None, unique=True)
+ https_base_url = StringCol(
+ notNull=False, default=None, unique=True)
ftp_base_url = StringCol(
notNull=False, default=None, unique=True)
rsync_base_url = StringCol(
@@ -155,7 +157,9 @@ class DistributionMirror(SQLBase):
@property
def base_url(self):
"""See IDistributionMirror"""
- if self.http_base_url is not None:
+ if self.https_base_url is not None:
+ return self.https_base_url
+ elif self.http_base_url is not None:
return self.http_base_url
else:
return self.ftp_base_url
@@ -677,6 +681,10 @@ class DistributionMirrorSet:
"""See IDistributionMirrorSet"""
return DistributionMirror.selectOneBy(http_base_url=url)
+ def getByHttpsUrl(self, url):
+ """See IDistributionMirrorSet"""
+ return DistributionMirror.selectOneBy(https_base_url=url)
+
def getByFtpUrl(self, url):
"""See IDistributionMirrorSet"""
return DistributionMirror.selectOneBy(ftp_base_url=url)
diff --git a/lib/lp/registry/scripts/distributionmirror_prober.py b/lib/lp/registry/scripts/distributionmirror_prober.py
index 916d4ba..c633fb8 100644
--- a/lib/lp/registry/scripts/distributionmirror_prober.py
+++ b/lib/lp/registry/scripts/distributionmirror_prober.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -12,6 +12,7 @@ import logging
import os.path
from StringIO import StringIO
+import OpenSSL
import requests
from six.moves import http_client
from six.moves.urllib.parse import (
@@ -20,14 +21,27 @@ from six.moves.urllib.parse import (
urlparse,
urlunparse,
)
+from treq.client import HTTPClient as TreqHTTPClient
from twisted.internet import (
defer,
protocol,
reactor,
)
-from twisted.internet.defer import DeferredSemaphore
+from twisted.internet.defer import (
+ CancelledError,
+ DeferredSemaphore,
+ )
+from twisted.internet.endpoints import HostnameEndpoint
+from twisted.internet.ssl import VerificationError
from twisted.python.failure import Failure
+from twisted.web.client import (
+ Agent,
+ BrowserLikePolicyForHTTPS,
+ ProxyAgent,
+ ResponseNeverReceived,
+ )
from twisted.web.http import HTTPClient
+from twisted.web.iweb import IResponse
from zope.component import getUtility
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -50,6 +64,7 @@ from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
# IMPORTANT: Changing these values can cause lots of false negatives when
# probing mirrors, so please don't change them unless you know what you're
# doing.
+
MIN_REQUEST_TIMEOUT_RATIO = 3
MIN_REQUESTS_TO_CONSIDER_RATIO = 30
@@ -57,6 +72,9 @@ MIN_REQUESTS_TO_CONSIDER_RATIO = 30
# We need to get rid of these global dicts in this module.
host_requests = {}
host_timeouts = {}
+# Set of invalid certificate (host, port) tuples, to avoid doing HTTPS calls
+# to hosts we already know they are not valid.
+invalid_certificate_hosts = set()
MAX_REDIRECTS = 3
@@ -161,6 +179,64 @@ class ProberProtocol(HTTPClient):
pass
+class HTTPSProbeFailureHandler:
+ """Handler to translate general errors into expected errors on HTTPS
+ connections."""
+ def __init__(self, factory):
+ self.factory = factory
+
+ def handleResponse(self, response):
+ """Translates any request with return code different from 200 into
+ an error in the callback chain.
+
+ Note that other 2xx codes that are not 200 are considered errors too.
+ This behaviour is the same as seen in ProberProtocol.handleStatus,
+ for HTTP responses.
+ """
+ status = response.code
+ if status == http_client.OK:
+ return response
+ else:
+ raise BadResponseCode(status, response)
+
+ def handleErrors(self, error):
+ """Handle exceptions in https requests.
+ """
+ if self.isInvalidCertificateError(error):
+ invalid_certificate_hosts.add(
+ (self.factory.request_host, self.factory.request_port))
+ reason = InvalidHTTPSCertificate(
+ self.factory.request_host, self.factory.request_port)
+ raise reason
+ if self.isTimeout(error):
+ raise ProberTimeout(self.factory.url, self.factory.timeout)
+ raise error
+
+ def isTimeout(self, error):
+ """Checks if the error was caused by a timeout.
+ """
+ return self._isErrorFromType(error, CancelledError)
+
+ def isInvalidCertificateError(self, error):
+ """Checks if the error was caused by an invalid certificate.
+ """
+ # It might be a raw SSL error, or a twisted-encapsulated
+ # verification error (such as DNSMismatch error when the
+ # certificate is valid for a different domain, for example).
+ return self._isErrorFromType(
+ error, OpenSSL.SSL.Error, VerificationError)
+
+ def _isErrorFromType(self, error, *types):
+ """Checks if the error was caused by any of the given types.
+ """
+ if not isinstance(error.value, ResponseNeverReceived):
+ return False
+ for reason in error.value.reasons:
+ if reason.check(*types) is not None:
+ return True
+ return False
+
+
class RedirectAwareProberProtocol(ProberProtocol):
"""A specialized version of ProberProtocol that follows HTTP redirects."""
@@ -216,6 +292,8 @@ class ProberFactory(protocol.ClientFactory):
connect_port = None
connect_path = None
+ https_agent_policy = BrowserLikePolicyForHTTPS
+
def __init__(self, url, timeout=config.distributionmirrorprober.timeout):
# We want the deferred to be a private attribute (_deferred) to make
# sure our clients will only use the deferred returned by the probe()
@@ -225,9 +303,14 @@ class ProberFactory(protocol.ClientFactory):
self.timeout = timeout
self.timeoutCall = None
self.setURL(url.encode('ascii'))
+ self.logger = logging.getLogger('distributionmirror-prober')
+
+ @property
+ def is_https(self):
+ return self.request_scheme == 'https'
def probe(self):
- logger = logging.getLogger('distributionmirror-prober')
+ logger = self.logger
# NOTE: We don't want to issue connections to any outside host when
# running the mirror prober in a development machine, so we do this
# hack here.
@@ -244,13 +327,46 @@ class ProberFactory(protocol.ClientFactory):
"host already." % self.url)
return self._deferred
+ if (self.request_host, self.request_port) in invalid_certificate_hosts:
+ reactor.callLater(
+ 0, self.failed, InvalidHTTPSCertificateSkipped(self.url))
+ logger.debug("Skipping %s as it doesn't have a valid HTTPS "
+ "certificate" % self.url)
+ return self._deferred
+
self.connect()
logger.debug('Probing %s' % self.url)
return self._deferred
+ def getHttpsClient(self):
+ # Should we use a proxy?
+ if not config.launchpad.http_proxy:
+ agent = Agent(
+ reactor=reactor, contextFactory=self.https_agent_policy())
+ else:
+ endpoint = HostnameEndpoint(
+ reactor, self.connect_host, self.connect_port)
+ agent = ProxyAgent(endpoint)
+ return TreqHTTPClient(agent)
+
def connect(self):
+ """Starts the connection and sets the self._deferred to the proper
+ task.
+ """
host_requests[self.request_host] += 1
- reactor.connectTCP(self.connect_host, self.connect_port, self)
+ if self.is_https:
+ treq = self.getHttpsClient()
+ self._deferred.addCallback(
+ lambda _: treq.head(
+ self.url, reactor=reactor, allow_redirects=True,
+ timeout=self.timeout))
+ error_handler = HTTPSProbeFailureHandler(self)
+ self._deferred.addCallback(error_handler.handleResponse)
+ self._deferred.addErrback(error_handler.handleErrors)
+ reactor.callWhenRunning(self._deferred.callback, None)
+ else:
+ reactor.connectTCP(self.connect_host, self.connect_port, self)
+
if self.timeoutCall is not None and self.timeoutCall.active():
self._cancelTimeout(None)
self.timeoutCall = reactor.callLater(
@@ -269,6 +385,8 @@ class ProberFactory(protocol.ClientFactory):
self.connector = connector
def succeeded(self, status):
+ if IResponse.providedBy(status):
+ status = str(status.code)
self._deferred.callback(status)
def failed(self, reason):
@@ -288,7 +406,7 @@ class ProberFactory(protocol.ClientFactory):
# https://bugs.squid-cache.org/show_bug.cgi?id=1758 applied. So, if
# you encounter any problems with FTP URLs you'll probably have to nag
# the sysadmins to fix squid for you.
- if scheme not in ('http', 'ftp'):
+ if scheme not in ('http', 'https', 'ftp'):
raise UnknownURLScheme(url)
if scheme and host:
@@ -376,14 +494,26 @@ class ProberTimeout(ProberError):
class BadResponseCode(ProberError):
- def __init__(self, status, *args):
+ def __init__(self, status, response=None, *args):
ProberError.__init__(self, *args)
self.status = status
+ self.response = response
def __str__(self):
return "Bad response code: %s" % self.status
+class InvalidHTTPSCertificate(ProberError):
+ def __init__(self, host, port, *args):
+ super(InvalidHTTPSCertificate, self).__init__(*args)
+ self.host = host
+ self.port = port
+
+ def __str__(self):
+ return "Invalid SSL certificate when trying to probe %s:%s" % (
+ self.host, self.port)
+
+
class RedirectToDifferentFile(ProberError):
def __init__(self, orig_path, new_path, *args):
@@ -409,6 +539,14 @@ class ConnectionSkipped(ProberError):
"host. It will be retried on the next probing run.")
+class InvalidHTTPSCertificateSkipped(ProberError):
+
+ def __str__(self):
+ return ("Connection skipped because the server doesn't have a valid "
+ "HTTPS certificate. It will be retried on the next "
+ "probing run.")
+
+
class UnknownURLScheme(ProberError):
def __init__(self, url, *args):
@@ -429,7 +567,13 @@ class UnknownURLSchemeAfterRedirect(UnknownURLScheme):
class ArchiveMirrorProberCallbacks(LoggingMixin):
- expected_failures = (BadResponseCode, ProberTimeout, ConnectionSkipped)
+ expected_failures = (
+ BadResponseCode,
+ ProberTimeout,
+ ConnectionSkipped,
+ InvalidHTTPSCertificate,
+ InvalidHTTPSCertificateSkipped,
+ )
def __init__(self, mirror, series, pocket, component, url, log_file):
self.mirror = mirror
@@ -574,6 +718,8 @@ class MirrorCDImageProberCallbacks(LoggingMixin):
ProberTimeout,
RedirectToDifferentFile,
UnknownURLSchemeAfterRedirect,
+ InvalidHTTPSCertificate,
+ InvalidHTTPSCertificateSkipped,
)
def __init__(self, mirror, distroseries, flavour, log_file):
diff --git a/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt b/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
index db17b40..3e49ebe 100644
--- a/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
+++ b/lib/lp/registry/stories/webservice/xx-distribution-mirror.txt
@@ -23,6 +23,7 @@ mirrors:
enabled: True
ftp_base_url: None
http_base_url: u'http://archive.ubuntu.com/ubuntu/'
+ https_base_url: None
name: u'canonical-archive'
official_candidate: True
owner_link: u'http://.../~mark'
@@ -52,6 +53,7 @@ And CD image mirrors:
enabled: True
ftp_base_url: None
http_base_url: u'http://releases.ubuntu.com/'
+ https_base_url: None
name: u'canonical-releases'
official_candidate: True
owner_link: u'http://.../~mark'
@@ -145,6 +147,7 @@ Mirror listing admins may see all:
enabled: True
ftp_base_url: None
http_base_url: u'http://localhost:11375/archive-mirror/'
+ https_base_url: None
name: u'archive-404-mirror'
official_candidate: True
owner_link: u'http://.../~name12'
@@ -227,6 +230,7 @@ While others can be set with the appropriate authorization:
enabled: True
ftp_base_url: None
http_base_url: u'http://releases.ubuntu.com/'
+ https_base_url: None
name: u'canonical-releases'
official_candidate: True
owner_link: u'http://.../~mark'
diff --git a/lib/lp/registry/stories/webservice/xx-distribution.txt b/lib/lp/registry/stories/webservice/xx-distribution.txt
index 1c0ed87..1486904 100644
--- a/lib/lp/registry/stories/webservice/xx-distribution.txt
+++ b/lib/lp/registry/stories/webservice/xx-distribution.txt
@@ -159,6 +159,7 @@ packages matching (substring) the given text.
enabled: True
ftp_base_url: None
http_base_url: u'http://releases.ubuntu.com/'
+ https_base_url: None
name: u'canonical-releases'
official_candidate: True
owner_link: u'http://.../~mark'
diff --git a/lib/lp/registry/templates/distributionmirror-index.pt b/lib/lp/registry/templates/distributionmirror-index.pt
index 78e934d..27f3139 100644
--- a/lib/lp/registry/templates/distributionmirror-index.pt
+++ b/lib/lp/registry/templates/distributionmirror-index.pt
@@ -118,6 +118,10 @@
<h2>Mirror location information</h2>
<ul class="webref" id="mirror-urls">
+ <li tal:condition="context/https_base_url" >
+ <a tal:content="context/https_base_url"
+ tal:attributes="href context/https_base_url">https://url/</a>
+ </li>
<li tal:condition="context/http_base_url" >
<a tal:content="context/http_base_url"
tal:attributes="href context/http_base_url">http://url/</a>
diff --git a/lib/lp/registry/templates/distributionmirror-macros.pt b/lib/lp/registry/templates/distributionmirror-macros.pt
index 18f82af..489f770 100644
--- a/lib/lp/registry/templates/distributionmirror-macros.pt
+++ b/lib/lp/registry/templates/distributionmirror-macros.pt
@@ -17,7 +17,7 @@
<tbody>
<tal:country_and_mirrors repeat="country_and_mirrors mirrors_by_country">
<tr class="head">
- <th colspan="2"
+ <th colspan="2"
tal:content="country_and_mirrors/country" />
<th tal:content="country_and_mirrors/throughput"/>
<th tal:condition="show_mirror_type">
@@ -35,6 +35,8 @@
tal:content="mirror/title">Mirror Name</a>
</td>
<td>
+ <a tal:condition="mirror/https_base_url"
+ tal:attributes="href mirror/https_base_url">https</a>
<a tal:condition="mirror/http_base_url"
tal:attributes="href mirror/http_base_url">http</a>
<a tal:condition="mirror/ftp_base_url"
diff --git a/lib/lp/registry/tests/distributionmirror_http_server.py b/lib/lp/registry/tests/distributionmirror_http_server.py
index e3e0a39..d12c9b3 100644
--- a/lib/lp/registry/tests/distributionmirror_http_server.py
+++ b/lib/lp/registry/tests/distributionmirror_http_server.py
@@ -1,6 +1,6 @@
#!/usr/bin/python
#
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from twisted.web.resource import Resource
@@ -21,10 +21,10 @@ class DistributionMirrorTestHTTPServer(Resource):
:error: Respond with a '500 Internal Server Error' status.
:redirect-to-valid-mirror/*: Respond with a '302 Found' status,
- redirecting to http://localhost:%(port)s/valid-mirror/*.
+ redirecting to http(s)://localhost:%(port)s/valid-mirror/*.
:redirect-infinite-loop: Respond with a '302 Found' status, redirecting
- to http://localhost:%(port)s/redirect-infinite-loop.
+ to http(s)://localhost:%(port)s/redirect-infinite-loop.
:redirect-unknown-url-scheme: Respond with a '302 Found' status,
redirecting to ssh://localhost/redirect-unknown-url-scheme.
@@ -32,11 +32,13 @@ class DistributionMirrorTestHTTPServer(Resource):
Any other path will cause the server to respond with a '404 Not Found'
status.
"""
+ protocol = "http"
def getChild(self, name, request):
+ protocol = self.protocol
port = request.getHost().port
if name == 'valid-mirror':
- leaf = DistributionMirrorTestHTTPServer()
+ leaf = self.__class__()
leaf.isLeaf = True
return leaf
elif name == 'timeout':
@@ -49,12 +51,14 @@ class DistributionMirrorTestHTTPServer(Resource):
'than one component.')
remaining_path = request.path.replace('/%s' % name, '')
leaf = RedirectingResource(
- 'http://localhost:%s/valid-mirror%s' % (port, remaining_path))
+ '%s://localhost:%s/valid-mirror%s' % (
+ protocol, port, remaining_path))
leaf.isLeaf = True
return leaf
elif name == 'redirect-infinite-loop':
return RedirectingResource(
- 'http://localhost:%s/redirect-infinite-loop' % port)
+ '%s://localhost:%s/redirect-infinite-loop' %
+ (protocol, port))
elif name == 'redirect-unknown-url-scheme':
return RedirectingResource(
'ssh://localhost/redirect-unknown-url-scheme')
@@ -65,6 +69,11 @@ class DistributionMirrorTestHTTPServer(Resource):
return "Hi"
+class DistributionMirrorTestSecureHTTPServer(DistributionMirrorTestHTTPServer):
+ """HTTPS version of DistributionMirrorTestHTTPServer"""
+ protocol = "https"
+
+
class RedirectingResource(Resource):
def __init__(self, redirection_url):
@@ -85,4 +94,3 @@ class FiveHundredResource(Resource):
def render_GET(self, request):
request.setResponseCode(500)
request.write('ASPLODE!!!')
- return NOT_DONE_YET
diff --git a/lib/lp/registry/tests/test_distributionmirror_prober.py b/lib/lp/registry/tests/test_distributionmirror_prober.py
index 39228d2..c3e363c 100644
--- a/lib/lp/registry/tests/test_distributionmirror_prober.py
+++ b/lib/lp/registry/tests/test_distributionmirror_prober.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""distributionmirror-prober tests."""
@@ -11,6 +11,7 @@ import logging
import os
from StringIO import StringIO
+from fixtures import MockPatchObject
from lazr.uri import URI
import responses
from six.moves import http_client
@@ -28,9 +29,14 @@ from testtools.twistedsupport import (
from twisted.internet import (
defer,
reactor,
+ ssl,
)
from twisted.python.failure import Failure
from twisted.web import server
+from twisted.web.client import (
+ BrowserLikePolicyForHTTPS,
+ ProxyAgent,
+ )
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -44,6 +50,8 @@ from lp.registry.scripts.distributionmirror_prober import (
BadResponseCode,
ConnectionSkipped,
InfiniteLoopDetected,
+ InvalidHTTPSCertificate,
+ InvalidHTTPSCertificateSkipped,
LoggingMixin,
MAX_REDIRECTS,
MIN_REQUEST_TIMEOUT_RATIO,
@@ -65,6 +73,7 @@ from lp.registry.scripts.distributionmirror_prober import (
)
from lp.registry.tests.distributionmirror_http_server import (
DistributionMirrorTestHTTPServer,
+ DistributionMirrorTestSecureHTTPServer,
)
from lp.services.config import config
from lp.services.daemons.tachandler import TacTestSetup
@@ -103,6 +112,186 @@ class HTTPServerTestSetup(TacTestSetup):
return os.path.join(self.root, 'distributionmirror_http_server.log')
+
+class LocalhostWhitelistedHTTPSPolicy(BrowserLikePolicyForHTTPS):
+ """HTTPS policy that bypasses SSL certificate check when doing requests
+ to localhost.
+ """
+
+ def creatorForNetloc(self, hostname, port):
+ # check if the hostname is in the the whitelist,
+ # otherwise return the default policy
+ if hostname == 'localhost':
+ return ssl.CertificateOptions(verify=False)
+ return super(LocalhostWhitelistedHTTPSPolicy, self).creatorForNetloc(
+ hostname, port)
+
+
+class TestProberHTTPSProtocolAndFactory(TestCase):
+ layer = TwistedLayer
+ run_tests_with = AsynchronousDeferredRunTestForBrokenTwisted.make_factory(
+ timeout=30)
+
+ def setUp(self):
+ super(TestProberHTTPSProtocolAndFactory, self).setUp()
+ root = DistributionMirrorTestSecureHTTPServer()
+ site = server.Site(root)
+ site.displayTracebacks = False
+ keys_path = os.path.join(config.root, "configs", "development")
+ keys = ssl.DefaultOpenSSLContextFactory(
+ os.path.join(keys_path, "launchpad.key"),
+ os.path.join(keys_path, "launchpad.crt"),
+ )
+ self.listening_port = reactor.listenSSL(0, site, keys)
+
+ self.addCleanup(self.listening_port.stopListening)
+
+ # Change the default policy to accept localhost self-signed
+ # certificates.
+ original_probefactory_policy = ProberFactory.https_agent_policy
+ original_redirect_policy = (
+ RedirectAwareProberFactory.https_agent_policy)
+ ProberFactory.https_agent_policy = LocalhostWhitelistedHTTPSPolicy
+ RedirectAwareProberFactory.https_agent_policy = (
+ LocalhostWhitelistedHTTPSPolicy)
+
+ for factory in (ProberFactory, RedirectAwareProberFactory):
+ self.useFixture(MockPatchObject(
+ factory, "https_agent_policy",
+ LocalhostWhitelistedHTTPSPolicy))
+
+ self.port = self.listening_port.getHost().port
+
+ self.urls = {'timeout': u'https://localhost:%s/timeout' % self.port,
+ '200': u'https://localhost:%s/valid-mirror' % self.port,
+ '500': u'https://localhost:%s/error' % self.port,
+ '404': u'https://localhost:%s/invalid-mirror' % self.port}
+ self.pushConfig('launchpad', http_proxy=None)
+
+ self.useFixture(MockPatchObject(
+ distributionmirror_prober, "host_requests", {}))
+ self.useFixture(MockPatchObject(
+ distributionmirror_prober, "host_timeouts", {}))
+ self.useFixture(MockPatchObject(
+ distributionmirror_prober, "invalid_certificate_hosts", set()))
+
+ def _createProberAndProbe(self, url):
+ prober = ProberFactory(url)
+ return prober.probe()
+
+ def test_timeout(self):
+ prober = ProberFactory(self.urls['timeout'], timeout=0.5)
+ d = prober.probe()
+ return assert_fails_with(d, ProberTimeout)
+
+ def test_500(self):
+ d = self._createProberAndProbe(self.urls['500'])
+ return assert_fails_with(d, BadResponseCode)
+
+ def test_notfound(self):
+ d = self._createProberAndProbe(self.urls['404'])
+ return assert_fails_with(d, BadResponseCode)
+
+ def test_config_no_https_proxy(self):
+ prober = ProberFactory(self.urls['200'])
+ self.assertThat(prober, MatchesStructure.byEquality(
+ request_scheme='https',
+ request_host='localhost',
+ request_port=self.port,
+ request_path='/valid-mirror',
+ connect_scheme='https',
+ connect_host='localhost',
+ connect_port=self.port,
+ connect_path='/valid-mirror'))
+
+ def test_RedirectAwareProber_follows_https_redirect(self):
+ url = 'https://localhost:%s/redirect-to-valid-mirror/file' % self.port
+ prober = RedirectAwareProberFactory(url)
+ self.assertEqual(prober.url, url)
+ deferred = prober.probe()
+
+ def got_result(result):
+ self.assertEqual(http_client.OK, result.code)
+ self.assertEqual(
+ 'https://localhost:%s/valid-mirror/file' % self.port,
+ result.request.absoluteURI)
+
+ return deferred.addCallback(got_result)
+
+ def test_https_prober_uses_proxy(self):
+ root = DistributionMirrorTestSecureHTTPServer()
+ site = server.Site(root)
+ proxy_listen_port = reactor.listenTCP(0, site)
+ proxy_port = proxy_listen_port.getHost().port
+ self.pushConfig(
+ 'launchpad', http_proxy='http://localhost:%s/valid-mirror/file'
+ % proxy_port)
+
+ url = 'https://localhost:%s/valid-mirror/file' % self.port
+ prober = RedirectAwareProberFactory(url)
+ self.assertEqual(prober.url, url)
+ deferred = prober.probe()
+
+ def got_result(result):
+ # We basically don't care about the result here. We just want to
+ # check that it did the request to the correct URI,
+ # and ProxyAgent was used pointing to the correct proxy.
+ agent = prober.getHttpsClient()._agent
+ self.assertIsInstance(agent, ProxyAgent)
+ self.assertEqual('localhost', agent._proxyEndpoint._hostText)
+ self.assertEqual(proxy_port, agent._proxyEndpoint._port)
+
+ self.assertEqual(
+ 'https://localhost:%s/valid-mirror/file' % self.port,
+ result.value.response.request.absoluteURI)
+
+ def cleanup(*args, **kwargs):
+ proxy_listen_port.stopListening()
+
+ # Doing the proxy checks on the error callback because the
+ # proxy is dummy and always returns 404.
+ deferred.addErrback(got_result)
+ deferred.addBoth(cleanup)
+ return deferred
+
+ def test_https_fails_on_invalid_certificates(self):
+ """Changes set back the default browser-like policy for HTTPS
+ request and make sure the request is failing due to invalid
+ (self-signed) certificate.
+ """
+ url = 'https://localhost:%s/valid-mirror/file' % self.port
+ prober = RedirectAwareProberFactory(url)
+ prober.https_agent_policy = BrowserLikePolicyForHTTPS
+ self.assertEqual(prober.url, url)
+ deferred = prober.probe()
+
+ def on_failure(result):
+ self.assertIsInstance(result.value, InvalidHTTPSCertificate)
+ self.assertIn(
+ ("localhost", self.port),
+ distributionmirror_prober.invalid_certificate_hosts)
+
+ def on_success(result):
+ if result is not None:
+ self.fail(
+ "Should have raised SSL error. Got '%s' instead" % result)
+
+ deferred.addErrback(on_failure)
+ deferred.addCallback(on_success)
+ return deferred
+
+ def test_https_skips_invalid_certificates_hosts(self):
+ distributionmirror_prober.invalid_certificate_hosts.add(
+ ("localhost", self.port))
+ url = 'https://localhost:%s/valid-mirror/file' % self.port
+ prober = RedirectAwareProberFactory(url)
+ prober.https_agent_policy = BrowserLikePolicyForHTTPS
+ self.assertEqual(prober.url, url)
+ deferred = prober.probe()
+
+ return assert_fails_with(deferred, InvalidHTTPSCertificateSkipped)
+
+
class TestProberProtocolAndFactory(TestCase):
layer = TwistedLayer
@@ -212,7 +401,7 @@ class TestProberProtocolAndFactory(TestCase):
self.assertTrue(prober.url == new_url)
self.assertTrue(result == str(http_client.OK))
- return deferred.addCallback(got_result)
+ return deferred.addBoth(got_result)
def test_redirectawareprober_detects_infinite_loop(self):
prober = RedirectAwareProberFactory(
@@ -737,12 +926,16 @@ class TestMirrorCDImageProberCallbacks(TestCaseWithFactory):
ConnectionSkipped,
RedirectToDifferentFile,
UnknownURLSchemeAfterRedirect,
+ InvalidHTTPSCertificate,
+ InvalidHTTPSCertificateSkipped,
]))
exceptions = [BadResponseCode(str(http_client.NOT_FOUND)),
ProberTimeout('http://localhost/', 5),
ConnectionSkipped(),
RedirectToDifferentFile('/foo', '/bar'),
- UnknownURLSchemeAfterRedirect('https://localhost')]
+ UnknownURLSchemeAfterRedirect('https://localhost'),
+ InvalidHTTPSCertificate('localhost', 443),
+ InvalidHTTPSCertificateSkipped("https://localhost/xx";)]
for exception in exceptions:
failure = callbacks.ensureOrDeleteMirrorCDImageSeries(
[(defer.FAILURE, Failure(exception))])
diff --git a/lib/lp/scripts/utilities/importpedant.py b/lib/lp/scripts/utilities/importpedant.py
index ec41baf..c7859a4 100644
--- a/lib/lp/scripts/utilities/importpedant.py
+++ b/lib/lp/scripts/utilities/importpedant.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from __future__ import absolute_import, print_function, unicode_literals
@@ -42,6 +42,8 @@ valid_imports_not_in_all = {
'textwrap': set(['dedent']),
'testtools.testresult.real': set(['_details_to_str']),
'twisted.internet.threads': set(['deferToThreadPool']),
+ # Even docs tell us to use this class. See docs on WebClientContextFactory.
+ 'twisted.web.client': set(['BrowserLikePolicyForHTTPS']),
'zope.component': set(
['adapter',
'ComponentLookupError',
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 85a05b6..6c0492f 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2,7 +2,7 @@
# NOTE: The first line above must stay first; do not move the copyright
# notice to the top. See http://www.python.org/dev/peps/pep-0263/.
#
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Testing infrastructure for the Launchpad application.
@@ -3527,13 +3527,13 @@ class BareLaunchpadObjectFactory(ObjectFactory):
return proberecord
def makeMirror(self, distribution, displayname=None, country=None,
- http_url=None, ftp_url=None, rsync_url=None,
+ http_url=None, https_url=None, ftp_url=None, rsync_url=None,
official_candidate=False):
"""Create a mirror for the distribution."""
if displayname is None:
displayname = self.getUniqueString("mirror")
# If no URL is specified create an HTTP URL.
- if http_url is None and ftp_url is None and rsync_url is None:
+ if http_url is https_url is ftp_url is rsync_url is None:
http_url = self.getUniqueURL()
# If no country is given use Argentina.
if country is None:
@@ -3547,6 +3547,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
display_name=displayname,
description=None,
http_base_url=http_url,
+ https_base_url=https_url,
ftp_base_url=ftp_url,
rsync_base_url=rsync_url,
official_candidate=official_candidate)
References
