Re: [PATCH] cifs: Add mount option named backup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 12, 2011 at 2:25 PM, Shirish Pargaonkar
<shirishpargaonkar@xxxxxxxxx> wrote:
> On Fri, Aug 12, 2011 at 12:19 PM, Steve French <smfrench@xxxxxxxxx> wrote:
>> Shirish or Manoj,
>> Could you give a little more detail on the failure scenario
>> and which files and information can not be accessed?
>>
>> A mount option may be the only choice, since there is
>> no equivalent for "open for backup intent" open flag,
>> and presumably no easy way to query whether a user is a member
>> of the "backup operators" group, but would a mount option be
>> a problem for other users of the same mount who don't have
>> backup operator privilege trying to open other files?
>>
>> On Fri, Aug 12, 2011 at 11:33 AM,  <shirishpargaonkar@xxxxxxxxx> wrote:
>>> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>>
>>>
>>> Add mount option backup.
>>>
>>> It allows an authenticated user to access files with the intent to back them
>>> up including their ACLs, who may not have access permission but has
>>> "Backup files and directories user right" on them (by virtue of being part
>>> of the built-in group Backup Operators.
>>>
>>> If an authenticated user is not part of the built-in group Backup Operators
>>> at the server, access to such files is denied.
>>>
>>>
>>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>> ---
>>>  fs/cifs/cifs_fs_sb.h |    1 +
>>>  fs/cifs/cifsacl.c    |   18 +++++++++++++-----
>>>  fs/cifs/cifsglob.h   |    4 +++-
>>>  fs/cifs/connect.c    |    5 +++++
>>>  fs/cifs/dir.c        |   11 +++++++++--
>>>  fs/cifs/file.c       |   14 ++++++++++++--
>>>  fs/cifs/link.c       |   18 +++++++++++++-----
>>>  7 files changed, 56 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>>> index 7260e11..8ba1b44 100644
>>> --- a/fs/cifs/cifs_fs_sb.h
>>> +++ b/fs/cifs/cifs_fs_sb.h
>>> @@ -43,6 +43,7 @@
>>>  #define CIFS_MOUNT_STRICT_IO   0x40000 /* strict cache mode */
>>>  #define CIFS_MOUNT_RWPIDFORWARD        0x80000 /* use pid forwarding for rw */
>>>  #define CIFS_MOUNT_POSIXACL    0x100000 /* mirror of MS_POSIXACL in mnt_cifs_flags */
>>> +#define CIFS_MOUNT_CIFS_BACKUP 0x200000 /* open file with backup intent bit */
>>>
>>>  struct cifs_sb_info {
>>>        struct rb_root tlink_tree;
>>> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
>>> index d0f59fa..24bc2dd 100644
>>> --- a/fs/cifs/cifsacl.c
>>> +++ b/fs/cifs/cifsacl.c
>>> @@ -945,7 +945,7 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
>>>  {
>>>        struct cifs_ntsd *pntsd = NULL;
>>>        int oplock = 0;
>>> -       int xid, rc;
>>> +       int xid, rc, create_options = 0;
>>>        __u16 fid;
>>>        struct cifs_tcon *tcon;
>>>        struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
>>> @@ -956,9 +956,13 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
>>>        tcon = tlink_tcon(tlink);
>>>        xid = GetXid();
>>>
>>> -       rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, READ_CONTROL, 0,
>>> -                        &fid, &oplock, NULL, cifs_sb->local_nls,
>>> -                        cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>> +
>>> +       rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, READ_CONTROL,
>>> +                       create_options, &fid, &oplock, NULL, cifs_sb->local_nls,
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>>        if (!rc) {
>>>                rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
>>>                CIFSSMBClose(xid, tcon, fid);
>>> @@ -995,7 +999,7 @@ static int set_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, const char *path,
>>>                struct cifs_ntsd *pnntsd, u32 acllen)
>>>  {
>>>        int oplock = 0;
>>> -       int xid, rc;
>>> +       int xid, rc, create_options = 0;
>>>        __u16 fid;
>>>        struct cifs_tcon *tcon;
>>>        struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
>>> @@ -1006,6 +1010,10 @@ static int set_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, const char *path,
>>>        tcon = tlink_tcon(tlink);
>>>        xid = GetXid();
>>>
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>> +
>>>        rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, WRITE_DAC, 0,
>>>                         &fid, &oplock, NULL, cifs_sb->local_nls,
>>>                         cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> index 95dad9d..3c6fd56 100644
>>> --- a/fs/cifs/cifsglob.h
>>> +++ b/fs/cifs/cifsglob.h
>>> @@ -179,6 +179,7 @@ struct smb_vol {
>>>        bool noperm:1;
>>>        bool no_psx_acl:1; /* set if posix acl support should be disabled */
>>>        bool cifs_acl:1;
>>> +       bool cifs_backup:1; /* to indicates backup intent during file open */
>>>        bool no_xattr:1;   /* set if xattr (EA) support should be disabled*/
>>>        bool server_ino:1; /* use inode numbers from server ie UniqueId */
>>>        bool direct_io:1;
>>> @@ -219,7 +220,8 @@ struct smb_vol {
>>>                         CIFS_MOUNT_OVERR_GID | CIFS_MOUNT_DYNPERM | \
>>>                         CIFS_MOUNT_NOPOSIXBRL | CIFS_MOUNT_NOSSYNC | \
>>>                         CIFS_MOUNT_FSCACHE | CIFS_MOUNT_MF_SYMLINKS | \
>>> -                        CIFS_MOUNT_MULTIUSER | CIFS_MOUNT_STRICT_IO)
>>> +                        CIFS_MOUNT_MULTIUSER | CIFS_MOUNT_STRICT_IO | \
>>> +                        CIFS_MOUNT_CIFS_BACKUP)
>>>
>>>  #define CIFS_MS_MASK (MS_RDONLY | MS_MANDLOCK | MS_NOEXEC | MS_NOSUID | \
>>>                      MS_NODEV | MS_SYNCHRONOUS)
>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>> index 80c2e3a..352d6d6 100644
>>> --- a/fs/cifs/connect.c
>>> +++ b/fs/cifs/connect.c
>>> @@ -883,6 +883,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>>>                        cFYI(1, "Null separator not allowed");
>>>                }
>>>        }
>>> +       vol->cifs_backup = 0; /* no backup intent */
>>>
>>>        while ((data = strsep(&options, separator)) != NULL) {
>>>                if (!*data)
>>> @@ -1405,6 +1406,8 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>>>                        vol->rwpidforward = 1;
>>>                } else if (strnicmp(data, "cifsacl", 7) == 0) {
>>>                        vol->cifs_acl = 1;
>>> +               } else if (strnicmp(data, "backup", 7) == 0) {
>>> +                       vol->cifs_backup = 1;
>>>                } else if (strnicmp(data, "nocifsacl", 9) == 0) {
>>>                        vol->cifs_acl = 0;
>>>                } else if (strnicmp(data, "acl", 3) == 0) {
>>> @@ -2763,6 +2766,8 @@ void cifs_setup_cifs_sb(struct smb_vol *pvolume_info,
>>>                cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_RWPIDFORWARD;
>>>        if (pvolume_info->cifs_acl)
>>>                cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_CIFS_ACL;
>>> +       if (pvolume_info->cifs_backup)
>>> +               cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_CIFS_BACKUP;
>>>        if (pvolume_info->override_uid)
>>>                cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_OVERR_UID;
>>>        if (pvolume_info->override_gid)
>>> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
>>> index ae576fb..fbecde6 100644
>>> --- a/fs/cifs/dir.c
>>> +++ b/fs/cifs/dir.c
>>> @@ -243,6 +243,9 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
>>>         */
>>>        if (!tcon->unix_ext && (mode & S_IWUGO) == 0)
>>>                create_options |= CREATE_OPTION_READONLY;
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>>
>>>        if (tcon->ses->capabilities & CAP_NT_SMBS)
>>>                rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
>>> @@ -357,6 +360,7 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, int mode,
>>>  {
>>>        int rc = -EPERM;
>>>        int xid;
>>> +       int create_options = CREATE_NOT_DIR | CREATE_OPTION_SPECIAL;
>>>        struct cifs_sb_info *cifs_sb;
>>>        struct tcon_link *tlink;
>>>        struct cifs_tcon *pTcon;
>>> @@ -431,9 +435,12 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, int mode,
>>>                return rc;
>>>        }
>>>
>>> -       /* FIXME: would WRITE_OWNER | WRITE_DAC be better? */
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>> +
>>>        rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE,
>>> -                        GENERIC_WRITE, CREATE_NOT_DIR | CREATE_OPTION_SPECIAL,
>>> +                        GENERIC_WRITE, create_options,
>>>                         &fileHandle, &oplock, buf, cifs_sb->local_nls,
>>>                         cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>>        if (rc)
>>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>>> index 9f41a10..39b06c4 100644
>>> --- a/fs/cifs/file.c
>>> +++ b/fs/cifs/file.c
>>> @@ -174,6 +174,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
>>>        int rc;
>>>        int desiredAccess;
>>>        int disposition;
>>> +       int create_options = CREATE_NOT_DIR;
>>>        FILE_ALL_INFO *buf;
>>>
>>>        desiredAccess = cifs_convert_flags(f_flags);
>>> @@ -210,9 +211,13 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
>>>        if (!buf)
>>>                return -ENOMEM;
>>>
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>> +
>>>        if (tcon->ses->capabilities & CAP_NT_SMBS)
>>>                rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
>>> -                        desiredAccess, CREATE_NOT_DIR, pnetfid, poplock, buf,
>>> +                        desiredAccess, create_options, pnetfid, poplock, buf,
>>>                         cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>>>                                 & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>>        else
>>> @@ -465,6 +470,7 @@ static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
>>>        char *full_path = NULL;
>>>        int desiredAccess;
>>>        int disposition = FILE_OPEN;
>>> +       int create_options = CREATE_NOT_DIR;
>>>        __u16 netfid;
>>>
>>>        xid = GetXid();
>>> @@ -524,6 +530,10 @@ static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
>>>
>>>        desiredAccess = cifs_convert_flags(pCifsFile->f_flags);
>>>
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>> +
>>>        /* Can not refresh inode by passing in file_info buf to be returned
>>>           by SMBOpen and then calling get_inode_info with returned buf
>>>           since file might have write behind data that needs to be flushed
>>> @@ -531,7 +541,7 @@ static int cifs_reopen_file(struct cifsFileInfo *pCifsFile, bool can_flush)
>>>           that inode was not dirty locally we could do this */
>>>
>>>        rc = CIFSSMBOpen(xid, tcon, full_path, disposition, desiredAccess,
>>> -                        CREATE_NOT_DIR, &netfid, &oplock, NULL,
>>> +                        create_options, &netfid, &oplock, NULL,
>>>                         cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
>>>                                CIFS_MOUNT_MAP_SPECIAL_CHR);
>>>        if (rc) {
>>> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
>>> index db3f18c..67e51b8 100644
>>> --- a/fs/cifs/link.c
>>> +++ b/fs/cifs/link.c
>>> @@ -183,14 +183,20 @@ CIFSFormatMFSymlink(u8 *buf, unsigned int buf_len, const char *link_str)
>>>  static int
>>>  CIFSCreateMFSymLink(const int xid, struct cifs_tcon *tcon,
>>>                    const char *fromName, const char *toName,
>>> -                   const struct nls_table *nls_codepage, int remap)
>>> +                   struct cifs_sb_info *cifs_sb)
>>>  {
>>>        int rc;
>>>        int oplock = 0;
>>> +       int remap;
>>> +       int create_options = CREATE_NOT_DIR;
>>>        __u16 netfid = 0;
>>>        u8 *buf;
>>>        unsigned int bytes_written = 0;
>>>        struct cifs_io_parms io_parms;
>>> +       struct nls_table *nls_codepage;
>>> +
>>> +       nls_codepage = cifs_sb->local_nls;
>>> +       remap = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR;
>>>
>>>        buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
>>>        if (!buf)
>>> @@ -202,8 +208,12 @@ CIFSCreateMFSymLink(const int xid, struct cifs_tcon *tcon,
>>>                return rc;
>>>        }
>>>
>>> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL &&
>>> +                       cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_BACKUP)
>>> +               create_options |= CREATE_OPEN_BACKUP_INTENT;
>>> +
>>>        rc = CIFSSMBOpen(xid, tcon, fromName, FILE_CREATE, GENERIC_WRITE,
>>> -                        CREATE_NOT_DIR, &netfid, &oplock, NULL,
>>> +                        create_options, &netfid, &oplock, NULL,
>>>                         nls_codepage, remap);
>>>        if (rc != 0) {
>>>                kfree(buf);
>>> @@ -559,9 +569,7 @@ cifs_symlink(struct inode *inode, struct dentry *direntry, const char *symname)
>>>        /* BB what if DFS and this volume is on different share? BB */
>>>        if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
>>>                rc = CIFSCreateMFSymLink(xid, pTcon, full_path, symname,
>>> -                                        cifs_sb->local_nls,
>>> -                                        cifs_sb->mnt_cifs_flags &
>>> -                                               CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> +                                       cifs_sb);
>>>        else if (pTcon->unix_ext)
>>>                rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
>>>                                           cifs_sb->local_nls);
>>> --
>>> 1.6.0.2
>>>
>>>
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>>
>
> On a Windows 2003 server, 'Back up files and directories policy is enabled
> and user named 'mahalcrow' is added to that policy.
>
> I mount a share from that Windows server by authenticating as user 'mahalcrow'.
>
> # mount -t cifs //cifstest1/myshare /mnt/smb_b -o
> backup,cifsacl,user=mahalcrow,pass=<passwd>
>
> # ls -l /mnt/smb_b/testfile2
> -rwx------ 1 administrator 10010 18 Aug  9 19:26 /mnt/smb_b/testfile2
>
> # /tmp/getcifsacl /mnt/smb_b/testfile2
> REVISION:0x1
> CONTROL:0x9404
> OWNER:CIFSTESTDOM\Administrator
> GROUP:CIFSTESTDOM\Domain Users
> ACL:CIFSTESTDOM\Administrator:ALLOWED/0x0/FULL
>
> So with the backup intent create option during open (nt_create_andx),
> an user who is neither owner of the file and not part of the only ACE
> in the ACL of this file, can still access the file.
>
>
> Now if  I remove this user, mahalcrow, from the policy 'Back up files
> and directories' under
>  Security Settings/Local Policies/user Rights Assignment/Security Options,
> unmount and remount the share and then try to access the file
> testfile2  from the
> mounted share, it fails, even with the backup intent mount option.
>
> # mount -t cifs //cifstest1/myshare /mnt/smb_b -o
> backup,cifsacl,user=mahalcrow,pass=<passwd>
> # ls -l /mnt/smb_b/testfile2
> -rwxr-xr-x 1 root root 18 Aug  9 19:26 /mnt/smb_b/testfile2
>
> # /tmp/getcifsacl /mnt/smb_b/testfile2
> getxattr error: 95
> REVISION:0x0
> CONTROL:0x0
>
>
> The change in policy on the server will not reflect on an existing smb
> connection though,
> if this user unmounts and remounts the same share, it would be effective.
>
> Regards,
>
> Shirish
>

Oh, forgot to mention, that without this mount option,  backup,
access to file fails.

# ls -l /mnt/smb_b/testfile2
ls: cannot access /mnt/smb_b/testfile2: Permission denied


So this mount option backup, alongwith cifsacl mount option, now enables a
program/tool like robocopy to function as it should.
If and how a user is shut out from any of such accesses is upto the
cifs/smb server.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux