Still looking into this and trying to fully wrap my head around it, but it seems that in some cases related to operations on streams, we fail to check if an fsp is a stream fsp in vfs_acl_xattr and pass it to vfswrap_fgetxattr(). As the system fd of the stream fsp is a "fake" fd created via pipe(), the fgetxattr() call will fail, so the vfs_acl_xattr module falls back to synthezising an ACL based on the configured "acl_xattr:default acl style" which defaults to "posix". As the POSIX mode is likely 0777 for directories or 0666 for files respectively, file access is likely much less restricted then the user set ACL from the xattr. Still investigating...
I guess while at this, we should also change the default of "acl_xattr:default acl style" to "windows" if "acl_xattr:ignore system acls" is enabled.
Couldn't we just check for fsp->base_fsp != NULL and use that when calling into FGETXATTR ?
Luckily this is not a security issue... When using vfs_streams_xattr, for a pathref handle of a stream the system fd will be a fake fd (creater by pipe() in vfs_fake_fd(). The code path is SMB_VFS_CREATE_FILE(..., "file:stream", ...) => open_file(): if (open_fd): -> taking the else branch: -> smbd_check_access_rights_fsp(stream_fsp) -> SMB_VFS_FGET_NT_ACL(stream_fsp) Calling SMB_VFS_FGET_NT_ACL(stream_fsp) on a handle of a named stream will try to retrieve the stored ACL when using vfs_acl_xattr, as it ends up calling fgetxattr(fake-fd) which returns EBADF. The vfs_acl_xattr code ignores the specific error and handles this as if there was no ACL stored and then runs the code to synthesize a default ACL accoding to the setting of "acl:default acl style". As the correct access check for streams has already been carried out by calling check_base_file_access() from create_file_unixpath(), the above problem is not a security issue: it can only lead to "decreased" permissions resulting in unexpected ACCESS_DENIED errors.
This bug was referenced in samba master: 0d3995cec10c5fae8c8b6a1df312062e38437e6f b26dc252aaf3f4b960bdfdb6a3dfe612b89fcdd5 451ad315a9bf32c627e1966ec30185542701c87e 92e0045d7ca7c0b94efd0244ba0e426cad0a05b6 23bc760ec5d61208c2d8778991e3d7e202eab352 c949e4b2a42423ac3851e86e489fd0c5d46d7f1f 4ab29e2a345b48ebba652d5154e96adf954a6757 03b9ce84736d536ab2dd8a5ce1a2656e6a90c8c8 55e55804bb2d0f21c1bbe207257bb40555f3b7a2 3af8f8e8741cc8c889bbf416ccd38a1b702917ec 06555c6bcb5644fc9eea35b3cbae8d8801c65ab6 f0299abf1b28a14518328710d9f84bef17fd2ecf 51243e3849736acbbf1d8f52cc02cdec5995fde4 fc45fcfde51b0b0bdcd524c82a0f9eabf7273045
Created attachment 17466 [details] git-am fix for 4.17.rcNext. Cherry-picked from master. Let me know if you want a back-port for 4.16.x also.
Comment on attachment 17466 [details] git-am fix for 4.17.rcNext. There's actually a bug in my changes. Currently working on a fix.
Oh bummer :-). Thanks for letting me know !
This bug was referenced in samba master: b5848d391be4f7633745d9c36e432ac8b1c9dba2 e74b10e17ee5df0f77ac5349242841be8d71c4e8 3f7d8db9945a325020e4d1574289dea9e8331c29 968a5ae89f0d0da219e7dd05dd1f7f7c96dbb910
Created attachment 17492 [details] Patch for 4.17 cherry-picked from master
Created attachment 17493 [details] Patch for 4.16 backported from master
Created attachment 17494 [details] Patch for 4.15 backported from master
Pushed to autobuild-v4-17-test. 4.16 and 4.15 will be pushed when the review is there.
Pushed to autobuild-v4-{16,15}-test.
This bug was referenced in samba v4-17-test: f23ef830bc74b52f615c3e45334bc37213de63c1 bae285ed7023bcee80148365d25024bfa0d33ce0 aa85dac1e9579194dd41d04f46787733fa70fd81 3994f71f039db76f5802b378c660112fd964e8dc ab76ab52c3926c85664f6d6dd852b56492e2d05a ba468a9b416bbc44338f00983d4f8f21d697504e 1434b66f2a1835563a6378158c2251f69192cf4c 814fd4e8e89a843985f187b75f76ec1427965c56 69742bab6679f3d13e691485664442769dfb7689 107af8fd98b6608df08ab346efb1f53addf13111 7c713f386f378152e9730bf2648ae87f5df54517 aca819549c3f60b29464678f3492fb1f6d09a0fb f2272106f360524cff023be42e27e7ea33e8dcfc 0d0eff660583c7ec1675323a43c181205ea9b2ae 81be412fb01a585337122e0a2fc58df338c322c9 9df07ee0fa5dfc9bb45070078169890c339b8835 25d6dcd88975368e6eaa90257a2546b51d414f82 4d37152c666d84e7c76d4e0e976d051c4bbaa9df
This bug was referenced in samba v4-17-stable (Release samba-4.17.0rc3): f23ef830bc74b52f615c3e45334bc37213de63c1 bae285ed7023bcee80148365d25024bfa0d33ce0 aa85dac1e9579194dd41d04f46787733fa70fd81 3994f71f039db76f5802b378c660112fd964e8dc ab76ab52c3926c85664f6d6dd852b56492e2d05a ba468a9b416bbc44338f00983d4f8f21d697504e 1434b66f2a1835563a6378158c2251f69192cf4c 814fd4e8e89a843985f187b75f76ec1427965c56 69742bab6679f3d13e691485664442769dfb7689 107af8fd98b6608df08ab346efb1f53addf13111 7c713f386f378152e9730bf2648ae87f5df54517 aca819549c3f60b29464678f3492fb1f6d09a0fb f2272106f360524cff023be42e27e7ea33e8dcfc 0d0eff660583c7ec1675323a43c181205ea9b2ae 81be412fb01a585337122e0a2fc58df338c322c9 9df07ee0fa5dfc9bb45070078169890c339b8835 25d6dcd88975368e6eaa90257a2546b51d414f82 4d37152c666d84e7c76d4e0e976d051c4bbaa9df
Looks like the patches for 4.16 fail with UNEXPECTED(failure): samba3.smb2.fileid.fileid(nt4_dc) Sorry! Looking...
Created attachment 17495 [details] Patch for 4.16 backported from master
Created attachment 17496 [details] Patch for 4.15 backported from master
Created attachment 17497 [details] Patch for 4.16 backported from master
Created attachment 17498 [details] Patch for 4.15 backported from master
The 4.15 and 4.16 backports where missing code to update itime and file_id as well in vfs_fget_dos_attributes/().
Can we take this patch for 4.15 and 4.16? I see in above comment, some things are missing?
We found additional problems which are reported and fixed via https://bugzilla.samba.org/show_bug.cgi?id=15161
This bug was referenced in samba master: 3dcdab86f13fabb7a8c6ce71c59a565287d11244 201e1969bf31af07e8bd52876ff7f4d72b48a848 3a37e4155c3cd82388652f89b611f2c46fee8525
Created attachment 17507 [details] Patch for 4.15 backported from master The previous patch had a bug in the backport of patch 19/20: somehow the knownfail file wasn't removed. Fix now, sorry!
This bug was referenced in samba v4-15-test: d6c0c4e1c551a33e0c48b5f47b6b09db01889b45 6d8a013942eb7d6ed4d3b34a48f7e4b4ecb5ae32 a3795100e42f1135733c77950eeaa43e4de409af 216000dbe6d844e0c2a593ef7e7ba18dbc3d74bf 2412d67678b7692b03d191bb62aeae5d727a68ee fc6121cade5f032c21900c14516efe45994bf1f4 f0a52d433730f38845cb0870a1afec04cf7a2ccf ff3798ae0ff2219b366b40d02a97f220b834ada0 95e658ad86685029e02b0ebf6eaaca2807402a7d b1ebf29f20229b01b408f4f9b99748604c407d46 0fb876b34b2cd280e70d6f13785d29a83be11f76 a3f3f26a6bfced21532a189a34fe9711fc972a83 82342c74390685a141874ee9ce76ce3ae849a496 1c5a02bfb41249e6df965880aaacaef5156ad028 6369f59f38a217de1099237f2d8a258dd5a70264 1115b311c3715d8943dea26d8135455bd9e68c0f 135b59d00a71caef5a41636e20bad359195a158e ab0f75acbbc8fc3c4f307c8a2fb9bfaf558af364 c5796b0c7a35f2cc96cab3c63502c21b6153abd8 fa6012b63ab36704dfcfd6f95958ae0e089a94b5 6b5792b0a2ca1b7d4114272165968aaea673fceb
This bug was referenced in samba v4-17-test: 3139a1063a07ddb8a0df705bdc67d179969960b2 930380d4746f57e3d8ae9b6e9b9e37fc12ad890d ed1d01126160d49aea9088805120f95050510fe6
This bug was referenced in samba v4-16-test: 56ab8361573e436c5c1517d5d23e980e45bcc815 b83ff1252ed7883e257e41ed7ead4e995ad070c8 6d66f432297629ee4a608c2a96052f12303fa497 00ce839865c7b08c57211817ae14009f02ee44f9 3e6566222c98cb42d7d60b5a3507fb8e294a1482 9823e91999480f7d6aecb31e92c9062b1f2ca6fd 11947a8e59abd95f9a4b1b1214edcddb2904d9d5 eab9c65b07512dc7c6bf6a6e4e50015466c493b5 39129be4fef0c54f631667154e249a610109180e 8d0581a8ab1e9a4d257fac336d0b6cc1502b730d 2ae309348ad18944a9f2a7a744eb54b0bf6bf8e7 06b5438132e06c4d56a33b68d7b67d71a180ac74 e661087a9e2268d3d3725bd416289d10c4e19e18 a13748d24272ae7dafdacb6461cbd295c636089c 6ee18ad9eaff5b97a720dfcca7cb561cf73a919d 3d54c1b6ebc3c876ab910af159e3aea44336654a 5a9aa7aa84e9492448b9bdba182c73cb4501f7e0 1d2444218383eccab86a89797c2d05b8e3b026a5 b807f3624d1f720ad3d60c7ee51a69d89183633f 7c83b7788ec022551a2fd9381a1a5ff8e4adf5bc 1761ad3dff2e887593a06a9d9d47828427133bfd
This bug was referenced in samba v4-17-stable (Release samba-4.17.0rc5): 3139a1063a07ddb8a0df705bdc67d179969960b2 930380d4746f57e3d8ae9b6e9b9e37fc12ad890d ed1d01126160d49aea9088805120f95050510fe6
Closing out bug report. Thanks!
This bug was referenced in samba v4-16-stable (Release samba-4.16.5): 56ab8361573e436c5c1517d5d23e980e45bcc815 b83ff1252ed7883e257e41ed7ead4e995ad070c8 6d66f432297629ee4a608c2a96052f12303fa497 00ce839865c7b08c57211817ae14009f02ee44f9 3e6566222c98cb42d7d60b5a3507fb8e294a1482 9823e91999480f7d6aecb31e92c9062b1f2ca6fd 11947a8e59abd95f9a4b1b1214edcddb2904d9d5 eab9c65b07512dc7c6bf6a6e4e50015466c493b5 39129be4fef0c54f631667154e249a610109180e 8d0581a8ab1e9a4d257fac336d0b6cc1502b730d 2ae309348ad18944a9f2a7a744eb54b0bf6bf8e7 06b5438132e06c4d56a33b68d7b67d71a180ac74 e661087a9e2268d3d3725bd416289d10c4e19e18 a13748d24272ae7dafdacb6461cbd295c636089c 6ee18ad9eaff5b97a720dfcca7cb561cf73a919d 3d54c1b6ebc3c876ab910af159e3aea44336654a 5a9aa7aa84e9492448b9bdba182c73cb4501f7e0 1d2444218383eccab86a89797c2d05b8e3b026a5 b807f3624d1f720ad3d60c7ee51a69d89183633f 7c83b7788ec022551a2fd9381a1a5ff8e4adf5bc 1761ad3dff2e887593a06a9d9d47828427133bfd
This bug was referenced in samba v4-15-stable (Release samba-4.15.10): d6c0c4e1c551a33e0c48b5f47b6b09db01889b45 6d8a013942eb7d6ed4d3b34a48f7e4b4ecb5ae32 a3795100e42f1135733c77950eeaa43e4de409af 216000dbe6d844e0c2a593ef7e7ba18dbc3d74bf 2412d67678b7692b03d191bb62aeae5d727a68ee fc6121cade5f032c21900c14516efe45994bf1f4 f0a52d433730f38845cb0870a1afec04cf7a2ccf ff3798ae0ff2219b366b40d02a97f220b834ada0 95e658ad86685029e02b0ebf6eaaca2807402a7d b1ebf29f20229b01b408f4f9b99748604c407d46 0fb876b34b2cd280e70d6f13785d29a83be11f76 a3f3f26a6bfced21532a189a34fe9711fc972a83 82342c74390685a141874ee9ce76ce3ae849a496 1c5a02bfb41249e6df965880aaacaef5156ad028 6369f59f38a217de1099237f2d8a258dd5a70264 1115b311c3715d8943dea26d8135455bd9e68c0f 135b59d00a71caef5a41636e20bad359195a158e ab0f75acbbc8fc3c4f307c8a2fb9bfaf558af364 c5796b0c7a35f2cc96cab3c63502c21b6153abd8 fa6012b63ab36704dfcfd6f95958ae0e089a94b5 6b5792b0a2ca1b7d4114272165968aaea673fceb