[Bug 1271653] Re: [MIR] libiscsi

Thu Feb 18 02:55:51 UTC 2016

I reviewed libiscsi version 1.12.0-2 as checked into xenial. This
shouldn't be considered a full security audit but rather a quick gauge of
maintainability.

- libiscsi provides user-space iscsi initiator support so applications can
  use iscsi targets without needing privileged access to the host.
- Build-Depends: debhelper, dh-autoreconf, libcunit1-dev
- Only does CHAP, MD5 cryptography
- Extensive networking support
- Does not itself daemonize
- Does not itself listen on the network
- No pre/post inst/rm
- No initscripts
- No dbus services
- No setuid executables
- iscsi-test-cu, iscsi-ls, iscsi-swp, iscsi-inq, iscsi-readcapacity16
  executables in path
- No sudo fragments
- No udev rules
- iscsi-test-cu looks like an incredible test suite, if it functions as
  advertised
- No cron jobs
- Clean build logs

- No subprocesses spawned
- Very careful memory management, nice per-scsi-task abstraction layer
- No file IO
- Extensive error logging, spot checks all looked careful
- Several environment variables are used:
  LD_ISCSI_GET_LBA_STATUS
  LD_ISCSI_DEBUG (not-packaged ld_iscsi.so)
  LIBISCSI_DEBUG
  LIBISCSI_TCP_USER_TIMEOUT
  LIBISCSI_TCP_KEEPCNT
  LIBISCSI_TCP_KEEPINTVL
  LIBISCSI_TCP_KEEPIDLE
  LIBISCSI_TCP_SYNCNT
  LIBISCSI_BIND_INTERFACES
  LIBISCSI_CHAP_USERNAME
  LIBISCSI_CHAP_PASSWORD
  Results were typically handed to atoi(3) and then used to set settings;
  maybe strtoul(3) would be more robust but this is fine
- No privileged operations
- Essentially no cryptography -- CHAP barely counts. Use this on trusted
  networks or over IPsec. (Trusted networks is the expected use, this
  isn't unreasonable.)
- Extensive networking; spot checks on networking syscalls all looked
  careful
- No portions of code looked more privileged than others
- No temporary file handling
- Does not use WebKit
- Clean cppcheck
- Clean shellcheck
- No PolicyKit

libiscsi looks professionally programmed; SCSI and TCP/IP aren't exactly
easy things but the design of this package looks careful and thoughtful. I
haven't inspected the SCSI state machine in any way but the methods I
inspected all looked like they inspected preconditions and logged
violations, all pieces feel like logical separations of concerns and
designed for testing.

The iscsi-test-cu test suite looks incredible if true. No tests are run
during the build but it would be difficult to test these functions deeply
during build.

The only bug I found is a series of slightly misleading error messages:

- lib/login.c has instances of 'aprintf failed' error strings but the
  memory allocation is stack-based buffers, and the failed function is
  snprintf().

ld_iscsi looks like a _very_ cute hack -- pity it is too immature to
enable it but I love the idea. (I did not review its code because it's
clearly labeled not-yet-ready for use.)

Security team ACK for promoting libiscsi to main. Please keep an eye on
ld_iscsi in future syncs with Debian to ensure it doesn't get released
before it is ready.

Thanks

** Changed in: libiscsi (Ubuntu)
     Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)

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

Title:
  [MIR] libiscsi

Status in libiscsi package in Ubuntu:
  New

Bug description:
  Checking against
  https://wiki.ubuntu.com/UbuntuMainInclusionRequirements:

  1: libiscsi is already available in universe: https://launchpad.net/ubuntu/+source/libiscsi
  =====

  2: Rationale:
  =====
  a) iSCSI has become the de-facto standard for connecting infrastructure servers to SAN/NAS storage
  b) qemu, which is already in main, depends on libiscsi for iscsi support:
  https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1271573

  3: Security
  =====
  a) no entries found in CVE database
  b) no entries found in Secunia database
  c) no entries found in Ubuntu CVE tracker
  d) executable security:

  NO executables which have the suid or sgid bit set.
  NO executables in /sbin, /usr/sbin.
  NO packages which install daemons (/etc/init.d/*)
  NO packages which open privileged ports (ports < 1024).
  NO add-ons and plugins to security-sensitive software (filters, scanners, UI skins, etc)

  4: QA
  =====

  a) no configuration required - installs shared library and supporting
  tools for inquiring iSCSI portals and targets

  b) no debconf questions

  c) no long-term outstanding important bugs

  d) 
  Debian PTS: http://packages.qa.debian.org/libi/libiscsi.html
  * 4 lintian warning about missing manpages for 2 binaries in the support tools
  * 2 buildd log warnings
  * 1 bug that a newer upstream version is available

  Ubuntu
  https://bugs.launchpad.net/ubuntu/+source/libiscsi/+bugs
  * 1 NEW bug complaining that libiscsi doesn't work with multipath on EMC

  Upstream:
  * no open bugs: https://github.com/sahlberg/libiscsi/issues?page=1&state=open

  e) Maintenance in Debian/Ubuntu is acceptable. However, several newer upstream releases are available: https://sites.google.com/site/libiscsitarballs/libiscsitarballs/
  --> this needs to be fixed in Debian. Upstream development is very active.

  f) no exotic hardware

  g) test-tool is available, but not run during build because it
  requires a running iSCSI portal / target, which cannot be expected to
  be available on a build server

  h) debian/watch does *not* exist, but debian/copyright contains a link to github
  --> this needs to be fixed in Debian

  5.) UI standards: n/a
  =====

  6.) Dependencies:
  =====

  * shared library has no external dependencies except for libc6, and only build-deps on debhelper and libpopt-dev which are already in main
  * binaries depend on the shared library libiscsi1 which is created out of this source package, and libpopt0, which is already in main

  7.) Standards compliance
  =====
  No issues found except for missing man pages for the support tools and some minor buildd warnings, already mentioned in 4d)

  8.) Maintenance
  =====

  Debian maintenance seems to be OK, so syncing should be enough.

  9.) Background info
  =====

  See 2.) Rationale

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/libiscsi/+bug/1271653/+subscriptions