Comment 6 for bug 2030482

I reviewed s300-tools 2.28.0-0ubuntu2 and 2.29.0-0ubuntu2 as released in
Mantic's development. This shouldn't be considered a full audit, but rather
a quick gauge of maintainability.

s390-tools already exists in main. This review only considers novel Rust
code which has not been included in main before.

Most Rust additions are bindings between s390-tools and other tools, such
as rust-openssl, rust-libc, and rust-curl. Vendored packages are not being
reviewed. rustc/cargo package contains ~700 vendored packages. s390-tools
defensively limits the number of vendored packages ++1

Note, that the s390x deb packages is ~4x larger than debs for other archs,
as expected. Security's uaudit tooling is designed for amd64. Due to
urgency, only amd64 was compiled and s390x debs between Lunar and Mantic
were compared for this review.

Please also note that Rust tools are not included in s390-tools-signed.

The Security team thanks Simon Choplin for his work to define Rust code in
main: https://wiki.ubuntu.com/RustCodeInMain

s390-tools are: Tools for use with the s390 Linux kernel and device
drivers

- CVE History:
  - only CVE in history only affects SUSE
  - see vendored security history above
  - GitHub Issue and PR tracker are maintained
  - changelog since 2.26.0 (lunar main) not concerning
- Build-Depends?
  - see new ./rust-vendor/
- pre/post inst/rm scripts?
  - d/s390-tools-statd.postinst loads systemd module
    - already existed in s390-tools lunar main
    - see systemd below
  - d/s390-tools-zkey.postinst adds zkeyadm group, and sets folders/files
    to 0770 root:zkeyadm
    - already existed in s390-tools lunar main
  - d/s390-tools.postinst no longer exists in mantic
    - d/debian/s390-tools.postinst.s390x does
  - many .install scripts
    - s390-tools-data.install is new
      - adds genprotimg bins
      - in lunar, genprotimg was in d/not-installed
      - not Rust related, future C review needed
      - https://github.com/ibm-s390-linux/s390-tools/tree/master/genprotimg
- init scripts?
  - none
- systemd units?
  - already existed in s390-tools lunar main
    - no re-review
    - see mon_tools and d/modules-load.d/s390-tools.conf
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - amd64 binaries:
    - all are root:root owned
    - /usr/bin/genprotimg
    - /usr/bin/pvattest
    - /usr/bin/pvextract-hdr
  - s390x binaries not reviewed
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - see MIR review notes on tests
  - some new Rust code contains tests
- cron jobs?
  - none
- Build logs:
  - see MIR review about noisy logs
  - overall, mantic's build logs are much better than lunar's

- note: only novel, non-vendored, Rust code is being considered for MIR
  - see header
  - related ./debian/ code is not a concern
  - diff -qr lunar/s390-tools-2.26.0/ mantic/s390-tools-2.29.0/
  - Only ./rust/ directory requires review!
- Processes spawned?
  - none detected
  - only std::process::ExitCode
- Memory management?
  - unsafe std::ptr::write_volatile used to zero secrets, ++1
  - defensive memory use
- File IO?
  - std::fs use
    - see ./rust/pv/src/uvdevice.rs
  - static path /sys/firmware/uv/prot_virt_guest
- Logging?
  - yes, see pv::misc::PvLogger
- Environment variable usage?
  - DEP_OPENSSL_CONF
  - DEP_OPENSSL_VERSION_NUMBER
  - S390_TOOLS_RELEASE
  - CARGO_PKG_NAME
  - CARGO_PKG_VERSION
- Use of privileged functions?
  - ioctl looks dangerous
    - likely less dangerous than asm predecessor
    - is uvdevice's test necessary?
      - do we ship mock_libc? if so, is there a benefit in the deb?
- Use of cryptography / random number sources etc?
  - OpenSSL
    - implementation not checked
    - if s390-tools is FIPS relevant, will request review post-MIR
- Use of temp files?
  - none
- Use of networking?
  - curl at al.
    - confined environments only
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - C results need investigation
    - not Rust related
- Any significant Coverity results?
  - many new results between 2.28.0 and 2.29.0
  - C results need investigation
    - especially genprotimg
    - see coverity-2.28.0.txt
    - not Rust related
  - is rust/pv/Cargo.toml:32 safe?
    - see coverity-2.29.0.txt
- Any significant shellcheck results?
  - worth review
    - not Rust related
- Any significant semgrep results?
  - C results look interesting

This brief security MIR is an exception. Security may conduct further
reviews and report issues to owning team--this is not a required condition
for Mantic's ACK.

genprotimg needs review.

Is ./usr/share/s390-tools/netboot/Dockerfile required in the s390x deb?

The s390-tools developers maintain a high quality code base and have taken
steps to ensure security.

Only s390-tools 2.29.0 in Mantic's proposed contains d/README.source!

Security team ACKs for promoting s390-tools main.