[MIR] protection-domain-mapper & qrtr

[Dimitri John Ledkov]

[Availability]
The package protection-domain-mapper is already in Ubuntu universe.
The package protection-domain-mapper build for the architectures it is designed to work on.
It currently builds and works for architectures: any, verified as working on arm64
Link to package https://launchpad.net/ubuntu/+source/protection-domain-mapper

[Rationale]

- The package protection-domain-mapper is required in Ubuntu main for
  ubuntu-desktop on ARM64, as it enables power-indicator (among other
  things) on most Windows on Arm laptops (qcom based laptops ~7 SKUs
  and more coming). There is no other way to implement this.

- protection-domain-mapper depends on qrtr for library and a systemd
  service it provides.

- There is no other/better way to solve this that is already in main
  or should go universe->main instead of this. As this is the only
  implementation of talking to the qcom hardware.

- The package protection-domain-mapper is required in Ubuntu main no
  later than today due to Mantic release, if we want to have the best
  impression of Ubuntu Desktop in the live session on x13s.

- If that fails, having it fixed as SRU is the next best option.

[Security]

- No CVEs/security issues in this software in the past. This is a
  reference open source implementation of these tools, which otherwise
  are used on qcom Android devices

- no `suid` or `sgid` binaries no executables in `/sbin` and
  `/usr/sbin`

- Package does install services: pd-mapper.service & qrtr-ns.service
  which allow runtime access to the qcom hardware which are run as
  root

- Security has been kept in mind and common isolation/risk-mitigation
patterns are in place utilizing the following features:

- Packages does not open privileged ports (ports < 1024).

- Package does not expose any external endpoints

- Packages does not contain extensions to security-sensitive software
  (filters, scanners, plugins, UI skins, ...)

[Quality assurance - function/usage]

- The package works well right after install, i.e. power indicator
  straight away starts to show accurate battery information

[Quality assurance - maintenance]
- The package is maintained well in Debian/Ubuntu/Upstream and does
  not have too many, long-term & critical, open bugs
  - Ubuntu https://bugs.launchpad.net/ubuntu/+source/protection-domain-mapper/+bug
  https://bugs.launchpad.net/ubuntu/+source/qrtr/+bug
  - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=protection-domain-mapper https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=qrtr
  - Upstream's bug tracker, e.g., GitHub Issues

- The package has important open bugs, listing them:
  https://bugs.launchpad.net/ubuntu/+source/protection-domain-mapper/+bug/2038944
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1045729 upstream fix at https://github.com/andersson/qrtr/pull/24/files

- The package does deal with exotic hardware, it is present at Lenovo
  X13s to be able to test, fix and verify bugs as many users at
  Canonical and Community have it. And it is available for purchase.

[Quality assurance - testing]

- The package does not run a test at build time because adequate
  testing requires exotic hardware & specifically kernel driver loaded

- The package does not run an autopkgtest because testing requires
  exotic hardware & specifically kernel driver loaded.

- The package does have not failing autopkgtests right now

- The package can not be well tested at build or autopkgtest time
  because it requires exotic hardware to test. To make up for that:
   - We have access to such hardware in the team (foundations & kernel)

   - We will add a run-once manual test case to iso tracker to ensure
     that "power indicator shows battery indicator %")

   - We will execute this test case on every upload of
     protection-domain-mapper qrtr and the underlying kernel, as well
     as image milestone testing

   - qrtr package is minimal and will be tested in a more wide
     reaching solution context protection-device-mapper, that is
     causing battery indicator to work.

[Quality assurance - packaging]

- debian/watch is present and works

- debian/control defines a correct Maintainer field

- This package does not yield massive lintian Warnings, Errors

  https://udd.debian.org/lintian/?email1=&email2=&email3=&packages=qrtr&ignpackages=&format=html&lt_error=on&lt_warning=on&lt_information=on&lt_pedantic=on&lt_experimental=on&lt_overridden=on&lt_masked=on&lt_classification=on&lintian_tag=#all

  lack of manpages, lack of systemd hardening features in systemd unit

  https://udd.debian.org/lintian/?email1=&email2=&email3=&packages=protection-domain-mapper&ignpackages=&format=html&lt_error=on&lt_warning=on&lt_information=on&lt_pedantic=on&lt_experimental=on&lt_overridden=on&lt_masked=on&lt_classification=on&lintian_tag=#all

  lack of manpage, lack of systemd hardening features in systemd unit

- Please link to a recent build log of the package

  https://launchpad.net/ubuntu/+source/qrtr/1.0-2

  https://launchpad.net/ubuntu/+source/protection-domain-mapper/1.0-4

- This package does not rely on obsolete or about to be demoted packages.

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

- Packaging and build is easy, link to debian/rules

  https://salsa.debian.org/DebianOnMobile-team/protection-domain-mapper/-/blob/debian/latest/debian/rules

  https://salsa.debian.org/DebianOnMobile-team/qrtr/-/blob/debian/latest/debian/rules

[UI standards]

- Application is not end-user facing (does not need translation)

[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 kernel-packages and I have their
  acknowledgement for that commitment

- This does not use static builds

- This does not use vendored code

- This does not use vendored code

- This package is not rust based

- The package successfully built during the most recent test rebuild

[Background information]
The Package description explains the package well
Upstream Name matches package name

Link to upstream project are: https://github.com/andersson/qrtr and
https://github.com/andersson/pd-mapper

This package unblocks announcement of Ubuntu Desktop on ARM64 Laptops
for the first time, on an arm64 laptop from a tier 1 OEM available for
sale now.

[Dimitri John Ledkov (xnox)]

dropped some stray todo's from the description

[Lukas Märdian (slyon)]

Review for Source Package: qrtr

[Summary]
qrtr is the userspace part for IPC communication with qualcomm processors. It is primarily used on mobile phones, but can also be relevant for ARM based laptops, such as the Lenovo X13s.

 MIR team ACK (pending security-team ACK), with some suggested Recommended TODOs.

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

List of specific binary packages to be promoted to main: libqrtr1, qrtr-tools
Specific binary packages built, but NOT to be promoted to main: None

Notes:
#0 requesting security review, because it's running a service as root, opening controll sockets, parsing protocol packages and having no hardening features enabled.
#1 This is needed for hardware enablement, no automatic testing can be provided, but the hardware is available and a test case described in the ISO tracker.

Required TODOs:
- None

Recommended TODOs:
#2 The package should get a team bug subscriber before being promoted
#3 Enablement of isolation/hardening features should be considered (e.g. as part of the systemd service)
#4 Adding (hardware independent) smoke tests during build-/autopkgtests should be considered, at least making sure the library can be compiled and loaded correctly
#5 Consider helping out uptream with adding documentation/man pages
#6 Consider supporting the Debian maintainer with more timely package updates after (rare) upstream releases are cut.
#7 Consider putting a condition into the systemd service, to make it run only when relevant hardware is detected

[Rationale, Duplication and Ownership]
There is no other package in main providing the same functionality.
A team is committed to own long term maintenance of this package.
The rationale given in the report seems valid and useful for Ubuntu

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - src:qrtr 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]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates,
  signing, ...)

Problems:
- does run a daemon as root
- doesn't make appropriate (for its exposure) use of established risk
  mitigation features (dropping permissions, using temporary environments,
  restri...

[Lukas Märdian (slyon)]

Review for Source Package: protection-domain-mapper

[Summary]
This is the reference implementation for Qualcomm's Protection Domain mapper service. It is required for userspace applications to access the various remote processors (Wi-Fi, modem, sensors...) on recent Qualcomm SoCs using the QRTR protocol. It is primarily used on mobile phones, but can also be relevant for ARM based laptops, such as the Lenovo X13s.

MIR team ACK (pending security-team ACK & qrtr promotion), with some Recommended TODOs.

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

List of specific binary packages to be promoted to main: protection-domain-mapper
Specific binary packages built, but NOT to be promoted to main: None

Notes:
#0 requesting security review, because it's running a service as root, parsing json and having no systemd hardening features enabled.
#1 This is needed for hardware enablement, no automatic testing can be provided, but the hardware is available and a test case described in the ISO tracker.

Required TODOs:
#2 libqrtr1 & qrtr-tools need to be promoted (LP: #2038942)

Recommended TODOs:
#3 The package should get a team bug subscriber before being promoted
#4 Enablement of isolation/hardening features should be considered (e.g. as part of the systemd service)
#5 Consider helping out uptream with adding documentation/man pages
#6 Consider putting a condition into the systemd service, to make it run only when relevant hardware is detected

[Rationale, Duplication and Ownership]
There is no other package in main providing the same functionality.
A team is committed to own long term maintenance of this package.
The rationale given in the report seems valid and useful for Ubuntu

[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 due to this: libqrtr1 & qrtr-tools need to be promoted (LP: #2038942)

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- does not expose any external endpoint (port/socket/... or similar)
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates, signing, ...)

Problems:
- does run a daemon as root
- does parse data formats (json) from an untrusted source (userland)
- doesn't make appropriate (for its exposure) use of established risk
  mitigation features (dropping permissions, using temporary environments,
  restricted users/groups, seccomp, systemd isolation features,
  apparmor, ...)

[Common blockers]
OK:
- does not FTBFS curren...

[Dimitri John Ledkov (xnox)]

#2 The package should get a team bug subscriber before being promoted

kernel-packages team is subscribed to both packages.

[Dimitri John Ledkov (xnox)]

#4 Enablement of isolation/hardening features should be considered (e.g. as part of the systemd service)
#6 Consider putting a condition into the systemd service, to make it run only when relevant hardware is detected

Addressed in a very strict manner in https://launchpad.net/ubuntu/+source/qrtr/1.0-2ubuntu1 and https://launchpad.net/ubuntu/+source/protection-domain-mapper/1.0-4ubuntu2

[Mark Esler (eslerm)]

I reviewed qrtr 1.0-2 as checked into mantic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

qrtr: Userspace reference for net/qrtr in the Linux kernel

- CVE History:
  - no CVE history
  - no security policy
  - CVE-2019-19079 and CVE-2021-29647 affect kernel implementation
- Build-Depends?
  - no explicit dependencies in d/control
- pre/post inst/rm scripts?
  - postinst configures and starts qrtr-ns.service
  - prerm stops qrtr-ns.service
  - postrm runs daemon-reload and purges qrtr-ns.service
- init scripts?
  - init
- systemd units?
  - ./lib/systemd/system/qrtr-ns.service
    - spartan documentation
    - starts qrtr-ns
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - ./usr/bin/qrtr-cfg
  - ./usr/bin/qrtr-lookup
  - ./usr/bin/qrtr-ns
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - tests, hardware tests, are needed
- cron jobs?
  - none
- Build logs:
  - mostly clean
  - see -proposed

- Processes spawned?
  - none
- Memory management?
  - memory use appears safe
  - if values are confidential, memset_s should be used
- File IO?
  - only sockets
- Logging?
  - yes, see PLOGE
  - string use looks safe
- Environment variable usage?
  - none
- Use of privileged functions?
  - none
- Use of cryptography / random number sources etc?
  - none
- Use of temp files?
  - none
- Use of networking?
  - heavy, most of codebase
  - nothing obviously concerning
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - rc appears to be false positive
  - src/ns.c:796:2 appears to be an infinite loop
- Any significant shellcheck results?
  - none
- Any significant bandit results?
  - none
  - ./qrtr.py is python2.7

We should be cautious of IPC routers running root permissions. Similar code has
enabled vendor backdoors [0].

Qualcomm IPC will only be enabled in kernels which require it, such as for the
x13s.

Some mitigations exist to prevent spoofing and non-local observers. Fuzzing
seems worthwhile.

Possibly zero in-line comments. No documentation. This is a major maintenance
issue.

slyon's recommendations are great!

Thank you for adding a hardened systemd profile to -proposed for promotion \o/
http://launchpadlibrarian.net/691288509/qrtr_1.0-2_1.0-2ubuntu1.diff.gz

Security team ACK for promoting qrtr to main.

[0] https://redmine.replicant.us/projects/replicant/wiki/samsunggalaxybackdoor

[Mark Esler (eslerm)]

I reviewed protection-domain-mapper as checked into mantic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

pd-mapper: [no upstream description]

- CVE History:
  - no CVE history
  - possible security issues in commit messages
  - no security policy
  - upstream may not be reporting security issues
    - this is a maintenance issue
- Build-Depends?
  - qrtr
- pre/post inst/rm scripts?
  - postinst configures and starts pd-mapper.service
  - prerm stops pd-mapper.service
  - postrm runs daemon-reload and purges pd-mapper.service
- init scripts?
  - none
- systemd units?
  - ./lib/systemd/system/pd-mapper.service
  - spartan documentation
  - starts pd-mapper
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - ./usr/bin/pd-mapper
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - tests, hardware tests, are needed
- cron jobs?
  - none
- Build logs:
  - no-manual-page

- Processes spawned?
  - none
- Memory management?
  - owning team has agreed to fix issues
- File IO?
  - pd_enumerate_jsons looks dangerous
    - statically opens /sys/class/remoteproc/
    - only root should own these files
    - if continues, without consequence
  - /lib/firmware/ also set
  - interprets json
- Logging?
  - minimal, mostly stderr
- Environment variable usage?
  - none
- Use of privileged functions?
  - none
- Use of cryptography / random number sources etc?
  - none
- Use of temp files?
  - none
- Use of networking?
  - through qrtr
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - high density for only ~1k loc
  - owning team agreed to address all positive results
- Any significant shellcheck results?
  - none
- Any significant bandit results?
  - none

This deserves fuzzing and a deeper review. Hardware enablement is a priority
and urgent.

slyon's recommendations are great!

There is no release or changelog history. Commit messages are the only context
Possibly zero in-line comments. No documentation. This project does not even
have a description. This is a major maintenance issue.

Thank you for adding a hardened systemd profile in -proposed for promotion \o/
http://launchpadlibrarian.net/691288606/protection-domain-mapper_1.0-4ubuntu1_1.0-4ubuntu2.diff.gz

Security team ACK for promoting protection-domain-mapper to main.

[Łukasz Zemczak (sil2100)]

I have promoted both packages to main:

$ change-override -S -s mantic -c main protection-domain-mapper
Override component to main
protection-domain-mapper 1.0-4ubuntu2 in mantic: universe/misc -> main
protection-domain-mapper 1.0-4ubuntu2 in mantic amd64: universe/net/optional/100% -> main
protection-domain-mapper 1.0-4ubuntu2 in mantic arm64: universe/net/optional/100% -> main
protection-domain-mapper 1.0-4ubuntu2 in mantic armhf: universe/net/optional/100% -> main
protection-domain-mapper 1.0-4ubuntu2 in mantic ppc64el: universe/net/optional/100% -> main
protection-domain-mapper 1.0-4ubuntu2 in mantic riscv64: universe/net/optional/100% -> main
protection-domain-mapper 1.0-4ubuntu2 in mantic s390x: universe/net/optional/100% -> main
Override [y|N]? y
7 publications overridden.

$ change-override -S -s mantic -c main qrtr
Override component to main
qrtr 1.0-2ubuntu1 in mantic: universe/misc -> main
libqrtr-dev 1.0-2ubuntu1 in mantic amd64: universe/libdevel/optional/100% -> main
libqrtr-dev 1.0-2ubuntu1 in mantic arm64: universe/libdevel/optional/100% -> main
libqrtr-dev 1.0-2ubuntu1 in mantic armhf: universe/libdevel/optional/100% -> main
libqrtr-dev 1.0-2ubuntu1 in mantic ppc64el: universe/libdevel/optional/100% -> main
libqrtr-dev 1.0-2ubuntu1 in mantic riscv64: universe/libdevel/optional/100% -> main
libqrtr-dev 1.0-2ubuntu1 in mantic s390x: universe/libdevel/optional/100% -> main
libqrtr1 1.0-2ubuntu1 in mantic amd64: universe/libs/optional/100% -> main
libqrtr1 1.0-2ubuntu1 in mantic arm64: universe/libs/optional/100% -> main
libqrtr1 1.0-2ubuntu1 in mantic armhf: universe/libs/optional/100% -> main
libqrtr1 1.0-2ubuntu1 in mantic ppc64el: universe/libs/optional/100% -> main
libqrtr1 1.0-2ubuntu1 in mantic riscv64: universe/libs/optional/100% -> main
libqrtr1 1.0-2ubuntu1 in mantic s390x: universe/libs/optional/100% -> main
qrtr-tools 1.0-2ubuntu1 in mantic amd64: universe/net/optional/100% -> main
qrtr-tools 1.0-2ubuntu1 in mantic arm64: universe/net/optional/100% -> main
qrtr-tools 1.0-2ubuntu1 in mantic armhf: universe/net/optional/100% -> main
qrtr-tools 1.0-2ubuntu1 in mantic ppc64el: universe/net/optional/100% -> main
qrtr-tools 1.0-2ubuntu1 in mantic riscv64: universe/net/optional/100% -> main
qrtr-tools 1.0-2ubuntu1 in mantic s390x: universe/net/optional/100% -> main
Override [y|N]? y
19 publications overridden.