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: 2022-09-28 15:41 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.
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 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