[MIR] gnome-snapshot

[Sebastien Bacher]

[Availability]
The package gnome-snapshot is already in Ubuntu universe.
The package gnome-snapshot build for the architectures it is designed to work on.
It currently builds and works for architectures: amd64 arm64 armhf ppc64el riscv64 s390x
Link to package https://launchpad.net/ubuntu/+source/gnome-snapshot

[Rationale]
- The package gnome-snapshot is required in Ubuntu main to replace cheese (which is unmaintained) as our default camera application. Cheese will go to universe as part of the transition.

- The package gnome-snapshot is required in Ubuntu main no later than February 29th due to the Noble feature freeze.

[Security]
- No CVEs/security issues in this software in the past

- no `suid` or `sgid` binaries
- no executables in `/sbin` and `/usr/sbin`
- Package does not install services, timers or recurring jobs
- Packages does not open privileged ports (ports < 1024).
- Package does not expose any external endpoints
- Packages does not contain extensions to security-sensitive software

[Quality assurance - function/usage]
- The package works well right after install

[Quality assurance - maintenance]
- The package is maintained well in Debian/Ubuntu/Upstream and does
  not have important issues listed
  - Ubuntu https://bugs.launchpad.net/ubuntu/+source/gnome-snapshot/+bug
  - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=gnome-snapshot
  - Upstream's bug tracker, https://gitlab.gnome.org/GNOME/snapshot/-/issues
- The package does not deal with exotic hardware we cannot support

[Quality assurance - testing]
- The package runs a test suite on build time, if it fails
  it makes the build fail, link to build log
https://launchpad.net/ubuntu/+source/gnome-snapshot/45.2+vendored-0ubuntu1/+latestbuild/amd64

- The package does not run an autopkgtest because it's a graphical application dealing with hardware and we don't have a proper way to include those in the autopkgtest infra today. Instead we have a manual testplan that we will use to validate updates before uploading: https://wiki.ubuntu.com/DesktopTeam/TestPlans/GnomeSnapshot

[Quality assurance - packaging]
- debian/watch is present and works

- debian/control defines a correct Maintainer

- This package has only one minor lintian warning

- Please link to a recent build log of the package https://launchpad.net/ubuntu/+source/gnome-snapshot/45.2+vendored-0ubuntu1/+latestbuild/amd64

- Log of `lintian --pedantic`
# lintian --pedantic gnome-snapshot_45.2-2_amd64.changes
W: snapshot: no-manual-page [usr/bin/snapshot]

- Lintian overrides are not present

- This package does not rely on obsolete or about to be demoted packages.
- This package has no python2 or GTK2 dependencies

- The package will be installed by default, but does not ask debconf questions

- Packaging and build is easy, link to debian/rules https://salsa.debian.org/gnome-team/snapshot/-/blob/debian/latest/debian/rules

[UI standards]
- Application is end-user facing, Translation is present, via standard gettext

- End-user applications that ships a standard conformant desktop file

[Dependencies]
- No further depends or recommends dependencies that are not yet in main

[Standards compliance]
- This package correctly follows FHS and Debian Policy

[Maintenance/Owner]
- The owning team will be desktop-packages and I have their acknowledgement for
  that commitment
- The future owning team is already subscribed to the package

- This package is rust based and vendors all non language-runtime dependencies

- The package has been built in the archive more recently than the last
  test rebuild

[Background information]
The Package description explains the package well
Upstream Name is snapshot
Link to upstream project https://gitlab.gnome.org/GNOME/snapshot

[Jeremy Bícha (jbicha)]

I updated the package today with a few notable changes:

- using upstream's "official" vendored tarball
- enabled the build tests
- renamed the binary package to gnome-snapshot as intended but accidentally not done earlier. This helps avoid future namespace concerns

Using the vendored tarball means our packaging is different than Debian's. The Desktop team has not decided yet where to host our packaging but I filed LP: #2054163 to track this issue.

https://launchpad.net/ubuntu/+source/gnome-snapshot/45.2+vendored-0ubuntu1

[Lukas Märdian (slyon)]

Review for Source Package: gnome-snapshot

[Summary]
gnome-snapshot is GNOME's photo and video capturing application, replacing "Cheese".
There are plenty of webcam application in the archive, but only src:cheese in main,
which is supposed to be demoted as part of this MIR.

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: gnome-snapshot
Specific binary packages built, but NOT to be promoted to main: <None>

Notes:
#0 Requesting security review due to parsing audio/video/image data and vendored dependency tracking

Required TODOs:
#1 src:cheese needs to be demoted as part of the promotion process
#2 try getting rid of pre-compiled static libraries
#3 Improve tracking of vendored Rust dependencies (e.g. using dh_cargo),
   see https://wiki.ubuntu.com/RustCodeInMain
#4 get gstreamer1.0-libcamera binary (src:libcamera) promoted, (LP: #1997560)
#5 Document how to refresh the vendored dependencies

Recommended TODOs:
#6 The package should get a team bug subscriber before being promoted (~desktop-packages subscribed)
#7 Consider implementing autopkgtests, using a simulated video device
#8 Give clearer guidance on where and how packaging will be hosted (e.g. using Vcs-Git control fields)
   And how the Ubuntu delta will be handled going forward, see bug #2054163
#9 consider fixing some of the more relevant lintian warnings:
   E: gnome-snapshot source: unpack-message-for-orig gnome-snapshot_45.2+vendored.orig.tar.xz . ar failed for snapshot-45.2/vendor/winapi[...].a
   W: gnome-snapshot: no-manual-page [usr/bin/snapshot]
   I: gnome-snapshot: hardening-no-fortify-functions [usr/bin/snapshot]

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

Problems:
- There is another package in main providing the same functionality: Cheese (to be demoted)
  rmadison -r noble -c main {cheese,kamoso,uvccapture,fswebcam,webcam,webcamd}

[Dependencies]
OK:
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems:
- other Dependencies to MIR/promote: gstreamer1.0-libcamera binary from src:libcamera (main)
  see bug #1997560

[Embedded sources and static linking]
OK:
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
 - Rust package that has all dependencies vendored. It does neither
   have *Built-Using (after build). Nor does the build log indicate
   built-in sources that are missed to be reported as Built-Using.
- Includes vendored code

Problems:
- embedded source present
- static linking, also pre-compile static libraries (mostly winap, but also vendor/rustix, vendor/system-deps): $ find . | grep "\.a"
- rust package not using dh_cargo (dh ... --buildsystem cargo)
- the package has documented how to refresh vendored code at <TBD>
- Vendored Rust dependency tracking m...

[Mark Esler (eslerm)]

There are unnecessary crates being vendored. I filed an upstream issue: https://gitlab.gnome.org/GNOME/snapshot/-/issues/137

This causes a bandwidth strain on mirrors or wherever the source package is needed.

To be clear, this is not a Security issue and does not impact Security's review (since owning team is responsible for maintaining security of vendored packages). This pattern has been raised as a MIR issue: https://github.com/canonical/ubuntu-mir/issues/51

[Jeremy Bícha (jbicha)]

I uploaded https://launchpad.net/ubuntu/+source/gnome-snapshot/46.0-1ubuntu1

It ended up being an overhaul of the packaging to try to meet MIR expectations. This is my subteam's first time with a Rust app so it's understandable that we're still learning.

It now uses the same minimal upstream tarball as Debian. And then we specifically run cargo vendor ourselves in the debian/missing-sources/ directory so that we can update the vendored crates at any time.

#2 try getting rid of pre-compiled static libraries
It looks to me like the only .a files now are in the Windows crates.
There was one false positive, an empty .a file for systemdeps but I've removed that from our vendoring with a slight hack in debian/rules to restore it when needed.

#3 Improve tracking of vendored Rust dependencies (e.g. using dh_cargo),
I at least am now using XS-Vendored-Sources-Rust. This app uses meson as its build system so it's unclear to me where I could inject dh_cargo commands. I can follow up later with Foundations to see if this can be improved but I'm guessing it meets the minimum requirement here.

#5 Document how to refresh the vendored dependencies

https://salsa.debian.org/ubuntu-dev-team/snapshot/-/blob/ubuntu/latest/debian/README.source

#7 Consider implementing autopkgtests, using a simulated video device
I don't hav

#8 Give clearer guidance on where and how packaging will be hosted (e.g. using Vcs-Git control fields)
   And how the Ubuntu delta will be handled going forward, see bug #2054163

The Vcs-Git fields are updated and a debian/README.source is provided to document details.

#9 consider fixing some of the more relevant lintian warnings:
No errors are emited now.

Snapshot has no relevant command line options so a manpage doesn't seem helpful.

hardening-no-fortify-functions is often a false positive. It is currently classified as "informational" which suggests that Debian isn't confident enough in this tag.

Vendoring actually increased the pedantic warnings with several instances of
maintainer-manual-page [debian/missing-sources/
package-does-not-install-examples [debian/missing-sources/
and one instance of source: unknown-field Vendored-Sources-Rust

Windows
-------
There isn't an easy way to remove the Windows crates. Possibly the crates could be patched to not require the Windows dependencies, but remember that the crates aren't part of the upstream source here so it would be patching after we run the crate vendor command, which is unusual but might be able to work. Those patches may need to be frequently rebased.

[Lukas Märdian (slyon)]

#1 still pending

#2 +1
$ find . | grep "\.a" does not return anything anymore

#3 +1
$ CARGO_VENDOR_DIR=debian/missing-sources /usr/share/cargo/bin/dh-cargo-vendored-sources is happy, so am I. This should fulfil the minimum requirement.

#4 still pending

#5 +1, thanks!

LGTM! MIR team ACK, just pending:
* security ACK
* demotion of src:cheese
* promotion of gstreamer1.0-libcamera binary (src:libcamera), which seems to be ready according to LP: #1997560

[Nick Galanis (nickgalanis)]

I reviewed gnome-snapshot 45.2+vendored-0ubuntu2 as checked into noble. This
shouldn't be
considered a full audit but rather a quick gauge of maintainability. Due to
time constraints, this report only took into account the package itself, and
not its significant number (244) of vendored libraries. Many of those should be
removed as mentioned in https://gitlab.gnome.org/GNOME/snapshot/-/issues/137.
However, none of the used ones are non-standard.
For now, the following audit is only going to report findings in the source
code of the Rust package gnome-snapshot.

gnome-snapshot is a camera application designed for GNOME environments,
offering straightforward functionality for capturing photos and videos across
various devices. It is a new package, and it serves as an updated replacement
for older GNOME camera apps.

- CVE History
  - None, package is new
- Build-Depends
  - Many vendored libraries, some of them not needed (eg:
https://gitlab.gnome.org/GNOME/snapshot/-/issues/137)
- pre/post inst/rm scripts
  - None
- init scripts
  - None
- systemd units
  - None
- dbus services
  - None
- setuid binaries
  - None
- binaries in PATH
  - None
- sudo fragments
  - None
- polkit files
  - None
- udev rules
  - None
- unit tests / autopkgtests
  - Unit tests ran during build
  - Absence of autopkgtests, already a recomended TODO by the MIR team
- cron jobs
  - None
- Build logs
  - No warnings / errors during build

- Processes spawned
  - Does not interact with user input, thus not susceptible to any command
injection attacks or unsafe arguments
- Memory management
  - Mostly using Rust's memory safe features
  - "unsafe" stanzas look non-problematic
- File IO
  - Nature of the package requires I/O interaction with local storage
  - Unsafe processing of user-owned files, already-existing in the system by
calling open_with_system() in gallery.js, which does not filter file types, as
mentioned in a TODO by the developers in line 301, and can result in the
processing of untrusted files. However, given the threat model (user-owned
files and user-owned machine), this is not concerning as even with filtering,
the user could rename the files and process them
- Logging
  - Safe, does not include user input
- Environment variable usage
  - Can not be abused
- Use of privileged functions
  - Only used for installing dependecies
- Use of cryptography / random number sources etc
  - None, not needed due to the nature of the package
- Use of temp files
  - None
- Use of networking
  - None, only inter-process communication to respond to GUI events
- Use of WebKit
  - None
- Use of PolicyKit
  - None

- Any significant cppcheck results
  - None
- Any significant Coverity results
  - None
- Any significant shellcheck results
  - None
- Any significant bandit results
  - None
- Any significant govulncheck results
  - None
- Any significant Semgrep results
  - None

Security team ACK for promoting gnome-snapshot to main.

Overall, the package is well-written, and developed in a memory-safe language,
something that makes us believe that it will be less susceptible to
vulnerabilities in the future. Moreover, it is maintained by GNOME, leading us
...

[Sebastien Bacher (seb128)]

Thanks, we hve a +1 from the security team now

I've
- promoted gstreamer1.0-libcamera to main
- demoted -S cheese
- subscribed the desktop-packages team to the package on launchpad

Which were the remaining 'required' items from the MIR side, we will try to address the recommended one still but that's enough for now so I'm promoted gnome-snapshot (the desktop seed has also been updated to cheese -> gnome-snapshot)

$ ./change-override -c main -S gnome-snapshot
Override component to main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble: universe/misc -> main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble amd64: universe/gnome/optional/100% -> main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble arm64: universe/gnome/optional/100% -> main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble armhf: universe/gnome/optional/100% -> main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble ppc64el: universe/gnome/optional/100% -> main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble riscv64: universe/gnome/optional/100% -> main
gnome-snapshot 45.2+vendored-0ubuntu1 in noble s390x: universe/gnome/optional/100% -> main
Override [y|N]? y
7 publications overridden.

[Lukas Märdian (slyon)]

Please make sure src:cheese gets demoted properly. It seems to be back in "main" now.

seb128> slyon, I think we need an upload of ubuntu-desktop which didn't happen yet

[Sebastien Bacher (seb128)]

it was demoted, unsure who promoted it back but yes it seems we need an ubuntu-desktop upload for the component mismatch to go away, updating the seed wasn't enough