[MIR] lerc

Bug #1977551 reported by Sebastien Bacher
18
This bug affects 1 person
Affects Status Importance Assigned to Milestone
lerc (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

[Availability]
The package lerc is already in Ubuntu universe.
The package lerc build for the architectures it is designed to work on.
It currently builds and works for architetcures: amd64 arm64 armhf ppc64el riscv64
s390x fails since upstream doesn't support big endian architecture
https://github.com/Esri/lerc/blob/master/src/LercLib/Defines.h#L56
which was discussed on in Debian
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=990789

Debian is built tiff without lerc support to workaround that issue.
Ideally it would be available on every architecture but it's probably not important for s390x users so we think the current solution is acceptable.
Link to package https://launchpad.net/ubuntu/+source/lerc

[Rationale]
- The package lerc is required in Ubuntu main as a new depends of tiff.

- The package lerc is required in Ubuntu main no later than aug 25 due to feature freeze

[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 not install services, timers or recurring jobs
- Packages does not open privileged ports (ports < 1024)
- Packages does not contain extensions to security-sensitive software

[Quality assurance - function/usage]
- The package works well right after install

[Quality assurance - maintenance]
The package is rather now but seems well maintained in Debian/Ubuntu and has no open bug reports
- 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/573017632/buildlog_ubuntu-jammy-amd64.lerc_3.0+ds-1_BUILDING.txt.gz

- The package runs an autopkgtest, and is currently passing on
  this amd64 arm64 armhf ppc64el list of architectures, link to test logs https://autopkgtest.ubuntu.com/packages/l/lerc

The i386 failure is due to python3 packages installability issues on that architecture. The s390x one is due to the fact that the library isn't build on that architecture as explained earlier. This is ok because neither of those are sign of problems with the library.

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

# lintian --pedantic
running with root privileges is not recommended!
P: lerc source: package-uses-old-debhelper-compat-version 12
P: lerc source: very-long-line-length-in-source-file CHANGELOG.md line 23 is 567 characters long (>512)
P: lerc source: very-long-line-length-in-source-file README.md line 23 is 620 characters long (>512)

those are only minor warnings

- Lintian overrides are not present

- This package does not rely on obsolete or about to be demoted packages.
- This package has no python2 or GTK2 dependencies

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

- Packaging and build is easy, link to d/rules https://salsa.debian.org/debian-gis-team/lerc/-/blob/master/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]
- Owning Team will be desktop-packages
- Team is not yet, but will subscribe to the package before promotion

- The package successfully built during the most recent test rebuild

[Background information]
The Package description explains the package well
Upstream Name is lerc
Link to upstream project https://github.com/Esri/lerc

Tags: sec-1089
Changed in lerc (Ubuntu):
assignee: nobody → Lukas Märdian (slyon)
Revision history for this message
Lukas Märdian (slyon) wrote :
Download full text (4.5 KiB)

Review for Package: src:lerc

[Summary]
https://www.osgeo.org/projects/lerc-limited-error-raster-compression/

LERC is an open-source image or raster format which supports rapid encoding
and decoding for any pixel type (not just RGB or Byte). The codebase seems
mature and didn't see many updates recently. It does provide an internal
test-suite that is being run at build time, using the just build binaries and
during autopkgtest using the installed library version.

MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

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

List of specific binary packages to be promoted to main: liblerc3, python3-lerc
Specific binary packages built, but NOT to be promoted to main: liblerc-dev

Notes:
#0 Big endian architectures (s390x) are not supported upstream.
#1 The -dev package contains a static build of the library.
   It should therefore be excluded from the promotion
#2 Requesting security review because it's parsing image/TIFF data

Required TODOs:
#3 d/rules uses "-c0" in dpkg-gensymbols/dh_makeshlibs, ignoring the symbols
   check. Why is this? Please explain, or fix this behavior. It should fail
   the build as usual, if symbols change.

Recommended TODOs:
#4 Lintian: lerc source: package-uses-old-debhelper-compat-version 12
   dh-compat should be updated or at least a bug should be reported against
   the Debian package
#5 SetuptoolsDeprecationWarning: setup.py install is deprecated.
   This deprecation warning should be fixed or at least a bug should be reported
   against the Debian package

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - SRCPKG 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 dependencies in main that are only superficially tested requiring
  more tests now.

Problems:
- exclude liblerc-dev, as it includes a statically linked library

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

Problems:
- statically linked library inside the -dev package, built in parallel to the
  dynamic library in "build-static" directory from debian/rules

[Security]
OK:
- history of CVEs does not look concerning
- 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)

Problems:
- does parse data formats (image/TIFF data)

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build up...

Read more...

Changed in lerc (Ubuntu):
assignee: Lukas Märdian (slyon) → Ubuntu Security Team (ubuntu-security)
milestone: none → ubuntu-22.10-feature-freeze
Revision history for this message
Lukas Märdian (slyon) wrote :

This can enter security queue while Desktop considers the remaining open required todos.
Assigning.

Also setting the milestone matching the requested 22.10 feature freeze deadline in August.

Changed in lerc (Ubuntu):
milestone: ubuntu-22.10-feature-freeze → ubuntu-22.08
tags: added: sec-1089
Revision history for this message
Sebastien Bacher (seb128) wrote :

#1 ack

#2 thanks!

#3 asked about the symbol to Debian on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013146 which they closed

'No, because C++ libraries have frequent disappearing symbols that are not ABI breaks. See:

 https://wiki.debian.org/UsingSymbolsFiles#C.2B-.2B-_libraries

The package uses pkgkde-symbolshelper that has better support for C++:

 https://qt-kde-team.pages.debian.net/symbolfiles.html

We don't want the build to fail during transitions just because some template was optimized out for example.

Kind Regards, '

#4 compat level report to Debian, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013147 which they responded by

'
Compat level 12 is recommended for packages maintained in the Debian GIS team:

 https://debian-gis-team.pages.debian.net/policy/policy.html#debhelper

We don't track the latest to ease backports for UbuntuGIS and OSGeoLive.

See:

 https://lists.debian.org/debian-gis/2021/09/msg00009.html'

Would you want us to carry a Delta in Ubuntu for it? (I would prefer not)

#5 reported deprecation warning to Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013148

Which got that reply

'This is not a problem.

Kind Regards,

Bas

PS. Please stop filing bugreports for minor cosmetic issues. '

:-(

Revision history for this message
Lukas Märdian (slyon) wrote :

#1 & #2 -> OK!
#4 & #5 -> Those are only recommended TODOs and won't block the MIR, thanks for pestering the DM about it, though. Those will most probably come back to us eventually, but it's fine for now!

#3: Thanks, I wasn't aware of pkgkde-symbolshelper!
So at least they are using some advanced tools to track their (C++) symbols.
The problem I see with this is that during testing it does not detect an API/ABI breakage, that I introduced on purpose. E.g. if I rename the "lerc_encodeForVersion" API to "lerc_encodeForVersionX", dpkg-buildpackage still passes the build, displaying a warning at least:
dh_makeshlibs -- -v3.0 -c0
dpkg-gensymbols: warning: some new symbols appeared in the symbols file: see diff output below
dpkg-gensymbols: warning: debian/liblerc3/DEBIAN/symbols doesn't match completely debian/liblerc3.symbols
--- debian/liblerc3.symbols (liblerc3_3.0_amd64)
+++ dpkg-gensymbolsaQREWf 2022-06-21 15:22:43.440908065 +0000
@@ -382,4 +382,5 @@
  lerc_decodeToDouble@Base 3.0
  lerc_encode@Base 3.0
  lerc_encodeForVersion@Base 3.0
+ lerc_encodeForVersionX@Base 3.0
  lerc_getBlobInfo@Base 3.0

But "lerc_encodeForVersion" is still there as a symbol, even though I removed/renamed it from the public header and implementation and the build doesn't fail (which is what I would expect in such case)... So I wonder if those tools are being used correctly.

Re-reading the MIR rules, we only ask for "symbols tracking is in place", not that it needs to fail the build... so this could be a tentative MIR team ACK.

But let me check back with the other MIR team members (while waiting for security-review anyway), just to be sure that this is what was intended here.

PS: C++ libraries seem to be a special case anyway, here's an interesting read:
https://wiki.debian.org/UsingSymbolsFiles#C.2B-.2B-_libraries

Revision history for this message
Lukas Märdian (slyon) wrote (last edit ):

After consultation with fellow MIR team members, we agreed upon requiring the build to fail on API/ABI changes and that mere "tracking" of symbols is not enough.

Seb, could you please make sure changed/removed symbols are correctly detected during the lerc build and make the build fail if this happens? Currently, removing (renaming) a symbol doesn't seem to be detected at all, see example given in the previous comment.

IMO this could either be done by dropping the "pkgkde_symbolshelper" and the "dh_makeshlibs" override. Or maybe pkgkde_symbolshelper can be proberly configured and combined with a dh_makeshlibs -c1 (or bigger) setting to make it work.

Security-review can still go on in parallel.

Thanks.

Changed in lerc (Ubuntu):
status: New → Incomplete
Revision history for this message
Sebastien Bacher (seb128) wrote :

We removed the dpkg-gensymbols override in https://launchpad.net/ubuntu/+source/lerc/3.0+ds-1ubuntu1

Changed in lerc (Ubuntu):
status: Incomplete → New
Revision history for this message
Mark Esler (eslerm) wrote :

I reviewed lerc 3.0+ds-1ubuntu1 as checked into kinetic as well as upstream's 4.0.0. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

Please note that LERC 4.0.0 was released after MIR process began (2022-06-15).

> LERC is an open-source image or raster format which supports rapid encoding and decoding for any pixel type (not just RGB or Byte). Users set the maximum compression error per pixel while encoding, so the precision of the original input image is preserved (within user defined error bounds).

- CVE History:
  - none
- Build-Depends?
  - C/C++
    - ld-linux-x86-64.so.2
    - libgcc_s.so.1
    - libm.so.6
    - libstdc++.so.6
    - linux-vdso.so.1
  - Python
    - numpy
    - ctypes
    - timeit
    - platform
    - os
- pre/post inst/rm scripts?
  - yes, generated by dh_python3
  - postint: compiles python3-lerc
  - prerm: attempts to remove python3-lerc
  - tested install and removal
- init scripts?
  - none
- systemd units?
  - none
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - none
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - has build tests and autopkgtests
- cron jobs?
  - none
- Build logs:
  - only setup.py being deprecated as previously mentioned

- Processes spawned?
  - none
- Memory management?
  - 86 memcpy calls, 1 malloc call
  - memcpy and malloc use looks good
  - defensive memory management tools are provided by functions of this library
    - see lerc_computeCompressedSize to help memory allocation
- File IO?
  - only for tests
- Logging?
  - none for C/C++, some commented out print lines
  - Python has some error messages and a verbose flag
- 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?
  - no, only js files not including in MIR
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - js results ignored
  - a minor bitshift error edgecase in private method
    - bitshift never errored as written
    - upstream addressed concern immediately with https://github.com/Esri/lerc/pull/224
- Any significant shellcheck results?
  - none
- Any significant bandit results?
  - none

LercTest/main.cpp and _lerc.py contain "TestLegacyData" which includes unshared files and a specific directory setup ('D:/GitHub/LercOpenSource_v2.5/testData/'). It would be nice if upstream moved this to ./testData and our test suite was expanded.

Upstream can make optional or remove timer() from _lerc.py when debug/performance information is not needed. https://github.com/Esri/lerc/issues/221

@seb128, thanks for reporting @slyon's #5. Deprecating setup.py would be nice to see.

Security team ACK for promoting lerc to main liblerc3 and python3-lerc. Packages using ./OtherLanguages/CSharp/ and ./OtherLanguages/js/ are not acknowledged for main and require a separate MIR.

Changed in lerc (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
status: New → In Progress
Revision history for this message
Luís Infante da Câmara (luis220413) wrote :

The version of libtiff5 in kinetic-proposed (4.4.0-4ubuntu2) depends on liblerc3.

Changed in lerc (Ubuntu):
status: In Progress → Fix Committed
Steve Langasek (vorlon)
Changed in lerc (Ubuntu):
status: Fix Committed → In Progress
Revision history for this message
Luís Infante da Câmara (luis220413) wrote :

ricotz mentioned yesterday on IRC that an i386 build is needed.

Revision history for this message
Sebastien Bacher (seb128) wrote :

it's building on i386 now

Revision history for this message
Sebastien Bacher (seb128) wrote :

which ended up depwaiting on python3-numpy, we will build libtiff with lerc on i386 instead

Revision history for this message
Jeremy Bicha (jbicha) wrote :

Updated tiff: https://launchpad.net/ubuntu/+source/tiff/4.4.0-4ubuntu3

And I forwarded our request to Debian to not build tiff with lerc on i386
https://bugs.debian.org/1017958

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

All required TODOs have been deal with. The situation of i386 is clear now too. +1 from the MIR team as discussed on IRC.

Changed in lerc (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Sebastien Bacher (seb128) wrote :

Override component to main
lerc 3.0+ds-1ubuntu1 in kinetic: universe/misc -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic amd64: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic arm64: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic armhf: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic ppc64el: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic riscv64: universe/libdevel/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic amd64: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic arm64: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic armhf: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic ppc64el: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic riscv64: universe/libs/optional/100% -> main
Override [y|N]? y

Changed in lerc (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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