Duplicate SMB file_ids leading to Windows client cache poisoning

[Andrew Walker]

On Fri, Dec 10, 2021 at 8:18 AM Ralph Boehme <slow at samba.org> wrote:

> On 12/10/21 12:49, Andrew Walker wrote:
> exactly. Thanks for digging out the references. Didn't remember I ever
> wrote such a comprehensive summary. :)
>

Took a little digging to find it :)


> So what shall we do? Back to inode numbers? I've been recommending
> fruit:zero_file_id=yes to customers who've been reporting strange issues
> with Mac clients. That causes Samba to return 0 file-ids which has been
> problematic in the past, but it seems to be working atm.
>


Looking through latest MacOS SMB code that I have on hand (which is _very_
old - smb-759.40.1 / Yosemite), zero file caused MacOS
SMB client to determine that file IDs are not supported by the SMB server,
which looks like it causes node lookups to be caused by
hash of filename rather than inode number...  see comments in
`kernel/smbfs/smbfs_io.c` and smbfs_hash() in `kernel/smbfs/smbfs_node.c`.

```
                                /*
                                 * Some applications use the inode as a
marker and expect it to
                                 * be persistent. If file IDs are not
supported by the server,
                                 * then our inode numbers are created by
hashing the name and
                                 * adding the parent inode number. Once a
node is created we
                                 * should try to keep the same inode number
through out its
                                 * life. The smbfs_nget will either create
the node or
                                 * return one found in the hash table. The
one that gets created
                                 * will use ctx->f_attr.fa_ino, but if its
in our hash table it
                                 * will have its original number. So in
either case set the file
                                 * number to the inode number that was used
when the node was
                                 * created.
                                 */
```

That seems annoying, but the MacOS source is also quite old. Do we know
that there are still issues with MacOS when an inode is reused during the
lifetime of an SMB session? Do we have an easy way to reproduce the MacOS
issue? Perhaps if we can determine that the issue only impacts legacy
versions of MacOS we can go back to just returning the inode and tell
people with EOL MacOS to upgrade :)

Just spitballing here, but what's the downside of just including the
current inode number in the file_id we're generating? (Trying to think of
what we can do that's minimally invasive).
```
diff --git a/source3/lib/file_id.c b/source3/lib/file_id.c
index 6bfcf1463d2..8f3d76e66f9 100644
--- a/source3/lib/file_id.c
+++ b/source3/lib/file_id.c
@@ -113,6 +113,7 @@ uint64_t make_file_id_from_itime(const struct stat_ex
*st)
        file_id = ((uint64_t)1) << 63;
        file_id |= (uint64_t)itime.tv_sec << 30;
        file_id |= file_id_low;
+       file_id |= ino;

        return file_id;
 }
```


> Tom, do you know if it using any of the mentioned fallbacks from MS-FSCC:
>
>    For file systems that do not support a 64-bit file ID,
>    this field MUST be set to 0, and MUST be ignored.
>
>    For files for which a unique 64-bit file ID cannot be
>    established, this field MUST be set to 0xffffffffffffffff,
>    and MUST be ignored.
>
> would be a way forward? Macs can deal with 0, not sure about UINT64_MAX.
>
> Thanks!
> -slow
>

This seems to be rather extreme for what is putatively a MacOS client issue.