[MIR] libmspub

Ugh. Again, identical packaging and concerns from MIR bug 1124074 and bug 1124092. I feel like I'm stuck. :)

-Simple, modern packaging
-No delta
-No test suite
-No symbols file, but it's C++, so that's understandable
-debian/copyright file is a little malformed (missing license stanza), but that's an issue for NEW, not for MIR. Plus, this is from Debian so not worth a delta.
-Would be nice to see a bug subscriber
-Forgot to mention this in other reviews, but no important bugs in Debian or Ubuntu for this package (and the others). Of course, this package is rather new.

Two problems though. This will need a security review, since it's a file parser. And unlike the other MIRs above, this doesn't seem to have been in main before via libreoffice. So passing to the security team for a quick audit.

Also, it does have the lintian warning hardening-no-fortify-functions, like the other MIRs above. Björn, can you just check if that warning is a false or true positive?

Identical comments as for libcdr and libvisio MIR bugs: The hardening-no-fortify-functions is a valid lintian warning. I sent a mail to the Debian maintainer containing a bunch of patches adding multi-arch support and fixing hardening-no-fortify-functions and other lintian complaints.

Judging from the Email exchange with Rene hardening-no-fortify-functions isnt a concern for this.

@bdrung: Can we unblock this as your other changes are helpful, but no blockers?

@Björn: It's not my decision. It's the decision of the MIR team.

@MIR team: Is it okay to wait for the next Debian upload to get hardening-no-fortify-functions fixed or should I fix hardening-no-fortify-functions in Ubuntu and get back in sync with Debian with their next upload?

I have uploaded libmspub 0.0.4-1ubuntu1 which fixes the hardening-no-fortify-functions lintian warning.

> Also, it does have the lintian warning hardening-no-fortify-functions, like the other
> MIRs above. Björn, can you just check if that warning is a false or true positive?

the lintian warning remains, now enabled verbose build logs to see that the package is built this way. The lintian check is just not sufficient. We should scan build logs for that.

The lintian warning remain? Neither my pbuilder builds of 0.0.4-1ubuntu1, nor the downloaded 0.0.4-1ubuntu2 .deb files emit hardening-no-fortify-functions.

I've asked the security team to provide me feedback on my report, before pasting it in here.

The version I audited had inconsistent stack protection and fortify, and missed PIE and BIND_NOW completely. I understand those are fixed in a newer upload.

The version I audited also did not have any kind of testing. Please provide some sample mspub files and compare the results of pub2xhtml against 'known good' versions in the build process so that we can have more confidence when maintaining this package in the future.

Thanks

@Seth:
Discussed this with upstream on IRC, they are open and welcoming it, thus filed a bug at:

 https://bugs.freedesktop.org/show_bug.cgi?id=61050

Note that mspub is really young and new, just doing their first releases thus there was no immediate need for tests as there was nothing they could regress against so far -- so the need for such tests is arising just now.

Seth, while I also am super sad about the lack of tests, we don't generally block a MIR on the absence of upstream tests (especially when Canonical is not upstream). I've filed bug 1128952 about the lack (which links to the upstream bug Björn mentioned.

So I wouldn't worry about that from an approval point of view. But I await your full security audit report.

@mterry: thanks for letting me know about a test suite not being
customary.

@Björn: thanks for filing the bug report upstream and talking with
them on IRC about a test suite. Full unit tests would be a superb bonus
and probably a development assistance upstream as well. :)

- No CVE history
- No init scripts, cron jobs, dbus services, fscaps, setuid, sudo
- No binaries use PIE or BIND_NOW
- One executable is missing stack protection
- The library is missing fortify
- All binaries use RELRO
- No testsuite
- No daemons
- No crypto, no networking
- No {pre,post}{inst,rm}
- Several doxygen warnings:
  - "warning: no matching class member found for .. Possible candidates .."
- Most memory allocations are C++ native
- Code rarely checks for error conditions, or emits error conditions that
  are not checked by calling functions; continuing in the face of errors
  may be suitable for conversion from under-documented format, but isn't
  necessarily ideal

The stack protection and fortify source should be enabled for the library
and both executables. PIE and BIND_NOW would be nice.

Provisional ACK assuming:
 - stack protection and fortify are enabled

Thanks

@Seth, just a quick follow up on tests and MIRs. It's a little odd. If there are tests, we usually block on enabling them and making them fail the build. But if there aren't tests, we don't generally require the often large effort of adding them, especially when we aren't upstream and that would imply a delta we'd carry just for the tests.

Which does tend to create a perverse incentive for an upstream that wants to pass a MIR to not add tests at all. :-/

@mterry: Current upload by bdung is using dh9 and thus has fortify. see also: https://bugs.launchpad.net/ubuntu/+source/libmspub/+bug/1124082/comments/5

$ hardening-check --verbose /usr/lib/libmspub-0.0.so.0.0.4
/usr/lib/libmspub-0.0.so.0.0.4:
 Position Independent Executable: no, regular shared library (ignored)
 Stack protected: yes
 Fortify Source functions: no, only unprotected functions found!
 unprotected: memset
 unprotected: memmove
 unprotected: memcpy
 Read-only relocations: yes
 Immediate binding: no, not found!

Looking at the build log of 0.0.4-1ubuntu2, all compiler calls have -D_FORTIFY_SOURCE=2.

$ hardening-check --verbose /usr/bin/pub2*
/usr/bin/pub2raw:
 Position Independent Executable: no, normal executable!
 Stack protected: no, not found!
 Fortify Source functions: yes
 protected: printf
 Read-only relocations: yes
 Immediate binding: no, not found!
/usr/bin/pub2xhtml:
 Position Independent Executable: no, normal executable!
 Stack protected: yes
 Fortify Source functions: unknown, no protectable libc functions used
 Read-only relocations: yes
 Immediate binding: no, not found!

-fstack-protector is passed to the compiler calls for both executables. Therefore I assume that all complaints are false positives.

Remaining lintian hardening warning is a false positive. Approved.

Override component to main
libmspub 0.0.4-1ubuntu2 in raring: universe/misc -> main
libmspub-0.0-0 0.0.4-1ubuntu2 in raring amd64: universe/libs/optional -> main
libmspub-0.0-0 0.0.4-1ubuntu2 in raring armhf: universe/libs/optional -> main
libmspub-0.0-0 0.0.4-1ubuntu2 in raring i386: universe/libs/optional -> main
libmspub-0.0-0 0.0.4-1ubuntu2 in raring powerpc: universe/libs/optional -> main
libmspub-dev 0.0.4-1ubuntu2 in raring amd64: universe/libdevel/optional -> main
libmspub-dev 0.0.4-1ubuntu2 in raring armhf: universe/libdevel/optional -> main
libmspub-dev 0.0.4-1ubuntu2 in raring i386: universe/libdevel/optional -> main
libmspub-dev 0.0.4-1ubuntu2 in raring powerpc: universe/libdevel/optional -> main
libmspub-doc 0.0.4-1ubuntu2 in raring amd64: universe/doc/optional -> main
libmspub-doc 0.0.4-1ubuntu2 in raring armhf: universe/doc/optional -> main
libmspub-doc 0.0.4-1ubuntu2 in raring i386: universe/doc/optional -> main
libmspub-doc 0.0.4-1ubuntu2 in raring powerpc: universe/doc/optional -> main
libmspub-tools 0.0.4-1ubuntu2 in raring amd64: universe/utils/optional -> main
libmspub-tools 0.0.4-1ubuntu2 in raring armhf: universe/utils/optional -> main
libmspub-tools 0.0.4-1ubuntu2 in raring i386: universe/utils/optional -> main
libmspub-tools 0.0.4-1ubuntu2 in raring powerpc: universe/utils/optional -> main
17 publications overridden.