Comment 15 for bug 1455644

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

I reviewed ippusbxd version 1.21.2-1 as checked into wily; this shouldn't
be considered a full security audit but rather a quick gauge of
maintainability.

- ippusbxd implements the usb-ipp standardized printer bridge;
  udev rules start the daemon when a supported printer is plugged in,
  exposing the printer to loopback interfaces.
- Build-Depends: debhelper, libusb-1.0-0-dev, cmake, pkg-config, dh-apparmor
- Provides a daemon
- start_daemon() does not properly daemonize:
  - doesn't set umask()
  - doesn't setsid()
  - doesn't chdir(/)
  - doesn't set signals to expected dispositions
- pre,post inst,rm scripts all automatically generated
- No initscripts
- No dbus services
- No setuid executables
- One executable, /usr/sbin/ippusbxd
- No sudo fragments
- No udev rules -- they must be packaged elsewhere?
- No tests
- No cronjobs
- One warning in build log looks harmless

- No subprocesses spawned
- Memory management looked careful
- No files are written to
- Logging looked safe
- No environment variables
- No privileged operations
- No cryptography
- No privileged portions of code
- No temporary file use
- No WebKit
- No javascript
- No policykit
- Clean cppcheck

The code quality is good, with a few caveats: the software doesn't
properly daemonize at startup; and the AppArmor profile doesn't look
like it's been used lately.

Till was very responsive to the in6addr_any issue and associated
requests.

Here's the remaining issues that I found, they may or may not be important
enough to fix, though the AppArmor profile probably needs to be updated to
allow the daemon to function:

- AppArmor profile needs to be updated
- usb_conn_acquire(), typo in text that may be user-visible, "aloc"
- there appears to be no way to stop the usb_pump_events() thread
- Really should setsid(), chdir(/), and set signal dispositions for
  reliable operation (umask is less important since the daemon creates no files)

Security team ACK to promote ippusbxd to main.

Thanks