I reviewed debsig-verify version 0.10 as checked into utopic. This
shouldn't be considered a full security audit, but rather a quick gauge
of maintainability.

debsig-verify is awkward. Extensive use is made of global state and much
of the program logic depends upon side-effects to this global state,
sometimes in function calls that happened thirty lines previously. It
would be extremely difficult to write function-level unit tests for
this program.

debsig-verify uses some library routines from dpkg; while I inspected
these calls and didn't see a problem, I must point out that dpkg was
only ever designed to handle packages that already passed the usual
hash-and-signatures check provided by apt and may not be suitable for
use on untrusted input. debsig-verify is taking a risk that the dpkg
implementations of these functions will not become a danger in the future,
even assuming they are fine today.

The following must be corrected before we can rely upon debsig-verify:

- Makefile tries to set -Wall, but it isn't used, obvious warnings are missing
- getSigKeyID() no error checks on fork()
- getSigKeyID() no error checks on t = fread(buf, 1, sizeof(buf), deb_fs);
- getSigKeyID() can be tricked into an infinite loop, no feof checks
- gpgVerify() no error checks on fork()

Because the signatures are embedded in the .deb files, necessarily only
certain sections are measured:

  debian-binary
  control.tar control.tar.gz control.tar.xz
  data.tar data.tar.gz  data.tar.xz  data.tar.bz2  data.tar.lzma
  - Possibly it will copy different objects with these names than 'ar' or
    'dpkg' will use when installing the package.
  - Installing a package will require at least twice the package size
    in free space before starting the process because a copy is made

Test cases should be prepared with member filenames including trailing
spaces, leading spaces, trailing / chars, etc. (I have not inspected how
'ar' or 'dpkg' unpack .deb files; if they similarly rewind the file
before unpacking named sections they will likely extract the same
sections that debsig-verify extracts. If they don't rewind for each
section they may unpack different sections.)

A "polyglot" test package with multiple control tarballs or data tarballs
should be tested as well to ensure the measured sections are the ones
chosen for installing.

I believe these may not be expected behaviours but they wouldn't be
under control of potentially malicious entities:

- verifyGroupRules() looks like it requires negative grp->min_opt in order
  for only mandatory match groups to suffice
- checkSelRules() looks like it requires negative grp->min_opt in order
  for only mandatory match groups to suffice

The following surprising facts are consequences of the broken CFLAGS
handling in the Makefile:

- getSigKeyID() 'deb' parameter is unused
- checkSelRules() 'deb' parameter is unused
- checkSelRules() 'deb' parameter shadows global 'deb' declaration
- verifyGroupRules() 'deb' parameter shadows global 'deb' declaration

It would be nice to fix these before shipment just so the presence of
these parameters won't be surprising in maintenance but they are unlikely
to be a harm at the moment.

And potentially surprising, 'gpg' looks like it may be executed often:

- verifyGroupRules() executes gpg via getKeyID() and getSigKeyID()
  potentially many times before performing the ultimate verification


debsig-verify is a significant complexity jump compared to using gpg to
verify a detached signature; debsig-verify's extra complexity is largely
due to three features:
 - embedding the signature in the .deb
 - an xml-based policy environment that delivers keys and
 - describes allowed and forbidden package types for those specific keys.

If we don't care about these three specific features we should use
something simpler. If one of these features will be useful, then we
can make debsig-verify work but I don't think it is ready for handling
malicious packages yet.

Thanks


** Changed in: debsig-verify (Ubuntu)
     Assignee: Seth Arnold (seth-arnold) => (unassigned)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1358272

Title:
  [MIR] debsig-verify

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/debsig-verify/+bug/1358272/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to