Use https config on https proxies for http servers

The settings used for unwrapping TLS connections depend on the access
and hostname we connect to more than what we eventually unwrap. The
bugreport mentions CaInfo, but all other https-settings should also
apply (regardless of generic or hostname specific) to an https proxy,
even if the connection we proxy through it is http-only.

Closes: #990555

5 files changed, 119 insertions, 69 deletions

diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index bd50e8078..afc761cc5 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -43,7 +43,71 @@ static bool hasDoubleColon(std::string const &n)
    return n.find("::") != std::string::npos;
 }
 
-class aptMethod : public pkgAcqMethod
+class aptConfigWrapperForMethods
+{
+protected:
+   std::vector<std::string> methodNames;
+public:
+   void setPostfixForMethodNames(char const * const postfix) APT_NONNULL(2)
+   {
+      methodNames.erase(std::remove_if(methodNames.begin(), methodNames.end(), hasDoubleColon), methodNames.end());
+      decltype(methodNames) toAdd;
+      for (auto && name: methodNames)
+	 toAdd.emplace_back(name + "::" + postfix);
+      std::move(toAdd.begin(), toAdd.end(), std::back_inserter(methodNames));
+   }
+
+   bool DebugEnabled() const
+   {
+      if (methodNames.empty())
+	 return false;
+      auto const sni = std::find_if_not(methodNames.crbegin(), methodNames.crend(), hasDoubleColon);
+      if (unlikely(sni == methodNames.crend()))
+	 return false;
+      auto const ln = methodNames[methodNames.size() - 1];
+      // worst case: all three are the same
+      std::string confln, confsn, confpn;
+      strprintf(confln, "Debug::Acquire::%s", ln.c_str());
+      strprintf(confsn, "Debug::Acquire::%s", sni->c_str());
+      auto const pni = sni->substr(0, sni->find('+'));
+      strprintf(confpn, "Debug::Acquire::%s", pni.c_str());
+      return _config->FindB(confln,_config->FindB(confsn, _config->FindB(confpn, false)));
+   }
+   std::string ConfigFind(char const * const postfix, std::string const &defValue) const APT_NONNULL(2)
+   {
+      for (auto name = methodNames.rbegin(); name != methodNames.rend(); ++name)
+      {
+	 std::string conf;
+	 strprintf(conf, "Acquire::%s::%s", name->c_str(), postfix);
+	 auto value = _config->Find(conf);
+	 if (not value.empty())
+	    return value;
+      }
+      return defValue;
+   }
+   std::string ConfigFind(std::string const &postfix, std::string const &defValue) const
+   {
+      return ConfigFind(postfix.c_str(), defValue);
+   }
+   bool ConfigFindB(char const * const postfix, bool const defValue) const APT_NONNULL(2)
+   {
+      return StringToBool(ConfigFind(postfix, defValue ? "yes" : "no"), defValue);
+   }
+   int ConfigFindI(char const * const postfix, int const defValue) const APT_NONNULL(2)
+   {
+      char *End;
+      std::string const value = ConfigFind(postfix, "");
+      auto const Res = strtol(value.c_str(), &End, 0);
+      if (value.c_str() == End)
+	 return defValue;
+      return Res;
+   }
+
+   explicit aptConfigWrapperForMethods(std::string const &name) : methodNames{{name}} {}
+   explicit aptConfigWrapperForMethods(std::vector<std::string> &&names) : methodNames{std::move(names)} {}
+};
+
+class aptMethod : public pkgAcqMethod, public aptConfigWrapperForMethods
 {
 protected:
    std::string const Binary;
@@ -397,61 +461,6 @@ protected:
       SendMessage("104 Warning", std::move(fields));
    }
 
-   std::vector<std::string> methodNames;
-   void setPostfixForMethodNames(char const * const postfix) APT_NONNULL(2)
-   {
-      methodNames.erase(std::remove_if(methodNames.begin(), methodNames.end(), hasDoubleColon), methodNames.end());
-      decltype(methodNames) toAdd;
-      for (auto && name: methodNames)
-	 toAdd.emplace_back(name + "::" + postfix);
-      std::move(toAdd.begin(), toAdd.end(), std::back_inserter(methodNames));
-   }
-   bool DebugEnabled() const
-   {
-      if (methodNames.empty())
-	 return false;
-      auto const sni = std::find_if_not(methodNames.crbegin(), methodNames.crend(), hasDoubleColon);
-      if (unlikely(sni == methodNames.crend()))
-	 return false;
-      auto const ln = methodNames[methodNames.size() - 1];
-      // worst case: all three are the same
-      std::string confln, confsn, confpn;
-      strprintf(confln, "Debug::Acquire::%s", ln.c_str());
-      strprintf(confsn, "Debug::Acquire::%s", sni->c_str());
-      auto const pni = sni->substr(0, sni->find('+'));
-      strprintf(confpn, "Debug::Acquire::%s", pni.c_str());
-      return _config->FindB(confln,_config->FindB(confsn, _config->FindB(confpn, false)));
-   }
-   std::string ConfigFind(char const * const postfix, std::string const &defValue) const APT_NONNULL(2)
-   {
-      for (auto name = methodNames.rbegin(); name != methodNames.rend(); ++name)
-      {
-	 std::string conf;
-	 strprintf(conf, "Acquire::%s::%s", name->c_str(), postfix);
-	 auto const value = _config->Find(conf);
-	 if (value.empty() == false)
-	    return value;
-      }
-      return defValue;
-   }
-   std::string ConfigFind(std::string const &postfix, std::string const &defValue) const
-   {
-      return ConfigFind(postfix.c_str(), defValue);
-   }
-   bool ConfigFindB(char const * const postfix, bool const defValue) const APT_NONNULL(2)
-   {
-      return StringToBool(ConfigFind(postfix, defValue ? "yes" : "no"), defValue);
-   }
-   int ConfigFindI(char const * const postfix, int const defValue) const APT_NONNULL(2)
-   {
-      char *End;
-      std::string const value = ConfigFind(postfix, "");
-      auto const Res = strtol(value.c_str(), &End, 0);
-      if (value.c_str() == End)
-	 return defValue;
-      return Res;
-   }
-
    bool TransferModificationTimes(char const * const From, char const * const To, time_t &LastModified) APT_NONNULL(2, 3)
    {
       if (strcmp(To, "/dev/null") == 0)
@@ -498,7 +507,7 @@ protected:
    }
 
    aptMethod(std::string &&Binary, char const *const Ver, unsigned long const Flags) APT_NONNULL(3)
-       : pkgAcqMethod(Ver, Flags), Binary(Binary), SeccompFlags(0), methodNames({Binary})
+       : pkgAcqMethod(Ver, Flags), aptConfigWrapperForMethods(Binary), Binary(std::move(Binary)), SeccompFlags(0)
    {
       try {
 	 std::locale::global(std::locale(""));
diff --git a/methods/connect.cc b/methods/connect.cc
index 044984403..bc2fe1de5 100644
--- a/methods/connect.cc
+++ b/methods/connect.cc
@@ -894,7 +894,8 @@ struct TlsFd : public MethodFd
 };
 
 ResultState UnwrapTLS(std::string const &Host, std::unique_ptr<MethodFd> &Fd,
-		      unsigned long Timeout, aptMethod *Owner)
+		      unsigned long const Timeout, aptMethod * const Owner,
+		      aptConfigWrapperForMethods const * const OwnerConf)
 {
    if (_config->FindB("Acquire::AllowTLS", true) == false)
    {
@@ -940,7 +941,7 @@ ResultState UnwrapTLS(std::string const &Host, std::unique_ptr<MethodFd> &Fd,
    }
 
    // Credential setup
-   std::string fileinfo = Owner->ConfigFind("CaInfo", "");
+   std::string fileinfo = OwnerConf->ConfigFind("CaInfo", "");
    if (fileinfo.empty())
    {
       // No CaInfo specified, use system trust store.
@@ -965,20 +966,20 @@ ResultState UnwrapTLS(std::string const &Host, std::unique_ptr<MethodFd> &Fd,
       }
    }
 
-   if (!Owner->ConfigFind("IssuerCert", "").empty())
+   if (not OwnerConf->ConfigFind("IssuerCert", "").empty())
    {
       _error->Error("The option '%s' is not supported anymore", "IssuerCert");
       return ResultState::FATAL_ERROR;
    }
-   if (!Owner->ConfigFind("SslForceVersion", "").empty())
+   if (not OwnerConf->ConfigFind("SslForceVersion", "").empty())
    {
       _error->Error("The option '%s' is not supported anymore", "SslForceVersion");
       return ResultState::FATAL_ERROR;
    }
 
    // For client authentication, certificate file ...
-   std::string const cert = Owner->ConfigFind("SslCert", "");
-   std::string const key = Owner->ConfigFind("SslKey", "");
+   std::string const cert = OwnerConf->ConfigFind("SslCert", "");
+   std::string const key = OwnerConf->ConfigFind("SslKey", "");
    if (cert.empty() == false)
    {
       if ((err = gnutls_certificate_set_x509_key_file(
@@ -993,7 +994,7 @@ ResultState UnwrapTLS(std::string const &Host, std::unique_ptr<MethodFd> &Fd,
    }
 
    // CRL file
-   std::string const crlfile = Owner->ConfigFind("CrlFile", "");
+   std::string const crlfile = OwnerConf->ConfigFind("CrlFile", "");
    if (crlfile.empty() == false)
    {
       if ((err = gnutls_certificate_set_x509_crl_file(tlsFd->credentials,
@@ -1017,9 +1018,9 @@ ResultState UnwrapTLS(std::string const &Host, std::unique_ptr<MethodFd> &Fd,
       return ResultState::FATAL_ERROR;
    }
 
-   if (Owner->ConfigFindB("Verify-Peer", true))
+   if (OwnerConf->ConfigFindB("Verify-Peer", true))
    {
-      gnutls_session_set_verify_cert(tlsFd->session, Owner->ConfigFindB("Verify-Host", true) ? tlsFd->hostname.c_str() : nullptr, 0);
+      gnutls_session_set_verify_cert(tlsFd->session, OwnerConf->ConfigFindB("Verify-Host", true) ? tlsFd->hostname.c_str() : nullptr, 0);
    }
 
    // set SNI only if the hostname is really a name and not an address
diff --git a/methods/connect.h b/methods/connect.h
index bd6507761..413484aa3 100644
--- a/methods/connect.h
+++ b/methods/connect.h
@@ -42,7 +42,8 @@ ResultState Connect(std::string To, int Port, const char *Service, int DefPort,
 		    std::unique_ptr<MethodFd> &Fd, unsigned long TimeOut, aptMethod *Owner);
 
 ResultState UnwrapSocks(std::string To, int Port, URI Proxy, std::unique_ptr<MethodFd> &Fd, unsigned long Timeout, aptMethod *Owner);
-ResultState UnwrapTLS(std::string const &To, std::unique_ptr<MethodFd> &Fd, unsigned long Timeout, aptMethod *Owner);
+ResultState UnwrapTLS(std::string const &To, std::unique_ptr<MethodFd> &Fd, unsigned long Timeout, aptMethod *Owner,
+		      aptConfigWrapperForMethods const * OwnerConf);
 
 void RotateDNS();
 
diff --git a/methods/http.cc b/methods/http.cc
index b6d754037..2a5ab2cd2 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -518,7 +518,9 @@ ResultState HttpServerState::Open()
 	 return result;
       if (Host == Proxy.Host && Proxy.Access == "https")
       {
-	 result = UnwrapTLS(Proxy.Host, ServerFd, TimeOut, Owner);
+	 aptConfigWrapperForMethods ProxyConf{std::vector<std::string>{"http", "https"}};
+	 ProxyConf.setPostfixForMethodNames(Proxy.Host.c_str());
+	 result = UnwrapTLS(Proxy.Host, ServerFd, TimeOut, Owner, &ProxyConf);
 	 if (result != ResultState::SUCCESSFUL)
 	    return result;
       }
@@ -531,7 +533,7 @@ ResultState HttpServerState::Open()
    }
 
    if (tls)
-      return UnwrapTLS(ServerName.Host, ServerFd, TimeOut, Owner);
+      return UnwrapTLS(ServerName.Host, ServerFd, TimeOut, Owner, Owner);
 
    return ResultState::SUCCESSFUL;
 }
diff --git a/test/integration/test-bug-990555-https-proxy-for-http b/test/integration/test-bug-990555-https-proxy-for-http
new file mode 100755
index 000000000..f43abfd92
--- /dev/null
+++ b/test/integration/test-bug-990555-https-proxy-for-http
@@ -0,0 +1,37 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+buildsimplenativepackage 'unrelated' 'all' '1' 'unstable'
+
+setupaptarchive --no-update
+changetowebserver --request-absolute='uri'
+changetohttpswebserver --no-rewrite
+
+msgtest 'Check that non-absolute paths are' 'not accepted'
+testfailure --nomsg aptget update --allow-insecure-repositories
+
+echo "Acquire::http::Proxy \"https://localhost:${APTHTTPSPORT}\";" > rootdir/etc/apt/apt.conf.d/99proxy
+
+msgtest 'Check that requests to https proxies' 'work from http'
+testsuccess --nomsg aptget update
+
+testsuccessequal 'Reading package lists...
+Building dependency tree...
+The following NEW packages will be installed:
+  unrelated
+0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst unrelated (1 unstable [all])
+Conf unrelated (1 unstable [all])' apt install unrelated -s
+
+testsuccess apt download unrelated --print-uris
+testfailure grep 'https:' rootdir/tmp/testsuccess.output
+
+cd downloaded
+testsuccess apt download unrelated
+testsuccess test -s 'unrelated_1_all.deb'
+cd ..