I reviewed zfs-linux version 0.6.5.4-0ubuntu2 as checked into xenial.
This should not be considered a full security audit but rather a gauge of
maintainability.
- zfs is a storage system with extensive features
- Build-Depends: autotools-dev, autoconf, autogen, automake, debhelper,
dh-autoreconf, dh-systemd, dkms, libselinux1-dev, libtool, uuid-dev,
zlib1g-dev,
- Is the dkms build-depend still necessary?
- Many lintian errors, warnings:
- package-name-doesnt-match-sonames
- postinst-must-call-ldconfig
- debian-changelog-line-too-long
- init.d-script-not-marked-as-conffile
- init.d-script-not-included-in-package
- systemd-no-service-for-init-script
- Does not (yet) do encryption
- Does not itself do networking, though zfs send | zfs receive often
operate over e.g. ssh
- Provides kernel modules, zed event daemon, utilities, test tools
- zed event daemon spawns 'zedlets' on specific events; care must be taken
when writing eventlets to ensure they properly terminate.
- zed has nice daemonization routines to send error messages to the parent
process
- pre/post inst/rm scripts are automatically generated
- init scripts / systemd units discover devices in storage pools, import
pools, mount filesystems, create zvol block devices, start zed, export
any zfs shared filesystems via nfs or cifs, etc.
- No dbus services
- No setuid executables
- programs in /sbin/:
- zpios
- zhack
- zfs
- zinject
- fsck.zfs
- ztest
- mount.zfs
- zdb
- zstreamdump
- zpool
/usr/sbin/:
- zed
- The /etc/sudoers.d/zfs file is entirely commented out; it suggests some
rules that can be used to allow passwordless access to the read-only zfs
and zpool subcommands. However, this file also includes some rules to
allow members of adm, admin, or staff groups access to full root
privileges. This is not what the comment at the top of the file suggests
and I see no reason for this file to contain these rules, even if
commented out.
- Extensive udev rules to create zvol block devices and /dev/disk/by-vdev/
for more-complicated vdev configurations
- No tests run during build
- Good selection of kernel smoke tests provided in debian/tests/
- There are no provided cron jobs but system administrators would be wise
to schedule pool scrubs frequently; typically weekly or monthly is
recommended based on drive quality, level of redundancy in the pool,
data read rate and working set size.
- More than the usual number of warnings from libtool
- Some process spawning: some via zed, some via the kernel, some via
utilities. They looked careful.
- Memory management looked careful
- Not much file IO in the usual sense
- Logging looked careful, very extensive error reporting
- A lot of environment variable use, some could have unexpected
consequences
- Extensive privileged commands; far too many to carefully audit
- No cryptography (yet)
- Does not itself do networking
- There are extensive privileged portions of code, including kernel
modules; some privilege checks that make sense in OpenSolaris are
stubbed out and ignored in Linux, we should investigate further.
- Minimal use of temp files, handled carefully
- Does not use WebKit
- Minor errors reported by cppcheck
- extensive shellcheck warnings on many of the scripts; I don't think any
are used in deployed systems
- Does not use PolicyKit
The ZFS code is professionally written: errors are checked extensively,
comments are tasteful, functions are just the right size, and the design
feels clean to the point of obviousness. The finer points are of course
very complicated and we'll need to rely upon upstream for help with
anything non-trivial.
I did not take sufficient time to fully evaluate the kernel
implementation:
- large amount of security engine in zfs:
- zfs_dozonecheck_impl() for example checks if the currently executing
task is executing in the global zone or in a local zone. INGLOBALZONE
appears to be defined to (1) making the check useless on Linux. This
should probably be extended to perform similar Linux container checks.
- Most OpenZFS implementations support NFS4 ACLs; I believe zfsonlinux
supports POSIX ACLs instead. It looks like there may be support for
both in the codebase, or perhaps framework to support both styles.
- Some other ZFS implementations allow delegating authority over
datasets to specific uids -- I understand this is entirely missing on
Linux but it feels like this deserves investigation.
- Extensive amount of API is controlled via ioctl; some amount of API
may be exposed to less-privileged users via .zfs/ directories. Both
deserve closer inspections.
We must validate that unprivileged containers cannot manipulate pools
or datasets.
I took some notes while reading that I hope are useful:
- the libraries install directly to /lib, is this intentional?
- efi_use_whole_disk() leaks &efi_label via VT_ENOSPC error return
- efi_get_info() hardcodes device names and name schemes in /dev/:
sd hd md vd xvd zd dm- ram loop
- nfs_check_exportfs() hardcodes /usr/sbin/exportfs
- ztest builds in some assumptions about pathnames, safety of
/proc/pid/exe, etc. It's not a general purpose utility but may still be
useful enough to continue packaging. It's only suitable for test
systems.
- vdev_elevator_switch() broken strncmp() test
- _finish_daemonize() if devnull is 0, 1, 2, the close(devnull) call will
leave the program without that fd
- zfs_fuid_node_add() modifies zfs_fuid_info_t list without obvious
locking -- are calls to this routine suitably locked elsewhere?
- ddt_zap_update() allocates cbuf[sizeof(dde_phys)] on the stack -- which
is an array DDT_PHYS_TYPES long of structs which themselves have an
embedded array of structs with further embedded array of integers. How
likely is it for this to blow out the stack depth?
- zpool_vdev_remove() includes misleading error message that top-level
devices can be removed
- zpool_vdev_remove() awkward error message "pool must be upgrade to
support log removal"
Environment variables used:
ZINJECT_DEBUG
ZFS_ABORT
ZPOOL_IMPORT_PATH
SPA_CONFIG_PATH
ZFS_STACK_SIZE
ZFS_DEBUG
TZ
UU_DIE_ABORTS
ZFS_MODULE_LOADING
ZFS_MODULE_TIMEOUT
__ZFS_POOL_RESTRICT
ZTEST_FD_DATA
LD_LIBRARY_PATH
Please remove the %adm %admin %staff section of /etc/sudoers.d/zfs.
Please investigate the lintian messages and libtool warnings.
Security team ACK for promoting zfs-linux to main is conditional upon
the sudoers file; please ensure that is addressed before 16.04 LTS is
released. (There's no need to fix 15.10 packages and no need to get
a CVE.)
Thanks
** Changed in: zfs-linux (Ubuntu)
Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1532198
Title:
[MIR] zfs-linux
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1532198/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
