Comment 10 for bug 1816548

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

I reviewed usbguard 0.7.4+ds-1 as checked into eoan. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

usbguard consists of a daemon which manages the authorization of new USB
devices via udev events. It provides an IPC interface (which by default is
only accessible by the root user but the Debian packaging adds the wheel
and usbguard groups) which allows it to be controlled - this uses protobuf
for serialization. A separate daemon, usbguard-dbus provides an interface
over the DBus system bus which uses PolicyKit to ensure only administrator
users can access privileged methods (such as authorizing new devices or
changing the overall policy etc). Also provides an audit backend so that it
can provide audit events. Includes support for restricting itself with a
built-in seccomp policy which is enabled in the Debian package.

A separate binary usbguard provides a CLI to interact with the daemon via
the IPC interface.

Finally, a separate binary package, usbguard-applet-qt provides a GUI to
control the daemon using it's own IPC interface. Users who use the applet
are assumed to be in the wheel or usbguard groups to enable them to control
the daemon.

- CVE History:
  - None
- Build-Depends
  - Huge list including Qt, GLib, DBus, PolicyKit, libseccomp, libaudit and
    docbook,xsltproc,pandoc etc for documentation generation
- pre/post inst/rm scripts
  - postinst creates empty copies of and fixes permissions on
    /etc/usbguard/rules.conf and /etc/usbguard/usbguard-daemon.conf so it
    is only accessible by the owner
    - Is this normal? I note this is flagged as a WARNING in csp-source.txt
      by uaudit.
  - postrm removes the above two configuration files and the configuration
    dir /etc/usbguard if empty.
- init scripts
  - None
- systemd units
  - usbguard daemon system service - runs usbguard-daemon with the default
    configuration and restarts on failure
  - usbguard-dbus system service - runs usbguard-dbus on DBus system bus
    with restart on failure
- dbus services
  - org.usbguard.system service
    - provides an interface to interact with
- setuid binaries
  - None
- binaries in PATH
  - /usr/bin/usbguard
  - /usr/sbin/usbguard-daemon
  - /usr/sbin/usbguard-dbus
- sudo fragments
  - None
- udev rules
  - None
- unit tests
  - Includes some unit tests which run during the build
  - No autopkgtests
- cron jobs
  - None
- Build logs:
  - No significant warnings / errors in the build log

- Processes spawned
  - usbguard watch command supports spawning a process as specified on the
    command-line - so no direct chance for shell command-injection here
- Memory management
  - There is not a lot of dynamic memory management - what there is appears
    to be quite clean and careful
- File IO
  - The configuration file for the daemon is read carefully and the path to
    this is hardcoded if NOT overridden by a command-line parameter
  - Files in sysfs are written to to authorize devices
  - umask is defensively set even though in general not many files are
    created by usbguard.
- Logging
  - Logging is careful
- Environment variable usage
  - Uses USBGUARD_DEBUG to enable more logging output
  - USBGUARD_DAEMON_CONF can be used to override the path to the daemon
    configuration
- Use of privileged functions
  - None
- Use of cryptography / random number sources etc
  - None
- Use of temp files
  - None
- Use of networking
  - None
- Use of WebKit
  - None
- Use of PolicyKit
  - Provides a PolicyKit policy to constrain access to the DBus interface -
    this looks sane

- No significant cppcheck results.
- No significant Coverity results.

Overall the code is quite clean and with no CVE history and no significant
issues noted during this security review. One question is do we just promote the
usbguard package itself to main or do we include the usbguard-applet-qt
package as well since until the GNOME integration lands (see
https://gitlab.gnome.org/GNOME/gnome-control-center/merge_requests/366 and
others) this has no user visible front-end to control it without the applet.

Security team ACK for promoting usbguard to main once the ownership of the
package is resolved and the various other issues identified in comment:4
above are addressed in some way.