[MIR] tracker-miners

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

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?

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.

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?

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.

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

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.

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

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

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.

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

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...

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

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.

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

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