Ubuntu
pappl package

[MIR] pappl

Bug #2004119 reported by Till Kamppeter on 2023-01-29

This bug affects 2 people

Affects		Status	Importance	Assigned to	Milestone
	pappl (Ubuntu)	In Progress	High	Unassigned	Ubuntu ubuntu-23.10-feature-freeze

Bug Description

MIR following https://github.com/canonical/ubuntu-mir

There is not yet a dependency on libpappl1 in Lunar, the following package will be packaged and MIRed and it depends on libpappl1:

- pappl-retrofit: https://github.com/OpenPrinting/pappl-retrofit
    Retro-fitting classic CUPS drivers into Printer Applications
    In our special case it will provide the Legacy Printer
    Application which will make classic CUPS drivers available to
    an installed CUPS Snap or CUPS 3.x.

Both pappl and pappl-retrofit will provide the possibility to easily
develop Printer Applications.

[Availability]

The package PAPPL is already in Ubuntu universe.
The package PAPPL builds for the architectures it is designed to work on.
It currently builds and works for architetcures:
amd64, arm64, armhf, ppc64el, riscv64, s390x

Link to package https://launchpad.net/ubuntu/+source/pappl

[Rationale]

- The package PAPPL is required in Ubuntu main for making the base
   for Printer Applications (emulations of IPP printers), the new
   format of printer drivers. Classic CUPS printer drivers are not
   supported any more by the CUPS Snap and by CUPS 3.x
   - The package pappl will generally be useful for a large part of
     our user base, for everyone with a legacy or specialty printer
     which does not do driverless IPP printing (AirPrint, IPP
     Everywhere, Mopria).
   - The package is also needed by pappl-retrofit
     (https://github.com/OpenPrinting/pappl-retrofit) which not only
     allows to easily convert classic CUPS drivers into Printer
     Applications but also provides the Legacy Printer Application
     which maps any classic driver installed into classic CUPS
     locations into an emulation of an IPP printer.

- The package PAPPL is required in Ubuntu main no later than 23.10
   due to the CUPS Snap being used as the standard print
   environment. The CUPS Snap does not support installing classic CUPS
   drivers.

[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 principally not install services, timers or recurring jobs
   - but its purpose is to provide the basic infrastructure to create
     Printer Applications, daemons which emulate IPP printers.
   - Security features for daemons are not provided, responsibility is
     with programs using this library.
- Package and daemons created with it do not open privileged ports
   (ports < 1024)
- 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 (it is only a library)

[Quality assurance - maintenance]
- The package is maintained well in Debian/Ubuntu and has not too many
   and long term critical bugs open
   - Ubuntu https://bugs.launchpad.net/ubuntu/+source/pappl/+bug
   - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=pappl
   - https://github.com/michaelrsweet/pappl/issues
- The package has no important open bugs, upstream issues are nearly
   all feature requests.
- 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://launchpadlibrarian.net/608122472/buildlog_ubuntu-kinetic-amd64.pappl_1.2.1-1_BUILDING.txt.gz
- The package runs an autopkgtest, and is currently passing on
   this list of architectures as under [Availability], test log attached.

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

- debian/control defines a correct Maintainer field

`lintian --pedantic`:

E: pappl changes: bad-distribution-in-changes-file unstable

--> Sync from Debian

E: pappl source: source-is-missing [doc/pappl.html]

   --> doc/ppapl.html is generated via Michael Sweet's codedoc utility
       https://github.com/michaelrsweet/codedoc, via doc/Makefile, source
       is doc/pappl-body.md plus comments in the *.c files in pappl/

E: pappl source: source-is-missing [pappl/test.html]

   --> File OK, it is actually manually created, for testing the CSS
       during development. See:
           https://github.com/michaelrsweet/pappl/issues/251

W: libpappl1: mismatched-override spelling-error-in-binary usr/lib/x86_64-linux-gnu/libpappl.so.1 Nam Name [usr/share/lintian/overrides/libpappl1:2]
N: 0 hints overridden; 1 unused override

- Lintian overrides are present, but ok because spelling error in binary
is minor reason, no risk of failure in functionality or security

- 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 (no debconf questions at all)

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

[UI standards]
- Library is end-user facing (provides the web admin interface for the
   Printer Application based on the library), Translation are present, via
   Michael Sweet's own https://github.com/michaelrsweet/stringsutil utility
   and Weblate, see doc/pappl-body.md
- Library does not ship desktop files as application created with it
   is a permanently running daemonn, user interface is web interface.

[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]
- Owning Team will be the Ubuntu Printing Team (ubuntu-printing)
- Team is already subscribed to the package

- This does not use static builds
- This does not use vendored code
- This package is not rust based

- 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 pappl
- Link to upstream project: https://github.com/michaelrsweet/pappl

See original description

Tags:

Revision history for this message

Till Kamppeter (till-kamppeter) wrote on 2023-01-29:

autopkgtest log Edit (45.0 KiB, text/plain)

Till Kamppeter (till-kamppeter) on 2023-01-29

description:

updated

Till Kamppeter (till-kamppeter) on 2023-01-30

description:	updated
description:	updated
description:	updated

Christian Ehrhardt  (paelzer) on 2023-01-31

Changed in pappl (Ubuntu):
assignee:	nobody → Christian Ehrhardt  (paelzer)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2023-02-01:

FYI: Fixed missing reference in the description

description:

updated

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2023-02-01:

Download full text (3.6 KiB)

Review for Package: pappl

[Summary]
This looks fine, small, well tested, modern packaging - all good.
MIR team ACK

But it does IMHO need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: libpappl1, libpappl-dev
Specific binary packages built, but NOT to be promoted to main: <none>

Notes:
- the -dev package says "... package contains the static library ..."
But it does not. Consider updating the description one day, but this is
very non-important - not worth a being hard todo for you, but worth an fyi.

Required TODOs:
- The package should get a team bug subscriber before being promoted

Recommended TODOs:
- none

[Duplication]
There are printin libs, sure - but the purpose and need for this is very
much tied to wrapping cups printer drivers to behave like printer applications.
That isn't provided elsewhere.
=> There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this (all are already in main)
- no -dev/-debug/-doc packages that need exclusion (also all in main)
- 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 (there are a few IDE files, but no code)
- no static linking
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard
- Does not include vendored code

Problems: None

[Security]
OK:
- history of CVEs does not look concerning, but it is also rather new.
  Generally the printer drivers have ahd lots of issues as they are a good
  attack vector, hence I'd not wonder if this would have issues found in the
  future.
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- 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 parse data formats (things to print) from an untrusted source.

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
- test suite fails will fail the build upon error.
- does have a non-trivial test suite that runs as autopkgtest
- This does not seem to need special HW for build or test
so it can't be
- no new python2 dependency

Problems: None

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place
- d/watch is present and looks ok
- Upstream update history is good, but has no long track record yet
- Debian/Ubuntu update history is good, but has no long track record yet
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- It is not on the lto-disabled list

Problems: None

[Upstream red flags]
OK:...

I reviewed pappl 1.3.1-2 as checked into Mantic. This shouldn't be considered a
full audit but rather a quick gauge of maintainability.

- CVE History:
  - No CVE assigned for PAPPL
  - There were some GitHub issues with security impact (for example, crashes
    mentioned in the issues #61, #121, and #272). But the delay between
    reporting and fixing is one day.
- Build-Depends
  - All libraries are with development files and headers.
  - Their purposes are:
    - Printing
        - libcups2-dev: CUPS library (printer and print job management
          capabilities)
        - libcupsimage2-dev: support for handling images within CUPS
    - Networking
        - libavahi-client-dev: Avahi client library (service discovery on a
          local network)
    - Cryptography
        - libgnutls28-dev: GnuTLS, for implementations for SSL, TLS, and DTLS
          protocols
    - I/O
        - libusb-1.0-0-dev: user-space access to USB devices
    - File formats
        - libpng-dev: operations with PNG images
        - libjpeg-dev: operations with JPEG images
        - zlib1g-dev: zlib compression and decompression
    - IAM
        - libpam0g-dev: authentication tasks involving Unix's PAM
- pre/post inst/rm scripts
  - N/A
- init scripts
  - N/A
- systemd units
  - N/A
- dbus services
  - N/A
- setuid binaries
  - N/A
- binaries in PATH
  - /usr/bin/pappl-makeresheader, for libpappl-dev: Creates a C header file
    suitable for inclusion in a printer application. Based on makeresheader.sh.
    All usages of the parameters (namely the filenames) are escaped with quotes
    (e.g. "/* $file */").
- sudo fragments
  - N/A
- polkit files
  - N/A
- udev rules
  - N/A
- unit tests / autopkgtests
  - TBR
- cron jobs
  - N/A
- Build logs:
  - False positive use-after-free warning in client-webif.c:214: The "body"
    pointer (which is created by strdup-ing another string) cannot be used
    after its free(). If the free() occurs, then the function already returns.
  - False positive spelling-error-in-binary warning from Lintian: already
    documented in libpappl1.lintian-overrides
- Processes spawned
  - Calls to system() in testpappl.c, but with hard-coded values
  - The same for posix_spawn, where no parameter is user-controlled
- Memory management
  - Only a calloc() without checking the return value, but in
    testsuite/pwg-driver.c:608
  - No strcpy. All operations are achieved with memcpy, which implies the
    specification of sizes.
- File IO
  - umask is not used.
  - Double close in printer-webif.c:1133, but the file descriptors are
    function-scoped
- Logging
  - All exposed functions in log.h (which internally are using write_log)
    - Enum value (represented in memory as an int) is not verified. If it's set
    other value, which should be less than a loglevel member from the system
    argument (set during the papplSystemCreate API call), then arbitrary memory
    read happens
      - Fixed in 4c0d022557df8babb08dacd365149192a8820e7e
    - If the string is not NULL terminated (e.g. by mistakingly using char
    string[2] = "ab"), then stack will be accessed (out-of-bound) until
    encountering a NULL.
- Environment variable usage
  - Copies of getenv's results are made with safe string copy operations,
    including buffers' length.
- Use of privileged functions
  - Only a ioctl call to set the printer's status
- Use of cryptography / random number sources etc
  - Strong private keys generated (e.g. RSA 4096 and ECDSA 384)
  - Reading from /dev/urandom for the papplGetRand function, but is used or 
    exposed only for creating nounced and UIDs. This aspect is clearly stated
    in the function's documentation.
  - The buffers in which sensitive data is stored (either allocated by OpenSSL
    or GnuTLS) are not zeroed after use. The same applies to cookies and
    password hashes, which are stored in PAPPL's data structures. APIs like
    OPENSSL_cleanse and BN_clear_free can be used to avoid situations in which
    the process memory is read, leaking sensitive information.
  - Timing attacks possible in papplClientHTMLAuthorize, for both cookies
    (client-webif.c:474) and passwords (client-webif.c:509 and
    system-webif.c:1354) due to the usage of a standard string compare
    functions (strcpy, strncpy)
      - Fixed in 3a0bbb2ca00df504cddd1f5541484a6eeeeabf73 and
        5c1acfee2caaf2244c0b54942201d94d8fbb1afb
- Use of temp files
  - No mktemp calls
  - papplCreateTempFile, a function exposed in the API, has a path traversal
    via the extension, which is not sanitized. (CHANGE)
      - An attacker with access to the folder with temporary files (either the
        one pointed in the TMPDIR envvar  or /tmp) can observe the filenames
        which are used. The first one is hard to deduce as it's based on
        /dev/urandom's content, but the next ones are computed with the
        Mersenne Twister PRNG.
      - This permits the attacker to predict the next filenames to be chosen,
        to create folders in advance for these filenames, and to use the
        extension "/.." * N + <absolute_target_path>.
      - The impact of this bug depends on how the path is further used by the
        application linking and using PAPPL. 
      - Fixed in ed1d3378baa0658f6390818aa202357ec1351325
- Use of networking
  - The majority of networking functions are only proxies, being exposed in
    the library API. The buffering is executed correctly, by considering the
    lengths to not create overflows.
- Use of WebKit
  - N/A
- Use of PolicyKit
  - N/A
- Any significant cppcheck results
  - Resource leak in pappl/parse-lock-log.c:93, when a return is made without
    freeing a file pointer (opening a file argument). There is only a file
    pointer (w/o any recurrence), so no concerns about memory exhaustion.
- Any significant Coverity results
  - Multiple TOCTOUs while creating directories with access + mkdir combo
    - Identified by Coverity in:
      - job.c:462
      - printer.c:887
      - system-webif.c:2329
      - system-webif.c:2329
      - system-webif.c:2340
      - system-webif.c:2557
      - system-webif.c:2568
      - mainloop-subcommands.c:783
      - mainloop-subcommands.c:1773
      - mainloop-subcommands.c:810
      - mainloop-subcommands.c:1798
    - The same situation was encountered in curl's codebase, with a fix already
    available here: https://github.com/curl/curl/commit/eb1592ec84bec8899a6642\
    01a7f2298ace059ea9.
    - Fixed in 6dcc29cb2a803c5715c7812d6925679a54a5ee23
- Any significant shellcheck results
  - Lots of errors, but in development scripts like configure and install-sh
- Any significant bandit results
  - N/A
- Any significant govulncheck results
  - N/A

The codebase is very well maintained and qualitative. The security team will
give the ACK for promoting pappl to main.

Thanks to Michel for having prompt responses, and a great openness to fix the
encountered bugs and update the security policy!

Changed in pappl (Ubuntu):
status:	New → In Progress
assignee:	Ubuntu Security Team (ubuntu-security) → nobody

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Bug attachments

autopkgtest log Edit

Add attachment

Remote bug watches

Bug watches keep track of this bug in other bug trackers.

Changed in pappl (Ubuntu):
assignee:	Christian Ehrhardt  (paelzer) → Ubuntu Security Team (ubuntu-security)

Changed in pappl (Ubuntu):
milestone:	ubuntu-23.04 → none

Changed in pappl (Ubuntu):
milestone:	none → ubuntu-23.10-feature-freeze

Ubuntupappl package

[MIR] pappl

Bug Description

Other bug subscribers

Bug attachments

Remote bug watches

Ubuntu
pappl package