Bug 15126 - acl_xattr VFS module may unintentionally use filesystem permissions instead of ACL from xattr
Summary: acl_xattr VFS module may unintentionally use filesystem permissions instead o...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: All All
: P5 critical (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-19 07:54 UTC by Ralph Böhme
Modified: 2023-07-20 16:33 UTC (History)
2 users (show)

See Also:


Attachments
git-am fix for 4.17.rcNext. (55.75 KB, patch)
2022-08-10 20:56 UTC, Jeremy Allison
slow: review-
Details
Patch for 4.17 cherry-picked from master (77.45 KB, patch)
2022-08-23 10:26 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.16 backported from master (80.93 KB, patch)
2022-08-23 11:05 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.15 backported from master (81.05 KB, patch)
2022-08-23 11:06 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.16 backported from master (75.00 KB, patch)
2022-08-24 07:53 UTC, Ralph Böhme
no flags Details
Patch for 4.15 backported from master (65.36 KB, patch)
2022-08-24 07:54 UTC, Ralph Böhme
no flags Details
Patch for 4.16 backported from master (81.29 KB, patch)
2022-08-24 09:29 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.15 backported from master (81.03 KB, patch)
2022-08-24 09:29 UTC, Ralph Böhme
metze: review+
Details
Patch for 4.15 backported from master (81.40 KB, patch)
2022-09-04 17:37 UTC, Ralph Böhme
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2022-07-19 07:54:09 UTC
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...
Comment 1 Ralph Böhme 2022-07-19 07:57:48 UTC
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.
Comment 2 Jeremy Allison 2022-07-19 18:17:54 UTC
Couldn't we just check for fsp->base_fsp != NULL and use that when calling into FGETXATTR ?
Comment 3 Ralph Böhme 2022-07-29 13:59:12 UTC
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.
Comment 4 Samba QA Contact 2022-08-10 16:33:03 UTC
This bug was referenced in samba master:

0d3995cec10c5fae8c8b6a1df312062e38437e6f
b26dc252aaf3f4b960bdfdb6a3dfe612b89fcdd5
451ad315a9bf32c627e1966ec30185542701c87e
92e0045d7ca7c0b94efd0244ba0e426cad0a05b6
23bc760ec5d61208c2d8778991e3d7e202eab352
c949e4b2a42423ac3851e86e489fd0c5d46d7f1f
4ab29e2a345b48ebba652d5154e96adf954a6757
03b9ce84736d536ab2dd8a5ce1a2656e6a90c8c8
55e55804bb2d0f21c1bbe207257bb40555f3b7a2
3af8f8e8741cc8c889bbf416ccd38a1b702917ec
06555c6bcb5644fc9eea35b3cbae8d8801c65ab6
f0299abf1b28a14518328710d9f84bef17fd2ecf
51243e3849736acbbf1d8f52cc02cdec5995fde4
fc45fcfde51b0b0bdcd524c82a0f9eabf7273045
Comment 5 Jeremy Allison 2022-08-10 20:56:01 UTC
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 6 Ralph Böhme 2022-08-12 08:08:21 UTC
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.
Comment 7 Jeremy Allison 2022-08-12 08:09:25 UTC
Oh bummer :-). Thanks for letting me know !
Comment 8 Samba QA Contact 2022-08-22 09:04:11 UTC
This bug was referenced in samba master:

b5848d391be4f7633745d9c36e432ac8b1c9dba2
e74b10e17ee5df0f77ac5349242841be8d71c4e8
3f7d8db9945a325020e4d1574289dea9e8331c29
968a5ae89f0d0da219e7dd05dd1f7f7c96dbb910
Comment 9 Ralph Böhme 2022-08-23 10:26:48 UTC
Created attachment 17492 [details]
Patch for 4.17 cherry-picked from master
Comment 10 Ralph Böhme 2022-08-23 11:05:38 UTC
Created attachment 17493 [details]
Patch for 4.16 backported from master
Comment 11 Ralph Böhme 2022-08-23 11:06:12 UTC
Created attachment 17494 [details]
Patch for 4.15 backported from master
Comment 12 Jule Anger 2022-08-23 11:45:45 UTC
Pushed to autobuild-v4-17-test.
4.16 and 4.15 will be pushed when the review is there.
Comment 13 Jule Anger 2022-08-23 13:44:33 UTC
Pushed to autobuild-v4-{16,15}-test.
Comment 14 Samba QA Contact 2022-08-23 14:27:11 UTC
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
Comment 15 Samba QA Contact 2022-08-23 14:50:44 UTC
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
Comment 16 Ralph Böhme 2022-08-23 16:44:50 UTC
Looks like the patches for 4.16 fail with

UNEXPECTED(failure): samba3.smb2.fileid.fileid(nt4_dc)

Sorry! Looking...
Comment 17 Ralph Böhme 2022-08-24 07:53:38 UTC
Created attachment 17495 [details]
Patch for 4.16 backported from master
Comment 18 Ralph Böhme 2022-08-24 07:54:06 UTC
Created attachment 17496 [details]
Patch for 4.15 backported from master
Comment 19 Ralph Böhme 2022-08-24 09:29:04 UTC
Created attachment 17497 [details]
Patch for 4.16 backported from master
Comment 20 Ralph Böhme 2022-08-24 09:29:27 UTC
Created attachment 17498 [details]
Patch for 4.15 backported from master
Comment 21 Ralph Böhme 2022-08-24 09:30:49 UTC
The 4.15 and 4.16 backports where missing code to update itime and file_id as well in vfs_fget_dos_attributes/().
Comment 22 Alok Chandra Mallick (smtp errors) 2022-09-01 18:14:37 UTC
Can we take this patch for 4.15 and 4.16? I see in above comment, some things are missing?
Comment 23 Ralph Böhme 2022-09-02 14:55:12 UTC
We found additional problems which are reported and fixed via

https://bugzilla.samba.org/show_bug.cgi?id=15161
Comment 24 Samba QA Contact 2022-09-02 15:57:03 UTC
This bug was referenced in samba master:

3dcdab86f13fabb7a8c6ce71c59a565287d11244
201e1969bf31af07e8bd52876ff7f4d72b48a848
3a37e4155c3cd82388652f89b611f2c46fee8525
Comment 25 Ralph Böhme 2022-09-04 17:37:57 UTC
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!
Comment 26 Jule Anger 2022-09-06 06:27:01 UTC
Pushed to autobuild-v4-{16,15}-test.
Comment 27 Samba QA Contact 2022-09-06 07:32:20 UTC
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
Comment 28 Samba QA Contact 2022-09-06 08:09:11 UTC
This bug was referenced in samba v4-17-test:

3139a1063a07ddb8a0df705bdc67d179969960b2
930380d4746f57e3d8ae9b6e9b9e37fc12ad890d
ed1d01126160d49aea9088805120f95050510fe6
Comment 29 Samba QA Contact 2022-09-06 08:50:42 UTC
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
Comment 30 Samba QA Contact 2022-09-06 14:25:25 UTC
This bug was referenced in samba v4-17-stable (Release samba-4.17.0rc5):

3139a1063a07ddb8a0df705bdc67d179969960b2
930380d4746f57e3d8ae9b6e9b9e37fc12ad890d
ed1d01126160d49aea9088805120f95050510fe6
Comment 31 Jule Anger 2022-09-06 14:33:05 UTC
Closing out bug report.

Thanks!
Comment 32 Samba QA Contact 2022-09-07 19:02:17 UTC
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
Comment 33 Samba QA Contact 2022-09-28 15:41:32 UTC
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