no_proxy ignored if https_proxy set

[Patrick Cable]

Description:
When using the https transport mechanism, $no_proxy is ignored if apt is getting it's proxy information from $https_proxy. If the source of proxy information is Acquire::https::Proxy set in apt.conf (or apt.conf.d), then $no_proxy is honored.

Package Versions:
1.0.1ubuntu2.13 on 14.04 LTS
1.2.10ubuntu1 on 16.04 LTS

Expected result:
Apt would pull down my packages from my internal repository without trying to put it through the proxy.

I've submitted a patch upstream that seems to fix the behavior, see https://github.com/Debian/apt/pull/14.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "fix-https-noproxy.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Patrick Cable (5-pc)]

Updated the patch with the help of the debian-apt upstream.

[Brian Murray (brian-murray)]

Would you mind updating the bug description according to the the Stable Release Update process?

https://wiki.ubuntu.com/StableReleaseUpdates#SRU_Bug_Template

Having a test case will help us get the bug SRUed.

[Patrick Cable (5-pc)]

Sure thing. SRU to follow:

[Impact]
This bug breaks users who use a proxy to access externally-hosted https apt repositories, but do not need to use a proxy to access internally hosted https apt repositories. Specifically, if a user has a https_proxy (or http_proxy and no https_proxy) setting in their environment, apt will no longer honor no_proxy.

The normal http transport mechanism handles this fine, so backporting will allow the expected behavior to work for both http and https transports and save system administrators in some proxy environments some frustration :-)

The previous behavior only checked for no_proxy in the event that Acquire::https::Proxy or Acquire::http::Proxy was set. If they weren't, then the https transport mechanism would pull environment variables, but not check the no_proxy variable. This patch moves the no_proxy check earlier in the code so that if no_proxy is set and the host matches the function returns earlier.

[Test Case]
1) Ensure that Acquire::http::Proxy and Acquire::https::Proxy are not set in /etc/apt/apt.conf or /etc/apt/apt.conf.d/*.
2) Set https_proxy and no_proxy in the environment.
3) Run apt-get update. Apt will still attempt to use https_proxy on a host that is named in no_proxy.

I tested this behavior in Trusty and Xenial. It'd be wonderful if I didn't have to maintain a patch package for Trusty, but I understand if you don't backport to out of date LTSes.

[Regression Potential]
The patch does not introduce any new code to the branch, only a) moves the placement of a check that would return out the function earlier and b) changes the control flow such that a check for https_proxy and http_proxy will be performed each time HttpsMethod::SetupProxy() is called. I would say the potential for regression is low, though now each time SetupProxy is called it will be running getenv(). The performance overhead of such a thing is not significant.

[Other Info]
I worked with the folks in the debian-apt channel on oftc on this to reach an easier-to-read refactor and intended functionality, and they've already merged it in upstream. See http://anonscm.debian.org/git/apt/apt.git/commit/?id=42ba3fa1ec004acbddf5266559bd76428d904206. Given that this is my first patch, I'm not sure what else to address here. Thanks!

[Julian Andres Klode (juliank)]

We are pushing for SRUing APT 1.2.12 in xenial in Bug #1580952

[Julian Andres Klode (juliank)]

Fixed in 1.2.12 and 1.3~exp1 in yakkety, xenial SRU is tracked in #1580952

[Patrick Cable (5-pc)]

Question: Is there a process by which to get this fix into Trusty as well? The code diff is the same.

[Patrick Cable (5-pc)]

I've created a debdiff for patching Trusty for this bug.

[Patrick Cable (5-pc)]

I've also created a debdiff for patching Precise for this bug.

[Brian Murray (brian-murray)]

I'll review these shortly as a part of my patch pilot shift today. Thanks for working on the debdiffs and the test case.

[Brian Murray (brian-murray)]

It'd probably be nice to fix this in wily too.

[Brian Murray (brian-murray)]

The trusty debdiff has UNRELEASED in the changelog, but I'll fix that.

[Patrick Cable (5-pc)]

I can toss together one for Wily tomorrow; I figured keeping unreleased in the changelog was important for debdiffs that hadn't been accepted yet.

[Martin Pitt (pitti)]

Hello Patrick, or anyone else affected,

Accepted apt into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/1.0.1ubuntu2.14 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Martin Pitt (pitti)]

Hello Patrick, or anyone else affected,

Accepted apt into precise-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/0.8.16~exp12ubuntu10.27 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Patrick Cable (5-pc)]

Precise: tested failure condition, added -proposed repo, updated apt and retested -- all good.
Trusty: tested failure condition, added -proposed repo, updated apt and retested -- all good.
Xenial: tested failure condition, added -proposed repo, updated apt and retested -- all good.

Not sure how to add the tags, must be missing something in the interface? "Fix Released" doesnt seem like the appropriate thing to select (for me anyways)

Will work on a debdiff for Wily now.

[Patrick Cable (5-pc)]

Here's a debdiff for fixing Wily.

[Brian Murray (brian-murray)]

I've sponsored the wily patch, thanks again for working on this.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apt - 0.8.16~exp12ubuntu10.27

---------------
apt (0.8.16~exp12ubuntu10.27) precise; urgency=low

  * When using the https transport mechanism, $no_proxy is ignored if apt is
    getting it's proxy information from $https_proxy (as opposed to
    Acquire::https::Proxy somewhere in apt config). If the source of proxy
    information is Acquire::https::Proxy set in apt.conf (or apt.conf.d),
    then $no_proxy is honored. This patch makes the behavior similar for
    both methods of setting the proxy. (LP: #1575877)

 -- Patrick Cable <email address hidden> Tue, 17 May 2016 13:49:45 -0700

[Martin Pitt (pitti)]

The verification of the Stable Release Update for apt has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apt - 1.0.1ubuntu2.14

---------------
apt (1.0.1ubuntu2.14) trusty; urgency=medium

  * When using the https transport mechanism, $no_proxy is ignored if apt is
    getting it's proxy information from $https_proxy (as opposed to
    Acquire::https::Proxy somewhere in apt config). If the source of proxy
    information is Acquire::https::Proxy set in apt.conf (or apt.conf.d),
    then $no_proxy is honored. This patch makes the behavior similar for
    both methods of setting the proxy. (LP: #1575877)

 -- Patrick Cable <email address hidden> Tue, 17 May 2016 13:45:06 -0700

[Martin Pitt (pitti)]

Hello Patrick, or anyone else affected,

Accepted apt into wily-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/1.0.10.2ubuntu3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Patrick Cable (5-pc)]

Just tested in Wily -- replicated failure condition, added -proposed repo, updated apt and retested and it's all good. Thanks!

[Simon Déziel (sdeziel)]

@Patrick, thanks for working on this. When you have done the verification, you can update the tags list that's below the initial issue description at the top. More details on that: https://wiki.ubuntu.com/StableReleaseUpdates#Verification

[Launchpad Janitor (janitor)]

This bug was fixed in the package apt - 1.0.10.2ubuntu3

---------------
apt (1.0.10.2ubuntu3) wily; urgency=medium

  * When using the https transport mechanism, $no_proxy is ignored if apt is
    getting it's proxy information from $https_proxy (as opposed to
    Acquire::https::Proxy somewhere in apt config). If the source of proxy
    information is Acquire::https::Proxy set in apt.conf (or apt.conf.d),
    then $no_proxy is honored. This patch makes the behavior similar for
    both methods of setting the proxy. (LP: #1575877)

 -- Patrick Cable <email address hidden> Wed, 18 May 2016 08:09:20 -0700