Add support for `--trusted-host` by charliermarsh · Pull Request #6591 · astral-sh/uv

charliermarsh · 2024-08-24T21:51:11Z

Summary

This PR revives #4944, which I think was a good start towards adding --trusted-host. Last night, I tried to add --trusted-host with a custom verifier, but we had to vendor a lot of reqwest code and I eventually hit some private APIs. I'm not confident that I can implement it correctly with that mechanism, and since this is security, correctness is the priority.

So, instead, we now use two clients and multiplex between them.

Closes #1339.

Test Plan

Created self-signed certificate, and ran python3 -m http.server --bind 127.0.0.1 4443 --directory . --certfile cert.pem --keyfile key.pem from the packse index directory.

Verified that cargo run pip install transitive-yanked-and-unyanked-dependency-a-0abad3b6 --index-url https://127.0.0.1:8443/simple-html failed with:

error: Request failed after 3 retries
  Caused by: error sending request for url (https://127.0.0.1:8443/simple-html/transitive-yanked-and-unyanked-dependency-a-0abad3b6/)
  Caused by: client error (Connect)
  Caused by: invalid peer certificate: Other(OtherError(CaUsedAsEndEntity))

Verified that cargo run pip install transitive-yanked-and-unyanked-dependency-a-0abad3b6 --index-url 'https://127.0.0.1:8443/simple-html' --trusted-host '127.0.0.1:8443' failed with the expected error (invalid resolution) and made valid requests.

Verified that cargo run pip install transitive-yanked-and-unyanked-dependency-a-0abad3b6 --index-url 'https://127.0.0.1:8443/simple-html' --trusted-host '127.0.0.2' -n also failed.

crates/uv-settings/src/settings.rs

charliermarsh · 2024-08-24T23:16:24Z

crates/uv-client/src/base_client.rs

+                }
+            })
+        {
+            &self.dangerous_client


The only way to access this client is via for_host. It's not exposed anywhere else.

crates/uv-cli/src/lib.rs

zanieb · 2024-08-25T00:24:12Z

Also might be helpful for #5726

gaby · 2024-08-25T00:30:57Z

What happens if --trusted-host is supplied multiple times, is that taken into account? Or do the later override the first one?

Edit: I see the code allows a list, which if it's like pip it would be space-separated.

crates/uv-cli/src/lib.rs

zanieb · 2024-08-25T14:22:16Z

crates/uv-configuration/src/trusted_host.rs

+            super::TrustedHost::HostPort("example.com".to_string(), 8080)
+        );
+
+        assert_eq!(


Can you add a test case for something with a path? e.g. https://example.com/hello/world?

zanieb · 2024-08-25T14:24:12Z

crates/uv-configuration/src/trusted_host.rs

+/// A trusted host, which could be a host or a host-port pair.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub enum TrustedHost {
+    Host(String),
+    HostPort(String, u16),
+}


I think you can / should use Realm in place of this (crates/uv-auth/src/realm.rs) — I think you can just move the from_str implementation over. I'm not sure if that adds complexity to the setting schema part — but it seems like we could just have Realm in the setting schema.

"should" because it's our canonical way to do this and there's From<&Url> for Realm.

Doesn't that require a scheme? These values don't require a scheme.

It does yeah, hm. But if someone provides a scheme here (which is allowed per the parser), shouldn't that match be enforced?

The scheme could become optional on Realm, I'm not sure if that causes problems. I don't feel strongly if it seems like a pain. There's not a lot of complexity in the matching anymore (Realm got simplified at some point, relying on some behaviors from reqwest)

We might need to test what happens if you set HTTPS_PROXY or ALL_PROXY to an http:// URL?

@zanieb That's typically what I use when using something like Squid-Proxy + Pypi. I set both HTTP_PROXY and HTTPS_PROXY to "http://squid-hostname`, and let Squid handle the request/redirect/etc.

We should allow http://. It's normal practice to run internal pypi mirror that runs on http. That's the whole point of trusting a host.

This is a reason as to why I'm suggesting a rename away from --trusted-host as it's not about trusting a host, rather skipping TLS verification 😅

@zanieb - I opted for now to keep TrustedHost, because the requirements are different than Realm... Realm requires a scheme, TrustedHost requires a host (Realm does not), etc. I was also hesitant to make uv-auth a dependency everywhere, and add Schemars as a dep to that crate.

pradyunsg · 2024-08-26T11:09:12Z

You’re receiving notifications because you were mentioned.

Not sure why GitHub thinks I have a mention here... this seems fine to me, in case someone mentioned me for asking my opinion on the idea. :)

zanieb

LGTM. Might want to add a note around https://docs.astral.sh/uv/configuration/authentication/#custom-ca-certificates

graingert · 2024-08-27T12:45:35Z

You’re receiving notifications because you were mentioned.

Not sure why GitHub thinks I have a mention here... this seems fine to me, in case someone mentioned me for asking my opinion on the idea. :)

You were tagged here #6591 (comment)

jonathanasdf · 2024-08-27T19:10:35Z

I seem to be getting some errors, eg.

[tool.uv]
allow-insecure-host = [
  "amazonaws.com",
]

warning: Failed to parse `pyproject.toml` during settings discovery:
  TOML parse error at line 93, column 1
     |
  93 | [tool.uv]
     | ^^^^^^^^^
  invalid type: string "amazonaws.com", expected struct TrustedHost

charliermarsh · 2024-08-27T19:11:07Z

Apologies, that's my bad. Can you open a separate issue? It should work on the command-line though.

charliermarsh · 2024-08-27T19:21:04Z

See: #6716

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.3.2` -> `0.3.5` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.3.5`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#035) [Compare Source](astral-sh/uv@0.3.4...0.3.5) ##### Enhancements - Add support for `--allow-insecure-host` (aliased to `--trusted-host`) ([#6591](astral-sh/uv#6591)) - Read requirements from `requires.txt` when available ([#6655](astral-sh/uv#6655)) - Respect `tool.uv.environments` in `pip compile --universal` ([#6663](astral-sh/uv#6663)) - Use relative paths by default in `uv add` ([#6686](astral-sh/uv#6686)) - Improve messages for empty solves and installs ([#6588](astral-sh/uv#6588)) ##### Bug fixes - Avoid reusing state across tool upgrades ([#6660](astral-sh/uv#6660)) - Detect musl and error for musl Python builds ([#6643](astral-sh/uv#6643)) - Ignore `send` errors in installer ([#6667](astral-sh/uv#6667)) ##### Documentation - Add development section to Docker guide and reference new example project ([#6666](astral-sh/uv#6666)) - Add docs for `constraint-dependencies` and `override-dependencies` ([#6596](astral-sh/uv#6596)) - Clarify package priority order in pip compatibility guide ([#6619](astral-sh/uv#6619)) - Fix docs for disabling build isolation with `uv sync` ([#6674](astral-sh/uv#6674)) - Improve consistency of directory lookup instructions in Docker ([#6665](astral-sh/uv#6665)) - Improve lockfile concept documentation, add coverage for upgrades ([#6698](astral-sh/uv#6698)) - Shift the order of some of the Docker guide content ([#6664](astral-sh/uv#6664)) - Use `python` to highlight requirements and use more content tabs ([#6549](astral-sh/uv#6549)) ### [`v0.3.4`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#034) [Compare Source](astral-sh/uv@0.3.3...0.3.4) ##### CLI - Show `--editable` on the `uv add` CLI ([#6608](astral-sh/uv#6608)) - Add `--refresh` to `tool run` warning for `--with` dependencies ([#6609](astral-sh/uv#6609)) ##### Bug fixes - Allow per dependency build isolation for `setup.py`-based projects ([#6517](astral-sh/uv#6517)) - Avoid un-strict syncing by-default for build isolation ([#6606](astral-sh/uv#6606)) - Respect `--no-build-isolation-package` in `uv sync` ([#6605](astral-sh/uv#6605)) - Respect extras and markers on virtual dev dependencies ([#6620](astral-sh/uv#6620)) - Support PEP 723 scripts in GUI files ([#6611](astral-sh/uv#6611)) - Update lockfile after setting minimum bounds in `uv add` ([#6618](astral-sh/uv#6618)) - Use relative paths for `--find-links` and local registries ([#6566](astral-sh/uv#6566)) - Use separate types to represent raw vs. resolver markers ([#6646](astral-sh/uv#6646)) - Parse wheels `WHEEL` and `METADATA` files as email messages ([#6616](astral-sh/uv#6616)) - Support unquoted hrefs in `--find-links` and other HTML sources ([#6622](astral-sh/uv#6622)) - Don't canonicalize paths to user requirements ([#6560](astral-sh/uv#6560)) ##### Documentation - Add FastAPI guide to overview ([#6603](astral-sh/uv#6603)) - Add docs for disabling build isolation with `uv sync` ([#6607](astral-sh/uv#6607)) - Add example of reading script from stdin using echo ([#6567](astral-sh/uv#6567)) - Add tip to use intermediate layers in Docker builds ([#6650](astral-sh/uv#6650)) - Clarify need to include `pyproject.toml` with `--no-install-project` ([#6581](astral-sh/uv#6581)) - Move `WORKDIR` directive in Docker examples ([#6652](astral-sh/uv#6652)) - Remove duplicate `WORKDIR` directive in Docker example ([#6651](astral-sh/uv#6651)) ### [`v0.3.3`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#033) [Compare Source](astral-sh/uv@0.3.2...0.3.3) ##### Enhancements - Add `uv sync --no-install-project` to skip installation of the project ([#6538](astral-sh/uv#6538)) - Add `uv sync --no-install-workspace` to skip installation of all workspace members ([#6539](astral-sh/uv#6539)) - Add `uv sync --no-install-package` to skip installation of specific packages ([#6540](astral-sh/uv#6540)) - Show previous version in self update message ([#6473](astral-sh/uv#6473)) ##### CLI - Add `--no-project` alias for `uv python pin --no-workspace` ([#6514](astral-sh/uv#6514)) - Ignore `.python-version` files in `uv venv` with `--no-config` ([#6513](astral-sh/uv#6513)) - Include virtual environment interpreters in `uv python find` ([#6521](astral-sh/uv#6521)) - Respect `-` as stdin channel for `uv run` ([#6481](astral-sh/uv#6481)) - Revert changes to pyproject.toml when sync fails duing `uv add` ([#6526](astral-sh/uv#6526)) ##### Configuration - Add `UV_COMPILE_BYTECODE` environment variable ([#6530](astral-sh/uv#6530)) ##### Bug fixes - Set `VIRTUAL_ENV` for `uv run` invocations ([#6543](astral-sh/uv#6543)) - Ignore errors in workspace discovery with `--no-project` ([#6554](astral-sh/uv#6554)) ##### Documentation - Add documentation for `uv python find` ([#6527](astral-sh/uv#6527)) - Add uv tool install example in Docker ([#6547](astral-sh/uv#6547)) - Document why we do lower bounds ([#6516](astral-sh/uv#6516)) - Fix to miss string termination in PowerShell commands for shell autocompletion documentation ([#6491](astral-sh/uv#6491)) - Fix incorrect workspace members keyword ([#6502](astral-sh/uv#6502)) - Use proper environment variables for Windows ([#6433](astral-sh/uv#6433)) - Improve caveat in `uvx` note ([#6546](astral-sh/uv#6546)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

charliermarsh added compatibility Compatibility with a specification or another tool configuration Settings and such security labels Aug 24, 2024

charliermarsh mentioned this pull request Aug 24, 2024

pip install --trusted-host support #1339

Closed

charliermarsh force-pushed the charlie/trust branch from 836152c to 2e5b71e Compare August 24, 2024 21:54

charliermarsh commented Aug 24, 2024

View reviewed changes

crates/uv-settings/src/settings.rs Outdated Show resolved Hide resolved

charliermarsh force-pushed the charlie/trust branch 2 times, most recently from 0c76816 to 8aafa26 Compare August 24, 2024 23:15

charliermarsh requested a review from zanieb August 24, 2024 23:15

charliermarsh commented Aug 24, 2024

View reviewed changes

charliermarsh force-pushed the charlie/trust branch 2 times, most recently from fba6f6a to 7c440dd Compare August 24, 2024 23:20

samypr100 reviewed Aug 24, 2024

View reviewed changes

crates/uv-cli/src/lib.rs Show resolved Hide resolved

charliermarsh force-pushed the charlie/trust branch from 7c440dd to d8d51f7 Compare August 24, 2024 23:54

gaby reviewed Aug 25, 2024

View reviewed changes

crates/uv-cli/src/lib.rs Show resolved Hide resolved

zanieb reviewed Aug 25, 2024

View reviewed changes

crates/uv-cli/src/lib.rs Outdated Show resolved Hide resolved

zanieb reviewed Aug 25, 2024

View reviewed changes

charliermarsh force-pushed the charlie/trust branch 2 times, most recently from 0cd60b3 to 64715d3 Compare August 25, 2024 15:49

charliermarsh requested a review from zanieb August 26, 2024 18:43

zanieb approved these changes Aug 27, 2024

View reviewed changes

charliermarsh added 4 commits August 27, 2024 09:16

Trusted host

c6ef656

Add settings

c53f6c6

Use custom type

944b756

Rename; add more context to dangers

e285658

charliermarsh force-pushed the charlie/trust branch from 64715d3 to 7e62208 Compare August 27, 2024 13:22

Enforce schema

7ead6fc

charliermarsh force-pushed the charlie/trust branch from 7e62208 to 7ead6fc Compare August 27, 2024 13:28

charliermarsh merged commit d86075f into main Aug 27, 2024

charliermarsh deleted the charlie/trust branch August 27, 2024 13:36

zanieb mentioned this pull request Aug 27, 2024

Add support for trusted-host pip option #4944

Closed

2 tasks

BrewTestBot mentioned this pull request Aug 27, 2024

uv 0.3.5 Homebrew/homebrew-core#182657

Merged

kohlrabi mentioned this pull request Aug 27, 2024

Support passing --cert and setting cert in uv.toml #6715

Open

baszalmstra mentioned this pull request Sep 5, 2024

Invalid peer certificate with --tls-no-verify prefix-dev/pixi#1982

Closed

2 tasks

MattTheCuber mentioned this pull request Jan 7, 2025

UV pip install requirements file breaks when it contains --trusted-host #10366

Closed

Comments

Conversation

charliermarsh commented Aug 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zanieb commented Aug 25, 2024

Uh oh!

gaby commented Aug 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zanieb Aug 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zanieb Aug 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pradyunsg commented Aug 26, 2024

Uh oh!

zanieb left a comment

Choose a reason for hiding this comment

Uh oh!

graingert commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanasdf commented Aug 27, 2024

Uh oh!

charliermarsh commented Aug 27, 2024

Uh oh!

charliermarsh commented Aug 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

charliermarsh commented Aug 24, 2024 •

edited

Loading

gaby commented Aug 25, 2024 •

edited

Loading

zanieb Aug 25, 2024 •

edited

Loading

zanieb Aug 25, 2024 •

edited

Loading

graingert commented Aug 27, 2024 •

edited

Loading