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

[Jeremy Allison]

On Fri, Jan 01, 2016 at 09:38:09PM +0800, QIU Quan wrote:
> 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.

Thanks QIU, can you log this as a bug at bugzilla.samba.org
so we can track it ?

Thanks,

Jeremy.