[MIR] tracker-miners

Bug #1770877 reported by Jeremy Bicha on 2018-05-12
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
tracker-miners (Ubuntu)
Undecided
Unassigned

Bug Description

Availability
============
Built for all supported architectures. In sync with Debian (except we dropped 2 Build-Depends: libiptcdata and libosinfo)

Rationale
=========
The Ubuntu Desktop team intends to include tracker by default in Ubuntu 18.10 because it is considered a core part of GNOME and strongly recommended by the Nautilus maintainer since it is required for several popular features to work correctly.

tracker is already in main. Last year, upstream split tracker-miners out of the tracker source, so technically this already used to be in main. The tracker MIR was LP: #1313996

Security
========
Tracker is a security-sensitive service. See Other Info for more about the 2016 issues.

https://security-tracker.debian.org/tracker/source-package/tracker-miners
https://security-tracker.debian.org/tracker/source-package/tracker
https://launchpad.net/ubuntu/+source/tracker-miners/+cve
https://launchpad.net/ubuntu/+source/tracker/+cve

Quality assurance
=================
- Ubuntu Desktop Bugs is subscribed

https://bugs.launchpad.net/ubuntu/+source/tracker-miners
https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=tracker-miners
https://bugzilla.gnome.org/buglist.cgi?quicksearch=product%3A"tracker"

Tests are run during the build.
No autopkgtests.

Dependencies
============
Some optional binary universe dependencies are enabled since they used to be in main
- libcue LP: #1770871
- libgsf LP: #1770874

Other optional binary universe dependencies are enabled in Debian but are disabled in Ubuntu:
- libiptcdata
- libosinfo

Standards compliance
====================
4.1.4, dh compat 11, dh7 style simple rules

Maintenance
===========
Maintained by the Debian GNOME team

https://salsa.debian.org/gnome-team/tracker-miners

upstream:
https://gitlab.gnome.org/GNOME/tracker-miners

Other Info
==========
https://blogs.gnome.org/carlosg/2016/12/08/oh-the-security/

https://scarybeastsecurity.blogspot.dk/2016/11/

Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in tracker-miners (Ubuntu):
status: New → Confirmed
Jeremy Bicha (jbicha) on 2018-05-14
description: updated
Matthias Klose (doko) wrote :

Not yet reviewed, however

 - there are two dozen crashes reported in Ubuntu. Are these all fixed?

 - Is the desktop team ready to support that package?

Changed in tracker-miners (Ubuntu):
status: Confirmed → Incomplete
Sebastien Bacher (seb128) wrote :

The segfaults are mostly in libraries that tracker use to get details about specific file formats, not in that component itself. The bugslist is small enough and there is no reason to believe the component is especially buggy

The desktop team is wanting to maintain that package and the desktop-bugs team has been subscribed to the package bugs.

Changed in tracker-miners (Ubuntu):
status: Incomplete → New
Jeremy Bicha (jbicha) on 2018-08-15
description: updated
Matthias Klose (doko) wrote :

Didier handled the previous tracker MIR, so assigning that to him. Are any of the libraries that were disabled for the MIR coming back into main with this upload?

Changed in tracker-miners (Ubuntu):
assignee: nobody → Didier Roche (didrocks)
Didier Roche (didrocks) wrote :

Looks mostly good. The debian packaging is fine, use dh_missing --fail-missing to ensure we ship everything we want. It has autopkgtests doing unit testing under X. The only missing thing is the trailing commas on deps! :p

Some questions:

* Any chance we can get a .symbol instead of a shlibs for better ABI and symbol tracking?
* debian/30-tracker.conf: do we have an idea of the impact of the global system for this settings?
* There is one lintian error (not overridden): E: tracker-extract: library-not-linked-against-libc usr/lib/x86_64-linux-gnu/tracker-miners-2.0/extract-modules/libextract-text.so (may be linked to the rpath issue mentioned above)
* (Wish) The override doesn't have any comment on why rpath is acceptable for the binary. I didn't find anything in debian/changelog mentioning it as well.
* (Wish) debian/tracker-miner-fs.preinst: would be good to use the internal maintainerscript tool to rm the conffiles, or just drop the whole conffiles removal as testing has 2.0.4-3, and it's not in stable, so nobody should get that case anymore.
* (Opened question/Wish) I'm unsure why we have dbus and autostart activation. It doesn't seem to me that the service pauses after a while. Is there a way we can really only on one way of activating it, like dbus only?

Notes:

> Other optional binary universe dependencies are enabled in Debian but are disabled in Ubuntu:
> - enca
> - libiptcdata
> - libosinfo
That looks indeed good, even if we are in sync with Debian. Note for security team: the conditional disabling is in debian/rules.

I have seen some local caches being world readable, I think many years after after another security review would be great.

Jeremy Bicha (jbicha) wrote :

1. I don't understand the lintian libc error. Cairo gets it too.
https://<email address hidden>#cairo

2. https://salsa.debian.org/gnome-team/tracker/commit/8d56fdf0
says "The rpath is used by tracker-store to load the private libraries in /usr/lib/tracker-0.10"
We can mention that in the lintian overrides files.

3. tracker-extract is a private library so that's why the shlib thing is used. See
https://salsa.debian.org/gnome-team/tracker-miners/commit/97c81629

So I think the rpath and shlib issues are connected.

4. debian/30-tracker.conf was added because Tracker depends on inotify watches. The default in Ubuntu Desktop is 8192 which isn't enough for users with a lot of indexed files. The tracker developers (and I guess distros like Fedora and Debian) don't believe there should be noticeable performance impact from using inotify watches. Some disagree. See bug 1666676 and https://askubuntu.com/q/154255/

5. The xdg autostart thing is bug 1697769 . The bug is annoying enough that it's mentioned in the Bionic Release Notes.

6. The preinst can't be removed yet because Debian stable only has 1.10.5-1 because it was still part of the tracker source package then. The complicated handling is because it's being moved from /etc/ to /usr/lib/ . I think sometimes we wouldn't bother keeping local modifications to that file, but since Michael already did the work, there's no reason to remove it now.
https://salsa.debian.org/gnome-team/tracker-miners/commit/fd0471478

Didier Roche (didrocks) wrote :

1. Would be great to have a good explanation for this. Mind looking?
2. ACK, please add a mention
3. 4. 5. 6. -> ack

Apart from 1. which would need further investigation, it looks like we are good on my side. I will still request the security team to review though, as per previous comments.

Changed in tracker-miners (Ubuntu):
assignee: Didier Roche (didrocks) → Ubuntu Security Team (ubuntu-security)
Jeremy Bicha (jbicha) wrote :

1. It looks like there is agreement that it is a false-positive in Lintian triggered by the Debian GNOME team being fond of -Wl,--as-needed.
https://bugs.debian.org/909267

2. This was done in git.
https://salsa.debian.org/gnome-team/tracker-miners/commit/be9b9ab0

Note that for anyone reviewing this. We had to pull the latest version of tracker and tracker-miners from Ubuntu 18.10 (in part because this isn't in main yet and we couldn't have nautilus depend on tracker yet. Some more details are at LP: #1793550 .)

So please review this for the latest code and packaging.

gbp clone https://salsa.debian.org/gnome-team/tracker-miners
gbp buildpackage

Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in tracker-miners (Ubuntu):
status: New → Confirmed
Iain Lane (laney) wrote :

Just wanted to update this bug that I'd like to upload Nautilus depending on tracker (and therefore this tracker-miners: this MIR) soon - we would like to have plenty of time to work out any issues if they arise. So if a security review could be forthcoming sooner rather than later that would help us out.

Alex Murray (alexmurray) on 2018-11-27
Changed in tracker-miners (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → Alex Murray (alexmurray)
Jeremy Bicha (jbicha) wrote :

Also, we fixed the 1793550 issue that I mentioned in comment 8. The latest stable release of tracker is in Ubuntu 19.04.

description: updated
Alex Murray (alexmurray) wrote :
Download full text (4.4 KiB)

I reviewed tracker-miners (2.0.4-2) as checked into disco. This is not
a full security audit but rather a quick gauge of maintainability.

tracker-miners is used to automatically crawl and index a users documents and
other files, extracting useful metadata from the documents, images, videos and
other various file types and storing it in the main tracker store so it can be
queried by other applications (gnome-documents, nautilus etc)

- Build dependencies:
  - libglib2.0-dev, libgstreamer1.0-dev, libgstreamer-plugins-base1.0-dev,
    libstemmer-dev, libtracker-miner-2.0-dev, libtracker-sparql-2.0-dev,
    libpoppler-glib-dev, libgsf-1-dev, libexif-dev, libgexiv2-dev, libpng-dev,
    libtiff-dev, libvorbis-dev, libflac-dev, libtotem-plparser-dev, zlib1g-dev,
    libexempi-dev, libxml2-dev, libupower-glib-dev, libenca-dev,
    libiptcdata0-dev, valac, libgif-dev, libgxps-dev, libosinfo-1.0-dev,
    libtagc0-dev, libcue-dev, libseccomp-dev, dbus
  - Note this includes valac however at no point in the build log does this
    appear to actually be used / executed - should this be removed?

- No CVE history itself but has been used to leverage other vulnerabilities
  automatically in the past
  https://scarybeastsecurity.blogspot.com/2016/11/0day-poc-risky-design-decisions-in.html
  - this is now mitigated by a seccomp sandbox - see below and
    https://blogs.gnome.org/carlosg/2016/12/08/oh-the-security/ for some
    discussion of this

- preinst script to remove old obsolete conffile /etc/sysctl.d/30-tracker.conf
- postinst script on configure to increases the maximum number of user inotify
  watches via sysctl
- three user systemd unit files, dbus-activated and auto-started on login
- no system dbus services
- no setuid files
- no binaries in PATH
- no sudo fragments
- no udev rules
- tests run during the build, quite a few tests, but I'm unsure of coverage
- no cron jobs
- clean build log

- one instance of spawning a process, to spawn gunzip to extract compressed
  postscript files; it uses g_spawn_async_with_pipes() with an array for argv
  and reads the decompressed postscript from stdin of the spawned process
- memory management looked careful
- extensive file IO - all opened read-only and a bunch of custom parsers for
  different file formats, as well as piggybacking of external libs to extract
  metadata
- logging looked fine
- Uses environment variables: TRACKER_USE_CONFIG_FILES, TRACKER_EXTRACTORS_DIR,
  TRACKER_EXTRACTOR_RULES_DIR, TRACKER_BUS_TYPE,
  TRACKER_TEST_DOMAIN_ONTOLOGY_RULE, TRACKER_DB_ONTOLOGIES_DIR,
  TRACKER_USE_LOG_FILES, TRACKER_VERBOSITY, LANG, HOME
  - Will also try and evaluate anything that looks like an environment variable
    within the list of user configured directories to index - so if user
    specified "$HOME/Downloads" this will evaluate $HOME etc
- uses ioctl() to read FAT filesystem attributes to check if is marked hidden
  or not
- Does not use cryptography
- DBus is used to communicate back to main tracker store with results of
  extracted files
- Does not use webkit
- Temporary file handling uses g_mkstemp_full() during atomic writeback to
  files
- Does not use javascript
- No cppcheck errors
- Does no...

Read more...

Changed in tracker-miners (Ubuntu):
assignee: Alex Murray (alexmurray) → nobody
Alex Murray (alexmurray) wrote :

Whoops - just noticed the comment re which version to review - will take a look at the suggested version in https://salsa.debian.org/gnome-team/tracker-miners

Alex Murray (alexmurray) on 2018-11-28
Changed in tracker-miners (Ubuntu):
assignee: nobody → Alex Murray (alexmurray)
Alex Murray (alexmurray) wrote :

I reviewed tracker-miners (2.1.5-3) as checked into git at
https://salsa.debian.org/gnome-team/tracker-miners from the debian/master
branch. This is not a full security audit but rather a quick gauge of
maintainability.

Review is identical to above (main changes are ensuring a bunch of metadata is extracted as UTF-8, fixing a number of memory leaks and changes to where functional tests are located).

Other than the list of build dependencies has increased slightly:
  - libglib2.0-dev, libgstreamer1.0-dev, libgstreamer-plugins-base1.0-dev,
    libstemmer-dev, libtracker-miner-2.0-dev, libtracker-sparql-2.0-dev,
    libpoppler-glib-dev, libgsf-1-dev, libexif-dev, libgexiv2-dev, libpng-dev,
    libtiff-dev, libvorbis-dev, libdbus-1-dev, libflac-dev,
    libtotem-plparser-dev, zlib1g-dev, libexempi-dev, libxml2-dev,
    libupower-glib-dev, libicu-dev, libiptcdata0-dev, tracker, valac,
    libgif-dev, libgxps-dev, libosinfo-1.0-dev, libtagc0-dev, libcue-dev,
    libseccomp-dev, dbus, dbus-x11, procps, shared-mime-info,

Security team ACK to promote to main.

Changed in tracker-miners (Ubuntu):
assignee: Alex Murray (alexmurray) → nobody
Iain Lane (laney) wrote :

Marking as good per comments #7 and #14, to make component-mismatches colour this a nice colour. Thanks all.

Changed in tracker-miners (Ubuntu):
status: Confirmed → Fix Committed
Sebastien Bacher (seb128) wrote :

tracker-miners 2.1.5-3ubuntu1 in disco: universe/utils -> main

Changed in tracker-miners (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.