Thanks for the review! I'd like to get a bit more details about some of the comments to understand what steps I can take to improve it. Please let me know if you prefer to take this discussion to another medium.
> - udev rules -- appear to be configured for works-by-default behaviour, > some examples on how to configure for authorization-required would be > nice I'm not sure I understand what examples and what configuration this comment refers to. > - No tests, a bit unfortunate This is about the package or the upstream project? Upstream we have some tests (using umockdev). See here for CI: https://travis-ci.org/intel/thunderbolt-software-user-space > - File IO done via RAII-C++ classes, not exactly obvious when it happens Any specific question that I can answer (or maybe even document in the code)? > - No PolicyKit My assumption (and is mentioned here and there in the code) is that eventually the tool will be better if it does start using PolicyKit for the privileged actions. It'd be nice to here what do you think about it or if there are guidelines from the distro on where, when and how to use it. > It uses std::random_device for security uses -- I believe this is safe but > direct use of getrandom(2) would not have questions about underlying C++ > library implementation choices. Noted. flags=0 is good enough, yes? (I will may have to make sure first that all the relevant distros include this syscall already.) Thanks again! -- You received this bug notification because you are a member of Kernel Packages, which is subscribed to thunderbolt-tools in Ubuntu. Matching subscriptions: Kernel Packages https://bugs.launchpad.net/bugs/1748157 Title: [MIR] thunderbolt-tools Status in thunderbolt-tools package in Ubuntu: Confirmed Bug description: == Overview == Intel Thunderbolt userspace components provides components for using Intel Thunderbolt controllers with security level features. Thunderbolt™ technology is a transformational high-speed, dual protocol I/O that provides unmatched performance with up to 40Gbps bi- directional transfer speeds. It provides flexibility and simplicity by supporting both data (PCIe, USB3.1) and video (DisplayPort) on a single cable connection that can daisy-chain up to six devices. [ See https://github.com/intel/thunderbolt-software-user-space ] == Answers to UbuntuMainInclusionRequirements == = Requirements = 1. Availability Package is in universe: https://launchpad.net/ubuntu/+source/thunderbolt-tools 2. Rationale Package a device enabler for users with Thunderbolt technology 3. Security: No security issues exposed so far. However, the tools have only been in Ubuntu since 2017-12-09, so this currently is less than the 90 days threshold. 4. Quality assurance: * Manual is provided * No debconf questions higher than medium * No major outstanding bugs. I'm also helping Intel fix issues that I'm finding with static analysis tools such as scan-build, cppcheck and CoverityScan Bugs outstanding: #883857 please backport for stretch-backports #882525 thunderbolt-tools: FTBFS on kFreeBSD: _ZN5boost6system15system_categoryEv undefined - I can fix this, but it makes no sense to run on kFreeBSD * Exotic Hardware: Only Thunderbolt supported H/W is required, this is an industry standard and the support for the tools are in the 4.13+ kernels * No Test Suite shipped with the package * Does not rely on obsolete or demoted packages 5. UI standards: * This is a CLI tool. Tool has normal CLI style short help and man pages * No desktop file required as it is a CLI tool. 6. Binary Dependencies: libboost-dev (main) libboost-filesystem-dev (main) libboost-program-options-dev (main) udev (main) 7. Standards compliance: lintian clean and meets the FHS + Debian Policy standards to the best of my knowledge 8. Maintenance * Package owning team: The Ubuntu Kernel Team * Debian package maintained by Colin Ian King (myself from the Kernel Team) 9. Background Information The user-space components implement device approval support: a. Easier interaction with the kernel module for approving connected devices. b. ACL for auto-approving devices white-listed by the user. Tools provided by this package: tbtacl - triggered by udev (see the udev rules in tbtacl.rules). It auto-approves devices that are found in ACL. tbtadm - user-facing CLI tool. It provides operations for device approval, handling the ACL and more. The user-space components operate in coordination with the upstream Thunderbolt kernel driver (found in v4.13) to provide the Thunderbolt functionalities. These components are NOT compatible with the old out-of-tree Thunderbolt kernel module. = Security checks = http://cve.mitre.org/cve/cve.html: Search in the National Vulnerability Database using the package as a keyword * No CVEs found http://secunia.com/advisories/search/: search for the package as a keyword * No security advisories found Ubuntu CVE Tracker http://people.ubuntu.com/~ubuntu-security/cve/main.html * No http://people.ubuntu.com/~ubuntu-security/cve/universe.html * No http://people.ubuntu.com/~ubuntu-security/cve/partner.html * No Check for security relevant binaries. If any are present, this requires a more in-depth security review. Executables which have the suid or sgid bit set. * Not applicable Executables in /sbin, /usr/sbin. * None in these paths Packages which install daemons (/etc/init.d/*) * No Packages which open privileged ports (ports < 1024). * No Add-ons and plugins to security-sensitive software (filters, scanners, UI skins, etc) * This does exec tbtacl from udev with new udev rules, so this needs security checking To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/thunderbolt-tools/+bug/1748157/+subscriptions -- Mailing list: https://launchpad.net/~kernel-packages Post to : kernel-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~kernel-packages More help : https://help.launchpad.net/ListHelp
