Created attachment 278137 [details] trace trace of directory access using backupuid option I am using "backupuid=<uid>" option for SMB mount, but I noticed that the "backup intent" flag is only set when client is accessing files, but not directories, resulting permission error during directory access. I attached a trace for directory access, and a trace for file access.
Created attachment 278139 [details] trace trace of file access using backupuid option
Created attachment 278141 [details] Proposed fix This is the patch I used to fix the problem. Note it also fixes a few places where backup intent flag is not being set.
Looks reasonable. Any thoughts about the two places in inode.c (rename) whether someone with backup_intent would ever rename? or whether it would make a difference for security?
Created attachment 278151 [details] proposed fix
Let me know if the patch works for you
Thanks. I will try the patch and let you know. Yeah, these places in inode.c should add as well (although I have not tested rename). <SMB2 CREATE Request> should have the flag set if client intends the file open/create for backup intent. From SMB2 document on the flag "FILE_OPEN_FOR_BACKUP_INTENT 0x00004000": The file is being opened for backup intent. That is, it is being opened or created for the purposes of either a backup or a restore operation. The server can check to ensure that the caller is capable of overriding whatever security checks have been placed on the file to allow a backup or restore operation to occur. The server can check for access rights to the file before checking the DesiredAccess field.
I have mixed feelings about whether rename is worth changing - but I did add a few more (reading alternate device and symlink formats e.g.)
Tested the patch using kernel version 4.13, works fine except creating directory under a ACL-restricted directory. I will attach a tcp trace for the mkdir error.
Created attachment 278157 [details] tcp trace of mkdir error
Creating a new file under the same directory is fine (the backup intent flag is set properly). In mkdir case, the backup intent flag is not set, hence the access denied failure.
Another problem is when I simply does a "touch" operation on a file (ACL restricted), the backup intent flag is not set and operation is denied: root@dynapod-colo-2136-105-7-107-134-vnode1:~# touch /tmp/mount/windows_dir/1.txt touch: setting times of '/tmp/mount/windows_dir/1.txt': Permission denied Surprisingly, I can edit the same file using vi and it went through fine. I will attached the tcp trace for the touch operation as well.
Created attachment 278179 [details] tcp trace of failed touch operation
chmod operation also has the same problem.
Looking at the code, I think smb2_open_op_close() function should also add the CREATE_OPEN_BACKUP_INTENT flag to create_options if backupuid matches. diff --git a/linux-4.13.0/fs/cifs/smb2inode.c b/linux-4.13.0/fs/cifs/smb2inode.c index 1238cd3..779cc11 100644 --- a/linux-4.13.0/fs/cifs/smb2inode.c +++ b/linux-4.13.0/fs/cifs/smb2inode.c @@ -56,6 +56,8 @@ smb2_open_op_close(const unsigned int xid, struct cifs_tcon *tcon, oparms.tcon = tcon; oparms.desired_access = desired_access; oparms.disposition = create_disposition; + if (backup_cred(cifs_sb)) + create_options |= CREATE_OPEN_BACKUP_INTENT; oparms.create_options = create_options; oparms.fid = &fid; oparms.reconnect = false; I can test this (diff is based on 4.13, but latest kernel appears to be the same).
Now "smb2_compound_op()" needs this (with Ronnie's compounding optimizations this actually makes it easy to hit multiple paths with the needed additional change).
open_op_close has changed to smb2_compound_op
Created attachment 278181 [details] tcp trace of failed chmod operation chmod trace is attached
Created attachment 278187 [details] fix compounded ops (like mkdir, setinfo etc.) to send backup intent on backupuid mounts Requires Ronnie's compounding patches (see linux-next build or equivalently cifs-2.6.git for-next branch) because open_query_close function is not used any more
I tested using 4.13 kernel (due to limitation of my side), and my previous patch in smb2_open_op_close() works (chmod/touch/mkdir). I'd assume smb2_compound_op() works the same way as smb2_open_op_close(). Thx.
From comment #19, it is confirmed the patches works? and we can close this bug?
The patch works fine (there is another problem but tracked in another bug, I believe)