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 <TBD> (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?
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 cleanups.
are waiting for these mentioned improvements/
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: /github. com/stefanberge r/swtpm/ issues/ 587
- 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:/
- 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] /autopkgtest. ubuntu. com/packages/ swtpm
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:/
- no new python2 dependency
- Python package, but using dh_python
- Go package, but using dh-golang
Problems: /launchpad. net/ubuntu/ +source/ swtpm/0. 6.1-0ubuntu2/ +build/ 22337599
- 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:/
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 <TBD> (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?