This comes with all the pros and cons of the rust package it is, but
other than a lot of noise due to that it was fine for now.

Review for Source Package: rust-sequoia-sq
Launchpad bug: https://bugs.launchpad.net/bugs/2121154
Target series: devel
Analysed source version: 1.3.1-10ubuntu1 (release pocket)
The most recent build version has not yet built. Substituted with 
1.3.1-10ubuntu1 (1.3.1-10ubuntu1 - built on amd64, amd64v3, arm64, armhf, 
ppc64el, riscv64, s390x).
Binary packages: sq

[Summary]
OK:
- Review for Source Package: rust-sequoia-sq
- Reporter MIR content found and used as context.
- All built binaries listed in promotion_status.binaries are already published 
in main (promotion_status.current_component).
  (The promotion_status evidence indicates that the binary package 'sq' is 
already in the main component, and the needs_promotion list is empty.)

MIR team ACK

Remember that you need to subscribe the team and there is nothing that
blocks you from doing so upfront (it is even useful to see bug flows
early)

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

[Rationale, Duplication and Ownership]
OK:
- The rationale given in the report seems valid and useful for Ubuntu
  (The reporter articulates a concrete Ubuntu user benefit (making PGP easier 
to use) and a strategic alignment with industry trends (RHEL adoption). 
Dependency analysis shows no blocking reverse dependencies, supporting the 
forward-looking nature of this request.)

Problems:
- The archive already ships gnupg and gpg (main), which provide the same core 
OpenPGP command-line functionality as sq. The reporter explicitly states the 
intent to eventually replace gnupg2 with Sequoia, so this overlap is 
intentional; the reviewer should confirm the migration rationale is acceptable.
  (gnupg (main) and gpg (main) are full OpenPGP implementations providing 
encryption, signing, and key management from the command line — the same 
functionality sq offers. The reporter acknowledges this duplication is 
deliberate, framing sq as a future replacement for gnupg2.)
- No owning team is subscribed to the bug, nor is there an explicit 
acknowledgement of long-term maintenance in the comments.
  (The team mapping evidence indicates no subscribed teams, and the bug 
comments lack a statement from a team confirming ownership.)

[Dependencies]
OK:
- no runtime dependencies outside main needing MIR
- no other build-time Dependencies with active code in the final binaries to 
MIR due to this
  (The build log shows a Vendored-Sources-Rust line enumerating all embedded 
Rust crate sources, which is the standard Ubuntu/Debian mechanism for tracking 
build-time dependencies with active code in Rust binaries. All runtime shared 
library dependencies (libbz2-1.0, libc6, libgcc-s1, libsqlite3-0, libssl3t64, 
openssl) are dynamically linked and in main, and no unusual vendored 
directories or static binary flags were detected.)
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring more 
tests now.
  (All runtime dependencies in main have autopkgtests enabled, with passing 
results on amd64 and other primary architectures for critical libraries like 
openssl and bzip2.)

Problems: none

[Embedded sources and static linking]
OK:
- no static linking in the classic sense, very much in the this is rust sense
- Built-Using entries present but appear to be standard toolchain entries: 
rust-defaults (= 1.93.1ubuntu1) - we know the vendored deps are lacking in this 
regard
- not a go package, no extra constraints to consider in that regard
- Rust Package — Rust-specific constraints apply (see ESL-9/10)
- rust package using dh_cargo (dh ... --buildsystem cargo)
- Rust package that has all dependencies vendored. It does neither have 
*Built-Using (after build). Nor does the build log indicate built-in sources 
missed as Built-Using.
- Includes vendored code, the package has documented how to refresh this code 
at debian/README.source using cargo-vendor-filterer

[Security]
OK:
- history of CVEs does not look concerning
  (All checked trackers report zero CVEs and zero historical CVEs for the 
package and its predecessor names.)
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not parse data formats (files [images, video, audio, xml, json, asn.1], 
network packets, structures, ...) from an untrusted source.
  (As a PGP command-line tool, sq processes OpenPGP packet structures that may 
originate from untrusted sources; however, the Sequoia project is purpose-built 
for secure OpenPGP handling in Rust, and its runtime dependencies (bzip2, 
sqlite3, openssl) are all in main. A human reviewer should confirm the OpenPGP 
packet parser validates and rejects malformed input defensively.)
- does not expose any external endpoint (port/socket/... or similar)
  (No systemd service files were present in the packaging source, and a 
deterministic scan of build logs for common network binding patterns 
(TcpListener, bind, listen, UdpSocket, UnixListener, accept) yielded zero 
matches. The sole produced binary is a command-line utility (sq) in the utils 
section, consistent with a non-daemon package.)
- does not process arbitrary web content
  (The package description identifies it as an OpenPGP command-line tool, and 
its runtime dependencies (libbz2, libc6, libsqlite3, libssl3, openssl) include 
no web browser or HTTP client libraries. No service files or external endpoints 
are present, and the reporter confirms the package does not expose any external 
endpoints.)
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
  (Runtime dependencies are limited to libc, libssl, libsqlite3, libbz2, and 
openssl — none of which are WebKit or JS engines. No desktop file is shipped, 
and the binary is in the utils section as a CLI tool, so there is no surface 
for arbitrary JS integration into the desktop.)
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
  (Runtime dependencies are limited to standard libraries (libc, libssl, 
libsqlite3, bzip2, openssl) with no TPM, secure boot, or PKCS#11 related 
packages. The package is a PGP tool that handles OpenPGP signatures, but this 
is distinct from platform security attestation.)
- this makes appropriate (for its exposure) use of established risk mitigation 
features (dropping permissions, using temporary environments, restricted 
users/groups, seccomp, systemd isolation features, apparmor, ...)
  (No service files, systemd units, cron jobs, setuid/setgid binaries, or 
AppArmor profiles were found, consistent with a user-facing command-line tool. 
The reporter confirms no privileged ports or external endpoints are exposed, 
and the memory-safe language (Rust) provides inherent mitigation appropriate 
for this exposure level.)

Problems:
- The package does handle cryptography — it is an OpenPGP implementation 
providing encryption, decryption, signing, and certificate/key management. A 
dedicated security review is required before promotion to main.
  (The package is explicitly an OpenPGP tool (Sequoia) intended to replace 
gnupg2, handling en-/decryption, signing, and certificates. Runtime 
dependencies on libssl3t64 and openssl confirm system crypto library usage, 
while vendored crates such as sequoia-openpgp, ring, sha2, 
sha1collisiondetection, and rustls implement or wrap cryptographic operations 
directly, placing this in the high-concern category per policy.)
=> That was obvious and expected - but it means a security review should happen.

[Common blockers]
OK:
- does not FTBFS currently; Launchpad build records pass for arches: amd64, 
amd64v3, arm64, armhf, ppc64el, riscv64, s390x
- A build-time test suite runs during the package build, and the build log 
contains test pass/fail output. There is no evidence that test failures are 
swallowed or ignored.
  This is super verbose, we see that it works building for ages in 
dh_auto_test. It is so much that it is hard to see.
- This does not need special HW for build or test
  (The reporter explicitly states the package does not deal with exotic 
hardware, and the build dependencies (cargo, rustc, libssl-dev, libbz2-dev, 
libsqlite3-dev, capnproto, pkgconf) are all standard software packages with no 
hardware-specific requirements. The package successfully builds on amd64, 
amd64v3, arm64, armhf, ppc64el, riscv64, and s390x.)
- no special hardware needed, so there is no compromise to accept
- no new python2 dependency
- not a go package, no extra constraints to consider in that regard

Problem:
- Does not have a non-trivial test suite that runs as autopkgtest, but the 
function of this is low level, I'd consider this ok with a good build time test 
to cover most of it.
  You already explained the same in the report - ok

Problems:
- Python package detected but dh_python/dh_python3 not found in debian/rules

[Packaging red flags]
OK:
- symbols tracking not applicable for this kind of code
  (the package ships no shared library (.so), so ABI symbol tracking does not 
apply)
- debian/watch is present and looks ok
- Upstream update history is (good/slow/sporadic)
  (The package history shows 33 releases spanning roughly five and a half 
years, averaging a new release every two months, which is classified as a 
'good' cadence.)
- Debian/Ubuntu update history is (good/slow/sporadic)
  (The computed release cadence descriptor is 'good', indicating roughly one 
upload every two months, which meets the threshold of one or more uploads per 
six months.)
- promoting this does not seem to cause issues for MOTUs that so far maintained 
the package
  (The ubuntu-upload-permission data shows the package is already in main with 
no team or individual uploaders listed, and the upload history shows empty 
uploader fields, indicating no MOTU maintainers would lose upload access.)
- no excessive lintian warnings
- debian/rules is rather clean
  (The main rule and both overrides constitute a base debhelper + cargo setup 
with only configuration/parametrization, which is clean. The vendor target is 
not part of the normal build flow but manipulates files in fragile ways (sed on 
.cargo-checksum.json) and performs git commits inside debian/rules, which is 
why a closer reviewer look is recommended.)
- It is not on the lto-disabled list
- debian/control defines a correct Maintainer field
- Delta is reasonable, mostly fixes for the MIR
- Current release packaged (There is 1.4.0 but this one is to match the 
counterpart)

Problems: none

[Upstream red flags]
OK:
- no incautious use of malloc/sprintf (the language has no direct MM)
  (The source package is identified as a Rust package, and the build log shows 
only rustc compilation invocations with no C/C++ compilation steps. Rust's 
ownership model and borrow checker eliminate the class of vulnerabilities 
associated with incautious malloc/sprintf usage.)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside tests)
- no important open bugs (crashers, etc) in Debian or Ubuntu
  (The only non-MIR bug in Ubuntu is a merge request for version 1.4.0, not a 
functional defect. Debian has zero open bugs and no RC or security issues.)
- no dependency on webkit, qtwebkit or libseed
- not part of the UI for extra checks
  (The binary package is in the utils section, has no GUI toolkit runtime 
dependencies (only libc, libssl, libsqlite3, libbz2, openssl), and is a CLI 
tool intended to replace gnupg2 — none of which indicate a graphical desktop 
application. No .desktop file is expected or required for this type of package.)
- not user-visible, translations not needed
- Errors/warnings during the build - there is ... way too much of everything.
  Spotted at least 2200 build warning(s) - but at least no errors.
  Inside the rust is that verbose domain it is not good but tolerable.

Problems:
- User 'nobody' found outside test context: 
./debian/rust-vendor/writeable/src/try_writeable.rs:62:///         let err = 
self.name.ok_or("nobody").try_write_to_parts(sink)?.err();; 
./debian/rust-vendor/writeable/src/try_writeable.rs:75:///         
self.name.ok_or("nobody").writeable_length_hint() + 8; 
./debian/rust-vendor/writeable/src/try_writeable.rs:245:    /// #        let _ 
= self.name.ok_or("nobody").try_write_to_parts(sink)?;
- setuid/setgid detected in source tree: 
./debian/rust-vendor/tokio/src/process/mod.rs:682:    /// `setuid` call in the 
child process. Failure in the `setuid`; 
./debian/rust-vendor/rustix/src/thread/prctl.rs:213:    /// Allows `setgid` 
manipulation. Allows `setgroups`. Allows forged gids on; 
./debian/rust-vendor/rustix/src/thread/id.rs:21:/// `setuid(uid)`
=> Actually this is in commented out code. While it is questionable to have so 
much code that is commented out that is no actual problem.

** Changed in: rust-sequoia-sq (Ubuntu)
     Assignee: (unassigned) => 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/2121154

Title:
   [MIR] rust-sequoia-sq

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/rust-sequoia-sq/+bug/2121154/+subscriptions


-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to