From d9601bce6e1fa61cbb3dc4d66e5492f9885eef8f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 26 Apr 2022 22:22:46 +0200 Subject: Allow AutoDetectProxy to work with more than just http(s) It is a bit unfair for third-party methods wrapping e.g. http to not have access to such tools, same for our old ftp and e.g. our tor even if in practice this isn't used much even for http. At least that is one less FIXME in the code. --- apt-pkg/acquire-worker.cc | 10 ++++---- apt-pkg/contrib/proxy.cc | 52 +++++++++++++++++++++++++++++----------- apt-pkg/contrib/proxy.h | 4 +++- doc/examples/configure-index | 18 +++++++++----- test/integration/test-apt-helper | 48 +++++++++++++++++++++++++++---------- 5 files changed, 93 insertions(+), 39 deletions(-) diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 4c2acb5cc..533089ea5 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -877,14 +877,12 @@ bool pkgAcquire::Worker::QueueItem(pkgAcquire::Queue::QItem *Item) } Message += "\nFilename: " + Item->Owner->DestFile; - // FIXME: We should not hard code proxy protocols here. - if (URL.Access == "http" || URL.Access == "https") + // AutoDetectProxy() checks this already by itself, but we don't want to access unknown configs + if (CanURIBeAccessedViaProxy(URL)) { AutoDetectProxy(URL); - if (_config->Exists("Acquire::" + URL.Access + "::proxy::" + URL.Host)) - { - Message += "\nProxy: " + _config->Find("Acquire::" + URL.Access + "::proxy::" + URL.Host); - } + if (auto const proxy = _config->Find("Acquire::" + URL.Access + "::proxy::" + URL.Host); not proxy.empty()) + Message.append("\nProxy: ").append(proxy); } HashStringList const hsl = Item->GetExpectedHashes(); diff --git a/apt-pkg/contrib/proxy.cc b/apt-pkg/contrib/proxy.cc index a99f44f49..9a9a9f29c 100644 --- a/apt-pkg/contrib/proxy.cc +++ b/apt-pkg/contrib/proxy.cc @@ -12,9 +12,11 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -22,6 +24,16 @@ #include "proxy.h" /*}}}*/ +bool CanURIBeAccessedViaProxy(URI const &URL) /*{{{*/ +{ + // for some methods a proxy doesn't make sense, so we don't have to fork + if (URL.Host.empty() || + APT::String::Startswith(URL.Access, "mirror+") || URL.Access.find("+mirror+") != std::string::npos || APT::String::Endswith(URL.Access, "+mirror")) + return false; + std::array const noproxy{"file", "copy", "store", "gpgv", "rred", "cdrom", "mirror"}; + return std::find(noproxy.begin(), noproxy.end(), URL.Access) == noproxy.end(); +} + /*}}}*/ // AutoDetectProxy - auto detect proxy /*{{{*/ // --------------------------------------------------------------------- /* */ @@ -29,16 +41,19 @@ static std::vector CompatibleProxies(URI const &URL) { if (URL.Access == "http" || URL.Access == "https") return {"http", "https", "socks5h"}; - return {URL.Access}; + if (URL.Access == "tor" || URL.Access == "tor+http" || URL.Access == "tor+https") + return {"socks5h"}; + if (URL.Access == "ftp") + return {"ftp"}; + return {}; } - bool AutoDetectProxy(URI &URL) { - // we support both http/https debug options - bool Debug = _config->FindB("Debug::Acquire::"+URL.Access,false); + if (not CanURIBeAccessedViaProxy(URL)) + return true; // the user already explicitly set a proxy for this host - if(_config->Find("Acquire::"+URL.Access+"::proxy::"+URL.Host, "") != "") + if (not _config->Find("Acquire::" + URL.Access + "::proxy::" + URL.Host, "").empty()) return true; // option is "Acquire::http::Proxy-Auto-Detect" but we allow the old @@ -49,6 +64,7 @@ bool AutoDetectProxy(URI &URL) if (AutoDetectProxyCmd.empty()) return true; + bool const Debug = _config->FindB("Debug::Acquire::" + URL.Access, false); if (Debug) std::clog << "Using auto proxy detect command: " << AutoDetectProxyCmd << std::endl; @@ -73,7 +89,7 @@ bool AutoDetectProxy(URI &URL) // and apt will use the generic proxy settings if (goodread == false) return true; - auto const cleanedbuf = _strstrip(buf); + APT::StringView const cleanedbuf = _strstrip(buf); // We warn about this as the implementor probably meant to use DIRECT instead if (cleanedbuf[0] == '\0') { @@ -82,17 +98,25 @@ bool AutoDetectProxy(URI &URL) } if (Debug) - std::clog << "auto detect command returned: '" << cleanedbuf << "'" << std::endl; + std::clog << "auto detect command returned: '" << cleanedbuf.data() << "'" << std::endl; - auto compatibleTypes = CompatibleProxies(URL); - bool compatible = strcmp(cleanedbuf, "DIRECT") == 0 || - compatibleTypes.end() != std::find_if(compatibleTypes.begin(), - compatibleTypes.end(), [cleanedbuf](std::string &compat) { - return strstr(cleanedbuf, compat.c_str()) == cleanedbuf; - }); + bool compatible = true; + if (cleanedbuf != "DIRECT") + { + if (auto const compatibleTypes = CompatibleProxies(URL); not compatibleTypes.empty()) + compatible = std::any_of(compatibleTypes.begin(), compatibleTypes.end(), + [cleanedbuf](std::string const &compat) + { + return cleanedbuf.substr(0, compat.size()) == compat; + }); + } + else if (URL.Access == "tor" || URL.Access == "tor+http" || URL.Access == "tor+https") + compatible = false; // Accepting DIRECT would silently disable tor if (compatible) - _config->Set("Acquire::"+URL.Access+"::proxy::"+URL.Host, cleanedbuf); + _config->Set("Acquire::"+URL.Access+"::proxy::"+URL.Host, cleanedbuf.data()); + else + _error->Warning("ProxyAutoDetect command returned incompatible proxy '%s' for access type %s", cleanedbuf.data(), URL.Access.c_str()); return true; } diff --git a/apt-pkg/contrib/proxy.h b/apt-pkg/contrib/proxy.h index f6d70ea8b..2a4c19fc8 100644 --- a/apt-pkg/contrib/proxy.h +++ b/apt-pkg/contrib/proxy.h @@ -9,8 +9,10 @@ #ifndef PKGLIB_PROXY_H #define PKGLIB_PROXY_H +#include + class URI; APT_PUBLIC bool AutoDetectProxy(URI &URL); - +APT_HIDDEN bool CanURIBeAccessedViaProxy(URI const &URL); #endif diff --git a/doc/examples/configure-index b/doc/examples/configure-index index 1b165702d..9623514b8 100644 --- a/doc/examples/configure-index +++ b/doc/examples/configure-index @@ -846,12 +846,18 @@ acquire::gpgv::dl-limit ""; acquire::store::dl-limit ""; acquire::mirror::dl-limit ""; methods::mirror::problemreporting ""; -acquire::http::proxyautodetect ""; -acquire::http::proxy-auto-detect ""; -acquire::http::proxy::* ""; -acquire::https::proxyautodetect ""; -acquire::https::proxy-auto-detect ""; -acquire::https::proxy::* ""; +acquire::*::proxyautodetect ""; +acquire::file::proxyautodetect ""; +acquire::copy::proxyautodetect ""; +acquire::store::proxyautodetect ""; +acquire::*::proxy-auto-detect ""; +acquire::file::proxy-auto-detect ""; +acquire::copy::proxy-auto-detect ""; +acquire::store::proxy-auto-detect ""; +acquire::file::proxy::* ""; +acquire::copy::proxy::* ""; +acquire::store::proxy::* ""; +acquire::*::proxy::* ""; // Options used by apt-ftparchive dir::archivedir ""; diff --git a/test/integration/test-apt-helper b/test/integration/test-apt-helper index ae1ca7456..4f883b440 100755 --- a/test/integration/test-apt-helper +++ b/test/integration/test-apt-helper @@ -74,10 +74,11 @@ E: Download Failed" setupproxydetect() { local METH="$1" shift - { - echo '#!/bin/sh -e' - echo "$@" - } > "${TMPWORKINGDIRECTORY}/apt-proxy-detect" + cat >"${TMPWORKINGDIRECTORY}/apt-proxy-detect" < rootdir/etc/apt/apt.conf.d/02proxy-detect } @@ -97,15 +98,38 @@ W: ProxyAutoDetect command returned an empty line" apthelper auto-detect-proxy h chmod -x "${TMPWORKINGDIRECTORY}/apt-proxy-detect" testfailureequal "E: ProxyAutoDetect command '${TMPWORKINGDIRECTORY}/apt-proxy-detect' can not be executed! - access (13: Permission denied)" apthelper auto-detect-proxy http://example.com/ - msgmsg "apt-helper $CONFNAME" 'http proxy' - setupproxydetect 'http' 'echo "http://some-proxy"' - testsuccessequal "Using proxy 'http://some-proxy' for URL 'http://www.example.com/'" apthelper auto-detect-proxy http://www.example.com - testsuccessequal "Using proxy '' for URL 'https://ssl.example.com/'" apthelper auto-detect-proxy https://ssl.example.com + for meth in 'http' 'https'; do + msgmsg "apt-helper $CONFNAME" "${meth} proxy" + for type in 'http' 'https' 'socks5h'; do + setupproxydetect "$meth" "echo '${type}://some-proxy'" + testsuccessequal "Using proxy '${type}://some-proxy' for URL '${meth}://www.example.com/'" apthelper auto-detect-proxy "${meth}://www.example.com" + if [ "$meth" = 'http' ]; then + testsuccessequal "Using proxy '' for URL 'https://ssl.example.com/'" apthelper auto-detect-proxy 'https://ssl.example.com' + else + testsuccessequal "Using proxy '' for URL 'http://no-ssl.example.com/'" apthelper auto-detect-proxy 'http://no-ssl.example.com' + fi + done + done + + msgmsg "apt-helper $CONFNAME" 'no strange proxy' + for meth in gpgv copy store file; do + setupproxydetect "$meth" "echo '${meth}://some-proxy'" + testsuccessequal "Using proxy '' for URL '${meth}:/foo/bar'" apthelper auto-detect-proxy "${meth}:/foo/bar" + testsuccessequal "Using proxy '' for URL '${meth}://./foo/bar'" apthelper auto-detect-proxy "${meth}://./foo/bar" + done + for url in 'cdrom://Moo Cow Rom/debian' 'cdrom://[The Debian 1.2 disk, 1/2 R16]/debian/'; do + setupproxydetect "${url%%:*}" "echo 'cdrom://some-proxy'" + testsuccessequal "Using proxy '' for URL '${url}'" apthelper auto-detect-proxy "$url" + done + for meth in tor tor+http tor+https; do + setupproxydetect "$meth" "echo 'socks5h://some-proxy'" + testsuccessequal "Using proxy 'socks5h://some-proxy' for URL '${meth}://example.org/'" apthelper auto-detect-proxy "${meth}://example.org" + + setupproxydetect "$meth" "echo 'DIRECT'" + testwarningequal "Using proxy '' for URL '${meth}://example.org/' +W: ProxyAutoDetect command returned incompatible proxy 'DIRECT' for access type ${meth}" apthelper auto-detect-proxy "${meth}://example.org" + done - msgmsg "apt-helper $CONFNAME" 'https proxy' - setupproxydetect 'https' 'echo "https://https-proxy"' - testsuccessequal "Using proxy '' for URL 'http://no-ssl.example.com/'" apthelper auto-detect-proxy http://no-ssl.example.com - testsuccessequal "Using proxy 'https://https-proxy' for URL 'https://ssl.example.com/'" apthelper auto-detect-proxy https://ssl.example.com rm -f rootdir/etc/apt/apt.conf.d/02proxy-detect "${TMPWORKINGDIRECTORY}/apt-proxy-detect" } -- cgit v1.2.3-70-g09d2