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; }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
** Changed in: provd (Ubuntu)
Assignee: Lukas Märdian (slyon) => Ubuntu Security Team (ubuntu-security)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2067373
Title:
[MIR] provd
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/provd/+bug/2067373/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
