Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support OS truststore for the TLS certificate verification #9685

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/repositories.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,11 @@ poetry config -- http-basic.pypi myUsername -myPasswordStartingWithDash

## Certificates

### OS Truststore

Poetry access the system truststore by default and retrieve the root certificates appropriately on Linux, Windows, and MacOS.
In addition to your OS root certificates, we still load the authorities provided by `certifi` as before.

### Custom certificate authority and mutual TLS authentication

Poetry supports repositories that are secured by a custom certificate authority as well as those that require
Expand Down
89 changes: 88 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ tomlkit = ">=0.11.4,<1.0.0"
trove-classifiers = ">=2022.5.19"
virtualenv = "^20.26.6"
xattr = { version = "^1.0.0", markers = "sys_platform == 'darwin'" }
wassima = "^1.1.3"

[tool.poetry.group.dev.dependencies]
pre-commit = ">=2.10"
Expand Down
4 changes: 4 additions & 0 deletions src/poetry/publishing/uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from poetry.publishing.hash_manager import HashManager
from poetry.utils.constants import REQUESTS_TIMEOUT
from poetry.utils.patterns import wheel_file_re
from poetry.utils.truststore import WithTrustStoreAdapter


if TYPE_CHECKING:
Expand Down Expand Up @@ -70,6 +71,9 @@ def auth(self, username: str | None, password: str | None) -> None:

def make_session(self) -> requests.Session:
session = requests.Session()
adapter = WithTrustStoreAdapter()
session.mount("http://", adapter)
session.mount("https://", adapter)
auth = self.get_auth()
if auth is not None:
session.auth = auth
Expand Down
4 changes: 2 additions & 2 deletions src/poetry/utils/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import requests.auth
import requests.exceptions

from cachecontrol import CacheControlAdapter
from cachecontrol.caches import FileCache
from requests_toolbelt import user_agent

Expand All @@ -28,6 +27,7 @@
from poetry.utils.constants import STATUS_FORCELIST
from poetry.utils.password_manager import HTTPAuthCredential
from poetry.utils.password_manager import PasswordManager
from poetry.utils.truststore import CacheControlWithTrustStoreAdapter


if TYPE_CHECKING:
Expand Down Expand Up @@ -137,7 +137,7 @@ def create_session(self) -> requests.Session:
if self._cache_control is None:
return session

adapter = CacheControlAdapter(
adapter = CacheControlWithTrustStoreAdapter(
cache=self._cache_control,
pool_maxsize=self._pool_size,
)
Expand Down
70 changes: 70 additions & 0 deletions src/poetry/utils/truststore.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from __future__ import annotations

import typing

from cachecontrol import CacheControlAdapter
from requests.adapters import HTTPAdapter
from wassima import RUSTLS_LOADED
from wassima import generate_ca_bundle


if typing.TYPE_CHECKING:
from urllib3 import HTTPConnectionPool


DEFAULT_CA_BUNDLE: str = generate_ca_bundle()


class WithTrustStoreAdapter(HTTPAdapter):
"""
Inject the OS truststore in Requests.
Certifi is still loaded in addition to the OS truststore for (strict) backward compatibility purposes.
See https://github.com/jawah/wassima for more details.
"""

def cert_verify(
self,
conn: HTTPConnectionPool,
url: str,
verify: bool | str,
cert: str | tuple[str, str] | None,
) -> None:
#: only apply truststore cert if "verify" is set with default value "True".
#: RUSTLS_LOADED means that "wassima" is not the py3 none wheel and does not fallback on "certifi"
#: if "RUSTLS_LOADED" is False then "wassima" just return "certifi" bundle instead.
if (
RUSTLS_LOADED
and url.lower().startswith("https")
and verify is True
and hasattr(conn, "ca_cert_data")
):
# url starting with https already mean that conn is a HTTPSConnectionPool
# the hasattr is to make mypy happy.
conn.ca_cert_data = DEFAULT_CA_BUNDLE

# still apply upstream logic as before
super().cert_verify(conn, url, verify, cert) # type: ignore[no-untyped-call]


class CacheControlWithTrustStoreAdapter(CacheControlAdapter):
"""
Same as WithTrustStoreAdapter but with CacheControlAdapter as its parent
class.
"""

def cert_verify(
self,
conn: HTTPConnectionPool,
url: str,
verify: bool | str,
cert: str | tuple[str, str] | None,
) -> None:
if (
RUSTLS_LOADED
and url.lower().startswith("https")
and verify is True
and hasattr(conn, "ca_cert_data")
):
conn.ca_cert_data = DEFAULT_CA_BUNDLE

super().cert_verify(conn, url, verify, cert) # type: ignore[no-untyped-call]
17 changes: 17 additions & 0 deletions src/poetry/vcs/git/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
from urllib.parse import urlparse
from urllib.parse import urlunparse

import certifi
import wassima

from dulwich import porcelain
from dulwich.client import HTTPUnauthorized
from dulwich.client import default_urllib3_manager
from dulwich.client import get_transport_and_path
from dulwich.config import ConfigFile
from dulwich.config import parse_submodules
Expand Down Expand Up @@ -204,6 +208,19 @@ def _fetch_remote_refs(cls, url: str, local: Repo) -> FetchPackResult:
kwargs["password"] = credentials.password

config = local.get_config_stack()

# we want to inject the system root CA if the transport is urllib3 (aka. http)
# so that our users won't need the "system" git.
# we combine the system trust store with the certifi bundle.
pool_manager = default_urllib3_manager(
config,
ca_certs=certifi.where(),
ca_cert_data=wassima.generate_ca_bundle()
if wassima.RUSTLS_LOADED
else None,
)
kwargs["pool_manager"] = pool_manager # type: ignore[assignment]

client, path = get_transport_and_path(url, config=config, **kwargs)

with local:
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/test_utils_vcs_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from typing import TYPE_CHECKING
from typing import Iterator
from typing import TypedDict
from unittest.mock import ANY
from urllib.parse import urlparse
from urllib.parse import urlunparse

Expand Down Expand Up @@ -357,6 +358,7 @@ def test_configured_repository_http_auth(
config=dummy_git_config,
username=GIT_USERNAME,
password=GIT_PASSWORD,
pool_manager=ANY,
)
spy_get_transport_and_path.assert_called_once()

Expand All @@ -383,6 +385,7 @@ def test_username_password_parameter_is_not_passed_to_dulwich(
spy_get_transport_and_path.assert_called_with(
location=source_url,
config=dummy_git_config,
pool_manager=ANY,
)
spy_get_transport_and_path.assert_called_once()

Expand Down
Loading