I reviewed policykit-unity8 version 0.1+16.10.20160829-0ubuntu1 as
checked into yakkety. This should not be considered a full security audit;
in fact, this code deserves a much closer inspection.
- policykit-unity8 provides a policykit agent for unity8, to integrate
dialogs from polkit into the unity8 interface.
- Build-Depends: astyle, clang-format | clang-format-3.4, click-dev,
cmake, cmake-extras, dbus, dbus-test-runner, debhelper (>= 9),
google-mock, libdbustest1-dev, libglib2.0-dev, libgtest-dev,
libnotify-dev, libproperties-cpp-dev, libpolkit-agent-1-dev,
python3-dbusmock
- Does not use encryption
- Does not use networking
- Does not provide a daemon
- Does not listen on an external interface
- Runs as user
- No pre/post inst/rm scripts
- No init scripts
- No dbus service files, extensive dbus use
- No setuid files
- No binaries in the PATH
- No sudo fragments
- No udev rules
- Does run three tests during the build -- I did not spot any
authentication failure tests, however
- No cron jobs
- Clean build logs
- No subprocesses spawned
- Memory management looked careful
- Only file IO was to /dev/rfkill
- Logging looked careful
- Only uses URFKILL_NO_SYSLOG environment variable
- Does not perform privileged operations
- Does not perform cryptography
- Does not perform networking
- No privileged portions of code
- No temporary files
- No WebKit
- No javascript
- clean cppcheck and shellcheck
- Uses polkit, including polkit_unix_session_new_for_process_sync()
How is this API safe to use? pids are not a reliable process identifier,
they are subject to race conditions. I did not see any polkit *session*
APIs that would use a safe process identifier.
This is complicated code, using complicated APIs, and I did not inspect
it as closely as it deserves. The 'small' picture looks good: errors
are checked, the coding looked professional, logs and static analysis
look good. I'm less clear about the larger picture.
I'm concerned that the polkit_unix_session_new_for_process_sync()
API may be unsuitable for use and yet not clearly documented as such; I'm
concerned that extensive use of threading to implement and dbus to
communicate security policy is racy and may allow confusing users or
software alike. I'm concerned that polkit is too generic to be useful and
yet may not be flexible enough to fully interact with PAM.
Please add -- or document here -- negative tests that exercise failure
and 'cancel' pathways. If they don't yet exist, please have them checked
into yakkety before release.
Security team ACK for promoting policykit-unity8 to main.
Thanks
** Changed in: policykit-unity8 (Ubuntu)
Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1617067
Title:
[MIR] PolicyKit Unity8
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/policykit-unity8/+bug/1617067/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
