mirror.fail - security issue in mirror:// - CVE-2018-0501

Bug #1787752 reported by Julian Andres Klode on 2018-08-18
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apt (Ubuntu)
High
Unassigned
Bionic
Undecided
Unassigned

Bug Description

Report from donkult to Debian team:

Hi,

Package: libapt-pkg5.0 (source: apt)
Affected: >= 1.6~alpha6, released Wed, 03 Jan 2018 22:33:37 +0000
 aka: NOT in stable, but sid/testing and derivatives based on it
 e.g. Ubuntu Bionic (18.04)
Severity: allows man-in-the-middle attackers to bypass repository-signing
 protection mechanisms if the mirror:// family of transports is used
Status: not public, found by the author of the bug, preview patch attached

APT in 1.6 saw me rewriting the mirror:// transport method, which works
comparable to the decommissioned httpredir.d.o "just" that apt requests
a mirror list and performs all the redirections internally with all the
bells like parallel download and automatic fallback (more details in the
apt-transport-mirror manpage included in the 1.6 release).

The automatic fallback is the problem here: The intend is that if a file
fails to be downloaded (e.g. because the mirror is offline, broken,
out-of-sync, …) instead of erroring out the next mirror in the list is
contacted for a retry of the download.

Internally the acquire process of an InRelease file (works with the
Release/Release.gpg pair, too) happens in steps: 1) download file and 2)
verify file, both handled as URL requests passed around. Due to an
oversight the fallbacks for the first step are still active for the
second step, so that the successful download from another mirror stands
in for the failed verification… *facepalm*

Note that the attacker can not judge by the request arriving for the
InRelease file if the user is using the mirror method or not. If entire
traffic is observed Eve might be able to observe the request for
a mirror list, but that might or might not be telling if following
requests for InRelease files will be based on that list or for another
sources.list entry not using mirror (Users have also the option to have
the mirror list locally (via e.g. mirror+file://) instead of on a remote
host). If the user isn't using mirror:// for this InRelease file apt
will fail very visibly as intended.

(The mirror list needs to include at least two mirrors and to work
reliably the attacker needs to be able to MITM all mirrors in the list.
For remotely accessed mirror lists this is no limitation as the attacker
is in full control of the file in that case)

Attached patch adds the one line fixing this (and moves a pimpl class
further to the top to make that valid compilable code). mirror:// is at
the moment the only user of this code infrastructure (for all others
this set is already empty), so there shouldn't be an opportunity for
regression here even through a central code area is changed. The patch
includes a test showcasing the problem and that it bypasses even
additional measures like signed-by.

Upgrade instructions: Given all apt-based frontends are affected and the
attack in progress is hardly visible in the progress reporting of an
update operation (the InRelease file is marked "Ign", but no fallback to
"Release/Release.gpg" is happening) and leaves no trace (expect files
downloaded from the attackers repository of course) the best course of
action might be to change the sources.list to not use the mirror family
of transports ({tor+,…}mirror{,+{http{,s},file,…}}) until a fixed
version of the src:apt packages are installed.

It might be best to coordinate Debian unstable/Ubuntu devel uploads with
Julian Andres Klode (CC'ed) as my free time is less predictable so
I will leave the schedule all up to you.

Codewise the patch should be complete already if nothing unforeseen
happens in further testing, so the final version should be "just"
a complete commit with message (consider this mail a draft for it),
CVE and stuff included.

Feel free to forward to anyone as needed; I have not contacted anyone
else about this.

Aside:
I am genuinely surprised that it was this easy to break apt as in
hindsight its perfectly obvious and should have raised a billion red
flags while implementing it … as I hope we can avoid calling that
"davidfail" I am proposing "mirrorfail" and a shattering mirror as icon.
On the upside: I always wanted to promote the use of mirror:// a bit
more – I guess I can cross that of the todolist now as its usage will no
doubt be discussed now while I will try to hide in shame behind a mirror.

Thanks, Sorry & Best regards

David Kalnischkies

CVE References

Julian Andres Klode (juliank) wrote :

Debian has been informed already

description: updated
summary: - security issue in mirror://
+ security issue in mirror:// - CVE-2018-0501

Summary and upgrade advise:

APT 1.6 and newer failed to verify InRelease files that were retrieved
as a fallback from one mirror to another when using the mirror:// method.

As the mirror method can thus not be trusted, it is suggested to replace
use of the mirror method in sources.list files with the standard http
method before upgrading to a fixed version.

Julian Andres Klode (juliank) wrote :

s/suggested/highly recommended/

Julian Andres Klode (juliank) wrote :
Changed in apt (Ubuntu Bionic):
status: New → Confirmed
Julian Andres Klode (juliank) wrote :

Debdiff for bionic attached. Tested by running autopkgtest on it - the test for this bug, test/integration/test-cve-2018-0501-mirror-alternatives must be executable, not sure if it is when applying the diff.

I plan to upload the change to Debian unstable as 1.6.4 and for experimental and cosmic as part of 1.7.0~alpha3 when the update is being released for bionic.

Julian Andres Klode (juliank) wrote :

Registered mirror.fail domain name for this bug :)

summary: - security issue in mirror:// - CVE-2018-0501
+ mirror.fail - security issue in mirror:// - CVE-2018-0501
Alex Murray (alexmurray) wrote :

Thanks for reporting this - since this does not affect Debian stable (only bionic and any distros based on bionic) there doesn't seem to be much need for wider disclosure of this issue at this stage (especially given that mirror:// is a non-standard, opt-in configuration and so not widely deployed by default and so the impact of this bug is not widespread)

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 1.6.3ubuntu0.1

---------------
apt (1.6.3ubuntu0.1) bionic-security; urgency=medium

  [ David Kalnischkies ]
  * SECURITY UPDATE: Fallback in the mirror method allowed a later server to
    supply any InRelease file without it having to be verified. (LP: #1787752)
    - apt-pkg/acquire-item.cc:: clear alternative URIs for mirror:// between
      steps
    - CVE-2018-0501

 -- Julian Andres Klode <email address hidden> Mon, 20 Aug 2018 09:48:01 +0200

Changed in apt (Ubuntu Bionic):
status: Confirmed → Fix Released
information type: Private Security → Public Security
tags: added: id-5b7a7ccffdbc885f187f02fb
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 1.7.0~alpha3

---------------
apt (1.7.0~alpha3) experimental; urgency=medium

  [ David Kalnischkies ]
  * SECURITY UPDATE: Fallback in the mirror method allowed a later server to
    supply any InRelease file without it having to be verified. (LP: #1787752)
    - apt-pkg/acquire-item.cc:: clear alternative URIs for mirror:// between steps
    - CVE-2018-0501
    - https://mirror.fail/

  [ Jean-Ralph Aviles ]
  * Add trailing newline to output of edit-sources.

  [ Julian Andres Klode ]
  * Add support for dpkg frontend lock (Closes: #869546)
  * Set DPKG_FRONTEND_LOCKED as needed when doing selection changes
  * Update symbols files

  [ Boyuan Yang ]
  * Simplified Chinese program translation update (Closes: #903695)

  [ David Kalnischkies ]
  * Report (soon) worthless keys if gpg uses fpr for GOODSIG

 -- Julian Andres Klode <email address hidden> Mon, 20 Aug 2018 17:44:19 +0200

Changed in apt (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers