Bug 12149 - smbd: cannot load a Windows device driver from a Samba share via SMB2
Summary: smbd: cannot load a Windows device driver from a Samba share via SMB2
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.5.0rc1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-12 20:35 UTC by Uri Simchoni
Modified: 2016-09-20 07:05 UTC (History)
3 users (show)

See Also:


Attachments
git-am fix for 4.5.0 (25.11 KB, patch)
2016-08-18 18:54 UTC, Uri Simchoni
ddiss: review+
Details
git-am fix for 4.4.next (25.05 KB, patch)
2016-08-18 18:58 UTC, Uri Simchoni
ddiss: review+
Details
git-am fix for 4.3.next (25.05 KB, patch)
2016-08-18 18:58 UTC, Uri Simchoni
ddiss: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Uri Simchoni 2016-08-12 20:35:46 UTC
The Windows service manager may load device drivers from an SMB share. In doing so it opens the driver file with FILE_EXECUTE and FILE_READ_ATTRIBUTES permission. Samba rejects the read for lack of FILE_READ_DATA in the access mask.

It turns out this works against a Windows server, due to an undocumented (as of ver 49 of MS-SMB2) behavior, which grants FILE_READ_DATA if FILE_EXECUTE is also granted.
Comment 1 Uri Simchoni 2016-08-12 20:40:05 UTC
To reproduce, using a member server and a Windows client also joined, run those commands from an elevated cmd prompt:

sc create mydriver type=kernel start=demand error=normal binpath=\\my-server.my-domain.local\my-share\mydriver.sys
sc start mydriver

The driver should have Execute permission for the client's machine account, otherwise it fails at open stage.
Comment 2 Jeremy Allison 2016-08-12 20:41:09 UTC
This one is interesting - is the implicit FILE_READ_DATA reported back as access granted when querying the current permissions of the handle ?
Comment 3 Uri Simchoni 2016-08-12 21:02:58 UTC
(In reply to Jeremy Allison from comment #2)

Haven't checked. Can you point me to the torture tests in Samba that fetch the granted access? (I will find them eventually if you don't have time - this needs to be verified along with the driver loading fix and copychunk behavior)

Thanks,
Uri
Comment 4 Jeremy Allison 2016-08-12 22:15:35 UTC
source4/torture/smb2/getinfo.c with info level RAW_FILEINFO_SMB2_ALL_INFORMATION will get the info needed.

We could also fix up cli_smb2_qfileinfo_basic() to expose more of the data returned via the parameters as it also does a SMB2_FILE_ALL_INFORMATION call, it just doesn't currently return the specific info.
Comment 5 Uri Simchoni 2016-08-13 18:00:58 UTC
(In reply to Jeremy Allison from comment #4)
Not reported, yet both READ and COPYCHUNK (for source handle) succeed if only FILE_EXECUTE is granted.

The correct model to think of this seems to be as follows:
1. The SMB2 server maintains an Open.GrantedAccess variable as documented in [MS-SMB2]
2. But an open file also has a file handle, and this handle has its own granted access mask maintained in the Windows kernel.
3. An SMB2 server would add FILE_READ_DATA to its copy of the mask if FILE_EXECUTE is granted, but the kernel file object does not have this FILE_READ_DATA (IOW, this is added AFTER the open in the kernel).
4. In some operations, the SMB2 server is doing the access check and then calling APIs which bypass security. READ falls under this category, and hence a READ is allowed because it relates to the SMB2 server's access mask
5. In other operations, the SMB2 server simply relays the request to the kernel. GETINFO is an example - no FILE_READ_DATA here.
6. Still in a third class, there's a mix. In the case of COPYCHUNK, the SMB2 server is doing the READ_DATA access check on source handle and WRITE/APPEND_DATA access check on dest handle, and if those pass, it calls ioctl (or whatever Windows equivalent). The ioctl code however, have bits that say if the handle must be readable, writeable, or both - in COPYCHUNK it has to be both (ever wondered why the dest handle has to be readable? that's why) - so this check is done within the kernel and hence FILE_EXECUTE on the dest handle won't buy you COPYCHUNK rights.

So for samba, it seems like we have to either maintain two masks, one for the SMB2 layer and one for the file system layer, or just hard-code workarounds for these quirks at the strategic places. I'm in favor of the second approach since the only difference that we know of between the kernel's view of access mask and the SMB2 server's view is this one, and adding a mask would create a TDB mess with durable file handles.

I'll add torture tests for READ, COPYCHUNK, and GETINFO, which seem to be the ones affected by this oddity, and post a proposed fix to the list.
Comment 6 Uri Simchoni 2016-08-18 18:54:00 UTC
Created attachment 12381 [details]
git-am fix for 4.5.0
Comment 7 Uri Simchoni 2016-08-18 18:58:18 UTC
Created attachment 12382 [details]
git-am fix for 4.4.next
Comment 8 Uri Simchoni 2016-08-18 18:58:58 UTC
Created attachment 12383 [details]
git-am fix for 4.3.next
Comment 9 Uri Simchoni 2016-08-18 19:00:25 UTC
The 4.4.x and 4.3.x fixes are same as master/4.5.0 fixes, but slight adjustments were required for the knownfail file. Other than that, the original master patches applied cleanly.
Comment 10 David Disseldorp 2016-08-23 22:27:07 UTC
@Karo: Please queue for the maintenance branches - thanks!
Comment 11 Stefan Metzmacher 2016-08-24 09:45:56 UTC
Pushed to autobuild-v4-5-test.
Comment 12 Stefan Metzmacher 2016-08-28 15:52:06 UTC
Pushed to v4-5-test.
Comment 13 Karolin Seeger 2016-09-13 09:55:31 UTC
Pushed to autobuild-v4-{4,3}-test.
Comment 14 Karolin Seeger 2016-09-20 07:05:07 UTC
Pushed to all branches.
Closing out bug report.

Thanks!