Comment 1 for bug 1936907

Revision history for this message
Lukas Märdian (slyon) wrote :

[Summary]
Overall this is looking fine, but we need to make sure to fix a few things around it. As this is a go binary with many vendorized dependencies, you also need to explicitly state your commitment to support the security team with security and dependency updates. Please do so in this bug report. Furthermore, you might add a team subscription to this package's bugs (though that can be done by an AA later on).

I'd also like you to take a look at some recommendations like fixing autopkgtests, fixing some lintian warnings and trying to avoid the "sudo" call in debian/control/tests. Those can be fixed after promotion to main as well.

This does need a security review, so I'll assign ubuntu-security (root daemon, vendorized code)
List of specific binary packages to be promoted to main: adsys

So this will be a MIR team ACK, once the required TODOs are fulfilled.

Notes:
This is a go package with many (>50) vendorized dependencies, but the owning team is using "dependabot" (from GitHub) to keep track of dependency/security updates.

Required TODOs:
- This is a static go binary, therefore the owning team must state the following commitment explicitly:
  * the owning team must state their commitment to test no-change-rebuilds triggered by a dependent library/compiler and to fix any issues found for the lifetime of the release (including ESM when included)
  * the owning team must provide timely testing of no-change-rebuilds from the security team, fixing the rebuilt package as necessary
  * the owning team must state their commitment to provide updates to the security team for any affected vendored code for the lifetime of the release (including ESM when included)
  * the owning team will provide timely, high quality updates for the security team to sponsor to fix issues in the affected vendored code

Recomended TODOs:
- Please add a team subscription (~desktop-packages?)
- Make autopkgtests pass (it's not a regression as the autopkgtests passed never before)
- Fix relevant lintian warnings (missing-notice-file-for-apache-license, hardening-no-pie)
- check if you could be using the "needs-root" restriction instead of "sudo" in debian/control/tests

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- Build-dependency: debhelper-compat is pure virtual, dh-apport is only a build-dependency and therefore OK to be in universe
- no -dev/-debug/-doc packages that need exclusion

Problems:
- NONE

[Embedded sources and static linking]
OK:
- Special case: this is a Go package, using dh-golang

Problems:
- embedded source present (53 vendorized go depdendencies)
- static linking (Built-Using)

[Security]
OK:
- history of CVEs does not look concerning, 0 CVEs so far, according to https://ubuntu.com/security/cve?q=&package=adsys&priority=&version=&status=
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port (but can be socket activated)
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop

Problems:
- does run a daemon as root (can be socket activated)
- does deal with system authentication (pam, polkit)
- does parse data formats (configuration files)

[Common blockers]
OK:
- does not FTBFS currently (builds for amd64, arm64, armhf, ppc64el, riscv64, s390x)
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- translation present in po/adsys.pot
- no new python2 dependency
- Go package that uses dh-golang

Problems:
- The package has no team bug subscriber (~desktop-packages?)
- this is a go package, extra constraints to consider in that regard
- does have a non-trivial test suite that runs as autopkgtest, but it has been failing every since

[Packaging red flags]
OK:
- Ubuntu does not carry a delta, it's a native Ubuntu package, no Debian upstream
- d/watch is not needed as this is a native package
- symbols tracking not applicable for this kind of code. (not a library)
- Upstream update history is good (but this is still a rather new package, first release in Feb'21, approx. 1 release per month since then)
- Debian/Ubuntu update history is good (following upstream)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far maintained the package (it's been maintained by the Desktop team, upstream and downstream)
- d/rules is rather clean
- Go Package that follows the Debian Go packaging guidelines (but not maintained by pkg-go team)
- is not on the lto-disabled list

Problems:
- Does have Built-Using (static Go binary)
- some Lintian warnings:
  E: adsys source: missing-notice-file-for-apache-license vendor/gopkg.in/yaml.v2/NOTICE vendor/gopkg.in/yaml.v3/NOTICE
  W: adsys: hardening-no-pie sbin/adsysd

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of user nobody
- no use of setuid (vendorized golang.org/x/sys dependency provides setuid functionality for UNIX/BSD, but not for Linux)
- 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 (will be installed in the background if AD is selected in the installer)
- use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (sudo is used in autopkgtests d/tests/control only – that's fine)

Problems:
- NONE