[Samba] Fix for CVE-2015-5299 denies access to ZFS snapshots due to overly strict condition checking

[QIU Quan]

OS platform: FreeBSD 10.1-RELEASE-p25
Filesystem: ZFS
Samba version: upgraded from 4.1.17 to 4.1.22

Problem:

I have been using the shadow_copy2 and zfsacl VFS modules to enable
access control and the Previous Versions feature for Windows clients.
With /usr/local/etc/smb4.conf configured this way (excerpt):

    vfs objects = shadow_copy2 zfsacl
    shadow:snapdir = .zfs/snapshot
    shadow:format = GMT-%Y.%m.%d-%H.%M.%S
    shadow:sort = desc

After updating the package to 4.1.22, which fixes several
vulnerabilities, I discovered that I could no longer view previous
versions of files or folders.

Checking out the log with log level at 10, I found some error
messages, which indicates access to the snapdir, .zfs/snapshots, is
denied.

Error messages:

    ../source3/modules/vfs_zfsacl.c:56(zfs_get_nt_acl_common)
      acl(ACE_GETACLCNT, /tank/share/.zfs/snapshot): Operation is not
supported on the filesystem where the file reside
    ../source3/smbd/open.c:128(smbd_check_access_rights)
      smbd_check_access_rights: Could not get acl on
/tank/share/.zfs/snapshot: NT_STATUS_NOT_SUPPORTED
    ../source3/modules/vfs_shadow_copy2.c:1170(check_access_snapdir)
      user does not have list permission on snapdir /tank/share/.zfs/snapshot
    ../source3/modules/vfs_shadow_copy2.c:1339(shadow_copy2_get_shadow_copy_data)
      access denied on listing snapdir /tank/share/.zfs/snapshot

Possible fix:

Reading the patch at
https://download.samba.org/pub/samba/patches/security/samba-4.1.21-security-2015-12-16.patch,
I see the CVE-2015-5299 fix simply takes the returned status from
smbd_check_access_rights() and only checks if it is OK, and if not,
then fails, also classifying other error statuses as access denied.

Meanwhile, smbd_check_access_rights() in source3/smbd/open.c indeed
returns NT_STATUS_ACCESS_DENIED in some way. I wonder if we change the
line

    if (!NT_STATUS_IS_OK(status)) {

in check_access_snapdir() in source3/modules/vfs_shadow_copy2.c to

    if (status == NT_STATUS_ACCESS_DENIED) {

would result in more accurate outcomes and avoid other statuses such
as NT_STATUS_NOT_SUPPORTED ending up access denied.


-- 
裘佺 (QIU Quan) <jackqq at gmail.com>