comparing 4.11 and 4.12 there is just one difference.

[2020/05/22 15:38:56.607571,  3] ../../source3/smbd/dir.c:659(dptr_create)
  creating new dirptr 0 for path stores, expect_close = 0

is missing in 4.12.

Can you upload a full debug level 10 log please, and also an ls -l <link-names> of the directory containing the DFS links. I'd like to see what the symlinks look like and also what smbd says when trying to process them.

Thanks !

I'd like to confirm that behaviour. It's easy to reproduce:

1: mkdir /tmp/relay; ln -s msdfs:srv0\\share0 /tmp/relay/srv0
2: smb.conf:
[global]
    map to guest = Bad User
    log file = /var/log/samba/log.%m
    HOST MSDFS = yes
[relay]
    path = /tmp/relay
    MSDFS ROOT = yes
    public = yes
    guest ok = yes
    guest only = yes
3: mapping "\\smbhost\relay" from a windows machine.

Works perfectly fine up to 4.11.9. and with 4.12.x 
"srv0" is void and functionless.

Sorry, I thought we were testing this in the regression tests. I'll take a look, but I might not get to this until after SambaXP.

Created attachment 16010 [details]
log level 10

[root@server]: /mnt/dfs/stores># ls -l *
lrwxrwxrwx 1 root root 20 22. Mai 15:06 4bay_usb3 -> 'msdfs:t420\4bay_usb3'
lrwxrwxrwx 1 root root 20 22. Mai 15:06 8bay_usb3 -> 'msdfs:t420\8bay_usb3'
lrwxrwxrwx 1 root root 14 15. Mai 11:37 hdd -> 'msdfs:t420\hdd'

(In reply to Bernd from comment #3)

Just checked your use-case:

mkdir /tmp/relay; ln -s msdfs:srv0\\share0 /tmp/relay/srv0

using smbclient locally on master. Works. So I'll keep looking.

right, but that's misleading and not the problem.

The use case is to have a mapped share ("relay") holding a list of 
junctions (e.g. "srv0").
Until 4.11.9 in windows explorer this pretty much looks like a list of 
folders and worked like an on demand mapping just by clicking on it.
Great feature.

(In reply to Bernd from comment #8)

Sure I agree. I was just checking that the "click-through" ability of these junction points still worked from our client tools. These are what we're using for regression tests, so that's why we missed the bug w.r.t. what Windows10 clients are doing.

I'll try and reproduce and then update our test suite with something that does the same as what Windows does.

Reproduced. We're not setting the FILE_ATTRIBUTE_REPARSE_POINT when returning these links. I'm looking into why now.

OK, I know how to fix this. For 4.12.next it'll be a trivial addition of an extra SMB_VFS_LSTAT() call inside is_msdfs_link() to make sure the returned smb_filename struct has a VALID_STAT returned. When we moved to calling SMB_VFS_READ_DFS_PATHAT() we dropped the previously implicit call to SMB_VFS_LSTAT() that was used to quickly see if it was a DFS pathname. Now we just do readlink() directly (as we know we're going to have to do that anyway) and we dropped the lstat call as not need. So it left the smb_fname->st structure uninitialized and we're returning garbage values when enumerating a directory containing MS-DFS links and not setting FILE_ATTRIBUTE_REPARSE_POINT (which is why the Windows10 client isn't seeing them as DFS-links anymore).

I can't fix it the "correct" way (explained below) in 4.12.next as that would require a VFS ABI change.

For master I'd like to fix it differently. I now realize I made a mistake in defining SMB_VFS_READ_DFS_PATHAT() to take a 'const struct smb_filename *'.

As SMB_VFS_READ_DFS_PATHAT() is actually called as part of filename meta-data lookup when enumerating a directory from SMB1/2/3 via smbd_dirptr_lanman2_entry(), it really has to fill in the values inside the smb_fname->st struct on return.

It's not just a "lookup dfs redirect paths" call, it's a "get all metadata about a DFS filename" call. That means it has to fill in smb_fname->st, so that when returning the info levels from the query directory we have all the needed info about the file.

Then we can correctly fill in the info level data being requested - the DOS attributes that contain FILE_ATTRIBUTE_REPARSE_POINT is just one thing we need to return (albeit the most important one).

So for 4.12.next, I'll add the extra LSTAT call to fill in smb_fname->st. That's not the "right" way as it still assumes that the name we're returning from the directory listing can be queried via an LSTAT call, still exposing that underlying it all it's being stored as a symlink - something I'm trying to move away from.

All a DFS redirect object in the filesystem really has to guarantee is that calling SMB_VFS_STAT() on it will fail, so the alternate lookup can then be invoked. So the alternate lookup of SMB_VFS_READ_DFS_PATHAT() needs to fill in smb_fname->st, so it can't be passed in as const.

All this is mostly for Ralph, so he can understand the design decision behind the changes I'm going to make for master.

First I will finish my regression test, then make sure the fix for master fixes it, then once that's passed CI and been pushed I'll do the "trivial" fix for the 4.12.next back-port and it should be in the next 4.12 release.

Created attachment 16013 [details]
git-am fix for master

What I currently have in gitlab-ci. I'll back-port a (modified) fix for 4.12 next.

Created attachment 16014 [details]
git-am fix for 4.12.next.

Checked here and works with Windows 10. Includes regression test to make sure we don't break in future.

Please let me know if this works for you. Once the master fix has passed ci and been pushed, I'll get this reviewed for 4.12.next.

Sorry for the problem.

(In reply to Jeremy Allison from comment #13)
Small nitpick for the 4.12 patch:

+	if (NT_STATUS_IS_OK(status)) {
+		int ret = SMB_VFS_LSTAT(conn, smb_fname);
+		if (ret < 0) {
+			status = map_nt_error_from_unix(errno);
+		}
+	}

Can you please split the ret variable declaration from the SMB_VFS_LSTAT() function call? Thanks!

looks good !

Patched against 4.12.3, tested for the posted trivial use-case and 
our real one.

Thank you very much ! Great work !

(In reply to Ralph Böhme from comment #14)

Will do when I update the patchset once it's gone into master.

Thanks !

Created attachment 16017 [details]
git-am fix for 4.12.next

(Mostly) cherry-picked from master, except for the last part which has to be done differently.

Comment on attachment 16017 [details]
git-am fix for 4.12.next

Andreas, we need to get this into 4.12.next in order for MSDFS links to work again. Thanks !

Comment on attachment 16017 [details]
git-am fix for 4.12.next

lgtm

Reassigning to Karolin for inclusion in 4.12.

(In reply to Ralph Böhme from comment #20)
Pushed to autobuild-v4-12-test.

(In reply to Karolin Seeger from comment #21)
Pushed to v4-12-test.
Closing out bug report.

Thanks!