fix(network): no proxying of localhost connections by default #564
Conversation
…overriding proxy vars at runtime
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #564 +/- ##
==========================================
- Coverage 71.10% 70.68% -0.42%
==========================================
Files 3 3
Lines 481 481
==========================================
- Hits 342 340 -2
- Misses 139 141 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -155,6 +155,7 @@ fi | |||
| if [ -e /etc/portage/make.conf ]; then | |||
| update_conf /etc/portage/make.conf "http_proxy=\"$PROXY_ADDR\" | |||
| https_proxy=\"$PROXY_ADDR\" | |||
| no_proxy=\"${NO_PROXY:-127.0.0.1}\" | |||
There was a problem hiding this comment.
I don't see it documented in make.conf(5) man page, do you have any source for this?
There was a problem hiding this comment.
It's not documented on that manpage but wget does support it (see gitlab link in separate reply).
|
Maybe stupid question, but you want to access a repo that is on "localhost" in the template, right? Why not simply configure it as |
|
But if you want to change that anyway, tests will need an update - currently they rely on accessing localhost repo via the proxy (and this is actually how it checks that proxy gets used). I wonder if anybody else relies on this feature... |
Well, it's not actually running from the local filesystem just because it's listening on But even if one is doing it from the local filesystem in the builder, again, we often run this script inside ephemeral containers or disposables, so the filesystem is seldom convenient. |
Do you think it would be enough to make the tests use an alternative loopback address ( |
|
No issue in the builder, but here it's about behavior of the final template, so the blast radius of a behavior change is bigger. What about having that qrexec proxy from updates proxy instead? It gives a bit more control to the updates proxy, but don't really need to use sys-net for it - it can be also a dedicated qube.
That may be a good idea anyway. |
Yeah, but:
I'm not sure I fully understand the concern/risk involved here.. Considering the update proxy out of the box will 403 these localhost connections, what is a (hypothetical/real) scenario this might be breaking for existing user? |
Ah, right. Then this change should be okay. Just update the tests first to use different loopback address. |
Sure! Would that be just the tests in this repo triggered here or any integration tests elsewhere to tackle? |
At least here: https://github.com/QubesOS/qubes-core-admin/blob/main/qubes/tests/integ/vm_update.py#L496-L538 |
So TBH I still haven't figured out how to make sure my test runs are using the fork 100%... At least I have those same tests passing with/without setting the env var when running them. From what I've seen so far, no changes should be needed there. Since they refer to Should be the same as with curl, I believe: vs |
|
Wait, so requests to https://github.com/QubesOS/qubes-core-agent-linux/blob/main/network/updates-blacklist vs |
|
Ok, then I guess no change is needed (will test). |
Ty!
Maybe it could be filtered only on whonix, then? (I guess that patch would solve my specific use-case separately from the slightly OT...The way this is set up looks like it could give some users the wrong idea resulting in dangerous behavior. Imagine a scenario where user... EDIT: nvm |
There are some corner cases here indeed, but also updates proxy is allowed only for templates by default, so it's pretty limited. |
FWIW the hypothetical scenario I described is also not valid because |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025032406-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025031804-4.3&flavor=update
Failed tests14 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/132953#dependencies 14 fixed
Unstable testsPerformance TestsPerformance degradation:17 performance degradations
Remaining performance tests:55 tests
|
NO_PROXY=127.0.0.1for pacman and gentoo package managers, to avoid proxying connections to repos already accessed over localhost.ALL_PROXYandNO_PROXYenvironment variablesRelated: