I reviewed libblockdev version 2.16-2 as checked into bionic. This should
not be considered a full security audit but rather a quick gauge of
maintainability.
- libblockdev is a plugin-based library to work with block devices,
providing an API based interface that knows how to either perform
underlying operations directly or call the necessary command line
applications as appropriate.
- libblockdev does not itself daemonize
- no networking
- automatically generated pre/post inst/rm scripts
- no initscripts
- no dbus services
- no setuid files
- no executables in PATH
- no sudo fragments
- no udev rules
- There is a test suite; it is not run during the build. I suspect keeping
this test suite disabled is the right approach.
- No cronjobs
- Noisy build logs, primarily from the documentation tools; not ideal, but
at least not much from the code itself
- Several plugins execute helper tools; the glib helpers make the
resulting code fairly complex, but it looks like the executions were
carefully written
- memory management is careful; allocated memory is quickly freed when it
is no longer used to simplify error handling
- normally 'files' being opened are block devices
- Logging functions looked careful
- No environment variable use
- Does not itself do cryptography -- drives LUKS, VeraCrypt, etc via
helpers
- Does not do networking
- Does not use temporary files
- Does not use WebKit
- Does not use PolicyKit
- Does not use JavaScript
- Mostly clean cppcheck results; s390 code has legitimate errors:
[src/plugins/s390.c:293]: (error) Used file that is not opened.
[src/plugins/s390.c:293]: (error) Resource handle 'fd' freed twice.
[src/plugins/s390.c:968]: (error) Resource leak: fd
The s390 code does not feel as mature as the rest of the code base.
I suspect a deeper inspection of the s390 sources would find more issues.
If it can be disabled without significant loss of functionality then I
recommend we disable it.
Security team ACK for promoting libblockdev to main.
Here's some notes I took while reviewing the code, in the hopes that they
are useful:
- bd_s390_dasd_online() double-closes fd, uses fd when it isn't open
- bd_s390_zfcp_offline() leaks fd in rc == EOF branch
- bd_s390_zfcp_online() calls fclose(fd) in a branch when fd is known to
be NULL
- bd_s390_zfcp_scsi_offline() does not check fopen(hba_path, "r") for an
error return
- bd_s390_zfcp_scsi_offline() does not check fopen(wwpn_path, "r") for an
error return
- bd_s390_zfcp_scsi_offline() does not check fopen(lun_path, "r") for an
error return
- bd_crypto_tc_open(), luks_format(), (and other functions) do not zero
out secrets such as passwords. (This is difficult to do; the goal is not
to be perfect, but to try. Simply adding memset_s() calls would be a
step in the right direction.)
- Many routines in src/plugins/crypto.c call strlen(passwd), some other
functions are described as taking binary data in password parameters. Is
there a chance of one interface setting, changing, or removing a
password, that other interfaces cannot work with?
- write_escrow_data_file() writes what looks like a secret key equivalent
to a file but does not itself set a safe umask before doing so.
I reviewed libblockdev version 2.16-2 as checked into bionic. This should
not be considered a full security audit but rather a quick gauge of
maintainability.
- libblockdev is a plugin-based library to work with block devices,
providing an API based interface that knows how to either perform
underlying operations directly or call the necessary command line
applications as appropriate.
- There are no CVEs in our database
- Build-Depends: debhelper, libtool, dh-python, python3:any, libglib2.0-dev ry1.0-dev, libcryptsetup-dev libdevmapper-dev libudev-dev introspection, pylint
libgireposito
libsystemd-dev, libdmraid-dev, libvolume-key-dev, libbytesize-dev,
libnss3-dev libparted-dev libmount-dev libblkid-dev libpython3-dev,
libkmod-dev gtk-doc-tools, gobject-
- libblockdev does not itself daemonize
- no networking
- automatically generated pre/post inst/rm scripts
- no initscripts
- no dbus services
- no setuid files
- no executables in PATH
- no sudo fragments
- no udev rules
- There is a test suite; it is not run during the build. I suspect keeping
this test suite disabled is the right approach.
- No cronjobs
- Noisy build logs, primarily from the documentation tools; not ideal, but
at least not much from the code itself
- Several plugins execute helper tools; the glib helpers make the plugins/ s390.c: 293]: (error) Used file that is not opened. plugins/ s390.c: 293]: (error) Resource handle 'fd' freed twice. plugins/ s390.c: 968]: (error) Resource leak: fd
resulting code fairly complex, but it looks like the executions were
carefully written
- memory management is careful; allocated memory is quickly freed when it
is no longer used to simplify error handling
- normally 'files' being opened are block devices
- Logging functions looked careful
- No environment variable use
- Does not itself do cryptography -- drives LUKS, VeraCrypt, etc via
helpers
- Does not do networking
- Does not use temporary files
- Does not use WebKit
- Does not use PolicyKit
- Does not use JavaScript
- Mostly clean cppcheck results; s390 code has legitimate errors:
[src/
[src/
[src/
The s390 code does not feel as mature as the rest of the code base.
I suspect a deeper inspection of the s390 sources would find more issues.
If it can be disabled without significant loss of functionality then I
recommend we disable it.
Security team ACK for promoting libblockdev to main.
Here's some notes I took while reviewing the code, in the hopes that they
are useful:
- bd_s390_ dasd_online( ) double-closes fd, uses fd when it isn't open zfcp_offline( ) leaks fd in rc == EOF branch zfcp_online( ) calls fclose(fd) in a branch when fd is known to zfcp_scsi_ offline( ) does not check fopen(hba_path, "r") for an zfcp_scsi_ offline( ) does not check fopen(wwpn_path, "r") for an zfcp_scsi_ offline( ) does not check fopen(lun_path, "r") for an
- bd_s390_
- bd_s390_
be NULL
- bd_s390_
error return
- bd_s390_
error return
- bd_s390_
error return
- bd_crypto_ tc_open( ), luks_format(), (and other functions) do not zero
out secrets such as passwords. (This is difficult to do; the goal is not
to be perfect, but to try. Simply adding memset_s() calls would be a
step in the right direction.)
- Many routines in src/plugins/ crypto. c call strlen(passwd), some other
functions are described as taking binary data in password parameters. Is
there a chance of one interface setting, changing, or removing a
password, that other interfaces cannot work with?
- write_escrow_ data_file( ) writes what looks like a secret key equivalent
to a file but does not itself set a safe umask before doing so.
Thanks