[MIR] libbackuppc-xs-perl

[Christian Ehrhardt ]

# libbackuppc-xs-perl

[Availability]
The package is in Ubuntu universe since bionic, builds for the architectures
it is designed to work on (arch any perl -> C).
This is the perl binding to backuppc and we'd need binary:libbackuppc-xs-perl

[Rationale]
This is part of the MIR activity for all dependencies of backuppc4.
That is out for quite a while and we wanted it for a while already.
v4 was a significant improvement over 3.x in terms of performance and storage
efficiency. While at the same time being compatible with the on disk format.
=> https://github.com/backuppc/backuppc/releases/tag/4.0.0
Later 4.1/4.2/4.3 further improved and stabilized things with 4.4 again
having some more good new features.

Said version 4.x needs this package as a requirement.

[Security]

Check the security History of the package
There were four XSS attacks and a config issue in 2009/2011 on backuppc itself.
But nothing on this lib.

[Quality assurance]

The package itself runs tests through autopkgtest-pkg-perl (testing the basics)
and backuppc itself does a backup/restore autopkgtest (testing the stack)

The package does not ask debconf questions higher than medium and is not going
to be installed by default.

There are no long-term outstanding bugs.

The package is maintained well in Debian/Ubuntu
The package does not deal with exotic hardware which we cannot support.

d/watch is present

No massive lintian warnings.

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

[UI standards]

Not an End-user application by itself.

[Dependencies]

No further dependencies that are not in main.

[Standards compliance]

The package does meet the FHS and Debian Policy standards.

Also, the source packaging is reasonably easy to understand and maintain.
Upstream packs an embedded zlib, but it is patches out and uses the system libs.

[Maintenance]

The Server team will subscribe for the package for maintenance

[Background]
Perl module with C backend for BackupPC 4

---

# backuppc-rsync

[Availability]
The package is in Ubuntu universe since <=precise and builds for all
architectures.

[Rationale]
This is part of the MIR activity for all dependencies of backuppc4.
That is out for quite a while and we wanted it for a while already.
v4 was a significant improvement over 3.x in terms of performance and storage
efficiency. While at the same time being compatible with the on disk format.
=> https://github.com/backuppc/backuppc/releases/tag/4.0.0
Later 4.1/4.2/4.3 further improved and stabilized things with 4.4 again
having some more good new features.

Said version 4.x needs this package as a requirement.

[Security]

Check the security History of the package:
No documented issues, but TBH it is a fork of rsync which is known to have
some issues. But since this is more or less tracking the an active stable
rsync release the same triage&fixes apply here as well.

Also most rsync CVEs are about rsync running as a server which isn't the case
here.

We still want to have security to nod about it.

[Quality assurance]

No autopkgtests present, but Backuppc does a backup/restore
autopkgtest (testing the stack) which includes this.

The package does not ask debconf questions higher than medium and is not going
to be installed by default.

There are no long-term outstanding bugs.

The package is maintained well in Debian/Ubuntu
The package does not deal with exotic hardware which we cannot support.

d/watch is present

No massive lintian warnings.

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

[UI standards]

Not an End-user application by itself (internal usage in backuppc4 only).

[Dependencies]

No further dependencies that are not in main.

[Standards compliance]

The package does meet the FHS and Debian Policy standards.

Also, the source packaging is reasonably easy to understand and maintain.
Upstream packs an embedded zlib, but it is patches out and uses the system libs.

[Maintenance]

The Server team will subscribe for the package for maintenance

[Background]
Rsync-bpc is a customized version of rsync that is used as part of
BackupPC, an open source backup system.

I'd wish this could be a module for normal rsync, but I don't consider this a
hard blocker.

It is used in a much smaller scope than "general rsync" since it isn't in a
public binary ath (only internal use). It does not build/provide a lot what
rsync is - no services, no extra tools, ...

Also this is a replacement for libfile-rsyncp-perl which can be demoted once
we get backuppc4 into main. Thereby while a "second rsync" isn't perfect we are
maintaining it anyway. And it might be better than a perl rewrite of the
same (which is what the old solution was).

The biggest issue IMHO is that it is slightly behind the main rsync in versions.
But it is not too far and e.g. currently on the Focal version which will have
lots of LTS support anyway. And backuppc-rsync has regular updates, they are
just aligning on the stable releases instead of new streams for features.

[Christian Ehrhardt  (paelzer)]

Prep work with all the checks for Filing a MIR done.
Ready for review.

[Christian Ehrhardt  (paelzer)]

[Summary]
While I'd prefer this to be an extension of real rsync instead of a fork
it seems reasonably maintained and reduced to just the bits needed for
backuppc4.

This does need a security review - almost more for the "duplication" than what
the code does, but still. I'll assign ubuntu-security

List of specific binary packages to be promoted to main: backuppc-rsync

[Duplication]
Obviously there is rsync itself, but that is so much more than the forked and
adapted version of it that is just for the use of backuppc.
And in turn - the functions needed by backuppc are not available in rsync.
So duplication is ok, with a small regret that a proper module or such for the
real rsync would be better.

[Dependencies]
OK:
- no other Dependencies to MIR due to this

[Embedded sources and static linking]
OK:
- there are zlib / popt code in the souce, but they are not used
  as d/rules configures it for --with-included-zlib=no --with-included-popt=no
- no static linking

[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 parse data formats
- does not open a port
- 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)

Problems:
- While there are no CVEs for this, it is derived from rsync which had issues.
  Those are mostly for the server components which are not present here, but
  still it is the same code.
  Furthermore we'd want security to ack on this being a rsync-fork to be
  supported.

[Common blockers]
OK:
- does not FTBFS currently
- The package has a team bug subscriber
- no translation present, but none needed for this case (user visible)?
- not a python/go package, no extra constraints to consider int hat regard
- no new python2 dependency

Problems:
- It does not have a test suite that runs at build time nor an autopkgtest
  Gladly - since this is mostly a component of backuppc4 - there is an
  autopkgtest for that which implicitly uses backuppc-rsync and thereby
  is kind of an end-to-end test covering this.
  Not perfect but ok.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok
- Upstream update history is good (somewhat behind proper rsync, but with
  regular updates)
- Debian/Ubuntu update history is ok
- 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
- Does not have Built-Using

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody (the source has refs to it, but not in the
  bits that are used for this build)
- no use of setuid (it does check for, but not use it)
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra...

[Christian Ehrhardt  (paelzer)]

MIR Team ACK for backuppc-rsync, assigning to security as explained above.

[Christian Ehrhardt  (paelzer)]

[Summary]
MIR team Ack, it is a small reasonable package without too much attack
surface nor any obvious flaws.

List of specific binary packages to be promoted to main: libbackuppc-xs-perl

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

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion

[Embedded sources and static linking]
OK:
- no static linking

Problems:
- embedded source zlib is ok (present, but patched out and not used)
- embedded source md5 is ok (this one is used, but it is from rsync which
  does not provide it as a reusable library). While there would be options
  from libbsd and libssl I think there are so many custom md5's out there,
  just many don't state it as external code - that I think this isn't a
  blocker)
  I filed https://github.com/backuppc/backuppc-xs/issues/7 for it

[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 parse data formats
- does not open a port
- 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)

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time (trivial test but yes)
  - test suite fails will fail the build upon error.
- does have a test suite that runs as autopkgtest
- The package has a team bug subscriber
- no translation present, but none needed for this case (user visible)?
- not a python/go package, no extra constraints to consider int hat regard
- no new python2 dependency

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok
- Upstream update history is good
- Debian update history is good
- 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
- Does not have Built-Using

[Upstream red flags]
OK:
- no Errors/warnings during the build (some minimal warnings due to the
  new compiler for Wformat-truncation, but nothing severe)
  => https://github.com/backuppc/backuppc-xs/issues/6
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks

[Bryce Harrington (bryce)]

We hit some issues with backuppc today, and are curious what next steps are for this MIR.
Are there any next actions we could help with?

[Seth Arnold (seth-arnold)]

Hello Bryce, I anticipate returning to work on this MIR Monday, I'd like to finish it up then, but you know how predicting progress goes. I think that's the final step before archive admins can promote the package. Thanks

[Bryce Harrington (bryce)]

Thanks Seth! Sounds great, and understood. Let us know if there's anything we can do to help.

[Seth Arnold (seth-arnold)]

I reviewed backuppc-rsync 3.1.3.0-3 as checked into hirsute. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

backuppc-rsync is a purpose-specific fork of rsync that is used by the
backuppc tool, to use their format for storing backups.

- CVE History:
  None in our database
- Build-Depends?
  debhelper-compat (= 13), libacl1-dev, libattr1-dev, libpopt-dev, zlib1g-dev,
- pre/post inst/rm scripts?
  None
- 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?
  Pretty sure None
- cron jobs?
  None
- Build logs:
  Some very-old-code kinds of warnings, not great, but probably not very
  useful in the threat model

- Processes spawned?
  Uses arrays
- Memory management?
  there's way too much old-school C stuff
- File IO?
  Quite a lot of it; controlled by remote execution.
- Logging?
  Looked fine
- Environment variable usage?
  Looked fine
- Use of privileged functions?
  Not much, looked fine
- Use of cryptography / random number sources etc?
  None
- Use of temp files?
  None
- Use of networking?
  Some, looked fine
- Use of WebKit?
  None
- Use of PolicyKit?
  None

- Any significant cppcheck results?
  Nothing too bad in the context of this tool
- Any significant Coverity results?
  Nothing too bad, upstream handled the issues quickly
- Any significant shellcheck results?
  None
- Any significant bandit results?
  None

backuppc-rsync is based on some *old* code, which is both good and bad.
It's a time-worn tool with some coding practices that aren't fantastic
today, in a language I wish we'd see less of, but being built on rsync is
no bad thing.

Upstream for backuppc-rsync was responsive, reached out for more
information for the issues I filed, and handled them well. None of the
issues were particularly pressing for the threat model here: it's not
like the remote endpoint is going to be run under control of malicious
entities.

Security team ACK for promoting backuppc-rsync to main.

Some notes I took while reviewing:

bpc_poolRefFileRead() fd leak in multiple error returns

read_delay_line() read() doesn't nul terminate deldelay_buf, but
strchr() expects it to be nul-terminated

bpc_attrib_dirRead() can pass fd to functions without being initialized?
hard to untangle

send_implied_dirs() buffer overwrite? hard to untangle -- 17999

send_extra_file_list() buffer overread? hard to untangle -- 17979

add_dirs_to_tree() buffer overwrite? hard to untangle, -- 17962

bpc_opendir() use-after-free, free(d->entries)

bpc_attrib_dirRead() can use nRead without initializing it first

bpc_attrib_xattrCopy() can leak 'key' if 'value' couldn't be allocated

add_dirs_to_tree() buffer overwrite? hard to untangle -- 17930

send_extra_file_list() buffer overread? hard to untangle -- 17894

bpc_poolWrite_copyToPool() appears to read and write eight bytes at a
time; is this a large operation? If this is responsible for a lot of IO
this could be pretty brutally bad.

bpc_poolWrite_copyToPool() can leak fdWrite if fdRead open() fails

add_cnk_list_entry() ...

[Christian Ehrhardt  (paelzer)]

Thank you Seth, this is fully ready now and just needs to be promoted.
It also is in component-mismatches already (for a long time)
I'll ping the archive admins to do so.

[Chris Halse Rogers (raof)]

Done! Apparently today Firefox will lock up if I try and paste anything into it, so you don't get the change-overrides output, but backuppc-rsync 3.1.3.0-3 source and binaries are now in impish main, as are libbackuppc-xs-perl 0.62-1build1 source and binaries.