Duplicate SMB file_ids leading to Windows client cache poisoning

[Andrew Walker]

On Tue, Dec 14, 2021 at 12:36 PM Tom Talpey <tom at talpey.com> wrote:

> On 12/13/2021 3:23 PM, Andrew Walker wrote:
> >
> >
> > On Fri, Dec 10, 2021 at 4:53 PM Andrew Walker <awalker at ixsystems.com
> > <mailto:awalker at ixsystems.com>> wrote:
> >
> >
> >
> >     On Fri, Dec 10, 2021 at 4:37 PM Tom Talpey <tom at talpey.com
> >     <mailto:tom at talpey.com>> wrote:
> >
> >         On 12/10/2021 4:23 PM, Christof Schmitt wrote:
> >          > On Fri, Dec 10, 2021 at 04:04:09PM -0500, Tom Talpey via
> >         samba-technical wrote:
> >          >> I believe the EXT, BTRFS, XFS and a few other Linux
> >         filesystems support
> >          >> retrieving the generation number via
> >         ioctl(FS_IOC_GETVERSION). But I'm
> >          >> not certain how universal this is. There being hundreds of
> >         file systems
> >          >> in Linux...
> >          >>
> >          >> Could Samba perhaps insert a kernel module, or use the SMB
> >         client kmod,
> >          >> to fetch this? It'd be ugly and will have security
> >         implications, so I
> >          >> would not go into it lightly.
> >          >
> >          > I missed FS_IOC_GETVERSION. That might be an option, since
> >         that is at
> >          > least supported on the most commonly used file systems (ext4,
> >         xfs,
> >          > btrfs). And if the call fails, we could log a warning, that
> >         this setup
> >          > might be unreliable for MacOS clients.
> >
> >         Looks like ZFS has its own idea, ZFS_IOC_OBJ_TO_STATS. But we
> could
> >         cover the basics with a handful of tries.
> >
> >         What about packing the dev_t, ino_t and generation number all
> into
> >         64 bits, without risking a collision? I think the dev_t is needed
> >         unless the Samba server can guarantee the share always maps to
> >         exactly the same one, which seems problematic.
> >
> >         Tom.
> >
> >
> >     With ZFS it looks like st_gen gets populated with the znode sequence
> >     number, which may increment unexpectedly for our purposes (for
> >     instance when timestamps incremented). I'll double-check with our
> >     ZFS devs tomorrow.
> >
> >
> > To clarify on this, FreeBSD return inode generation in stat(2) output.
> > stat.st_gen. In the case of ZFS on FreeBSD though, this value increments
> > on every file change. You can view by running
>
> Oddly, I don't see st_gen in any online FreeBSD manpage. Totally
> agree that it's useless to bump it on any file change. How wide
> is the field? Historically, generation numbers were small, 8-16b.
>
> But I do see that FreeBSD does provide an st_birthtim. That seems
> like a better option, if populated?
>
> Tom.


```
#if defined(_WANT_FREEBSD11_STAT) || defined(_KERNEL)
struct freebsd11_stat {
        __uint32_t st_dev;              /* inode's device */
        __uint32_t st_ino;              /* inode's number */
        mode_t    st_mode;              /* inode protection mode */
        __uint16_t st_nlink;            /* number of hard links */
        uid_t     st_uid;               /* user ID of the file's owner */
        gid_t     st_gid;               /* group ID of the file's group */
        __uint32_t st_rdev;             /* device type */
        struct  timespec st_atim;       /* time of last access */
        struct  timespec st_mtim;       /* time of last data modification */
        struct  timespec st_ctim;       /* time of last file status change
*/
        off_t     st_size;              /* file size, in bytes */
        blkcnt_t st_blocks;             /* blocks allocated for file */
        blksize_t st_blksize;           /* optimal blocksize for I/O */
        fflags_t  st_flags;             /* user defined flags for file */
        __uint32_t st_gen;              /* file generation number */
        __int32_t st_lspare;
        struct timespec st_birthtim;    /* time of file creation */
        /*
         * Explicitly pad st_birthtim to 16 bytes so that the size of
         * struct stat is backwards compatible.  We use bitfields instead
         * of an array of chars so that this doesn't require a C99 compiler
         * to compile if the size of the padding is 0.  We use 2 bitfields
         * to cover up to 64 bits on 32-bit machines.  We assume that
         * CHAR_BIT is 8...
         */
        unsigned int :(8 / 2) * (16 - (int)sizeof(struct timespec));
        unsigned int :(8 / 2) * (16 - (int)sizeof(struct timespec));
};
#endif /* _WANT_FREEBSD11_STAT || _KERNEL */
```
st_gen is uint32_t. On ZFS it's returning txg in which file was created,
and so will typically be higher than you would see on an FS where number is
monotonically incrementing. On FreeBSD tmpfs and a few other FSes it is
populated via arc4random().

Birthtime on FreeBSD may be changed via utimensat(2) and futimens(2), and
so it should not be relied on to uniquely identify a file.