[MIR] gpsd

Bug #1790855 reported by Christian Ehrhardt 
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gpsd (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

FYI: We want to only seed the two binary packages:
- gpsd
- libgpsd23
But none of the others (further bindings, tools, ...)
They will stay "only" a suggest from Chrony, but we want to add them to the supported seed to reflect their elevated support status.

Availability: GPSD is available since quite a while and builds for all architectures

Rationale:
- The package is the de-facto way to feed GPS HW-based time info into chrony which became the main NTP server with Bionic.
- All users using HW assisted NTP would be glad to have this in main
- It is not a dependency for chrony, but we'd seed it to get into main and add a suggest to chrony (while HW people want it the majority of the community is good without, so no depends/recommend)
- in some way the replacement ntp->chrony was only half of it as ntp had ntp-server AND GPS reading capabilties. This MIR fills the gap created by that.

Security:
- there two (fairly old) CVEs aganst GPSD
  => https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=gpsd
- since the above nothing came up, the project itself is active and vital IMHO
  => https://www.openhub.net/p/gpsd
- One of the issues has a USN, maybe the security team remembers if that was ok or bad back then
  => https://usn.ubuntu.com/1820-1/

Quality assurance:
- After installing the package just needs to be told on which device to work, then it will gather GPS data (that is as minimal as it can be I'd think).
- no debconf on install
- long term this had a few crashes back in 2012-2014 but not much since then (a few actually unrelatred apport reports on postinst issues); nothing should stop considering this for main IMHO
=> https://bugs.launchpad.net/ubuntu/+source/gpsd
=> https://bugs.debian.org/cgi-bin/pkgreport.cgi?repeatmerged=yes&src=gpsd
- The one related important bug IMHO is bug 1790496 which will add apparmor to GPSD which I'd prefer when we grant it main (I wait on a security review there)
- "exotic hardware" is part of the GPSD story we (server team) have two kinds of receivers to test but there is a vast array of potential receivers which we will not be able to test all of them.
- a debian/watch file is in place

UI standards:
- not a UI package

Dependencies:
- Dependencies are sane (all in main and not deprecated)
  GPSD:
  Depends: netbase | systemd-sysv, lsb-base (>= 3.2-13), adduser (>= 3.34), libbluetooth3 (>= 4.91), libc6 (>= 2.27), libdbus-1-3 (>= 1.9.14), libusb-1.0-0 (>= 2:1.0.8), libgps23 (= 3.17-5build1)
  Recommends: udev, python
  LIBGPS23
  Depends: libc6 (>= 2.15), libdbus-1-3 (>= 1.9.14), libstdc++6 (>= 5)
- There are a few universe build-depends, but nothing totally outdated IMHO

Standards compliance:
- meets the FHS
- follows (an older) standard 3.9.2

Maintenance:
- so far was mostly a sync, only now we pick up more work on it.
- DPB confirmed the server team would take over package subscription and maintainership as owning team

Background information:
Receiving GPS signals just to do so would be no core value of Ubuntu and not main-worthy. But being the de-facto way to feed the main ntp server (chrony) in Ubunutu with GPS data to improve time makes it a candidate.

Related branches

CVE References

Revision history for this message
David Britton (dpb) wrote :

+1 for server team ownership of gpsd

Matthias Klose (doko)
Changed in gpsd (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (cyphermox)
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote : Re: [Bug 1790855] [NEW] [MIR] gpsd

Since you're only Suggesting from chrony, this does not actually require
a MIR -- it's place in universe is fine for Suggests, and the package
will be available for installation.

Also, I'm very concerned by this being python2-only. We're still working
hard to replace all python2 things with python3, it doesn't feel right
to add more python2 things in.

And finally, this is a complex package that has a number of lintian
warnings, including missing hardening flags -- assigning to security
team for a more in-depth code review.

NAK for promoting gpsd to main at this time, at the very least until the
package's lintian warnings are fixed and the package modernized to use /
provide python3 bindings instead of python2; and a clear list of the
binary packages to be promoted.

--
Mathieu Trudel-Lapierre <email address hidden>
Freenode: cyphermox, Jabber: <email address hidden>
4096R/65B58DA1 818A D123 0992 275B 23C2 CF89 C67B B4D6 65B5 8DA1

Changed in gpsd (Ubuntu):
assignee: Mathieu Trudel-Lapierre (cyphermox) → Ubuntu Security Team (ubuntu-security)
status: New → Incomplete
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
I wanted to give an update on the current status.
First of all thanks Mathieu for the reivew.

1. Dependeny:
Yes we are only suggesting from chrony and don't "need" it in main for that (we do not intent to depend/recommened there). Instead we want to seed gpsd to be in main (no default install, just be in main) to make up for the lost function of Time-HW support due to the demotion of ntp.

2. python2 concerns
I agree to your concern to not add new py2, but we only want the gpsd package itself, not all the auxiliary tools in main. And this is a compiled binary, without any python. It will dpeend on libgpsd23 library, but that is non-python as well.
file $(dpkg -L libgps23 gpsd | xargs) | cut -c 42-108 | sort | uniq -c
      7 ASCII text
      3 ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamic
      1 POSIX shell script, ASCII text executable
     24 directory
      7 gzip compressed data, max compression, from Unix, original size
      1 symbolic link to ../libgps23/changelog.Debian.gz
      1 symbolic link to libgps.so.23.0.0
      1 0: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamic
      1 gz: gzip compressed data, max compression, from Unix, original size

TL;DR: the binaries we want is gpsd + libgps23 (due to dependency), no python concerns on these

3. Lintian warnings
You are right, I missed to check these (self-slap) I'd try to work on these somewhen next cycle to get things right.
Note to myself: the base package has a python recommends that is not required IMHO

I hope that after the work for #3 and my explanations for #1 and #2 above we are good from the MIR teams side.
Once I'm done on that I'll open it up for your re-review.

I talked with the security team and they will try to check the package. Hopefully we will get all together for 19.04 - updated packaging, apparmor profile, security review - and can then make this available in main.

description: updated
description: updated
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

You misunderstand, I think. Why put it in main at all, if you're not going to be installing it by default? There's just about no use; it's still only Suggested to install. People can install it from universe when they want a hardware time source.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Re: [Bug 1790855] Re: [MIR] gpsd

On Tue, Oct 23, 2018 at 5:02 PM Mathieu Trudel-Lapierre <
<email address hidden>> wrote:

> You misunderstand, I think. Why put it in main at all, if you're not
> going to be installing it by default? There's just about no use; it's
> still only Suggested to install. People can install it from universe
> when they want a hardware time source.
>

To grant it the full statement of support, which we want to provide for at
least one way to get HW time.
As an example strongswan is not default installed, but seeded to be in main
for the same reasons.

I'm on a conference this week, but lets sync on that next weeks MIR meeting.
I want to make clear I really understand your concern.

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

That isn't necessary. You should just be clear that you just want to seed it in the 'supported' seed.

description: updated
Revision history for this message
Alex Murray (alexmurray) wrote :

@cyphermox - this is assigned to the security team for security review but is still marked Incomplete from your questions earlier - plus looks like you also NAK'd it above - is this now ACK'd from your side or is it still blocked - and hence should I un-assign it from the security team?

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi Alex, it was NAK'd from packaging issues.
Those are already split in the Debian repository [1][2] and only wait for an upload to happen.

Since we knew this would resolve the security Team is assigned to do the review already, so that we are ready when the packaging changes make their way into Disco.

That said, for a code review please look at 3.18.1 (which is the version that I expect to have fixed the packaging issues) not the current 3.17.x in Ubuntu Disco.

[1]: https://github.com/bzed/pkg-gpsd/pull/1
[2]: https://github.com/bzed/pkg-gpsd/pull/2

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
all packaging quality issues are adressed in 3.18.1-1 which is in Debian experimental.
The maintainer wants to wait to do the transition until Buster is released.

But we could continue our evaluation here.
1. I'd ask to re-evaluate the package at version 3.18.1-1
   That has fixed all lintian --pedantic warnings and got rid of python2
2. once that is ok we could ask the security Team to do the reivew to eventually be ready once
   Debian makes the move to 3.18.1-1

Setting the state back to new to be re-evaluated.

Changed in gpsd (Ubuntu):
status: Incomplete → New
assignee: Ubuntu Security Team (ubuntu-security) → MIR approval team (ubuntu-mir)
Revision history for this message
Bernd Zeimetz (bzed) wrote :

Actually I do not want to wait, but given the fact that most reverse dependencies do not build due to an undocumented API/ABI change, I just did not have the time to figure out what the correct way to fix this for each reverse dependency is.
The new main gpsd upstream developer was not willing to fix the manpages and documentation after being asked if he could do it. He said he doesn't care about distributions and that the documentation is the code...

So my motivation was not really high enough to work on it.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I didn't realize (or I forgot over Christmas?) that was the reason to make the transition harder than usual :-/
Thanks for the Info Bernd!

Maybe I can make that part of my mission on the bug squashing party, too late for a migration but since we want to resolve it sooner or later.

The main change to the API that I've seen was chaning gps_sock_read in [1]
And for those not interested in the new things this seems to be ok:
- int status = gps_read(gpsdata);
+ int status = gps_read(gpsdata, NULL, 0);

Did you see other breaking API changes that I missed?

[1]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=6bba8b329fc7687b15863d30471d5af402467802

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Wow, that diff is really gross. (And the "bump on incompatible changes" define is even sitting there in the context.) Are we sure this is the best tool for the job?

Thanks

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Yeah I formerly checked only libgps.h, but the list is somewhat bigger but still seems manageable:

API:
7.0 - add gps_fix_t.ecef (February 2018) [3]
      changed prototype of gps_read() to add buffer parameters [1]
      increased length of devconfig_t.subtype [2]
      add gnssid:svid:sigid to satellite_t [4][5]
      add mtime to attitude_t [6]

[1]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=6bba8b329fc7687b15863d30471d5af402467802
[2]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=52aab46cc0c3dca4bcd33d8b1c5e0eb11749ced6
[3]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=9f6d73c779168a6ec276b02a3d08ece8bfc15d51
[4]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=49748a4a8dc2709ec1a3c11ca62d1a2e7d49e733
[5]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=77c19c5f519616f6b82fc7e6d64d918a785f9ba4
[6]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=0220c30ee51a2a776b45564b3eeb0a8a5dc2aaaf

I agree there are non encouraging comments in [3], but this is only extra data no need to use (yet) it in consuming projects.
[2] seems to only need a recompile, but no changes.
[6] needs a rebuild for the struct change, but nothing else as the new field is unused for now.
Finally the complexity of adapting to [4][5] depends on the usage of PRN which might not be used in all projects - those two feel the hardest to me atm.

@Seth - it is the "best tool for the job", because it is the only one that feeds HW time data into chrony.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I further found potential changes that need adaptations in:
- Savannah issue #53671: policy_t -> gps_policy_t. [7]
- ATT: centralize clearing of the attitude data. [8]

[7]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=25edf0e836200ee4f6cf86fa4456635ee210484f
[8]: https://git.savannah.gnu.org/cgit/gpsd.git/commit/?id=a7dc77f0c6730f97e5dfbaefa70bfa3730caaeff

[7] might only need a rename if the name is used externally, [8] seems to be a nop for consuming code unless you want to use the new gps_clear_att.

I'm stopping this preevaluation for now, until I find time to really look at all the rev-deps using GPS and trying to fix them.

Revision history for this message
Bernd Zeimetz (bzed) wrote :

Over the last years, the gpsd upstream usually did a very good job on ABI/API changes. This time its ... sad.

Revision history for this message
Bernd Zeimetz (bzed) wrote :

I've subscribed Eric S. Raymond on this bug so he can see whats going on...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

While I'll continue to work with Bernd to get the newer GPSD in (post Buster) which will make it qualify for MAIN inclusion I wanted to share that I just realized that it was already in main in precise - I thought it is worth to share that nugget of information.

Revision history for this message
Seth Arnold (seth-arnold) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

3.19-2 is now in Debian/unstable and will soon sync to Ubuntu.
That means the changes in 3.18 to resolve the packaging issues are in too.
Thanks Bernd for all the work.

In addition 3.19 has an apparmor profile and while 3.18 added py3 3.19 now dropped py2 which makes this a clear MIR ack now.

It already is on the security Teams list, lets also reflect that in the bug assignment.

Changed in gpsd (Ubuntu):
assignee: MIR approval team (ubuntu-mir) → Ubuntu Security Team (ubuntu-security)
Changed in gpsd (Ubuntu):
status: New → Triaged
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I helped to get the lib transition done and 3.19-3 is not only on proposed but in focal-release for real.

@Security - this was in your queues already, but now it is all yours - no other known todo left except sec-review and then seeding+promotion.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI - server team now is already subscribed to this, that way we see ahead of time how much traffic is on this package and we can't forget it later on :-) Thanks Josh.

Revision history for this message
Alex Murray (alexmurray) wrote :
Download full text (7.4 KiB)

I reviewed gpsd 3.20-1 as checked into focal. This shouldn't be considered
a full audit but rather a quick gauge of maintainability. As noted in the
MIR description, this review only covers code and binaries which are
contained in the gpsd and libgps25 packages.

gpsd is a daemon (and associated library) to handle input from GPS
devices. The daemon translates the various GPS reciever inputs into a
standard format and multiplexes this out to the various connected
clients. It can also be used to connect to a remote gpsd instance as a
source as well. For the purpose of this MIR, gpsd is the standard way to
provide GPS time sources to chrony, which is now used as the time
synchronisation daemon in Ubuntu.

- CVE History:
  - 3 CVEs in project history (CVE-2004-1388 CVE-2013-2038 CVE-2018-17937)
  - 1 is still open for xenial, bionic and disco (CVE-2018-17937)
  - All fixed upstream in a timely manner
- Build-Depends
  - Nothing of note here (no crypto / networking libraries)
- pre/post inst/rm scripts
  - gpsd.postinst setups up the gpsd user account
  - gpsd.postrm removes the gpsd user account on purge
- No init scripts
- systemd units are used to spawn gpsdctl (see below) for GPS devices, and
  to manage the local GPSD socket and hence launch the GPSD daemon on
  demand when a local client connects
- No dbus services
- No setuid binaries
- gpsd binary package provides the following binaries in PATH
  - /usr/bin/ppscheck
  - /usr/sbin/gpsd
  - /usr/sbin/gpsdctl
- AppArmor policy:
  - /etc/apparmor.d/usr.sbin.gpsd
    - This looks quite sensible and provides access to the commonly used
      components and paths plus includes a site-specific
      <local/usr.sbin.gpsd> profile to allow for easy customisation
- No sudo fragments
- Uses udev rules to ensure GPS devices have correct name (/dev/gps%n) and
  the right systemd service (gpsdctl@%k) is spawned - the gpsdctl service
  spawns /usr/sbin/gpsdctl for each device which inturn spawns a gpsd
  instance to read from that device and set the device permissions so that
  the gpsd binary can still read from the device once it drops privileges
- unit tests / autopkgtests
  - Some unit tests are run during the build
  - autopkgtest exists - this does a basic test that the gpsd service and
    socket are available and active and that the output from the socket is
    the bare-minimum expected
- No cron jobs
- Build logs:
  - A single compile-time warning comes from an included system Qt header
    file using a deprecated copy method
    - /usr/include/x86_64-linux-gnu/qt5/QtCore/qvariant.h:275:25: warning: implicitly-declared 'constexpr QVariant::Private& QVariant::Private::operator=(const QVariant::Private&)' is deprecated [-Wdeprecated-copy]
  - Warning when generating man pages:
    - W: libgps-dev: manpage-has-errors-from-man usr/share/man/man5/gpsd_json.5.gz 272: warning [p 2, 2.2i, div '3tbd15,3', 0.8i]: can't break line
  - dpkg warnings:
    - dpkg-gensymbols: warning: debian/libqgpsmm25/DEBIAN/symbols doesn't match completely debian/libqgpsmm25.symbols
    - dpkg-gencontrol: warning: Recommends field of package gpsd: substitution variable ${python:Depends} used, but is not defined
      (this...

Read more...

Revision history for this message
Alex Murray (alexmurray) wrote :
Changed in gpsd (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Changed in gpsd (Ubuntu):
status: Triaged → In Progress
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Great, thanks Alex.
This is ready from MIR+Security POV now.
=> Set in progress according to our state chart

The latest version is in Focal already:
 gpsd | 3.20-1 | focal/universe | source, amd64, arm64, armhf, ppc64el, s390x

=> There will be no change in e.g. chrony required to support gpsd.
=> This will pull in `gpsd` (the daemon) and `libgps25` (lib used by the daemon).
   None of the other binaries in this source should be pulled in by that and that
   is exactly what we want.
=> due to auto inclusion once source is in main this will also pull in `libgps-dev`
   and `libgps-dev`, those two are safe as they only depend on things in main and
   fromt he gpsd package - but it will bring in `python3-gps` as well
=> the source also contains `libqgpsmm-dev` but this part of it is not meant to be supported.
   Therefore there will be an Extra-Exclude: libqgpsmm-dev for that.

Therefore this has two MPs:
- https://code.launchpad.net/~paelzer/ubuntu-seeds/+git/platform/+merge/377415
- https://code.launchpad.net/~paelzer/ubuntu-seeds/+git/ubuntu/+merge/377416

Revision history for this message
Bernd Zeimetz (bzed) wrote :

On 2020-01-10 08:18, Christian Ehrhardt  wrote:
> Great, thanks Alex.
> This is ready from MIR+Security POV now.
> => Set in progress according to our state chart
>
> The latest version is in Focal already:
> gpsd | 3.20-1 | focal/universe | source, amd64, arm64,
> armhf, ppc64el, s390x

Please note that this missed an soname bump, upstream forgot it and
I didn't spot the changes. There will be -3 with libgps26 soon.

--
  Bernd Zeimetz Debian GNU/Linux Developer
  http://bzed.de http://www.debian.org
  GPG Fingerprint: ECA1 E3F2 8E11 2432 D485 DD95 EB36 171A 6FF9 435F

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This had too many dependencies yesterday.
But when I was looking for that today I found that Steve already added

commit 7193ff159faf338718240e3fb9c0ae90d91d3624
Author: Steve Langasek <email address hidden>
    Exclude gpsd-dbg along with libqgpsmm-dev to keep Qt out of main
- * Extra-Exclude: libqgpsmm-dev
+ * Extra-Exclude: libqgpsmm-dev gpsd-dbg

With that in place the mismatch matches what we expected and gpsd + libgps26 is in main now.
Setting fix released.

Changed in gpsd (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

root@f:~# apt-cache policy libgps-dev gpsd
libgps-dev:
  Installed: (none)
  Candidate: 3.20-3
  Version table:
     3.20-3 500
        500 http://archive.ubuntu.com/ubuntu focal/main amd64 Packages
gpsd:
  Installed: (none)
  Candidate: 3.20-3
  Version table:
     3.20-3 500
        500 http://archive.ubuntu.com/ubuntu focal/main amd64 Packages

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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