[Bug 1271653] Re: [MIR] libiscsi

[Seth Arnold]

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
Server Team, which is subscribed to libiscsi in Ubuntu.
https://bugs.launchpad.net/bugs/1271653

Title:
  [MIR] libiscsi

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