Bug 15435 - regression DFS not working with widelinks = true
Summary: regression DFS not working with widelinks = true
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.17.10
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-26 18:50 UTC by Noel Power
Modified: 2023-09-07 09:02 UTC (History)
1 user (show)

See Also:


Attachments
backport for 4.18 & 4.17 (7.73 KB, patch)
2023-07-29 19:42 UTC, Noel Power
jra: review+
Details
backport for 4.19 (7.70 KB, patch)
2023-08-03 10:24 UTC, Noel Power
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Noel Power 2023-07-26 18:50:11 UTC
when widelinks = yes is set DFS (at least in the case of local DFS pointing at local shares fail)

e.g.

with following setup


[global]
map to guest = Bad User
server role = standalone server
wide links = yes
unix extensions = no

[guest]
path = /srv/samba/guest/
read only = no
guest ok = yes
guest only = yes
msdfs root = yes


[1] create local share dir

mkdir -p /srv/samba/guest

[2] create msdfs link

ln -snf msdfs:$(hostname -f)\\guest /srv/samba/guest/linka

[3] start smbd

[4] try to access dfs link

 smbclient --no-pass //localhost/guest -c "cd linka"

will result in NT_STATUS_OBJECT_NAME_NOT_FOUND
Comment 1 Noel Power 2023-07-26 18:56:34 UTC
seems this was introduced in 4.17 by d20b60c3200b5e1881cdf4b59da154d1af7e3994 but the code around this has changed since again I think

basically it seems with the current code with widelinks enabled the vfs stack goes as follows

openat_pathref_fsp_case_insensitive

-> openat_pathref_fullname
  -> fd_openat
    -> non_widelink_open
     -> SMB_VFS_OPENAT
       -> widelinks_openat

which ends up failing with NT_STATUS_OBJECT_NAME_NOT_FOUND *but* with the associated stat buf invalide (because widelinks follow the symlink which in the case of a dfs link is invalid so it resolves to a bad / non existent object

in the normal case it is the link that is attempted to open (which fails for the normal reasons) *but* the associated stat buf is valid so the subsequent tests can determine it is a link, a dfs link etc. and process appropriatly
Comment 2 Jeremy Allison 2023-07-26 19:02:13 UTC
Is there a place in that call-stack where we have a valid stat for the dfs link path ?

If so we might be able to do something different in the 'widelinks = yes' case.
Comment 3 Jeremy Allison 2023-07-26 19:04:31 UTC
widelinks_openat() is ugly, dirty and specific enough we might be able to do something in there to restore the old behavior :-).
Comment 4 Noel Power 2023-07-26 19:08:22 UTC
(In reply to Jeremy Allison from comment #2)

>Is there a place in that call-stack where we have a valid stat for the dfs link >path ?
>
>If so we might be able to do something different in the 'widelinks = yes' case.

I didn't see one, but I could easily have missed, I opened this draft with a (as it says in the commit message) 'dodgy (tm)' attempt at fixing

https://gitlab.com/samba-team/samba/-/merge_requests/3195
Comment 5 Jeremy Allison 2023-07-26 19:12:17 UTC
I.e. if the SMB_VFS_NEXT_OPENAT() call in widelinks_openat() fails with -1, errno =ENOENT, then do an SMB_VFS_NEXT_LSTAT() on the name and see if it matches a DFS name, fill in the stat field if so.

This may not be the right place of course, so look in the "normal" DFS path case (when widelinks=no) and see where the returned struct stat gets filled in.
Comment 6 Samba QA Contact 2023-07-29 00:44:03 UTC
This bug was referenced in samba master:

b57cdfd7efb161cf96b3a39dc7a1652db817e602
3d2e9db8b95f9f45d486f8272e53584975f177fa
2668dcd0968133cca4f8410bf8c41ed0483f5d87
0bf8b25aacdf2f5c746922320b32e3f0886c81f5
Comment 7 Noel Power 2023-07-29 19:42:00 UTC
Created attachment 18013 [details]
backport for 4.18 & 4.17

backport should apply to 4.17 & 4.18
Comment 8 Jeremy Allison 2023-08-02 19:46:48 UTC
Re-assigning to Jule for inclusion in 4.18.next, 4.17.next.
Comment 9 Jule Anger 2023-08-03 07:01:55 UTC
Pushed to autobuild-v4-{18,17}-test.

Could you please also create a patch for 4.19?
The 4.18/4.17 patch did not apply to 4.19.
Comment 10 Samba QA Contact 2023-08-03 08:45:05 UTC
This bug was referenced in samba v4-18-test:

e50f377b4ab853b11ea17778a3e5ea712548bc22
5db858c1afde2fd0a20c81360f03f165eee2d53b
e949750d4f533d0c2a04ada4a02236f1b012107a
c40f1619d96c0332d3ad9d9b8e63a4fbc10f332f
Comment 11 Noel Power 2023-08-03 09:59:13 UTC
(In reply to Jule Anger from comment #9)
>Could you please also create a patch for 4.19?
>The 4.18/4.17 patch did not apply to 4.19.

so sorry, I didn't realise we had branched :-( (will backport now)
Comment 12 Noel Power 2023-08-03 10:24:22 UTC
Created attachment 18022 [details]
backport for 4.19

I don't think a a re(review) is needed for this patch version (only context diffs here) but please let me know if it is needed
Comment 13 Jule Anger 2023-08-03 11:36:57 UTC
Thanks :-)
Pushed to autobuild-v4-19-test.
Comment 14 Samba QA Contact 2023-08-03 12:47:04 UTC
This bug was referenced in samba v4-17-test:

9ace53099ed5cc9cb3cabe2f104882b0f260ea43
98a53e95a0f36e60355e3e2d2719018cd485f436
b63c917cf742682f909597349d2f1f513c422ec7
10f3fafc6f4fedbc182894c3d03fd2939cfcee18
Comment 15 Samba QA Contact 2023-08-03 14:31:03 UTC
This bug was referenced in samba v4-19-test:

d59392056e796c2c491e5f53dac2f9161b329bf4
ece48278912e14432f34f6d0edcd26768e299406
368b3e6102b5a7d9df0f243ced7d75fe8605eb79
1231268c219a2f00ad62c7d109db96129b0d2388
Comment 16 Jule Anger 2023-08-04 07:00:36 UTC
Closing out bug report.

Thanks!
Comment 17 Samba QA Contact 2023-08-08 07:22:33 UTC
This bug was referenced in samba v4-19-stable (Release samba-4.19.0rc2):

d59392056e796c2c491e5f53dac2f9161b329bf4
ece48278912e14432f34f6d0edcd26768e299406
368b3e6102b5a7d9df0f243ced7d75fe8605eb79
1231268c219a2f00ad62c7d109db96129b0d2388
Comment 18 Samba QA Contact 2023-08-16 16:57:57 UTC
This bug was referenced in samba v4-18-stable (Release samba-4.18.6):

e50f377b4ab853b11ea17778a3e5ea712548bc22
5db858c1afde2fd0a20c81360f03f165eee2d53b
e949750d4f533d0c2a04ada4a02236f1b012107a
c40f1619d96c0332d3ad9d9b8e63a4fbc10f332f
Comment 19 Samba QA Contact 2023-09-07 09:02:47 UTC
This bug was referenced in samba v4-17-stable (Release samba-4.17.11):

9ace53099ed5cc9cb3cabe2f104882b0f260ea43
98a53e95a0f36e60355e3e2d2719018cd485f436
b63c917cf742682f909597349d2f1f513c422ec7
10f3fafc6f4fedbc182894c3d03fd2939cfcee18