Review for Package: swtpm The package is overall in good shape, already has plenty of users and use-cases despite being rather new to the archive. Nevertheless some polishing seems to be needed. This does need a security review - even more so for any potential guest->host attack surface, so I'll assign ubuntu-security To make quick progress I'll already pass it on to security despite that we are waiting for these mentioned improvements/cleanups. MIR team ACK, under the condition of fulfillling the outlined required and at least looking at recommended TODOs. - List of specific binary packages to be promoted to main: swtpm, swtpm-tools - Specific binary packages built, but NOT to be promoted to main: none Required TODOs: - Please cut the dependency of swtpm-tools to gnutls-bin as discussed as it would otherwise imply more MIRs. - libtpms-dev doesn't exist on ppc64el and thereby IMHO blocks too many important use cases from being generally working. Please investigate to add that and/or explain why this shall be considered not a problem. - The autopkgtest suite needs to pass (actually run) on arm64 (stalled by long queue) Recommended TODOs: - While the lib is internal, .symbols tracking usually is cheap and protects even internal libs from some mistakes, consider adding it. - Version 0.7.0 seems rather close please update it later this cycle https://github.com/stefanberger/swtpm/issues/587 - The package should get a team bug subscriber before being promoted Right now I do not see it subscribed by "foundations-bugs" yet - evaluate the possibility and impact of having "tcsd" in the build environment - Have a look at bug 1949155 if you want to change anything for it Already completed TODOs: - Please fixup the user/group creation (see bug 1949060) [Duplication] There is no other package in main providing the same functionality. [Dependencies] OK: - no other Dependencies to MIR due to this (This = bin:swtpm) - checked with check-mir - not listed in seeded-in-ubuntu - none of the built reverse-depends are in universe - no -dev/-debug/-doc packages that need exclusion - No dependencies in main that are only superficially tested requiring more tests now. Problems: - Dependencies of swtpm-tools: - swtpm-tools would depend on gnutls-bin which is a binary not in main from src:gnutls28 in main. - This in turn depends pkg:libopts25 on from autogen (universe) After chatting with Steve, we want to keep swtpm-tools to be promoted (as it is needed by e.g. libvirt), but cut the dependency of swtpm-tools to gnutls-bin. [Embedded sources and static linking] - no embedded source present - no static linking - does not have odd Built-Using entries - not a go package, no extra constraints to consider in that regard [Security] OK: - does not use webkit1,2 - does not use lib*v8 directly - does not process arbitrary web content - does not use centralized online accounts - does not integrate arbitrary javascript into the desktop - does not deal with system authentication (eg, pam), etc) Problems: - does not run a daemon as root, but is often run with too high privilegded (user tss), which is why we have spawned bug 1948880 and bug 1949060 - does parse data formats (partically VM guest controlled) - does open a port/socket (in the common use-case) - does deal with security attestation - history of CVEs does not look concerning, but the use case make it a target [Common blockers] OK: - does not FTBFS currently - does have a test suite that runs at build time - test suite fails will fail the build upon error. - does have a non-trivial test suite that runs as autopkgtest (same as the build time test) => https://autopkgtest.ubuntu.com/packages/swtpm - no new python2 dependency - Python package, but using dh_python - Go package, but using dh-golang Problems: - Does not build on ppc64el, since this is meant to be used for encrypted disk and VM tpm usage I think it might need to work on all architectures. => https://launchpad.net/ubuntu/+source/swtpm/0.6.1-0ubuntu2/+build/22337599 libtpms-dev isn't available there, I guess we need to have a look - The package is present on arm64 swtpm | 0.6.1-0ubuntu2 | jammy/universe | source, amd64, arm64, armhf, riscv64, s390x And the test suite only has: Restrictions: allow-stderr Depends: @, @builddeps@ Nevertheless, it is not run on arm64 yet (super long test queue). Before promotion we should see this working. [Packaging red flags] OK: - Ubuntu does not carry a delta (not yet in Debian) - d/watch is present and looks ok - Upstream update history is good - promoting this does not seem to cause issues for MOTUs that so far maintained the package - d/rules is rather clean - It is not on the lto-disabled list Problems: - The current release is packaged, but we are just a bit away from 0.7.0 which would be nice to get before Ubutnu 22.04 completes - Debian/Ubuntu update history is undefined (rather new) - symbols tracking is not in place, I know it is only an internal lib But the increased effort usually is minimal and it helps to catch e.g. unwanted effects of backports. - Some Lintian warnings around postinst issues, I've brought them up as part of bug 1949060 already [Upstream red flags] OK: - no incautious use of malloc/sprintf (as far as we can check it) - no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside tests) - no use of user nobody - no use of setuid - use of setuid, but ok because (prefer systemd to set those for services) - no important open bugs (crashers, etc) in Debian or Ubuntu - no dependency on webkit, qtwebkit, seed or libgoa-* - not part of the UI for extra checks - no translation present, but none needed for this case (user visible)? Problems: - use of user "tss" which comes with device permissions we do not want/need. I know you are aware, but since I found a few related issues I spawned bug 1949060 for it. - Errors/warnings during the build show "configure: WARNING: tcsd could not be found; typically need it for tss user account and tests" I wonder if evaluating that could give us more testing or anything else?