Comment 5 for bug 1752056

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

Hello, I reviewed bolt version 0.1-0ubuntu1 as packaged in bionic. This
should not be considered a full security review but rather a quick gauge
of maintainability.

- There are no CVEs in our database
- Bolt provides an interface to authorize Thunderbolt devices on a
  computer; Thunderbolt devices have immense power over the safety of the
  computer.
- Build-Depends: debhelper, meson, libglib2.0-dev, libudev-dev,
  libumockdev-dev, libpolkit-gobject-1-dev, systemd, udev
- boltd runs as a daemon (but does not daemonize)
- boltd owns the org.freedesktop.bolt name and similar path, interface,
  and device interface names, on dbus
- boltctl connects to the daemon via dbus and provides subcommands:
  authorize, enroll, forget, info, list, and monitor
- pre/post init/rm scripts automatically generated
- bolt.service file is Type=dbus and uses systemd Protect* and Restrict*
  fields
- dbus activation file
- no setuid files
- only boltctl in PATH -- boltd is in /usr/lib/x86_64-linux-gnu/boltd
- no sudo fragments
- udev configuration, starts bolt service via systemd on
  subsystem==thunderbolt

- No subprocesses spawned
- sysfs files are opened and written to for authorizing and providing keys
- logging looked careful
- Three environment variables used, TERM, BOLT_DBPATH, GIO_USE_VFS,
  appeared to be used safely
- No cryptography
- No networking
- No privileged portions of code
- No temporary files
- No WebKit
- No JavaScript
- Clean cppcheck
- Uses polkit polkit_authority_check_authorization_sync()

bolt is perhaps more complex than it needs to be due to using GObject and
glib extensively; it can be difficult to track the flow of execution
through the layers of indirection. That said, error returns are checked
extensively, and code quality is high.

I'm concerned that using polkit's auth_admin_keep rule may allow a
malicious device to add new, unexpected, capabilities after an initial
authorization: https://github.com/gicmo/bolt/issues/73

There's a warning during the build:
https://github.com/gicmo/bolt/issues/74

meson.build:355: WARNING: Trying to compare values of different types
(str, bool) using !=.

Security team ACK for promoting bolt to main.

Thanks