From 0d0459ac31f40d90270de0a9da8e1eac0b57784d Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 18 Sep 2017 15:35:13 +0200 Subject: [feat] check if there are newest configuration files here we port the if-modified-since conditional mechanism, so that we only write the config if it is newer than whan we have. we also add a line with the status code to the logs, so that it's easier to debug. note that the 'configs.json' file is never returning 304. - Resolves: #8773 --- Makefile | 1 + src/leap/bitmask/bonafide/__init__.py | 1 - src/leap/bitmask/bonafide/_http.py | 48 +++++++++++++++-- src/leap/bitmask/bonafide/bootstrap.py | 0 src/leap/bitmask/bonafide/config.py | 97 +++++++++++++--------------------- src/leap/bitmask/bonafide/session.py | 8 +-- tests/e2e/conditional_downloads.py | 40 ++++++++++++++ 7 files changed, 124 insertions(+), 71 deletions(-) delete mode 100644 src/leap/bitmask/bonafide/bootstrap.py create mode 100755 tests/e2e/conditional_downloads.py diff --git a/Makefile b/Makefile index 3c33a094..7ff10d1e 100644 --- a/Makefile +++ b/Makefile @@ -45,6 +45,7 @@ test: test_e2e: install_helpers tests/e2e/e2e-test-mail.sh tests/e2e/e2e-test-vpn.sh + tests/e2e/conditional_downloads.py test_functional_setup: pip install -U behave selenium diff --git a/src/leap/bitmask/bonafide/__init__.py b/src/leap/bitmask/bonafide/__init__.py index 9dfb19db..a007f8c5 100644 --- a/src/leap/bitmask/bonafide/__init__.py +++ b/src/leap/bitmask/bonafide/__init__.py @@ -3,7 +3,6 @@ # exersise the codebase. import config -import bootstrap import session import provider import service diff --git a/src/leap/bitmask/bonafide/_http.py b/src/leap/bitmask/bonafide/_http.py index 8f05b421..94d0bcb9 100644 --- a/src/leap/bitmask/bonafide/_http.py +++ b/src/leap/bitmask/bonafide/_http.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # _http.py -# Copyright (C) 2015 LEAP +# Copyright (C) 2015-2017 LEAP # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -18,10 +18,14 @@ """ twisted.web utils for bonafide. """ -import base64 + import cookielib +import os import urllib +from leap.common.files import get_mtime + +from twisted.logger import Logger from twisted.internet import defer, protocol, reactor from twisted.internet.ssl import Certificate from twisted.python.filepath import FilePath @@ -32,6 +36,9 @@ from twisted.web.iweb import IBodyProducer from zope.interface import implements +log = Logger() + + def cookieAgentFactory(verify_path, connectTimeout=30): customPolicy = BrowserLikePolicyForHTTPS( Certificate.loadPEM(FilePath(verify_path).getContent())) @@ -40,19 +47,36 @@ def cookieAgentFactory(verify_path, connectTimeout=30): return CookieAgent(agent, cookiejar) -def httpRequest(agent, url, values={}, headers={}, method='POST', token=None): +class Unchanged(Exception): + pass + + +# TODO this should be ported to use treq client. + +def httpRequest(agent, url, values={}, headers={}, method='POST', token=None, + saveto=None): data = '' if values: data = urllib.urlencode(values) headers['Content-Type'] = ['application/x-www-form-urlencoded'] + if saveto is not None: + # TODO - I think we need a force parameter, because we might have a + # malformed file. Or maybe just remove the file if sanity check does + # not pass. + mtime = get_mtime(saveto) + if mtime is not None: + headers['if-modified-since'] = [mtime] if token: headers['Authorization'] = ['Token token="%s"' % (bytes(token))] def handle_response(response): - # print "RESPONSE CODE", response.code + log.debug("RESPONSE %s %s %s" % (method, response.code, url)) if response.code == 204: d = defer.succeed('') + if response.code == 304: + log.debug('304 (Not modified): %s' % url) + raise Unchanged() else: class SimpleReceiver(protocol.Protocol): def __init__(s, d): @@ -70,12 +94,28 @@ def httpRequest(agent, url, values={}, headers={}, method='POST', token=None): response.deliverBody(SimpleReceiver(d)) return d + def passthru(failure): + failure.trap(Unchanged) + d = agent.request(method, url, Headers(headers), StringProducer(data) if data else None) d.addCallback(handle_response) + if saveto: + d.addCallback(lambda body: _write_to_file(body, saveto)) + d.addErrback(passthru) return d +def _write_to_file(content, path): + folder = os.path.split(path)[0] + if not os.path.isdir(folder): + os.makedirs(folder) + with open(path, 'w') as f: + f.write(content) + # touch it to update its utime + os.utime(path, None) + + class StringProducer(object): implements(IBodyProducer) diff --git a/src/leap/bitmask/bonafide/bootstrap.py b/src/leap/bitmask/bonafide/bootstrap.py deleted file mode 100644 index e69de29b..00000000 diff --git a/src/leap/bitmask/bonafide/config.py b/src/leap/bitmask/bonafide/config.py index 6857a3f8..d0468a49 100644 --- a/src/leap/bitmask/bonafide/config.py +++ b/src/leap/bitmask/bonafide/config.py @@ -34,13 +34,15 @@ from urlparse import urlparse from twisted.internet import defer, reactor from twisted.logger import Logger from twisted.web.client import downloadPage +from twisted.web.client import readBody +from leap.bitmask.bonafide._http import httpRequest from leap.bitmask.bonafide.provider import Discovery from leap.bitmask.bonafide.errors import NotConfiguredError, NetworkError from leap.common.check import leap_assert from leap.common.config import get_path_prefix as common_get_path_prefix -from leap.common.files import mkdir_p +from leap.common.files import mkdir_p, get_mtime from leap.common.http import HTTPClient @@ -56,6 +58,7 @@ def get_path_prefix(standalone=False): _preffix = get_path_prefix() +log = Logger() def get_provider_path(domain, config='provider.json'): @@ -78,19 +81,6 @@ def get_ca_cert_path(domain): return os.path.join('providers', domain, 'keys', 'ca', 'cacert.pem') -def get_modification_ts(path): - """ - Gets modification time of a file. - - :param path: the path to get ts from - :type path: str - :returns: modification time - :rtype: datetime object - """ - ts = os.path.getmtime(path) - return datetime.datetime.fromtimestamp(ts) - - def update_modification_ts(path): """ Sets modification time of a file to current time. @@ -101,7 +91,7 @@ def update_modification_ts(path): :rtype: datetime object """ os.utime(path, None) - return get_modification_ts(path) + return get_mtime(path) def is_file(path): @@ -189,18 +179,17 @@ class Provider(object): if not is_configured: if autoconf: self.log.debug( - 'provider %s not configured: downloading files...' % - domain) + 'BOOTSTRAP: provider %s not initialized, ' + 'downloading files...' % domain) self.bootstrap() else: raise NotConfiguredError("Provider %s is not configured" % (domain,)) else: - self.log.debug('Provider already initialized') - self.first_bootstrap[self._domain] = defer.succeed( - 'already_initialized') - self.ongoing_bootstrap[self._domain] = defer.succeed( - 'already_initialized') + self.log.debug( + 'BOOTSTRAP: Provider is already initialized, checking if ' + 'there are newest config files...') + self.bootstrap(replace_if_newer=True) @property def domain(self): @@ -241,7 +230,7 @@ class Provider(object): " not found in provider " + self._domain) return config - def bootstrap(self): + def bootstrap(self, replace_if_newer=False): domain = self._domain self.log.debug('Bootstrapping provider %s' % domain) ongoing = self.ongoing_bootstrap.get(domain) @@ -263,7 +252,7 @@ class Provider(object): self.first_bootstrap[domain].errback(failure) return failure - d = self.maybe_download_provider_info() + d = self.maybe_download_provider_info(replace=replace_if_newer) d.addCallback(self.maybe_download_ca_cert) d.addCallback(self.validate_ca_cert) d.addCallbacks(first_bootstrap_done, first_bootstrap_error) @@ -291,6 +280,7 @@ class Provider(object): # TODO handle pre-seeded providers? # or let client handle that? We could move them to bonafide. provider_json = self._get_provider_json_path() + if is_file(provider_json) and not replace: return defer.succeed('provider_info_already_exists') @@ -304,8 +294,9 @@ class Provider(object): shutil.rmtree(folders) raise NetworkError(failure.getErrorMessage()) - d = self._http.request(uri, method=met) - d.addCallback(_write_to_file, provider_json) + d = httpRequest( + self._http._agent, + uri, method=met, saveto=provider_json) d.addCallback(lambda _: self._load_provider_json()) d.addErrback(errback) return d @@ -378,12 +369,6 @@ class Provider(object): # UNAUTHENTICATED if we try to get the services # See: # https://leap.se/code/issues/7906 - def check_error(content): - c = json.loads(content) - if 'error' in c: - raise Exception(c['error']) - return content - def further_bootstrap_needs_auth(ignored): self.log.warn('Cannot download services config yet, need auth') pending_deferred = defer.Deferred() @@ -391,10 +376,8 @@ class Provider(object): return defer.succeed('ok for now') uri, met, path = self._get_configs_download_params() - - d = self._http.request(uri, method=met) - d.addCallback(check_error) - d.addCallback(_write_to_file, path) + d = httpRequest( + self._http._agent, uri, method=met, saveto=path) d.addCallback(lambda _: self._load_provider_json()) d.addCallback( lambda _: self._get_config_for_all_services(session=None)) @@ -414,17 +397,13 @@ class Provider(object): d.addCallback(lambda _: stuck.callback('continue!')) return d - if not self.has_fetched_services_config(): - self._load_provider_json() - uri, met, path = self._get_configs_download_params() - d = session.fetch_provider_configs(uri, path, met) - d.addCallback(verify_provider_configs) - d.addCallback(complete_bootstrapping) - return d - else: - d = defer.succeed('already downloaded') - d.addCallback(complete_bootstrapping) - return d + self._load_provider_json() + uri, met, path = self._get_configs_download_params() + + d = session.fetch_provider_configs(uri, path, met) + d.addCallback(verify_provider_configs) + d.addCallback(complete_bootstrapping) + return d def _get_configs_download_params(self): uri = self._disco.get_configs_uri() @@ -519,10 +498,11 @@ class Provider(object): uri = base + str(services_dict[subservice]) path = self._get_service_config_path(subservice) if session: - d = session.fetch_provider_configs(uri, path) + d = session.fetch_provider_configs( + uri, path, method='GET') else: d = self._fetch_provider_configs_unauthenticated( - uri, path) + uri, path, method='GET') pending.append(d) return defer.gatherResults(pending) @@ -534,9 +514,8 @@ class Provider(object): def _fetch_provider_configs_unauthenticated(self, uri, path): self.log.info('Downloading config for %s...' % uri) - d = self._http.request(uri) - d.addCallback(_write_to_file, path) - return d + return httpRequest( + self._http._agent, uri, saveto=path) class Record(object): @@ -547,16 +526,14 @@ class Record(object): return self.__dict__ -def _write_to_file(content, path): - with open(path, 'w') as f: - f.write(content) - - if __name__ == '__main__': + from twisted.internet.task import react + + def main(reactor, *args): + provider = Provider('cdev.bitmask.net', autoconf=True) + return provider.callWhenReady(print_done) def print_done(): print '>>> bootstrapping done!!!' - provider = Provider('cdev.bitmask.net') - provider.callWhenReady(print_done) - reactor.run() + react(main, []) diff --git a/src/leap/bitmask/bonafide/session.py b/src/leap/bitmask/bonafide/session.py index 711e9ee2..988cbb99 100644 --- a/src/leap/bitmask/bonafide/session.py +++ b/src/leap/bitmask/bonafide/session.py @@ -165,12 +165,9 @@ class Session(object): # right path. uri = self._api.get_smtp_cert_uri() met = self._api.get_smtp_cert_method() - print met, "to", uri return self._request(self._agent, uri, method=met) def _request(self, *args, **kw): - # TODO: we are not pinning the TLS cert of the API - # maybe we can use leap.common.http kw['token'] = self._token return httpRequest(*args, **kw) @@ -201,9 +198,8 @@ class Session(object): @defer.inlineCallbacks def fetch_provider_configs(self, uri, path, method='GET'): - config = yield self._request(self._agent, uri, method=method) - with open(path, 'w') as cf: - cf.write(config) + config = yield self._request( + self._agent, uri, method=method, saveto=path) defer.returnValue('ok') diff --git a/tests/e2e/conditional_downloads.py b/tests/e2e/conditional_downloads.py new file mode 100755 index 00000000..6b9a8409 --- /dev/null +++ b/tests/e2e/conditional_downloads.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python +""" +Exercises the bonafide http module against a real server. +Checks for the conditional header behavior, ensuring that we do not write a +config file if we receive a 304 Not Modified response. +""" +import uuid +import os +import tempfile +import shutil + +from twisted.internet import defer +from twisted.internet.task import react + +from leap.bitmask.bonafide._http import httpRequest +from leap.common import http + +URI = 'https://demo.bitmask.net/1/configs/eip-service.json' +tmp = tempfile.mkdtemp() + + +@defer.inlineCallbacks +def main(reactor, *args): + client = http.HTTPClient() + fname = os.path.join(tmp, str(uuid.uuid4())) + yield httpRequest(client._agent, URI, method='GET', saveto=fname) + filesize = os.path.getsize(fname) + assert filesize > 1 + # touch file to 5 minutes in the past + past = int(os.path.getmtime(fname)) - 300 + os.utime(fname, (past, past)) + assert os.path.getmtime(fname) == past + yield httpRequest(client._agent, URI, method='GET', saveto=fname) + # it was not modified + assert os.path.getmtime(fname) == past + print 'OK' + shutil.rmtree(tmp) + + +react(main, []) -- cgit v1.2.3