Merge remote-tracking branch 'kali/feature/provider_check_against_ca_bundle' into develop

6 files changed, 56 insertions, 35 deletions

diff --git a/src/leap/bitmask/gui/mainwindow.py b/src/leap/bitmask/gui/mainwindow.py
index 200d68aa..d8d0ac21 100644
--- a/src/leap/bitmask/gui/mainwindow.py
+++ b/src/leap/bitmask/gui/mainwindow.py
@@ -36,9 +36,9 @@ from leap.bitmask.gui import statemachines
 from leap.bitmask.gui.statuspanel import StatusPanelWidget
 from leap.bitmask.gui.wizard import Wizard
 
+from leap.bitmask.provider.providerbootstrapper import ProviderBootstrapper
 from leap.bitmask.services.eip.eipbootstrapper import EIPBootstrapper
 from leap.bitmask.services.eip.eipconfig import EIPConfig
-from leap.bitmask.services.eip.providerbootstrapper import ProviderBootstrapper
 # XXX: Soledad might not work out of the box in Windows, issue #2932
 from leap.bitmask.services.soledad.soledadbootstrapper import \
     SoledadBootstrapper
diff --git a/src/leap/bitmask/gui/wizard.py b/src/leap/bitmask/gui/wizard.py
index 45734b81..bb38b136 100644
--- a/src/leap/bitmask/gui/wizard.py
+++ b/src/leap/bitmask/gui/wizard.py
@@ -14,7 +14,6 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
 """
 First run wizard
 """
@@ -27,15 +26,13 @@ from functools import partial
 from PySide import QtCore, QtGui
 from twisted.internet import threads
 
-from leap.bitmask.config import flags
 from leap.bitmask.config.providerconfig import ProviderConfig
 from leap.bitmask.crypto.srpregister import SRPRegister
-from leap.bitmask.util.privilege_policies import is_missing_policy_permissions
+from leap.bitmask.provider.providerbootstrapper import ProviderBootstrapper
+from leap.bitmask.services import get_service_display_name, get_supported
 from leap.bitmask.util.request_helpers import get_content
 from leap.bitmask.util.keyring_helpers import has_keyring
 from leap.bitmask.util.password import basic_password_checks
-from leap.bitmask.services.eip.providerbootstrapper import ProviderBootstrapper
-from leap.bitmask.services import get_service_display_name, get_supported
 
 from ui_wizard import Ui_Wizard
 
diff --git a/src/leap/bitmask/services/eip/providerbootstrapper.py b/src/leap/bitmask/provider/providerbootstrapper.py
index 3b7c9899..751da828 100644
--- a/src/leap/bitmask/services/eip/providerbootstrapper.py
+++ b/src/leap/bitmask/provider/providerbootstrapper.py
@@ -14,7 +14,6 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
 """
 Provider bootstrapping
 """
@@ -32,11 +31,11 @@ from leap.bitmask.util import get_path_prefix
 from leap.bitmask.util.constants import REQUEST_TIMEOUT
 from leap.bitmask.services.abstractbootstrapper import AbstractBootstrapper
 from leap.bitmask.provider.supportedapis import SupportedAPIs
+from leap.common import ca_bundle
 from leap.common.certs import get_digest
 from leap.common.files import check_and_fix_urw_only, get_mtime, mkdir_p
 from leap.common.check import leap_assert, leap_assert_type, leap_check
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -85,16 +84,32 @@ class ProviderBootstrapper(AbstractBootstrapper):
         self._provider_config = None
         self._download_if_needed = False
 
+    @property
+    def verify(self):
+        """
+        Verify parameter for requests.
+
+        :returns: either False, if checks are skipped, or the
+                  path to the ca bundle.
+        :rtype: bool or str
+        """
+        if self._bypass_checks:
+            verify = False
+        else:
+            verify = ca_bundle.where()
+        return verify
+
     def _check_name_resolution(self):
         """
         Checks that the name resolution for the provider name works
         """
         leap_assert(self._domain, "Cannot check DNS without a domain")
-
         logger.debug("Checking name resolution for %s" % (self._domain))
 
         # We don't skip this check, since it's basic for the whole
         # system to work
+        # err --- but we can do it after a failure, to diagnose what went
+        # wrong. Right now we're just adding connection overhead. -- kali
         socket.gethostbyname(self._domain)
 
     def _check_https(self, *args):
@@ -102,24 +117,29 @@ class ProviderBootstrapper(AbstractBootstrapper):
         Checks that https is working and that the provided certificate
         checks out
         """
-
         leap_assert(self._domain, "Cannot check HTTPS without a domain")
-
         logger.debug("Checking https for %s" % (self._domain))
 
         # We don't skip this check, since it's basic for the whole
-        # system to work
+        # system to work.
+        # err --- but we can do it after a failure, to diagnose what went
+        # wrong. Right now we're just adding connection overhead. -- kali
 
         try:
             res = self._session.get("https://%s" % (self._domain,),
-                                    verify=not self._bypass_checks,
+                                    verify=self.verify,
                                     timeout=REQUEST_TIMEOUT)
             res.raise_for_status()
-        except requests.exceptions.SSLError:
+        except requests.exceptions.SSLError as exc:
+            logger.exception(exc)
             self._err_msg = self.tr("Provider certificate could "
                                     "not be verified")
             raise
-        except Exception:
+        except Exception as exc:
+            # XXX careful!. The error might be also a SSL handshake
+            # timeout error, in which case we should retry a couple of times
+            # more, for cases where the ssl server gives high latencies.
+            logger.exception(exc)
             self._err_msg = self.tr("Provider does not support HTTPS")
             raise
 
@@ -129,11 +149,13 @@ class ProviderBootstrapper(AbstractBootstrapper):
         """
         leap_assert(self._domain,
                     "Cannot download provider info without a domain")
-
         logger.debug("Downloading provider info for %s" % (self._domain))
 
-        headers = {}
+        # --------------------------------------------------------------
+        # TODO factor out with the download routines in services.
+        # Watch out! We're handling the verify paramenter differently here.
 
+        headers = {}
         provider_json = os.path.join(get_path_prefix(), "leap", "providers",
                                      self._domain, "provider.json")
         mtime = get_mtime(provider_json)
@@ -142,16 +164,18 @@ class ProviderBootstrapper(AbstractBootstrapper):
             headers['if-modified-since'] = mtime
 
         uri = "https://%s/%s" % (self._domain, "provider.json")
-        verify = not self._bypass_checks
+        verify = self.verify
 
         if mtime:  # the provider.json exists
-            provider_config = ProviderConfig()
-            provider_config.load(provider_json)
+        # So, we're getting it from the api.* and checking against
+        # the provider ca.
             try:
-                verify = provider_config.get_ca_cert_path()
+                provider_config = ProviderConfig()
+                provider_config.load(provider_json)
                 uri = provider_config.get_api_uri() + '/provider.json'
+                verify = provider_config.get_ca_cert_path()
             except MissingCACert:
-                # get_ca_cert_path fails if the certificate does not exists.
+                # no ca? then download from main domain again.
                 pass
 
         logger.debug("Requesting for provider.json... "
@@ -165,6 +189,9 @@ class ProviderBootstrapper(AbstractBootstrapper):
         # Not modified
         if res.status_code == 304:
             logger.debug("Provider definition has not been modified")
+        # --------------------------------------------------------------
+        # end refactor, more or less...
+        # XXX Watch out, have to check the supported api yet.
         else:
             provider_definition, mtime = get_content(res)
 
@@ -181,8 +208,8 @@ class ProviderBootstrapper(AbstractBootstrapper):
             else:
                 api_supported = ', '.join(SupportedAPIs.SUPPORTED_APIS)
                 error = ('Unsupported provider API version. '
-                         'Supported versions are: {}. '
-                         'Found: {}.').format(api_supported, api_version)
+                         'Supported versions are: {0}. '
+                         'Found: {1}.').format(api_supported, api_version)
 
                 logger.error(error)
                 raise UnsupportedProviderAPI(error)
@@ -230,7 +257,8 @@ class ProviderBootstrapper(AbstractBootstrapper):
         """
         Downloads the CA cert that is going to be used for the api URL
         """
-
+        # XXX maybe we can skip this step if
+        # we have a fresh one.
         leap_assert(self._provider_config, "Cannot download the ca cert "
                     "without a provider config!")
 
@@ -244,7 +272,7 @@ class ProviderBootstrapper(AbstractBootstrapper):
             return
 
         res = self._session.get(self._provider_config.get_ca_cert_uri(),
-                                verify=not self._bypass_checks,
+                                verify=self.verify,
                                 timeout=REQUEST_TIMEOUT)
         res.raise_for_status()
 
diff --git a/src/leap/bitmask/provider/tests/__init__.py b/src/leap/bitmask/provider/tests/__init__.py
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/src/leap/bitmask/provider/tests/__init__.py
diff --git a/src/leap/bitmask/services/eip/tests/test_providerbootstrapper.py b/src/leap/bitmask/provider/tests/test_providerbootstrapper.py
index b0685676..9b47d60e 100644
--- a/src/leap/bitmask/services/eip/tests/test_providerbootstrapper.py
+++ b/src/leap/bitmask/provider/tests/test_providerbootstrapper.py
@@ -14,15 +14,12 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
-
-
 """
 Tests for the Provider Boostrapper checks
 
 These will be whitebox tests since we want to make sure the private
 implementation is checking what we expect.
 """
-
 import os
 import mock
 import socket
@@ -39,13 +36,12 @@ from nose.twistedtools import deferred, reactor
 from twisted.internet import threads
 from requests.models import Response
 
-from leap.bitmask.services.eip.providerbootstrapper import ProviderBootstrapper
-from leap.bitmask.services.eip.providerbootstrapper import \
-    UnsupportedProviderAPI
-from leap.bitmask.services.eip.providerbootstrapper import WrongFingerprint
-from leap.bitmask.provider.supportedapis import SupportedAPIs
 from leap.bitmask.config.providerconfig import ProviderConfig
 from leap.bitmask.crypto.tests import fake_provider
+from leap.bitmask.provider.providerbootstrapper import ProviderBootstrapper
+from leap.bitmask.provider.providerbootstrapper import UnsupportedProviderAPI
+from leap.bitmask.provider.providerbootstrapper import WrongFingerprint
+from leap.bitmask.provider.supportedapis import SupportedAPIs
 from leap.common.files import mkdir_p
 from leap.common.testing.https_server import where
 from leap.common.testing.basetest import BaseLeapTest
diff --git a/src/leap/bitmask/util/constants.py b/src/leap/bitmask/util/constants.py
index 63f6b1f7..e6a6bdce 100644
--- a/src/leap/bitmask/util/constants.py
+++ b/src/leap/bitmask/util/constants.py
@@ -16,4 +16,4 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 SIGNUP_TIMEOUT = 5
-REQUEST_TIMEOUT = 10
+REQUEST_TIMEOUT = 15