NXDRIVE-2695: Better handling of custom SSL certificates (#2880)

* NXDRIVE-2695: Add a function to validate a SSL certificate file

* NXDRIVE-2695: Refactor ca_bundle and ssl_no_verify usages

* NXDRIVE-2695: Better handling of custom SSL certificates

* NXDRIVE-2695: Use a more specific file name for the final certificate

To prevent unwanted deletions.

[skip ci]

* NXDRIVE-2695: Add test for certifi or certificate(s) updates

* NXDRIVE-2695: Remove duplicate test case

* NXDRIVE-2695: Allow to pass a final certificate to ca-bundle

docs/changes/5.2.3.md

@@ -5,6 +5,7 @@ Release date: `2021-xx-xx`
 ## Core
 
 - [NXDRIVE-2533](https://jira.nuxeo.com/browse/NXDRIVE-2533): Create the new migrations engine
+- [NXDRIVE-2695](https://jira.nuxeo.com/browse/NXDRIVE-2695): Better handling of custom SSL certificates
 - [NXDRIVE-2704](https://jira.nuxeo.com/browse/NXDRIVE-2704): Force usage of idempotent requests for large files
 - [NXDRIVE-2708](https://jira.nuxeo.com/browse/NXDRIVE-2708): Handle removed folder when dequeuing local folder scan
 - [NXDRIVE-2709](https://jira.nuxeo.com/browse/NXDRIVE-2709): Handle unauthorized error when adding the top-level sync root
@@ -48,4 +49,10 @@ Release date: `2021-xx-xx`
 - Removed `FoldersOnly.children`
 - Renamed `ManagerDAO.schema_version` to `ManagerDAO.old_migrations_max_schema_version`
 - Added dao/migrations/migration_engine.py
+- Added utils.py::`concat_all_certificates()`
+- Added `cert_file` keyword argument to utils.py::`get_certificate_details()`
+- Added utils.py::`get_final_certificate()`
+- Added utils.py::`get_final_certificate_from_folder()`
 - Added utils.py::`is_large_file()`
+- Added utils.py::`is_valid_ssl_certificate()`
+- Added utils.py::`requests_verify()`

nxdrive/client/proxy.py

@@ -7,7 +7,7 @@
 
 from ..metrics.utils import user_agent
 from ..options import Options
-from ..utils import client_certificate, decrypt, encrypt, force_decode
+from ..utils import client_certificate, decrypt, encrypt, force_decode, requests_verify
 
 if TYPE_CHECKING:
     from ..dao.engine import EngineDAO  # noqa
@@ -184,14 +184,13 @@ def save_proxy(proxy: Proxy, dao: "EngineDAO", *, token: str = None) -> None:
 
 
 def validate_proxy(proxy: Proxy, url: str, /) -> bool:
-    verify = Options.ca_bundle or not Options.ssl_no_verify
     headers = {"User-Agent": user_agent()}
     try:
         with requests.get(
             url,
             headers=headers,
             proxies=proxy.settings(url=url),
-            verify=verify,
+            verify=requests_verify(Options.ca_bundle, Options.ssl_no_verify),
             cert=client_certificate(),
         ):
             return True

nxdrive/engine/engine.py

@@ -57,6 +57,7 @@
     force_decode,
     grouper,
     if_frozen,
+    requests_verify,
     safe_filename,
     safe_long_path,
     set_path_readonly,
@@ -956,11 +957,6 @@ def _load_configuration(self) -> None:
         self.force_ui = self.dao.get_config("force_ui")
         self.remote_user = self.dao.get_config("remote_user")
         self._remote_token = self._load_token()
-        self._ssl_verify = self.dao.get_bool("ssl_verify", default=True)
-        if Options.ssl_no_verify:
-            self._ssl_verify = False
-        self._ca_bundle = Options.ca_bundle or self.dao.get_config("ca_bundle")
-        self._client_certificate = client_certificate()
 
         if not self._remote_token:
             self.set_invalid_credentials(
@@ -1271,10 +1267,6 @@ def init_remote(self) -> Remote:
         # Used for FS synchronization operations
         args = (self.server_url, self.remote_user, self.manager.device_id, self.version)
 
-        verify = self._ca_bundle
-        if not (verify and self._ssl_verify):
-            verify = self._ssl_verify
-
         kwargs = {
             "password": self._remote_password,
             "timeout": self.timeout,
@@ -1283,8 +1275,8 @@ def init_remote(self) -> Remote:
             "upload_callback": self.suspend_client,
             "dao": self.dao,
             "proxy": self.manager.proxy,
-            "verify": verify,
-            "cert": self._client_certificate,
+            "verify": requests_verify(Options.ca_bundle, Options.ssl_no_verify),
+            "cert": client_certificate(),
         }
         return self.remote_cls(*args, **kwargs)
 
@@ -1318,13 +1310,6 @@ def bind(self, binder: Binder, /) -> None:
         if check_fs:
             self._setup_local_folder(check_fs)
 
-        # Persist the user preference about the SSL behavior.
-        # It can be tweaked via ca-bundle or ssl-no-verify options. But also
-        # from the ponctual bypass-ssl window prompted at the account creation.
-        self._ssl_verify = not Options.ssl_no_verify
-        self._ca_bundle = Options.ca_bundle
-        self._client_certificate = client_certificate()
-
         if check_credentials:
             self.remote = self.init_remote()
             if not self._remote_token:
@@ -1337,8 +1322,6 @@ def bind(self, binder: Binder, /) -> None:
         self.dao.update_config("server_url", self.server_url)
         self.dao.update_config("remote_user", self.remote_user)
         self._save_token(self._remote_token)
-        self.dao.store_bool("ssl_verify", self._ssl_verify)
-        self.dao.update_config("ca_bundle", self._ca_bundle)
 
         # Check for the root
         # If the top level state for the server binding doesn't exist,

nxdrive/gui/api.py

@@ -47,6 +47,7 @@
     get_date_from_sqlite,
     get_default_local_folder,
     normalized_path,
+    save_config,
     sizeof_fmt,
     test_url,
 )
@@ -519,6 +520,12 @@ def _get_ssl_error(self, server_url: str, /) -> str:
             if self.application.accept_unofficial_ssl_cert(hostname):
                 Options.ca_bundle = None
                 Options.ssl_no_verify = True
+                save_config(
+                    {
+                        "ca_bundle": Options.ca_bundle,
+                        "ssl_no_verify": Options.ssl_no_verify,
+                    }
+                )
                 return self._get_ssl_error(server_url)
         except MissingClientSSLCertificate as exc:
             log.warning(exc)

nxdrive/gui/application.py

@@ -79,6 +79,7 @@
     normalize_event_filename,
     normalized_path,
     parse_protocol_url,
+    requests_verify,
     short_name,
     sizeof_fmt,
     today_is_special,
@@ -1099,7 +1100,7 @@ def auth() -> None:
                 host=url,
                 auth=(user, pwd),
                 proxies=self.manager.proxy.settings(url=url),
-                verify=Options.ca_bundle or not Options.ssl_no_verify,
+                verify=requests_verify(Options.ca_bundle, Options.ssl_no_verify),
                 cert=client_certificate(),
             )
             try:

nxdrive/manager.py

@@ -61,6 +61,7 @@
     get_default_local_folder,
     if_frozen,
     normalized_path,
+    requests_verify,
     save_config,
 )
 
@@ -722,7 +723,7 @@ def get_server_login_type(
                 headers=headers,
                 proxies=self.proxy.settings(url=url),
                 timeout=STARTUP_PAGE_CONNECTION_TIMEOUT,
-                verify=Options.ca_bundle or not Options.ssl_no_verify,
+                verify=requests_verify(Options.ca_bundle, Options.ssl_no_verify),
                 cert=client_certificate(),
             ) as resp:
                 status = resp.status_code

nxdrive/utils.py

@@ -17,6 +17,7 @@
 from itertools import islice
 from logging import getLogger
 from pathlib import Path
+from tempfile import gettempdir
 from threading import get_native_id
 from typing import (
     TYPE_CHECKING,
@@ -30,6 +31,7 @@
     Union,
 )
 from urllib.parse import parse_qsl, urlparse, urlsplit, urlunsplit
+from uuid import uuid4
 
 from nuxeo.utils import get_digest_algorithm, get_digest_hash
 
@@ -594,6 +596,12 @@ def retrieve_ssl_certificate(hostname: str, /, *, port: int = 443) -> str:
             return ssl.DER_cert_to_PEM_cert(cert_data)
 
 
+def is_valid_ssl_certificate(file: Path) -> bool:
+    """Check weither the given *file* is a valid SSL certificate."""
+    details = get_certificate_details(cert_file=file)
+    return bool(details["subject"])
+
+
 def client_certificate() -> Optional[Tuple[str, str]]:
     """
     Fetch the paths to the certification file and it's key from the option.
@@ -607,38 +615,135 @@ def client_certificate() -> Optional[Tuple[str, str]]:
 
 @lru_cache(maxsize=4)
 def get_certificate_details(
-    *, hostname: str = "", cert_data: str = ""
+    *, hostname: str = "", cert_data: str = "", cert_file: Path = None
 ) -> Dict[str, Any]:
     """
     Get SSL certificate details from a given certificate content or hostname.
 
     Note: This function uses a undocumented method of the _ssl module.
           It is continuously tested in our CI to ensure it still
           available after any Python upgrade.
-          Certified working as of Python 3.8.6.
+          Certified working as of Python 3.9.5.
     """
 
     import ssl
 
     defaults = deepcopy(DEFAULTS_CERT_DETAILS)
-    cert_file = Path("c.crt")
 
-    try:
-        certificate = cert_data or retrieve_ssl_certificate(hostname)
-        cert_file.write_text(certificate, encoding="utf-8")
+    if cert_file:
+        certificate = cert_file
+        cleanup = False
+    else:
         try:
-            # Taken from https://stackoverflow.com/a/50072461/1117028
-            # pylint: disable=protected-access
-            details = ssl._ssl._test_decode_cert(cert_file)  # type: ignore
-            defaults.update(details)
-        finally:
-            cert_file.unlink()
+            data = cert_data or retrieve_ssl_certificate(hostname)
+        except Exception:
+            log.warning("Error while retrieving the SSL certificate", exc_info=True)
+            return defaults
+
+        certificate = Path(gettempdir()) / f"cert-{uuid4()}.crt"
+        certificate.write_text(data, encoding="utf-8")
+        cleanup = True
+
+    try:
+        # Taken from https://stackoverflow.com/a/50072461/1117028
+        # pylint: disable=protected-access
+        details = ssl._ssl._test_decode_cert(certificate)  # type: ignore
+        defaults.update(details)
     except Exception:
-        log.warning("Error while retrieving the SSL certificate", exc_info=True)
+        log.warning("Error while decoding the SSL certificate", exc_info=True)
+    finally:
+        if cleanup:
+            certificate.unlink()
 
     return defaults
 
 
+def concat_all_certificates(files: List[Path]) -> Optional[Path]:
+    """Craft a all-in-one certificate with ones from cacert and custom ones."""
+    from hashlib import md5
+
+    import certifi
+
+    cert_files = [Path(certifi.where())]
+    certificates = cert_files[0].read_bytes()
+    for count, file in enumerate(sorted(files)):
+        if not file.is_file():
+            continue
+
+        if not is_valid_ssl_certificate(file):
+            log.info(f"Skipped invalid certificate {str(file)!r}")
+            continue
+
+        certificate = file.read_bytes()
+
+        if len(certificates) + len(certificate) > 5 * 1024 * 1024:  # 5 MiB
+            # Way too big for simple text files, stop now
+            log.warning(
+                "No all certificate where processed due to the maximum file size (5 MiB). "
+                f"{len(cert_files) - 1} files were processed, {len(files) - count} were left."
+            )
+            break
+
+        certificates += certificate
+        cert_files.append(file)
+
+    if len(cert_files) == 1:
+        log.warning("No valid certificate found.")
+        return None
+
+    name = md5(certificates).hexdigest()
+    folder = Options.nxdrive_home
+    final_file: Path = folder / f"ndrive_{name}.pem"
+
+    if not final_file.is_file():
+        # Either the certificate does not exist yet, either it is obsolete.
+        # Let's clean-up all of them.
+        for obsolete_file in folder.glob("ndrive_*.pem"):
+            log.info(f"Removed obsolete certificate {str(obsolete_file)!r}")
+            obsolete_file.unlink()
+
+        log.info(f"Saved the final certificate to {str(final_file)!r}, including:")
+        for cert_file in cert_files:
+            log.info(f" >>> {str(cert_file)!r}")
+        final_file.write_bytes(certificates)
+    else:
+        log.info(f"Will use the final certificate from {str(final_file)!r}")
+
+    return final_file
+
+
+def get_final_certificate(file: Path) -> Optional[Path]:
+    """Return the all-in-one certificate if the provided *file* is a certificate
+    else *file* without modifications.
+
+    If the *file* is already a all-in-one certificate, then return it without changes.
+    It will ease testing customer's files.
+    """
+    if file.stem.startswith("ndrive_") and file.suffix == ".pem":
+        return file
+    return concat_all_certificates([file])
+
+
+def get_final_certificate_from_folder(folder: Path) -> Optional[Path]:
+    """Return the all-in-one certificate if the provided *folder* contains
+    valid certificate(s) else *folder* without modifications.
+    """
+    return concat_all_certificates(list(folder.glob("*")))
+
+
+def requests_verify(ca_bundle: Optional[Path], ssl_no_verify: bool) -> Any:
+    """Return the appropriate value for the *verify* keyword argument of *requests* calls."""
+    if ca_bundle:
+        path = ca_bundle
+        if path.is_file():
+            path = get_final_certificate(path) or path
+        elif path.is_dir():
+            path = get_final_certificate_from_folder(path) or path
+        return path
+
+    return not ssl_no_verify
+
+
 def _cryptor(key: bytes, iv: bytes) -> "Cipher":
     """Instantiate a new AES (de|en)cryptor."""
     from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
@@ -1104,7 +1209,7 @@ def test_url(
 
     kwargs: Dict[str, Any] = {
         "timeout": timeout,
-        "verify": Options.ca_bundle or not Options.ssl_no_verify,
+        "verify": requests_verify(Options.ca_bundle, Options.ssl_no_verify),
         "cert": client_certificate(),
         "headers": {"User-Agent": user_agent()},
     }