Review for Source Package: upki Launchpad bug: https://bugs.launchpad.net/bugs/2166514 Target series: devel Analysed source version: 1.0.0~beta.3-0ubuntu1 (release pocket) Binary packages: libupki-dev, libupki-openssl-dev, libupki-openssl1, libupki1, upki, upki-mirror
[Summary] OK: - Review for Source Package: upki - Reporter MIR content found and used as context. - The following binaries need to be promoted to main: upki, libupki1, libupki-openssl1. (These binaries are currently published in universe and are listed in the set of packages requiring promotion.) - Package has team subscriber(s): foundations-bugs MIR team ACK under the constraint to resolve listed below This does need a security review, so I'll assign ubuntu-security It also needs security to answer the question in regard to their tracking of rust libs (see @security below) Required TODOs: Yes to all tasks you already acknowledged - #1 add apparmor profiles for service: https://warthogs.atlassian.net/browse/FR-14470 - #2 fix autopkgtest https://warthogs.atlassian.net/browse/FR-14471 - #3 fix unit tests https://warthogs.atlassian.net/browse/FR-14473 A few more - #4 Depending tests also fail, could be the struggling infra but before promotion we'd want to seem them passing (e.g. you change a crypto lib to use it, then all those reverse dependency tests need to pass - this is implied by proposed migration but I wanted to mention it for completeness) - #5 AFAICS this set[uh]id is all just prep code (see setuid below) but still the question is: How do we protect us from todays "no implemented" to some day on an updated of vendored crates to be implemented. Please answer or code up something why it would be safe now (proably trivial) and why it will stay safe on updates in regard to setuid/setguid. - #6 @security: I have to accept the statement littered all over d/rules "dh-cargo-built-using is horribly broken ..." but at the end of the day I'm unsure how in this flux the seucrity team tracks things. I'll call them explicitly if they would need built-using or else (as I'm afraid that has been washed away and forgotten, not your fault but the trigger to find out) Recommended TODOs: - #7 symbols tracking isn't in place; add a debian/*.symbols file as you ship an -so file and even call is v1 ABI wise. If not doable, document why tracking is impractical and what else is used to stay safe against unwanted ABI/API changes. - #8 debian/rules is quite complex and full of workarounds. If over time this can get cleaner that would be very helpful for long term maintenance. Think of it like this, if some other packager has to follow up and they are not allowed to speak to you for reasoning - they should have no angry thought towards you :-) - #9 You need to decide what to do about libupki-openssl-dev and libupki-dev. They will not be depended on, but auto-include will fetch them for promotion. If the intention is to get them into main as well update the description which currently says "The binary packages upki, libupki1, libupki-openssl1 need to be in main to achieve integration into crypto libraries by default". If they shall not be in main add them to Extra-exclude referring to this bug please. [Rationale, Duplication and Ownership] OK: - There is no other package in main providing the same functionality. (The dup-search evidence lists candidates, but the reporter correctly identifies that the main candidates (openssl, ssl-cert, libstrongswan, libpq5) are low-level utilities or wrappers, not dedicated PKI management systems. The universe candidates that do fit the PKI description (e.g., easy-rsa, gnomint) are not in main.) - A team is committed to own long term maintenance of this package. (The team mapping lists foundations-bugs as a subscribed team for upki, and the MIR report explicitly states that foundations-bugs is aware of the implications of vendored code and commits to ongoing maintenance, testing, and security backport responsibilities.) - I know about the concrete plans for main packages to depend on upki (e.g., seed entries, reverse-depends) that justify the "most applications" claim Problems: none [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 vendored Rust crates compiled from a rust-vendor/ directory and linked into the final binaries, with X-Cargo-Built-Using: rust-defaults (= 1.95.0ubuntu1) present on both upki and upki-mirror packages. All external runtime dependencies (libc6, libgcc-s1, libssl4, ca-certificates, systemd) are in main; the only universe dependencies are same-source packages from upki itself.) Problems: - Verify if the failing autopkgtests for systemd, glibc, and gcc-16 and so on. But we know the infra struggles these days and you already acknowledged fixing tests and others in comment #1 - -dev/-debug/-doc packages that need exclusion libupki-openssl-dev and libupki-dev [Embedded sources and static linking] OK: - no static linking (except the one that is conceptually meant to be due to rust) - does not have unexpected Built-Using entries - No Go Package for extra checks in that regard - Rust Package that follows the Debian Ruest packaging guidelines (The package uses dh-cargo and cargo-based build tooling. Vendored Rust crates are shipped in a dedicated vendor directory with documented reproduction instructions and comprehensive per-crate copyright coverage, which is the current accepted Ubuntu Rust packaging practice.) - vendoring is used, but the reasoning is sufficiently explained (The package ships a rust-vendor directory containing vendored crates, and debian/README.source documents the vendoring process including how to regenerate the vendor tarball. Per-crate copyright fragments are also maintained in debian/copyright, providing adequate documentation of the vendored sources.) - 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 (Vendored code is shipped in the binary packages — a third-party directory within the vendored ring crate is present and not confined to test/example paths. The README.source file documents that running 'debian/rules vendor-tarball' recreates the rust-vendor directory and builds an additional orig tarball, satisfying the refresh documentation requirement.) Problems: none [Security] OK: - history of CVEs does not look concerning (The Ubuntu CVE tracker, NVD enrichment, and cvelist scan all returned zero results for current and historical vulnerabilities.) But to admit, it is fresh, so that means nothing :-) - does not run a daemon as root - does not use webkit1,2 - does not use lib*v8 directly - does not expose any external endpoint (port/socket/... or similar) (Pattern searches for TcpListener, UdpSocket, UnixListener, bind, listen, 0.0.0.0, and INADDR_ANY returned no matches, and searches for Listen/Socket directives in service-related content also returned nothing. However, the upki-fetch.service file content was not directly available in the evidence, so the reviewer should confirm it does not open any listening sockets.) - does not process arbitrary web content (The package name, upstream project (rustls/upki), and the MIR reporter's description all indicate a PKI management utility, not a web browser or content renderer. A fetch service and timer exist, and runtime dependencies include ca-certificates and libssl4, suggesting it retrieves certificate data over TLS from configured endpoints — not arbitrary web content. The source package description is empty, so full confirmation from that source was not possible.) On the other hand, the fetch service does in some way, not uncontrolled but fetched content - but that is conceptually needed and needs to be part of the review. See below in problems. - does not use centralized online accounts - does not integrate arbitrary javascript into the desktop (All runtime dependencies are system libraries and utilities (libc6, libgcc-s1, libssl4, ca-certificates, systemd) with no WebKit or JavaScript engine packages. The package is a Rust-based PKI library and CLI tool with no desktop file, and its binaries are in utils/libs/libdevel sections rather than desktop-oriented sections.) - does not deal with system authentication (eg, pam), etc) Problems: - does parse data formats (files [images, video, audio, xml, json, asn.1], network packets, structures, ...) from an untrusted source. (The package is a PKI/certificate infrastructure tool that includes vendored crates for parsing X.509 certificates, ASN.1/DER, CRLs, and JSON, and ships a systemd service (upki-fetch.service) that fetches certificate data from network sources. These are inherently untrusted data formats processed from untrusted origins.) - The package does handle cryptography: it is a PKI management tool that depends on OpenSSL (libssl4) and ca-certificates, ships a libupki-openssl1 binary, and vendors cryptographic libraries including ring and aws-lc-sys. A dedicated security review is required before main inclusion. (The package name and binary packages (libupki-openssl1, libupki-openssl-dev) indicate PKI and certificate handling; runtime dependencies include libssl4 (from openssl) and ca-certificates; and vendored Rust dependencies include ring and aws-lc-sys, both of which provide low-level cryptographic implementations rather than purely delegating to system libraries.) - Hardening of debian/upki-fetch.service (User=, NoNewPrivileges, ProtectSystem, PrivateTmp, CapabilityBoundingSet, SystemCallFilter, etc.) for this network-facing periodic fetch service. You already acknowledged that. - does deal with security attestation (not secure boot, tpm, signatures - but certificate handling which is part of it) (No TPM, secure boot, or PKCS#11 references were found in the source tree or its dependencies. Cryptographic signature handling is limited to conventional X.509 certificate operations via vendored crates such as x509-parser and rustls, which is expected PKI functionality rather than security attestation.) [Common blockers] OK: - does not FTBFS currently; Launchpad build records pass for arches: amd64, amd64v3, arm64, armhf, ppc64el, riscv64, s390x - This does not need special HW for build or test (The reporter confirms builds pass on amd64, amd64v3, arm64, armhf, ppc64el, riscv64, and s390x with no mention of exotic hardware. The declared test uses only python3 and the package itself with isolation-container and needs-root restrictions, which are standard autopkgtest constraints rather than dedicated hardware requirements.) - no special hardware needed, so there is no compromise to accept - if a non-trivial test on this level does not make sense..., is the overall solution (app+libs) extensively covered i.e. via end to end autopkgtest ? (The autopkgtest-db indicates the package has autopkgtests, and reverse-deps shows no consumers exist.) - no new python2 dependency - not a python package, no extra constraints to consider in that regard - Not a Go package, no extra constraints to consider in that regard Problems: none - The package's debian/rules skips a build-time test suite - but you acknolwedged to know and fix that - Does not have non-trivial autopkgtest, but failing - which you know and work on (The declared test `upki-fetch` is a functional test with dedicated test data and helper scripts, not a bare import or smoke check, satisfying the non-trivial requirement. While the autopkgtest infrastructure confirms the test exists and runs, all results are currently failing across every architecture, which warrants human investigation before final sign-off.) [Packaging red flags] OK: - debian/watch is present and looks ok - Upstream update history is - too short to decide, but under out controlö - Debian/Ubuntu update history is too short to matter - promoting this does not seem to cause issues for MOTUs that so far maintained the package (The package has no individual or team uploaders listed in ubuntu-upload-permission, and the upload history shows no specific uploader names, indicating it is likely synced or handled by the archive admins rather than a dedicated MOTU maintainer.) - no excessive lintian warnings - It is not on the lto-disabled list - debian/control defines a correct Maintainer field - This is ubuntu first, no need to discuss delta Problems: - Shared library is shipped but no debian/*.symbols file was found (shared library package(s) libupki-openssl1, libupki1 ship a .so but no debian/*.symbols file; for C++ libraries where tracking is impractical, document why (or use abigail/abi-compliance-check in CI, or bump SOVER on every update)) - Package is somewhat behind upstream: 1.0.0~beta.3 vs upki-1.0.0 - The debian/rules file is not a simple debhelper setup: it disables the test suite, contains substantial hand-rolled build and install logic, manipulates vendored crate checksums in fragile ways, and relies on patched dh-cargo helpers shipped in debian/bin. (The override_dh_auto_test target has the cargo test invocation commented out, effectively disabling tests. The override_dh_auto_install target runs multiple manual cargo install/cinstall commands and help2man, and the vendor-deps target removes test directories and rewrites .cargo-checksum.json files across all vendored crates, which is fragile and a maintenance hazard.) [Upstream red flags] OK: - no incautious use of malloc/sprintf (the language has no direct MM) (The package is built with cargo and rustc (Rust 1.95), confirming it is a memory-safe language with no direct malloc/sprintf usage. The build succeeded and no memory-safety-related compiler warnings were observed in the provided build log excerpts.) - no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside tests) - user 'nobody' matches are in commented-out code only (inactive, reported for completeness): ./rust-vendor/writeable/src/try_writeable.rs:62:/// let err = self.name.ok_or("nobody").try_write_to_parts(sink)?.err();; ./rust-vendor/writeable/src/try_writeable.rs:75:/// self.name.ok_or("nobody").writeable_length_hint() + 8 - no important open bugs (crashers, etc) in Debian or Ubuntu (Launchpad shows 2 open bugs for upki: one for AppArmor integration and the MIR bug itself, with no critical or security issues. Debian BTS reports zero open bugs, and the upstream GitHub tracker shows zero open issues.) - no dependency on webkit, qtwebkit or libseed - not part of the UI for extra checks (The source produces library packages (libupki1, libupki-dev, etc.) and CLI/daemon binaries (upki, upki-mirror) in the utils, libs, and libdevel sections, with no GUI toolkit dependencies among runtime deps — only libc, libgcc, libssl, ca-certificates, and systemd. Systemd service and timer files further confirm a non-graphical service/CLI nature.) - not user-visible, translations not needed Problems: - setuid/setgid detected in source tree: ./rust-vendor/rustix/src/thread/id.rs:85: backend::thread::syscalls::setuid_thread(uid); ./rust-vendor/rustix/src/thread/id.rs:139: backend::thread::syscalls::setgid_thread(gid); ./rust-vendor/rustix/src/not_implemented.rs:83: not_implemented!(setuid); - no Errors/warnings during the build (Can't decide: review 351 build warning(s) and decide if acceptable: rehash: warning: skipping ca-certificates.crt, it does not contain exactly one certificate or CRL; warning: ignoring 'sbom' config, pass `-Zsbom` to enable it; warning: [email protected]: ignoring 'hints.mostly-unused', pass `-Zprofile-hint-mostly-unused` to enable it) ** Changed in: upki (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/2166514 Title: [MIR] upki To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/upki/+bug/2166514/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
