From d0ef571416e1ff6266c89e6285898d269768cf8f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 1 Aug 2016 17:52:55 +0200 Subject: suggest transport-packages based on established namescheme apt-transports not shipped in apt directly are usually named apt-transport-% with % being what is in the name of the transport. tor additional introduced aliases via %+something, which isn't a bad idea, so be strip the +something part from the method name before suggesting the installation of an apt-transport-% package. This avoids us the maintainance of a list of existing transports creating a two class system of known and unknown transports which would be quite arbitrary and is unfriendly to backports. --- apt-pkg/acquire-worker.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 9ed7b5b28..39cc55bdf 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -97,8 +97,10 @@ bool pkgAcquire::Worker::Start() if (FileExists(Method) == false) { _error->Error(_("The method driver %s could not be found."),Method.c_str()); - if (Access == "https") - _error->Notice(_("Is the package %s installed?"), "apt-transport-https"); + std::string const A(Access.cbegin(), std::find(Access.cbegin(), Access.cend(), '+')); + std::string pkg; + strprintf(pkg, "apt-transport-%s", A.c_str()); + _error->Notice(_("Is the package %s installed?"), pkg.c_str()); return false; } -- cgit v1.2.3-70-g09d2 From 57401c48fadc0c78733a67294f9cc20a57e527c9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 2 Aug 2016 22:44:50 +0200 Subject: detect redirection loops in acquire instead of workers Having the detection handled in specific (http) workers means that a redirection loop over different hostnames isn't detected. Its also not a good idea have this implement in each method independently even if it would work --- apt-pkg/acquire-item.cc | 69 +++++++++++++++++++++--------- apt-pkg/acquire-item.h | 5 ++- apt-pkg/acquire-worker.cc | 10 +++++ methods/aptmethod.h | 3 +- methods/http.cc | 6 +++ methods/http.h | 1 + methods/https.h | 1 + methods/server.cc | 75 ++++++++++++++------------------- methods/server.h | 1 + test/integration/test-apt-redirect-loop | 23 ++++++++++ 10 files changed, 129 insertions(+), 65 deletions(-) create mode 100755 test/integration/test-apt-redirect-loop (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index ad8cb7f24..f13d2f6ae 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -685,10 +685,15 @@ class APT_HIDDEN CleanupItem : public pkgAcqTransactionItem /*{{{*/ /*}}}*/ // Acquire::Item::Item - Constructor /*{{{*/ +class pkgAcquire::Item::Private +{ +public: + std::vector PastRedirections; +}; APT_IGNORE_DEPRECATED_PUSH pkgAcquire::Item::Item(pkgAcquire * const owner) : FileSize(0), PartialSize(0), Mode(0), ID(0), Complete(false), Local(false), - QueueCounter(0), ExpectedAdditionalItems(0), Owner(owner), d(NULL) + QueueCounter(0), ExpectedAdditionalItems(0), Owner(owner), d(new Private()) { Owner->Add(this); Status = StatIdle; @@ -699,6 +704,7 @@ APT_IGNORE_DEPRECATED_POP pkgAcquire::Item::~Item() { Owner->Remove(this); + delete d; } /*}}}*/ std::string pkgAcquire::Item::Custom600Headers() const /*{{{*/ @@ -766,32 +772,40 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con } string const FailReason = LookupTag(Message, "FailReason"); - enum { MAXIMUM_SIZE_EXCEEDED, HASHSUM_MISMATCH, WEAK_HASHSUMS, OTHER } failreason = OTHER; + enum { MAXIMUM_SIZE_EXCEEDED, HASHSUM_MISMATCH, WEAK_HASHSUMS, REDIRECTION_LOOP, OTHER } failreason = OTHER; if ( FailReason == "MaximumSizeExceeded") failreason = MAXIMUM_SIZE_EXCEEDED; else if ( FailReason == "WeakHashSums") failreason = WEAK_HASHSUMS; + else if (FailReason == "RedirectionLoop") + failreason = REDIRECTION_LOOP; else if (Status == StatAuthError) failreason = HASHSUM_MISMATCH; if(ErrorText.empty()) { + std::ostringstream out; + switch (failreason) + { + case HASHSUM_MISMATCH: + out << _("Hash Sum mismatch") << std::endl; + break; + case WEAK_HASHSUMS: + out << _("Insufficient information available to perform this download securely") << std::endl; + break; + case REDIRECTION_LOOP: + out << "Redirection loop encountered" << std::endl; + break; + case MAXIMUM_SIZE_EXCEEDED: + out << LookupTag(Message, "Message") << std::endl; + break; + case OTHER: + out << LookupTag(Message, "Message"); + break; + } + if (Status == StatAuthError) { - std::ostringstream out; - switch (failreason) - { - case HASHSUM_MISMATCH: - out << _("Hash Sum mismatch") << std::endl; - break; - case WEAK_HASHSUMS: - out << _("Insufficient information available to perform this download securely") << std::endl; - break; - case MAXIMUM_SIZE_EXCEEDED: - case OTHER: - out << LookupTag(Message, "Message") << std::endl; - break; - } auto const ExpectedHashes = GetExpectedHashes(); if (ExpectedHashes.empty() == false) { @@ -822,10 +836,8 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con } out << "Last modification reported: " << LookupTag(Message, "Last-Modified", "") << std::endl; } - ErrorText = out.str(); } - else - ErrorText = LookupTag(Message,"Message"); + ErrorText = out.str(); } switch (failreason) @@ -833,6 +845,7 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con case MAXIMUM_SIZE_EXCEEDED: RenameOnError(MaximumSizeExceeded); break; case HASHSUM_MISMATCH: RenameOnError(HashSumMismatch); break; case WEAK_HASHSUMS: break; + case REDIRECTION_LOOP: break; case OTHER: break; } @@ -976,6 +989,24 @@ std::string pkgAcquire::Item::HashSum() const /*{{{*/ return hs != NULL ? hs->toStr() : ""; } /*}}}*/ +bool pkgAcquire::Item::IsRedirectionLoop(std::string const &NewURI) /*{{{*/ +{ + if (d->PastRedirections.empty()) + { + d->PastRedirections.push_back(NewURI); + return false; + } + auto const LastURI = std::prev(d->PastRedirections.end()); + // redirections to the same file are a way of restarting/resheduling, + // individual methods will have to make sure that they aren't looping this way + if (*LastURI == NewURI) + return false; + if (std::find(d->PastRedirections.begin(), LastURI, NewURI) != LastURI) + return true; + d->PastRedirections.push_back(NewURI); + return false; +} + /*}}}*/ pkgAcqTransactionItem::pkgAcqTransactionItem(pkgAcquire * const Owner, /*{{{*/ pkgAcqMetaClearSig * const transactionManager, IndexTarget const &target) : diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index ac4994738..e6e5ea12b 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -304,6 +304,8 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/ */ virtual ~Item(); + bool APT_HIDDEN IsRedirectionLoop(std::string const &NewURI); + protected: /** \brief The acquire object with which this item is associated. */ pkgAcquire * const Owner; @@ -357,7 +359,8 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/ virtual std::string GetFinalFilename() const; private: - void * const d; + class Private; + Private * const d; friend class pkgAcqMetaBase; friend class pkgAcqMetaClearSig; diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 39cc55bdf..1ee78d070 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -269,6 +269,16 @@ bool pkgAcquire::Worker::RunMessages() for (auto const &Owner: ItmOwners) { pkgAcquire::ItemDesc &desc = Owner->GetItemDesc(); + if (Owner->IsRedirectionLoop(NewURI)) + { + std::string msg = Message; + msg.append("\nFailReason: RedirectionLoop"); + Owner->Failed(msg, Config); + if (Log != nullptr) + Log->Fail(Owner->GetItemDesc()); + continue; + } + if (Log != nullptr) Log->Done(desc); diff --git a/methods/aptmethod.h b/methods/aptmethod.h index d3c948636..0963ea21e 100644 --- a/methods/aptmethod.h +++ b/methods/aptmethod.h @@ -17,7 +17,8 @@ class aptMethod : public pkgAcqMethod { - char const * const Binary; +protected: + std::string Binary; public: virtual bool Configuration(std::string Message) APT_OVERRIDE diff --git a/methods/http.cc b/methods/http.cc index c61ca1c3f..cf5eae06d 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -465,6 +465,12 @@ bool HttpServerState::RunData(FileFd * const File) return Owner->Flush() && !_error->PendingError(); } /*}}}*/ +bool HttpServerState::RunDataToDevNull() /*{{{*/ +{ + FileFd DevNull("/dev/null", FileFd::WriteOnly); + return RunData(&DevNull); +} + /*}}}*/ bool HttpServerState::ReadHeaderLines(std::string &Data) /*{{{*/ { return In.WriteTillEl(Data); diff --git a/methods/http.h b/methods/http.h index ac5f52314..2ac3b8c9b 100644 --- a/methods/http.h +++ b/methods/http.h @@ -106,6 +106,7 @@ struct HttpServerState: public ServerState virtual void Reset() APT_OVERRIDE { ServerState::Reset(); ServerFd = -1; }; virtual bool RunData(FileFd * const File) APT_OVERRIDE; + virtual bool RunDataToDevNull() APT_OVERRIDE; virtual bool Open() APT_OVERRIDE; virtual bool IsOpen() APT_OVERRIDE; diff --git a/methods/https.h b/methods/https.h index 8592570c6..85cc7824e 100644 --- a/methods/https.h +++ b/methods/https.h @@ -39,6 +39,7 @@ class HttpsServerState : public ServerState /** \brief Transfer the data from the socket */ virtual bool RunData(FileFd * const /*File*/) APT_OVERRIDE { return false; } + virtual bool RunDataToDevNull() APT_OVERRIDE { return false; } virtual bool Open() APT_OVERRIDE { return false; } virtual bool IsOpen() APT_OVERRIDE { return false; } diff --git a/methods/server.cc b/methods/server.cc index 6d147fe12..672a3d16d 100644 --- a/methods/server.cc +++ b/methods/server.cc @@ -297,12 +297,33 @@ ServerMethod::DealWithHeaders(FetchResult &Res) else NextURI.clear(); NextURI.append(DeQuoteString(Server->Location)); + if (Queue->Uri == NextURI) + { + SetFailReason("RedirectionLoop"); + _error->Error("Redirection loop encountered"); + if (Server->HaveContent == true) + return ERROR_WITH_CONTENT_PAGE; + return ERROR_UNRECOVERABLE; + } return TRY_AGAIN_OR_REDIRECT; } else { NextURI = DeQuoteString(Server->Location); URI tmpURI = NextURI; + if (tmpURI.Access == "http" && Binary == "https+http") + { + tmpURI.Access = "https+http"; + NextURI = tmpURI; + } + if (Queue->Uri == NextURI) + { + SetFailReason("RedirectionLoop"); + _error->Error("Redirection loop encountered"); + if (Server->HaveContent == true) + return ERROR_WITH_CONTENT_PAGE; + return ERROR_UNRECOVERABLE; + } URI Uri = Queue->Uri; // same protocol redirects are okay if (tmpURI.Access == Uri.Access) @@ -486,10 +507,6 @@ bool ServerMethod::Fetch(FetchItem *) // ServerMethod::Loop - Main loop /*{{{*/ int ServerMethod::Loop() { - typedef vector StringVector; - typedef vector::iterator StringVectorIterator; - map Redirected; - signal(SIGTERM,SigTerm); signal(SIGINT,SigTerm); @@ -720,46 +737,16 @@ int ServerMethod::Loop() File = 0; break; } - - // Try again with a new URL - case TRY_AGAIN_OR_REDIRECT: - { - // Clear rest of response if there is content - if (Server->HaveContent) - { - File = new FileFd("/dev/null",FileFd::WriteExists); - Server->RunData(File); - delete File; - File = 0; - } - - /* Detect redirect loops. No more redirects are allowed - after the same URI is seen twice in a queue item. */ - StringVector &R = Redirected[Queue->DestFile]; - bool StopRedirects = false; - if (R.empty() == true) - R.push_back(Queue->Uri); - else if (R[0] == "STOP" || R.size() > 10) - StopRedirects = true; - else - { - for (StringVectorIterator I = R.begin(); I != R.end(); ++I) - if (Queue->Uri == *I) - { - R[0] = "STOP"; - break; - } - - R.push_back(Queue->Uri); - } - - if (StopRedirects == false) - Redirect(NextURI); - else - Fail(); - - break; - } + + // Try again with a new URL + case TRY_AGAIN_OR_REDIRECT: + { + // Clear rest of response if there is content + if (Server->HaveContent) + Server->RunDataToDevNull(); + Redirect(NextURI); + break; + } default: Fail(_("Internal error")); diff --git a/methods/server.h b/methods/server.h index af0914b9c..b23b0e50a 100644 --- a/methods/server.h +++ b/methods/server.h @@ -91,6 +91,7 @@ struct ServerState /** \brief Transfer the data from the socket */ virtual bool RunData(FileFd * const File) = 0; + virtual bool RunDataToDevNull() = 0; virtual bool Open() = 0; virtual bool IsOpen() = 0; diff --git a/test/integration/test-apt-redirect-loop b/test/integration/test-apt-redirect-loop new file mode 100755 index 000000000..b54f74276 --- /dev/null +++ b/test/integration/test-apt-redirect-loop @@ -0,0 +1,23 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" + +setupenvironment +configarchitecture "i386" + +insertpackage 'stable' 'apt' 'all' '1' +setupaptarchive --no-update + +echo 'alright' > aptarchive/working +changetohttpswebserver +webserverconfig 'aptwebserver::redirect::replace::/redirectme3/' '/redirectme/' +webserverconfig 'aptwebserver::redirect::replace::/redirectme2/' '/redirectme3/' +webserverconfig 'aptwebserver::redirect::replace::/redirectme/' '/redirectme2/' + +testfailure apthelper download-file "http://localhost:${APTHTTPPORT}/redirectme/working" httpfile +testsuccess grep 'Redirection loop encountered' rootdir/tmp/testfailure.output + +testfailure apthelper download-file "https://localhost:${APTHTTPSPORT}/redirectme/working" httpsfile +testsuccess grep 'Redirection loop encountered' rootdir/tmp/testfailure.output -- cgit v1.2.3-70-g09d2 From 61db48241f2d46697a291bfedaf398a1ca9a70e3 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 4 Aug 2016 08:45:38 +0200 Subject: implement socks5h proxy support for http method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Socks support is a requested feature in sofar that the internet is actually believing Acquire::socks::Proxy would exist. It doesn't and this commit isn't adding it as that isn't how our configuration works, but it allows Acquire::http::Proxy="socks5h://…". The HTTPS method was changed already to support socks proxies (all versions) via curl. This commit implements only SOCKS5 (RFC1928) with no auth or pass&user auth (RFC1929), but not GSSAPI which is required by the RFC. The 'h' in the protocol name further indicates that DNS resolution is delegated to the socks proxy rather than performed locally. The implementation works and was tested with Tor as socks proxy for which implementing socks5h only can actually be considered a feature. Closes: 744934 --- apt-pkg/contrib/fileutl.cc | 31 ++++ apt-pkg/contrib/fileutl.h | 1 + methods/http.cc | 191 ++++++++++++++++++++++--- methods/http.h | 2 +- test/integration/skip-method-http-socks-client | 116 +++++++++++++++ 5 files changed, 318 insertions(+), 23 deletions(-) create mode 100755 test/integration/skip-method-http-socks-client (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 342f9830d..a9d51a6bf 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -2443,6 +2443,37 @@ bool FileFd::Read(void *To,unsigned long long Size,unsigned long long *Actual) } return FileFdError(_("read, still have %llu to read but none left"), Size); +} +bool FileFd::Read(int const Fd, void *To, unsigned long long Size, unsigned long long * const Actual) +{ + ssize_t Res = 1; + errno = 0; + if (Actual != nullptr) + *Actual = 0; + *static_cast(To) = '\0'; + while (Res > 0 && Size > 0) + { + Res = read(Fd, To, Size); + if (Res < 0) + { + if (errno == EINTR) + { + Res = 1; + errno = 0; + continue; + } + return _error->Errno("read", _("Read error")); + } + To = static_cast(To) + Res; + Size -= Res; + if (Actual != 0) + *Actual += Res; + } + if (Size == 0) + return true; + if (Actual != nullptr) + return true; + return _error->Error(_("read, still have %llu to read but none left"), Size); } /*}}}*/ // FileFd::ReadLine - Read a complete line from the file /*{{{*/ diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index c13613171..4a1676dc2 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -86,6 +86,7 @@ class FileFd return Read(To,Size); } bool Read(void *To,unsigned long long Size,unsigned long long *Actual = 0); + bool static Read(int const Fd, void *To, unsigned long long Size, unsigned long long * const Actual = 0); char* ReadLine(char *To, unsigned long long const Size); bool Flush(); bool Write(const void *From,unsigned long long Size); diff --git a/methods/http.cc b/methods/http.cc index 0378d7c60..1ed2e3629 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -44,6 +44,7 @@ #include #include #include +#include #include #include @@ -151,9 +152,9 @@ bool CircleBuf::Read(int Fd) // CircleBuf::Read - Put the string into the buffer /*{{{*/ // --------------------------------------------------------------------- /* This will hold the string in and fill the buffer with it as it empties */ -bool CircleBuf::Read(string Data) +bool CircleBuf::Read(string const &Data) { - OutQueue += Data; + OutQueue.append(Data); FillOut(); return true; } @@ -296,6 +297,24 @@ HttpServerState::HttpServerState(URI Srv,HttpMethod *Owner) : ServerState(Srv, O // HttpServerState::Open - Open a connection to the server /*{{{*/ // --------------------------------------------------------------------- /* This opens a connection to the server. */ +static bool TalkToSocksProxy(int const ServerFd, std::string const &Proxy, + char const * const type, bool const ReadWrite, uint8_t * const ToFrom, + unsigned int const Size, unsigned int const Timeout) +{ + if (WaitFd(ServerFd, ReadWrite, Timeout) == false) + return _error->Error("Waiting for the SOCKS proxy %s to %s timed out", URI::SiteOnly(Proxy).c_str(), type); + if (ReadWrite == false) + { + if (FileFd::Read(ServerFd, ToFrom, Size) == false) + return _error->Error("Reading the %s from SOCKS proxy %s failed", type, URI::SiteOnly(Proxy).c_str()); + } + else + { + if (FileFd::Write(ServerFd, ToFrom, Size) == false) + return _error->Error("Writing the %s to SOCKS proxy %s failed", type, URI::SiteOnly(Proxy).c_str()); + } + return true; +} bool HttpServerState::Open() { // Use the already open connection if possible. @@ -337,29 +356,156 @@ bool HttpServerState::Open() if (CheckDomainList(ServerName.Host,getenv("no_proxy")) == true) Proxy = ""; } - - // Determine what host and port to use based on the proxy settings - int Port = 0; - string Host; - if (Proxy.empty() == true || Proxy.Host.empty() == true) + + if (Proxy.Access == "socks5h") { - if (ServerName.Port != 0) - Port = ServerName.Port; - Host = ServerName.Host; + if (Connect(Proxy.Host, Proxy.Port, "socks", 1080, ServerFd, TimeOut, Owner) == false) + return false; + + /* We implement a very basic SOCKS5 client here complying mostly to RFC1928 expect + * for not offering GSSAPI auth which is a must (we only do no or user/pass auth). + * We also expect the SOCKS5 server to do hostname lookup (aka socks5h) */ + std::string const ProxyInfo = URI::SiteOnly(Proxy); + Owner->Status(_("Connecting to %s (%s)"),"SOCKS5h proxy",ProxyInfo.c_str()); + auto const Timeout = Owner->ConfigFindI("TimeOut", 120); + #define APT_WriteOrFail(TYPE, DATA, LENGTH) if (TalkToSocksProxy(ServerFd, ProxyInfo, TYPE, true, DATA, LENGTH, Timeout) == false) return false + #define APT_ReadOrFail(TYPE, DATA, LENGTH) if (TalkToSocksProxy(ServerFd, ProxyInfo, TYPE, false, DATA, LENGTH, Timeout) == false) return false + if (ServerName.Host.length() > 255) + return _error->Error("Can't use SOCKS5h as hostname %s is too long!", ServerName.Host.c_str()); + if (Proxy.User.length() > 255 || Proxy.Password.length() > 255) + return _error->Error("Can't use user&pass auth as they are too long (%lu and %lu) for the SOCKS5!", Proxy.User.length(), Proxy.Password.length()); + if (Proxy.User.empty()) + { + uint8_t greeting[] = { 0x05, 0x01, 0x00 }; + APT_WriteOrFail("greet-1", greeting, sizeof(greeting)); + } + else + { + uint8_t greeting[] = { 0x05, 0x02, 0x00, 0x02 }; + APT_WriteOrFail("greet-2", greeting, sizeof(greeting)); + } + uint8_t greeting[2]; + APT_ReadOrFail("greet back", greeting, sizeof(greeting)); + if (greeting[0] != 0x05) + return _error->Error("SOCKS proxy %s greets back with wrong version: %d", ProxyInfo.c_str(), greeting[0]); + if (greeting[1] == 0x00) + ; // no auth has no method-dependent sub-negotiations + else if (greeting[1] == 0x02) + { + if (Proxy.User.empty()) + return _error->Error("SOCKS proxy %s negotiated user&pass auth, but we had not offered it!", ProxyInfo.c_str()); + // user&pass auth sub-negotiations are defined by RFC1929 + std::vector auth = {{ 0x01, static_cast(Proxy.User.length()) }}; + std::copy(Proxy.User.begin(), Proxy.User.end(), std::back_inserter(auth)); + auth.push_back(static_cast(Proxy.Password.length())); + std::copy(Proxy.Password.begin(), Proxy.Password.end(), std::back_inserter(auth)); + APT_WriteOrFail("user&pass auth", auth.data(), auth.size()); + uint8_t authstatus[2]; + APT_ReadOrFail("auth report", authstatus, sizeof(authstatus)); + if (authstatus[0] != 0x01) + return _error->Error("SOCKS proxy %s auth status response with wrong version: %d", ProxyInfo.c_str(), authstatus[0]); + if (authstatus[1] != 0x00) + return _error->Error("SOCKS proxy %s reported authorization failure: username or password incorrect? (%d)", ProxyInfo.c_str(), authstatus[1]); + } + else + return _error->Error("SOCKS proxy %s greets back having not found a common authorization method: %d", ProxyInfo.c_str(), greeting[1]); + union { uint16_t * i; uint8_t * b; } portu; + uint16_t port = htons(static_cast(ServerName.Port == 0 ? 80 : ServerName.Port)); + portu.i = &port; + std::vector request = {{ 0x05, 0x01, 0x00, 0x03, static_cast(ServerName.Host.length()) }}; + std::copy(ServerName.Host.begin(), ServerName.Host.end(), std::back_inserter(request)); + request.push_back(portu.b[0]); + request.push_back(portu.b[1]); + APT_WriteOrFail("request", request.data(), request.size()); + uint8_t response[4]; + APT_ReadOrFail("first part of response", response, sizeof(response)); + if (response[0] != 0x05) + return _error->Error("SOCKS proxy %s response with wrong version: %d", ProxyInfo.c_str(), response[0]); + if (response[2] != 0x00) + return _error->Error("SOCKS proxy %s has unexpected non-zero reserved field value: %d", ProxyInfo.c_str(), response[2]); + std::string bindaddr; + if (response[3] == 0x01) // IPv4 address + { + uint8_t ip4port[6]; + APT_ReadOrFail("IPv4+Port of response", ip4port, sizeof(ip4port)); + portu.b[0] = ip4port[4]; + portu.b[1] = ip4port[5]; + port = ntohs(*portu.i); + strprintf(bindaddr, "%d.%d.%d.%d:%d", ip4port[0], ip4port[1], ip4port[2], ip4port[3], port); + } + else if (response[3] == 0x03) // hostname + { + uint8_t namelength; + APT_ReadOrFail("hostname length of response", &namelength, 1); + uint8_t hostname[namelength + 2]; + APT_ReadOrFail("hostname of response", hostname, sizeof(hostname)); + portu.b[0] = hostname[namelength]; + portu.b[1] = hostname[namelength + 1]; + port = ntohs(*portu.i); + hostname[namelength] = '\0'; + strprintf(bindaddr, "%s:%d", hostname, port); + } + else if (response[3] == 0x04) // IPv6 address + { + uint8_t ip6port[18]; + APT_ReadOrFail("IPv6+port of response", ip6port, sizeof(ip6port)); + portu.b[0] = ip6port[16]; + portu.b[1] = ip6port[17]; + port = ntohs(*portu.i); + strprintf(bindaddr, "[%02X%02X:%02X%02X:%02X%02X:%02X%02X:%02X%02X:%02X%02X:%02X%02X:%02X%02X]:%d", + ip6port[0], ip6port[1], ip6port[2], ip6port[3], ip6port[4], ip6port[5], ip6port[6], ip6port[7], + ip6port[8], ip6port[9], ip6port[10], ip6port[11], ip6port[12], ip6port[13], ip6port[14], ip6port[15], + port); + } + else + return _error->Error("SOCKS proxy %s destination address is of unknown type: %d", + ProxyInfo.c_str(), response[3]); + if (response[1] != 0x00) + { + char const * errstr; + switch (response[1]) + { + case 0x01: errstr = "general SOCKS server failure"; Owner->SetFailReason("SOCKS"); break; + case 0x02: errstr = "connection not allowed by ruleset"; Owner->SetFailReason("SOCKS"); break; + case 0x03: errstr = "Network unreachable"; Owner->SetFailReason("ConnectionTimedOut"); break; + case 0x04: errstr = "Host unreachable"; Owner->SetFailReason("ConnectionTimedOut"); break; + case 0x05: errstr = "Connection refused"; Owner->SetFailReason("ConnectionRefused"); break; + case 0x06: errstr = "TTL expired"; Owner->SetFailReason("Timeout"); break; + case 0x07: errstr = "Command not supported"; Owner->SetFailReason("SOCKS"); break; + case 0x08: errstr = "Address type not supported"; Owner->SetFailReason("SOCKS"); break; + default: errstr = "Unknown error"; Owner->SetFailReason("SOCKS"); break; + } + return _error->Error("SOCKS proxy %s didn't grant the connect to %s due to: %s (%d)", ProxyInfo.c_str(), bindaddr.c_str(), errstr, response[1]); + } + else if (Owner->DebugEnabled()) + ioprintf(std::clog, "http: SOCKS proxy %s connection established to %s\n", ProxyInfo.c_str(), bindaddr.c_str()); + + if (WaitFd(ServerFd, true, Timeout) == false) + return _error->Error("SOCKS proxy %s reported connection, but timed out", ProxyInfo.c_str()); + #undef APT_ReadOrFail + #undef APT_WriteOrFail } - else if (Proxy.Access != "http") - return _error->Error("Unsupported proxy configured: %s", URI::SiteOnly(Proxy).c_str()); else { - if (Proxy.Port != 0) - Port = Proxy.Port; - Host = Proxy.Host; + // Determine what host and port to use based on the proxy settings + int Port = 0; + string Host; + if (Proxy.empty() == true || Proxy.Host.empty() == true) + { + if (ServerName.Port != 0) + Port = ServerName.Port; + Host = ServerName.Host; + } + else if (Proxy.Access != "http") + return _error->Error("Unsupported proxy configured: %s", URI::SiteOnly(Proxy).c_str()); + else + { + if (Proxy.Port != 0) + Port = Proxy.Port; + Host = Proxy.Host; + } + return Connect(Host,Port,"http",80,ServerFd,TimeOut,Owner); } - - // Connect to the remote server - if (Connect(Host,Port,"http",80,ServerFd,TimeOut,Owner) == false) - return false; - return true; } /*}}}*/ @@ -707,7 +853,7 @@ void HttpMethod::SendReq(FetchItem *Itm) but while its a must for all servers to accept absolute URIs, it is assumed clients will sent an absolute path for non-proxies */ std::string requesturi; - if (Server->Proxy.empty() == true || Server->Proxy.Host.empty()) + if (Server->Proxy.Access != "http" || Server->Proxy.empty() == true || Server->Proxy.Host.empty()) requesturi = Uri.Path; else requesturi = Uri; @@ -756,7 +902,8 @@ void HttpMethod::SendReq(FetchItem *Itm) else if (Itm->LastModified != 0) Req << "If-Modified-Since: " << TimeRFC1123(Itm->LastModified, false).c_str() << "\r\n"; - if (Server->Proxy.User.empty() == false || Server->Proxy.Password.empty() == false) + if (Server->Proxy.Access == "http" && + (Server->Proxy.User.empty() == false || Server->Proxy.Password.empty() == false)) Req << "Proxy-Authorization: Basic " << Base64Encode(Server->Proxy.User + ":" + Server->Proxy.Password) << "\r\n"; diff --git a/methods/http.h b/methods/http.h index 4d22c88f9..644e576f0 100644 --- a/methods/http.h +++ b/methods/http.h @@ -67,7 +67,7 @@ class CircleBuf // Read data in bool Read(int Fd); - bool Read(std::string Data); + bool Read(std::string const &Data); // Write data out bool Write(int Fd); diff --git a/test/integration/skip-method-http-socks-client b/test/integration/skip-method-http-socks-client new file mode 100755 index 000000000..ee08f6d34 --- /dev/null +++ b/test/integration/skip-method-http-socks-client @@ -0,0 +1,116 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" + +setupenvironment + +# We don't do a real proxy here, we just look how the implementation +# reacts to certain responses from a "proxy" provided by socat +# Checks HTTP, but requesting https instead will check HTTPS (curl) which +# uses different error messages through – also: https://github.com/curl/curl/issues/944 + +# FIXME: Not run automatically as it uses a hardcoded port (5555) + +msgtest 'Check that everything is installed' 'socat' +if dpkg-checkbuilddeps -d 'socat' /dev/null >/dev/null 2>&1; then + msgpass +else + msgskip "$(command dpkg -l socat)" + exit +fi + +runclient() { + # this doesn't need to be an actually reachable webserver for this test + # in fact, its better if it isn't. + rm -f index.html + apthelper download-file http://localhost:2903/ index.html \ + -o Acquire::http::Proxy="socks5h://${1}localhost:5555" \ + -o Acquire::http::Timeout=2 -o Debug::Acquire::http=1 > client.output 2>&1 || true +} +runserver() { + socat -x tcp-listen:5555,reuseaddr \ + system:"echo -n '$*' | xxd -r -p; echo 'HTTP/1.1 200 OK'; echo 'Content-Length: 5'; echo 'Connection: close'; echo; echo 'HTML'" \ + > server.output 2>&1 & +} +PROXY="socks5h://localhost:5555" + +msgmsg 'SOCKS does not run' +runclient +testsuccess grep 'Could not connect to localhost:5555' client.output + +msgmsg 'SOCKS greets back with wrong version' +runserver '04 00' +runclient +testsuccess grep 'greets back with wrong version: 4' client.output + +msgmsg 'SOCKS tries GSSAPI auth we have not advertised' +runserver '05 01' +runclient +testsuccess grep 'greets back having not found a common authorization method: 1' client.output + +msgmsg 'SOCKS tries user&pass auth we have not advertised' +runserver '05 02' +runclient +testsuccess grep 'pass auth, but we had not offered it' client.output + +msgmsg 'SOCKS user:pass wrong version' +runserver '05 02' '05 00' +runclient 'user:pass@' +testsuccess grep 'auth status response with wrong version: 5' client.output + +msgmsg 'SOCKS user:pass wrong auth' +runserver '05 02' '01 01' +runclient 'user:pass@' +testsuccess grep 'reported authorization failure: username or password incorrect? (1)' client.output + +msgmsg 'SOCKS user:pass request not granted no hostname' +runserver '05 02' '01 00' '05 01 00 03 00 1f 90' +runclient 'user:pass@' +testsuccess grep 'grant the connect to :8080 due to: general SOCKS server failure (1)' client.output + +msgmsg 'SOCKS user:pass request not granted with hostname' +runserver '05 02' '01 00' '05 01 00 03 09 68 6f 73 74 6c 6f 63 61 6c 1f 90' +runclient 'user:pass@' +testsuccess grep 'grant the connect to hostlocal:8080 due to: general SOCKS server failure (1)' client.output + +msgmsg 'SOCKS user:pass request not granted ipv4' +runserver '05 02' '01 00' '05 04 00 01 ac 10 fe 01 1f 90' +runclient 'user:pass@' +testsuccess grep 'grant the connect to 172.16.254.1:8080 due to: Host unreachable (4)' client.output + +msgmsg 'SOCKS user:pass request not granted ipv6' +runserver '05 02' '01 00' '05 12 00 04 20 01 0d b8 ac 10 fe 00 00 00 00 00 00 00 00 00 1f 90' +runclient 'user:pass@' +testsuccess grep 'grant the connect to \[2001:0DB8:AC10:FE00:0000:0000:0000:0000\]:8080 due to: Unknown error (18)' client.output + +msgmsg 'SOCKS user:pass request granted ipv4' +runserver '05 02' '01 00' '05 00 00 01 ac 10 fe 01 1f 90' +runclient 'user:pass@' +testequal "http: SOCKS proxy $PROXY connection established to 172.16.254.1:8080" head -n 1 client.output +testfileequal index.html 'HTML' + +msgmsg 'SOCKS user:pass request granted ipv6' +runserver '05 02' '01 00' '05 00 00 04 20 01 0d b8 ac 10 fe 00 00 00 00 00 00 00 00 00 1f 90' +runclient 'user:pass@' +testequal "http: SOCKS proxy $PROXY connection established to [2001:0DB8:AC10:FE00:0000:0000:0000:0000]:8080" head -n 1 client.output +testfileequal index.html 'HTML' + +msgmsg 'SOCKS no auth no hostname' +runserver '05 00 05 00 00 03 00 1f 90' +runclient +testequal "http: SOCKS proxy $PROXY connection established to :8080" head -n 1 client.output +testfileequal index.html 'HTML' + +msgmsg 'SOCKS no auth with hostname' +runserver '05 00 05 00 00 03 09 68 6f 73 74 6c 6f 63 61 6c 1f 90' +runclient +testequal "http: SOCKS proxy $PROXY connection established to hostlocal:8080" head -n 1 client.output +testfileequal index.html 'HTML' + +msgmsg 'SOCKS user-only request granted ipv4' +runserver '05 02' '01 00' '05 00 00 01 ac 10 fe 01 1f 90' +runclient 'apt@' +testequal "http: SOCKS proxy $PROXY connection established to 172.16.254.1:8080" head -n 1 client.output +testfileequal index.html 'HTML' -- cgit v1.2.3-70-g09d2 From c9c910695185b59aa27b787c1a250497e47b492b Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 6 Aug 2016 19:59:57 +0200 Subject: allow methods to be disabled and redirected via config To prevent accidents like adding http-sources while using tor+http it can make sense to allow disabling methods. It might even make sense to allow "redirections" and adding "symlinked" methods via configuration. This could e.g. allow using different options for certain sources by adding and configuring a "virtual" new method which picks up the config based on the name it was called with like e.g. http does if called as tor+http. --- apt-pkg/acquire-worker.cc | 31 +++++++++++++++++----- test/integration/test-apt-https-no-redirect | 15 +++++++++++ .../test-apt-update-failure-propagation | 21 ++++++++++----- test/integration/test-bug-738785-switch-protocol | 20 ++------------ .../test-different-methods-for-same-source | 15 ++--------- 5 files changed, 58 insertions(+), 44 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 1ee78d070..7a4f8177f 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -93,9 +93,22 @@ pkgAcquire::Worker::~Worker() bool pkgAcquire::Worker::Start() { // Get the method path - string Method = _config->FindDir("Dir::Bin::Methods") + Access; + constexpr char const * const methodsDir = "Dir::Bin::Methods"; + std::string const confItem = std::string(methodsDir) + "::" + Access; + std::string Method; + if (_config->Exists(confItem)) + Method = _config->FindFile(confItem.c_str()); + else + Method = _config->FindDir(methodsDir) + Access; if (FileExists(Method) == false) { + if (flNotDir(Method) == "false") + { + _error->Error(_("The method '%s' is explicitly disabled via configuration."), Access.c_str()); + if (Access == "http" || Access == "https") + _error->Notice(_("If you meant to use Tor remember to use %s instead of %s."), ("tor+" + Access).c_str(), Access.c_str()); + return false; + } _error->Error(_("The method driver %s could not be found."),Method.c_str()); std::string const A(Access.cbegin(), std::find(Access.cbegin(), Access.cend(), '+')); std::string pkg; @@ -103,9 +116,15 @@ bool pkgAcquire::Worker::Start() _error->Notice(_("Is the package %s installed?"), pkg.c_str()); return false; } + std::string const Calling = _config->FindDir(methodsDir) + Access; if (Debug == true) - clog << "Starting method '" << Method << '\'' << endl; + { + std::clog << "Starting method '" << Calling << "'"; + if (Calling != Method) + std::clog << " ( via " << Method << " )"; + std::clog << endl; + } // Create the pipes int Pipes[4] = {-1,-1,-1,-1}; @@ -130,11 +149,9 @@ bool pkgAcquire::Worker::Start() SetCloseExec(STDIN_FILENO,false); SetCloseExec(STDERR_FILENO,false); - const char *Args[2]; - Args[0] = Method.c_str(); - Args[1] = 0; - execv(Args[0],(char **)Args); - cerr << "Failed to exec method " << Args[0] << endl; + const char * const Args[] = { Calling.c_str(), nullptr }; + execv(Method.c_str() ,const_cast(Args)); + std::cerr << "Failed to exec method " << Calling << " ( via " << Method << ")" << endl; _exit(100); } diff --git a/test/integration/test-apt-https-no-redirect b/test/integration/test-apt-https-no-redirect index 69e2ba57f..d6c630d5f 100755 --- a/test/integration/test-apt-https-no-redirect +++ b/test/integration/test-apt-https-no-redirect @@ -14,6 +14,7 @@ echo 'alright' > aptarchive/working changetohttpswebserver webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "http://localhost:${APTHTTPPORT}/" webserverconfig 'aptwebserver::redirect::replace::/redirectme2/' "https://localhost:${APTHTTPSPORT}/" +echo 'Dir::Bin::Methods::https+http "https";' > rootdir/etc/apt/apt.conf.d/99add-https-http-method msgtest 'download of a file works via' 'http' testsuccess --nomsg downloadfile "http://localhost:${APTHTTPPORT}/working" httpfile @@ -22,9 +23,23 @@ testfileequal httpfile 'alright' msgtest 'download of a file works via' 'https' testsuccess --nomsg downloadfile "https://localhost:${APTHTTPSPORT}/working" httpsfile testfileequal httpsfile 'alright' +rm -f httpfile httpsfile + +msgtest 'download of http file works via' 'https+http' +testsuccess --nomsg downloadfile "http://localhost:${APTHTTPPORT}/working" httpfile +testfileequal httpfile 'alright' + +msgtest 'download of https file works via' 'https+http' +testsuccess --nomsg downloadfile "https://localhost:${APTHTTPSPORT}/working" httpsfile +testfileequal httpsfile 'alright' +rm -f httpfile httpsfile msgtest 'download of a file does not work if' 'https redirected to http' testfailure --nomsg downloadfile "https://localhost:${APTHTTPSPORT}/redirectme/working" redirectfile msgtest 'libcurl has forbidden access in last request to' 'http resource' testsuccess --nomsg grep -q -E -- "Redirection from https to 'http://.*' is forbidden" rootdir/tmp/testfailure.output + +msgtest 'download of a file does work if' 'https+http redirected to https' +testsuccess --nomsg downloadfile "https+http://localhost:${APTHTTPPORT}/redirectme2/working" redirectfile +testfileequal redirectfile 'alright' diff --git a/test/integration/test-apt-update-failure-propagation b/test/integration/test-apt-update-failure-propagation index ec6bf4a48..1361b1b93 100755 --- a/test/integration/test-apt-update-failure-propagation +++ b/test/integration/test-apt-update-failure-propagation @@ -27,10 +27,11 @@ for FILE in rootdir/etc/apt/sources.list.d/*-sid-* ; do done pretest() { + msgmsg "$@" rm -rf rootdir/var/lib/apt/lists testsuccessequal 'N: Unable to locate package foo' aptcache policy foo } -pretest +pretest 'initialize test' 'update' testsuccess aptget update testsuccessequal "foo: Installed: (none) @@ -41,7 +42,7 @@ testsuccessequal "foo: 1 500 500 https://localhost:${APTHTTPSPORT} stable/main all Packages" aptcache policy foo -pretest +pretest 'not found' 'release files' mv aptarchive/dists/stable aptarchive/dists/stable.good testfailuremsg "E: The repository 'https://localhost:${APTHTTPSPORT} stable Release' does not have a Release file. N: Updating from such a repository can't be done securely, and is therefore disabled by default. @@ -76,7 +77,16 @@ posttest() { } posttest -pretest +pretest 'method disabled' 'https' +echo 'Dir::Bin::Methods::https "false";' > rootdir/etc/apt/apt.conf.d/99disable-https +testfailuremsg "E: The method 'https' is explicitly disabled via configuration. +N: If you meant to use Tor remember to use tor+https instead of https. +E: Failed to fetch https://localhost:${APTHTTPSPORT}/dists/stable/InRelease +E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update +rm -f rootdir/etc/apt/apt.conf.d/99disable-https +posttest + +pretest 'method not installed' 'https' rm "${NEWMETHODS}/https" testfailuremsg "E: The method driver ${TMPWORKINGDIRECTORY}/rootdir/usr/lib/apt/methods/https could not be found. N: Is the package apt-transport-https installed? @@ -84,13 +94,12 @@ E: Failed to fetch https://localhost:${APTHTTPSPORT}/dists/stable/InRelease E: Some index files failed to download. They have been ignored, or old ones used instead." aptget update posttest -ln -s "$OLDMETHODS/https" "$NEWMETHODS" -pretest +pretest 'https connection refused' 'doom port' for FILE in rootdir/etc/apt/sources.list.d/*-stable-* ; do # lets see how many testservers run also Doom sed -i -e "s#:${APTHTTPSPORT}/#:666/#" "$FILE" done -testwarning aptget update +testwarning aptget update -o Dir::Bin::Methods::https="${OLDMETHODS}/https" testequalor2 "W: Failed to fetch https://localhost:666/dists/stable/InRelease Failed to connect to localhost port 666: Connection refused W: Some index files failed to download. They have been ignored, or old ones used instead." "W: Failed to fetch https://localhost:666/dists/stable/InRelease couldn't connect to host W: Some index files failed to download. They have been ignored, or old ones used instead." tail -n 2 rootdir/tmp/testwarning.output diff --git a/test/integration/test-bug-738785-switch-protocol b/test/integration/test-bug-738785-switch-protocol index fa6702eb0..690e8727e 100755 --- a/test/integration/test-bug-738785-switch-protocol +++ b/test/integration/test-bug-738785-switch-protocol @@ -38,28 +38,12 @@ cd - >/dev/null testsuccess aptget install apt -y testdpkginstalled 'apt' -# install a slowed down file: otherwise its to fast to reproduce combining -NEWMETHODS="$(readlink -f rootdir)/usr/lib/apt/methods" -OLDMETHODS="$(readlink -f rootdir/usr/lib/apt/methods)" -rm "$NEWMETHODS" -mkdir "$NEWMETHODS" -backupIFS="$IFS" -IFS="$(printf "\n\b")" -for METH in $(find "$OLDMETHODS" -maxdepth 1 ! -type d); do - ln -s "$OLDMETHODS/$(basename "$METH")" "$NEWMETHODS" -done -IFS="$backupIFS" -rm "$NEWMETHODS/https" - cd downloaded -testfailureequal "E: The method driver $(readlink -f './../')/rootdir/usr/lib/apt/methods/https could not be found. -N: Is the package apt-transport-https installed?" aptget download apt +testfailureequal "E: The method 'https' is explicitly disabled via configuration. +N: If you meant to use Tor remember to use tor+https instead of https." aptget download apt -o Dir::Bin::Methods::https=false testfailure test -e apt_1.0_all.deb cd - >/dev/null -# revert to all methods -ln -s "$OLDMETHODS/https" "$NEWMETHODS" - # check that downgrades from https to http are not allowed webserverconfig 'aptwebserver::support::http' 'true' sed -i -e "s#:${APTHTTPPORT}/redirectme#:${APTHTTPSPORT}/downgrademe#" -e 's# http:# https:#' rootdir/etc/apt/sources.list.d/* diff --git a/test/integration/test-different-methods-for-same-source b/test/integration/test-different-methods-for-same-source index 40138ad5d..791a84cb7 100755 --- a/test/integration/test-different-methods-for-same-source +++ b/test/integration/test-different-methods-for-same-source @@ -10,21 +10,10 @@ insertpackage 'stable' 'foo' 'all' '1' insertsource 'stable' 'foo' 'all' '1' setupaptarchive --no-update -# install a slowed down file: otherwise its to fast to reproduce combining -NEWMETHODS="$(readlink -f rootdir)/usr/lib/apt/methods" -OLDMETHODS="$(readlink -f rootdir/usr/lib/apt/methods)" -rm "$NEWMETHODS" -mkdir "$NEWMETHODS" -backupIFS="$IFS" -IFS="$(printf "\n\b")" -for METH in $(find "$OLDMETHODS" -maxdepth 1 ! -type d); do - ln -s "$OLDMETHODS/$(basename "$METH")" "$NEWMETHODS" -done -IFS="$backupIFS" -ln -s "${OLDMETHODS}/http" "${NEWMETHODS}/http-ng" - changetowebserver webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "http://localhost:${APTHTTPPORT}/" + +echo 'Dir::Bin::Methods::http-ng "http";' > rootdir/etc/apt/apt.conf.d/99add-http-ng-method sed -i -e 's# http:# http-ng:#' $(find rootdir/etc/apt/sources.list.d -name '*-deb-src.list') testsuccess apt update -o Debug::Acquire::http-ng=1 -- cgit v1.2.3-70-g09d2