From 9533c269adaa6aa620767783829badd1c5f8579b Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sun, 29 May 2022 21:39:46 +0200 Subject: [PATCH] config: allow bool values for repo cert This change allows certificates..cert configuration to accept boolean values in addition to certificate paths. This allows for repositories to skip TLS certificate validation for cases where self-signed certificats are used by package sources. In addition to the above, the certificate configuration handling has now been delegated to a dedicated dataclass. Co-authored-by: Celeborn2BeAlive Co-authored-by: Maayan Bar --- docs/configuration.md | 5 ++- docs/repositories.md | 15 +++++++ src/poetry/console/commands/config.py | 28 +++++++----- src/poetry/installation/pip_installer.py | 14 ++++-- src/poetry/publishing/publisher.py | 11 +++-- src/poetry/publishing/uploader.py | 8 ++-- src/poetry/repositories/http.py | 15 ++----- src/poetry/utils/authenticator.py | 57 +++++++++++++++++------- src/poetry/utils/helpers.py | 17 ------- tests/console/commands/test_config.py | 14 +++++- tests/installation/test_pip_installer.py | 37 +++++++++++++-- tests/publishing/test_publisher.py | 10 ++--- tests/utils/test_authenticator.py | 50 +++++++++++++++++++++ tests/utils/test_helpers.py | 23 ---------- 14 files changed, 197 insertions(+), 107 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 4ed52b91934..30f8ff55e62 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -315,12 +315,15 @@ for more information. ### `certificates..cert`: -**Type**: string +**Type**: string | bool Set custom certificate authority for repository ``. See [Repositories - Configuring credentials - Custom certificate authority]({{< relref "repositories#custom-certificate-authority-and-mutual-tls-authentication" >}}) for more information. +This configuration can be set to `false`, if TLS certificate verification should be skipped for this +repository. + ### `certificates..client-cert`: **Type**: string diff --git a/docs/repositories.md b/docs/repositories.md index 490d78bf785..d356d661953 100644 --- a/docs/repositories.md +++ b/docs/repositories.md @@ -384,6 +384,21 @@ poetry config certificates.foo.cert /path/to/ca.pem poetry config certificates.foo.client-cert /path/to/client.pem ``` +{{% note %}} +The value of `certificates..cert` can be set to `false` if certificate verification is +required to be skipped. This is useful for cases where a package source with self-signed certificates +are used. + +```bash +poetry config certificates.foo.cert false +``` +{{% /note %}} + +{{% warning %} +Disabling certificate verification is not recommended as it is does not conform to security +best practices. +{{% /warning %} + ## Caches Poetry employs multiple caches for package sources in order to improve user experience and avoid duplicate network diff --git a/src/poetry/console/commands/config.py b/src/poetry/console/commands/config.py index 11ba60d25a3..eacb058573b 100644 --- a/src/poetry/console/commands/config.py +++ b/src/poetry/console/commands/config.py @@ -3,6 +3,7 @@ import json import re +from pathlib import Path from typing import TYPE_CHECKING from typing import Any from typing import cast @@ -11,7 +12,11 @@ from cleo.helpers import option from poetry.config.config import PackageFilterPolicy +from poetry.config.config import boolean_normalizer +from poetry.config.config import boolean_validator +from poetry.config.config import int_normalizer from poetry.console.commands.command import Command +from poetry.locations import DEFAULT_CACHE_DIR if TYPE_CHECKING: @@ -48,13 +53,6 @@ class ConfigCommand(Command): @property def unique_config_values(self) -> dict[str, tuple[Any, Any, Any]]: - from pathlib import Path - - from poetry.config.config import boolean_normalizer - from poetry.config.config import boolean_validator - from poetry.config.config import int_normalizer - from poetry.locations import DEFAULT_CACHE_DIR - unique_config_values = { "cache-dir": ( str, @@ -275,20 +273,26 @@ def handle(self) -> int | None: return 0 # handle certs - m = re.match( - r"(?:certificates)\.([^.]+)\.(cert|client-cert)", self.argument("key") - ) + m = re.match(r"certificates\.([^.]+)\.(cert|client-cert)", self.argument("key")) if m: + repository = m.group(1) + key = m.group(2) + if self.option("unset"): config.auth_config_source.remove_property( - f"certificates.{m.group(1)}.{m.group(2)}" + f"certificates.{repository}.{key}" ) return 0 if len(values) == 1: + new_value: str | bool = values[0] + + if key == "cert" and boolean_validator(values[0]): + new_value = boolean_normalizer(values[0]) + config.auth_config_source.add_property( - f"certificates.{m.group(1)}.{m.group(2)}", values[0] + f"certificates.{repository}.{key}", new_value ) else: raise ValueError("You must pass exactly 1 value") diff --git a/src/poetry/installation/pip_installer.py b/src/poetry/installation/pip_installer.py index 60acb85b86e..536f6cdce04 100644 --- a/src/poetry/installation/pip_installer.py +++ b/src/poetry/installation/pip_installer.py @@ -63,11 +63,17 @@ def install(self, package: Package, update: bool = False) -> None: args += ["--trusted-host", parsed.hostname] if isinstance(repository, HTTPRepository): - if repository.cert: - args += ["--cert", str(repository.cert)] + certificates = repository.certificates - if repository.client_cert: - args += ["--client-cert", str(repository.client_cert)] + if certificates.cert: + args += ["--cert", str(certificates.cert)] + + if parsed.scheme == "https" and not certificates.verify: + assert parsed.hostname is not None + args += ["--trusted-host", parsed.hostname] + + if certificates.client_cert: + args += ["--client-cert", str(certificates.client_cert)] index_url = repository.authenticated_url diff --git a/src/poetry/publishing/publisher.py b/src/poetry/publishing/publisher.py index 95f79afdab1..dd31e5c00e3 100644 --- a/src/poetry/publishing/publisher.py +++ b/src/poetry/publishing/publisher.py @@ -6,8 +6,6 @@ from poetry.publishing.uploader import Uploader from poetry.utils.authenticator import Authenticator -from poetry.utils.helpers import get_cert -from poetry.utils.helpers import get_client_cert if TYPE_CHECKING: @@ -72,9 +70,10 @@ def publish( username = auth.username password = auth.password - resolved_client_cert = client_cert or get_client_cert( - self._poetry.config, repository_name - ) + certificates = self._authenticator.get_certs_for_repository(repository_name) + resolved_cert = cert or certificates.cert or certificates.verify + resolved_client_cert = client_cert or certificates.client_cert + # Requesting missing credentials but only if there is not a client cert defined. if not resolved_client_cert and hasattr(self._io, "ask"): if username is None: @@ -96,7 +95,7 @@ def publish( self._uploader.upload( url, - cert=cert or get_cert(self._poetry.config, repository_name), + cert=resolved_cert, client_cert=resolved_client_cert, dry_run=dry_run, skip_existing=skip_existing, diff --git a/src/poetry/publishing/uploader.py b/src/poetry/publishing/uploader.py index fc60648a5aa..99557561c3f 100644 --- a/src/poetry/publishing/uploader.py +++ b/src/poetry/publishing/uploader.py @@ -3,6 +3,7 @@ import hashlib import io +from pathlib import Path from typing import TYPE_CHECKING from typing import Any @@ -25,8 +26,6 @@ if TYPE_CHECKING: - from pathlib import Path - from cleo.io.null_io import NullIO from poetry.poetry import Poetry @@ -114,15 +113,14 @@ def get_auth(self) -> tuple[str, str] | None: def upload( self, url: str, - cert: Path | None = None, + cert: Path | bool = True, client_cert: Path | None = None, dry_run: bool = False, skip_existing: bool = False, ) -> None: session = self.make_session() - if cert: - session.verify = str(cert) + session.verify = str(cert) if isinstance(cert, Path) else cert if client_cert: session.cert = str(client_cert) diff --git a/src/poetry/repositories/http.py b/src/poetry/repositories/http.py index c453b0cdd7d..8d413b6c0e9 100644 --- a/src/poetry/repositories/http.py +++ b/src/poetry/repositories/http.py @@ -31,6 +31,7 @@ if TYPE_CHECKING: from poetry.config.config import Config from poetry.inspection.info import PackageInfo + from poetry.utils.authenticator import RepositoryCertificateConfig class HTTPRepository(CachedRepository, ABC): @@ -59,18 +60,8 @@ def url(self) -> str: return self._url @property - def cert(self) -> Path | None: - cert = self._authenticator.get_certs_for_url(self.url).get("verify") - if cert: - return Path(cert) - return None - - @property - def client_cert(self) -> Path | None: - cert = self._authenticator.get_certs_for_url(self.url).get("cert") - if cert: - return Path(cert) - return None + def certificates(self) -> RepositoryCertificateConfig: + return self._authenticator.get_certs_for_url(self.url) @property def authenticated_url(self) -> str: diff --git a/src/poetry/utils/authenticator.py b/src/poetry/utils/authenticator.py index ba153111167..81ab081b934 100644 --- a/src/poetry/utils/authenticator.py +++ b/src/poetry/utils/authenticator.py @@ -8,6 +8,7 @@ import urllib.parse from os.path import commonprefix +from pathlib import Path from typing import TYPE_CHECKING from typing import Any @@ -20,21 +21,42 @@ from poetry.config.config import Config from poetry.exceptions import PoetryException -from poetry.utils.helpers import get_cert -from poetry.utils.helpers import get_client_cert from poetry.utils.password_manager import HTTPAuthCredential from poetry.utils.password_manager import PasswordManager if TYPE_CHECKING: - from pathlib import Path - from cleo.io.io import IO logger = logging.getLogger(__name__) +@dataclasses.dataclass(frozen=True) +class RepositoryCertificateConfig: + cert: Path | None = dataclasses.field(default=None) + client_cert: Path | None = dataclasses.field(default=None) + verify: bool = dataclasses.field(default=True) + + @classmethod + def create( + cls, repository: str, config: Config | None + ) -> RepositoryCertificateConfig: + config = config if config else Config.create() + + verify: str | bool = config.get( + f"certificates.{repository}.verify", + config.get(f"certificates.{repository}.cert", True), + ) + client_cert: str = config.get(f"certificates.{repository}.client-cert") + + return cls( + cert=Path(verify) if isinstance(verify, str) else None, + client_cert=Path(client_cert) if client_cert else None, + verify=verify if isinstance(verify, bool) else True, + ) + + @dataclasses.dataclass class AuthenticatorRepositoryConfig: name: str @@ -47,11 +69,8 @@ def __post_init__(self) -> None: self.netloc = parsed_url.netloc self.path = parsed_url.path - def certs(self, config: Config) -> dict[str, Path | None]: - return { - "cert": get_client_cert(config, self.name), - "verify": get_cert(config, self.name), - } + def certs(self, config: Config) -> RepositoryCertificateConfig: + return RepositoryCertificateConfig.create(self.name, config) @property def http_credential_keys(self) -> list[str]: @@ -91,7 +110,7 @@ def __init__( self._io = io self._sessions_for_netloc: dict[str, requests.Session] = {} self._credentials: dict[str, HTTPAuthCredential] = {} - self._certs: dict[str, dict[str, Path | None]] = {} + self._certs: dict[str, RepositoryCertificateConfig] = {} self._configured_repositories: dict[ str, AuthenticatorRepositoryConfig ] | None = None @@ -186,14 +205,13 @@ def request( stream = kwargs.get("stream") certs = self.get_certs_for_url(url) - verify = kwargs.get("verify") or certs.get("verify") - cert = kwargs.get("cert") or certs.get("cert") + verify = kwargs.get("verify") or certs.cert or certs.verify + cert = kwargs.get("cert") or certs.client_cert if cert is not None: cert = str(cert) - if verify is not None: - verify = str(verify) + verify = str(verify) if isinstance(verify, Path) else verify settings = session.merge_environment_settings( # type: ignore[no-untyped-call] prepared_request.url, proxies, stream, verify, cert @@ -332,6 +350,11 @@ def get_http_auth( repository=repository, username=username ) + def get_certs_for_repository(self, name: str) -> RepositoryCertificateConfig: + if name.lower() == "pypi" or name not in self.configured_repositories: + return RepositoryCertificateConfig() + return self.configured_repositories[name].certs(self._config) + @property def configured_repositories(self) -> dict[str, AuthenticatorRepositoryConfig]: if self._configured_repositories is None: @@ -352,7 +375,7 @@ def add_repository(self, name: str, url: str) -> None: self.configured_repositories[name] = AuthenticatorRepositoryConfig(name, url) self.reset_credentials_cache() - def get_certs_for_url(self, url: str) -> dict[str, Path | None]: + def get_certs_for_url(self, url: str) -> RepositoryCertificateConfig: if url not in self._certs: self._certs[url] = self._get_certs_for_url(url) return self._certs[url] @@ -398,11 +421,11 @@ def _get_repository_config_for_url( return candidates[0] - def _get_certs_for_url(self, url: str) -> dict[str, Path | None]: + def _get_certs_for_url(self, url: str) -> RepositoryCertificateConfig: selected = self.get_repository_config_for_url(url) if selected: return selected.certs(config=self._config) - return {"cert": None, "verify": None} + return RepositoryCertificateConfig() _authenticator: Authenticator | None = None diff --git a/src/poetry/utils/helpers.py b/src/poetry/utils/helpers.py index cfc6c3b66b9..69743be2f52 100644 --- a/src/poetry/utils/helpers.py +++ b/src/poetry/utils/helpers.py @@ -18,7 +18,6 @@ from poetry.core.packages.package import Package from requests import Session - from poetry.config.config import Config from poetry.utils.authenticator import Authenticator @@ -33,22 +32,6 @@ def module_name(name: str) -> str: return canonicalize_name(name).replace(".", "_").replace("-", "_") -def get_cert(config: Config, repository_name: str) -> Path | None: - cert = config.get(f"certificates.{repository_name}.cert") - if cert: - return Path(cert) - else: - return None - - -def get_client_cert(config: Config, repository_name: str) -> Path | None: - client_cert = config.get(f"certificates.{repository_name}.client-cert") - if client_cert: - return Path(client_cert) - else: - return None - - def _on_rm_error(func: Callable[[str], None], path: str, exc_info: Exception) -> None: if not os.path.exists(path): return diff --git a/tests/console/commands/test_config.py b/tests/console/commands/test_config.py index 2fbca6f1062..052b726c2fd 100644 --- a/tests/console/commands/test_config.py +++ b/tests/console/commands/test_config.py @@ -175,16 +175,26 @@ def test_set_client_cert( ) +@pytest.mark.parametrize( + ("value", "result"), + [ + ("path/to/ca.pem", "path/to/ca.pem"), + ("true", True), + ("false", False), + ], +) def test_set_cert( tester: CommandTester, auth_config_source: DictConfigSource, mocker: MockerFixture, + value: str, + result: str | bool, ): mocker.spy(ConfigSource, "__init__") - tester.execute("certificates.foo.cert path/to/ca.pem") + tester.execute(f"certificates.foo.cert {value}") - assert auth_config_source.config["certificates"]["foo"]["cert"] == "path/to/ca.pem" + assert auth_config_source.config["certificates"]["foo"]["cert"] == result def test_config_installer_parallel( diff --git a/tests/installation/test_pip_installer.py b/tests/installation/test_pip_installer.py index 797e7874eba..ec824048a2f 100644 --- a/tests/installation/test_pip_installer.py +++ b/tests/installation/test_pip_installer.py @@ -14,6 +14,7 @@ from poetry.installation.pip_installer import PipInstaller from poetry.repositories.legacy_repository import LegacyRepository from poetry.repositories.pool import Pool +from poetry.utils.authenticator import RepositoryCertificateConfig from poetry.utils.env import NullEnv @@ -21,6 +22,7 @@ from pytest_mock import MockerFixture from poetry.utils.env import VirtualEnv + from tests.conftest import Config @pytest.fixture @@ -120,15 +122,15 @@ def test_install_with_non_pypi_default_repository(pool: Pool, installer: PipInst @pytest.mark.parametrize( ("key", "option"), [ - ("cert", "client-cert"), - ("verify", "cert"), + ("client_cert", "client-cert"), + ("cert", "cert"), ], ) def test_install_with_certs(mocker: MockerFixture, key: str, option: str): client_path = "path/to/client.pem" mocker.patch( "poetry.utils.authenticator.Authenticator.get_certs_for_url", - return_value={key: client_path}, + return_value=RepositoryCertificateConfig(**{key: client_path}), ) default = LegacyRepository("default", "https://foo.bar") @@ -204,3 +206,32 @@ def copy_only(source: Path, dest: Path) -> None: # any command in the virtual environment should trigger the error message output = tmp_venv.run("python", "-m", "site") assert not re.match(rf"Error processing line 1 of .*{pth_file}", output) + + +def test_install_with_trusted_host(config: Config): + config.merge({"certificates": {"default": {"cert": False}}}) + + default = LegacyRepository("default", "https://foo.bar") + pool = Pool() + pool.add_repository(default, default=True) + + null_env = NullEnv() + + installer = PipInstaller(null_env, NullIO(), pool) + + foo = Package( + "foo", + "0.0.0", + source_type="legacy", + source_reference=default.name, + source_url=default.url, + ) + + installer.install(foo) + + assert len(null_env.executed) == 1 + cmd = null_env.executed[0] + assert "--trusted-host" in cmd + cert_index = cmd.index("--trusted-host") + # Need to do the str(Path()) bit because Windows paths get modified by Path + assert cmd[cert_index + 1] == "foo.bar" diff --git a/tests/publishing/test_publisher.py b/tests/publishing/test_publisher.py index 2b3cf01c627..afc992f828a 100644 --- a/tests/publishing/test_publisher.py +++ b/tests/publishing/test_publisher.py @@ -38,7 +38,7 @@ def test_publish_publishes_to_pypi_by_default( assert [("foo", "bar")] == uploader_auth.call_args assert [ ("https://upload.pypi.org/legacy/",), - {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False}, + {"cert": True, "client_cert": None, "dry_run": False, "skip_existing": False}, ] == uploader_upload.call_args @@ -70,7 +70,7 @@ def test_publish_can_publish_to_given_repository( assert [("foo", "bar")] == uploader_auth.call_args assert [ ("http://foo.bar",), - {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False}, + {"cert": True, "client_cert": None, "dry_run": False, "skip_existing": False}, ] == uploader_upload.call_args assert "Publishing my-package (1.2.3) to foo" in io.fetch_output() @@ -104,7 +104,7 @@ def test_publish_uses_token_if_it_exists( assert [("__token__", "my-token")] == uploader_auth.call_args assert [ ("https://upload.pypi.org/legacy/",), - {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False}, + {"cert": True, "client_cert": None, "dry_run": False, "skip_existing": False}, ] == uploader_upload.call_args @@ -159,7 +159,7 @@ def test_publish_uses_client_cert( assert [ ("https://foo.bar",), { - "cert": None, + "cert": True, "client_cert": Path(client_cert), "dry_run": False, "skip_existing": False, @@ -186,5 +186,5 @@ def test_publish_read_from_environment_variable( assert [("bar", "baz")] == uploader_auth.call_args assert [ ("https://foo.bar",), - {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False}, + {"cert": True, "client_cert": None, "dry_run": False, "skip_existing": False}, ] == uploader_upload.call_args diff --git a/tests/utils/test_authenticator.py b/tests/utils/test_authenticator.py index 2467959b6f5..aaf560b2064 100644 --- a/tests/utils/test_authenticator.py +++ b/tests/utils/test_authenticator.py @@ -16,6 +16,7 @@ from cleo.io.null_io import NullIO from poetry.utils.authenticator import Authenticator +from poetry.utils.authenticator import RepositoryCertificateConfig if TYPE_CHECKING: @@ -599,3 +600,52 @@ def test_authenticator_git_repositories( three = authenticator.get_credentials_for_git_url("https://foo.bar/org/three.git") assert not three.username assert not three.password + + +@pytest.mark.parametrize( + ("ca_cert", "client_cert", "result"), + [ + (None, None, RepositoryCertificateConfig()), + ( + "path/to/ca.pem", + "path/to/client.pem", + RepositoryCertificateConfig( + Path("path/to/ca.pem"), Path("path/to/client.pem") + ), + ), + ( + None, + "path/to/client.pem", + RepositoryCertificateConfig(None, Path("path/to/client.pem")), + ), + ( + "path/to/ca.pem", + None, + RepositoryCertificateConfig(Path("path/to/ca.pem"), None), + ), + (True, None, RepositoryCertificateConfig()), + (False, None, RepositoryCertificateConfig(verify=False)), + ( + False, + "path/to/client.pem", + RepositoryCertificateConfig(None, Path("path/to/client.pem"), verify=False), + ), + ], +) +def test_repository_certificate_configuration_create( + ca_cert: str | bool | None, + client_cert: str | None, + result: RepositoryCertificateConfig, + config: Config, +) -> None: + cert_config = {} + + if ca_cert is not None: + cert_config["cert"] = ca_cert + + if client_cert is not None: + cert_config["client-cert"] = client_cert + + config.merge({"certificates": {"foo": cert_config}}) + + assert RepositoryCertificateConfig.create("foo", config) == result diff --git a/tests/utils/test_helpers.py b/tests/utils/test_helpers.py index fa452749f0b..7be34c336d5 100644 --- a/tests/utils/test_helpers.py +++ b/tests/utils/test_helpers.py @@ -1,19 +1,10 @@ from __future__ import annotations -from pathlib import Path -from typing import TYPE_CHECKING - import pytest from poetry.core.utils.helpers import parse_requires from poetry.utils.helpers import canonicalize_name -from poetry.utils.helpers import get_cert -from poetry.utils.helpers import get_client_cert - - -if TYPE_CHECKING: - from tests.conftest import Config def test_parse_requires(): @@ -72,20 +63,6 @@ def test_parse_requires(): assert result == expected -def test_get_cert(config: Config): - ca_cert = "path/to/ca.pem" - config.merge({"certificates": {"foo": {"cert": ca_cert}}}) - - assert get_cert(config, "foo") == Path(ca_cert) - - -def test_get_client_cert(config: Config): - client_cert = "path/to/client.pem" - config.merge({"certificates": {"foo": {"client-cert": client_cert}}}) - - assert get_client_cert(config, "foo") == Path(client_cert) - - test_canonicalize_name_cases = [ ("flask", "flask"), ("Flask", "flask"),