Comment 1 for bug 2067373

Review for Source Package: provd

[Summary]
This is the backend for Ubuntu "initial setup" provisioning story for Desktop
systems, similar to gnome-initial-setup, but enhanced by Ubuntu Pro components
and others. It's a relatively new Ubuntu native package, supposed to replace
"gnome-initial-setup" and "oem-config" in main.

MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: provd
Specific binary packages built, but NOT to be promoted to main: <None>

Notes:
#0 Generic questions
#0.a Why not ship this as part of the "ubuntu-desktop-init" snap?
     (i.e. together with its frontend)
#0.b Could you please briefly differentiate this tool from cloud-init,
     which is also used as part of the new Desktop installer?
#1 team bug subscriber ~desktop-packages is already subscribed

Required TODOs:
#2 Dependency on "gnome-initial-setup" needs to be dropped
#3 "gnome-initial-setup" & "oem-config" need to be demoted
#4 Improve Go packaging, I'm not an expert here, but I think we should at least have an "Built-Using" in debian/control, to indicate the toolchain that was used to build this
#5 Add files (debian/README.source) that explains how to refresh the vendored sources
#5.a Please give rational why all the vendoring is needed (c.f. recommendation #6)

Recommended TODOs:
#6 Consider using more "golang-*-dev" packages from the archive where possible, indicated by "Static-Built-Using" in debian/control to avoid some vendoring
#7 Consider using more mitigation features (dropping permissions, using temporary environments, restricted users/groups, seccomp, systemd isolation features, apparmor, ...), especially setting suid via systemd
#8 Consider running more complex integration tests as autopkgtest (e.g. integration with the ubuntu-desktop-init" snap), in addition to the "go test" unit-tests.
#9 Consider fixing some of the lintian warnings (see "Packaging red flags" below)
#10 Consider fixing some of the build-time warnings (see "Upstream red flags" below)

[Rationale, Duplication and Ownership]
- A team is committed to own long term maintenance of this package. (~desktop-packages)
- The rationale given in the report seems valid and useful for Ubuntu

Problems:
- Depends on gnome-initial-setup
- There are other package in main providing the same functionality.
  => gnome-initial-setup and ubiquity/oem-config (to be demoted)
  => cloud-init (used in desktop-installer) – please differentiate

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - SRCPKG checked with `check-mir`
  - all dependencies can be found in `seeded-in-ubuntu` (already in main)
  - none of the (potentially auto-generated) dependencies (Depends
    and Recommends) that are present after build are not in main
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
Problems:
- embedded source present
- static linking
- lacking [Static-]Built-Using entries in control file (usage of 'golang-*-dev' is encouraged)
dpkg-gencontrol: warning: package provd: substitution variable ${misc:Built-Using} unused, but is defined
dpkg-gencontrol: warning: package provd: substitution variable ${misc:Static-Built-Using} unused, but is defined
dpkg-gencontrol: warning: package provd: substitution variable ${misc:Built-Using} unused, but is defined
dpkg-gencontrol: warning: package provd: substitution variable ${misc:Static-Built-Using} unused, but is defined
- Go Package that follows the Debian Go packaging guidelines
- not a rust package, no extra constraints to consider in that regard

Problems:
- golang: static builds are used, the team did not yet confirmed their
  commitment to the additional responsibilities implied by static builds.
- vendoring is used, but the reasoning is not sufficiently explained
- Includes vendored code, the package has documented how to refresh this
  code at debian/README.source <= This file is missing!

[Security]
OK:
- history of CVEs does not look concerning (new package)
- does run a daemon (but not as as root??)
- does not use webkit1,2
- does not use lib*v8 directly
- does not process arbitrary web content
- does not integrate arbitrary javascript into the desktop
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- new package
- running a daemon (as non-root?)
- does parse data formats (e.g Ubuntu Pro response) from an untrusted source.
  Lots of parsers in vendored code, too.
- does expose external endpoint (socket:/run/gnome-initial-setup/desktop-provision/init.socket)
- does use centralized online accounts (Ubuntu Pro)
- does deal with system authentication (gdm setup, active directory)
- does deal with cryptography (password hashes)
- this high risk application could make more use of mitigation features
  (dropping permissions, using temporary environments, restricted users/groups,
  seccomp, systemd isolation features, apparmor, ...)

[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
- This does not need special HW for build or test
- no new python2 dependency
- Go package, but using dh-golang

Problems:
- running "go test" unit tests as autopkgtest. Maybe some integration tests
  using the "ubuntu-desktop-init" snap would be helpful, too.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta (Ubuntu native package)
- symbols tracking not applicable for this kind of code.
- debian/watch is not present but also not needed (e.g. native)
- Upstream update history is sporadic – its a native package after all
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- debian/rules is rather clean
- It is not on the lto-disabled list

Problems:
- Debian/Ubuntu update history is sporadic (not a lot of history, being a new package)
- the current release is not packaged, upstream (GitHub) contains 0.1.4, should be an easy upgrade from 0.1.2
- lintian warnings:
I: provd source: missing-built-using-field-for-golang-package (in section for provd) [debian/control:18]
I: provd source: unused-license-paragraph-in-dep5-copyright gpl-3 [debian/copyright:353]
Nitpicks:
I: provd: spelling-error-in-binary acccessed accessed [usr/libexec/provd]
I: provd: spelling-error-in-binary compatibile compatible [usr/libexec/provd]
I: provd: spelling-error-in-binary divison division [usr/libexec/provd]
I: provd: spelling-error-in-binary standar standard [usr/libexec/provd]
I: provd: spelling-error-in-binary standart standard [usr/libexec/provd]

[Upstream red flags]
OK:
- no Errors during the build
- no incautious use of malloc/sprintf (the language has no direct MM)
  => some "unsafe" code in internal/services/user/password_hash.go
- no use of user nobody
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit or libseed
- not part of the UI for extra checks
- no translation present, but none needed for this case

Problems:
- use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
  => sprovd/sprovd.go wraps "sudo pro attach"

- use of setuid / setgid in /usr/libexec/sprovd
  => prefer systemd to set those flags, or give rationale why it's done another way

- Warnings during the build:
go: warning: "./..." matched no packages
? github.com/canonical/ubuntu-desktop-provision/provd [no test files]

# github.com/linuxdeepin/go-gir/gobject-2.0
fix_gobject.c: In function ‘_g_type_param_value_array’:
fix_gobject.c:74:13: warning: Deprecated pre-processor symbol
   74 | GType _g_type_param_value_array() { return G_TYPE_PARAM_VALUE_ARRAY; }
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~